-
Notifications
You must be signed in to change notification settings - Fork 5
NEW (SFGE) @W-17915999@ Added SFGE telemetry support #269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -188,7 +201,8 @@ function handleRunStdOut(stdOutMsg: string, emitLog: (logLevel: LogLevel, msg: s | |||
for (const sfgeMessage of sfgeMessages) { | |||
if (isSfgeLogMessage(sfgeMessage)) { | |||
if (sfgeMessage.messageSeverity === 'TELEMETRY') { | |||
// TODO: TELEMETRY | |||
const telemetryData: TelemetryData = JSON.parse(sfgeMessage.args[0]) as TelemetryData; | |||
emitTelemetry(telemetryData.eventName as string, telemetryData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, this means that the Telemetry Data will have its own redundant eventName
parameter, but I don't believe that's worth getting into a tizzy over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whenever possible, we should put JSON.parse in a try/catch in case we don't get the text in full or whatever. Probably could just log with trace level logging if it does fail and just continue as usual. But if you don't like the redundant stuff, then just recreate the data structure over here with what you want. Up to you.
const engine: SfgeEngine = new SfgeEngine(configWithLowTimeout, fixedClock); | ||
const workspace: Workspace = new Workspace('id', [path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace')]); | ||
const engine: SfgeEngine = new SfgeEngine(DEFAULT_SFGE_ENGINE_CONFIG, fixedClock); | ||
const workspace: Workspace = new Workspace('id', [path.join(TEST_DATA_FOLDER, 'sampleUnhandledWorkspace')]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched this test to work by scanning code that SFGE can't currently handle, instead of setting an absurdly low timeout. For whatever reason, the timeout failure wasn't reliably producing telemetry from within SFGE. That's something we may want to look into, but it's not within the scope of this story.
@@ -0,0 +1,13 @@ | |||
global virtual class MyStringHelper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was pulled from Scanner issue #1177.
43eb178
to
7c84e87
Compare
7c84e87
to
0483d6a
Compare
This PR allows the SFGE engine to transmit telemetry events from the underlying Java module back up through the Typescript layer up to Core.