-
Notifications
You must be signed in to change notification settings - Fork 1
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
@W-14219242@: Fixing behavioral issues with telemetry. #27
Conversation
@@ -79,14 +79,34 @@ export class CoreExtensionService { | |||
* | |||
* @returns The {@link TelemetryService} object exported by the Core Extension if available, else null. | |||
*/ | |||
public static getTelemetryService(): TelemetryService|null { | |||
public static _getTelemetryService(): CoreTelemetryService|null { |
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.
The leading underscore is a JS social convention indicating that the method should be treated by developers as private even though it's technically public.
if (CoreExtensionService.initialized) { | ||
return CoreExtensionService.telemetryService; | ||
} | ||
throw new Error(messages.error.coreExtensionServiceUninitialized); | ||
} | ||
} | ||
|
||
export class TelemetryService { |
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.
Moving TelemetryService
into core-extension-service.ts
allowed me to make it so all telemetry events have to go through this class instead of calling CoreExtensionService._getTelemetryService()
directly.
export class TelemetryService { | ||
public static sendCommandEvent(key: string, data: Properties): void { | ||
const coreTelemetryService = CoreExtensionService.getTelemetryService(); | ||
coreTelemetryService.sendCommandEvent(key, undefined, data); |
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 needed to be coreTelemetryService?.sendCommandEvent
, like how it's done in sendException
. That error has been corrected in the version that now exists in core-extension-service.ts
.
No description provided.