Switch GH telemetry to use vscode extension telemetry (exp roll out)#3331
Switch GH telemetry to use vscode extension telemetry (exp roll out)#3331vijayupadya wants to merge 4 commits intomainfrom
Conversation
vijayupadya
changed the title
There was a problem hiding this comment.
Pull request overview
Adds an experiment-gated path for routing GitHub telemetry through @vscode/extension-telemetry instead of the legacy applicationinsights sender, enabling A/B rollout of the new pipeline.
Changes:
- Introduces an experiment flag (
chat.advanced.telemetry.useVSCodeTelemetryLibForGH) to toggle the new GH telemetry sender behavior. - Adds a GH telemetry
TelemetrySenderadapter that can route events to either the legacyAzureInsightReporteror the newTelemetryReporter. - Updates
@vscode/extension-telemetrydependency to^1.4.0.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/telemetry/vscode-node/telemetryServiceImpl.ts | Injects IInstantiationService and lazily evaluates the experiment flag to decide GH telemetry routing. |
| src/platform/telemetry/vscode-node/githubTelemetrySender.ts | Adds adapter that chooses between legacy and new telemetry reporters and constructs the new TelemetryReporter config. |
| src/platform/telemetry/common/ghTelemetrySender.ts | Adjusts token store handling and adds a tagOverrides field into the payload returned for logging. |
| src/platform/configuration/common/configurationService.ts | Adds the new experiment-backed config key controlling GH telemetry routing. |
| package.json | Bumps @vscode/extension-telemetry to ^1.4.0. |
| package-lock.json | Locks dependency bump to @vscode/extension-telemetry@1.4.0. |
Comments suppressed due to low confidence (1)
src/platform/telemetry/vscode-node/githubTelemetrySender.ts:22
- The new experiment-gated routing logic (TelemetryReporterAdapter choosing old vs new reporter and unwrapping event names) isn’t covered by existing telemetry tests. Add unit tests that exercise both flag states and verify the final event name and destination match the legacy behavior, especially for
wrapEventNameForPrefixRemoval(...)events.
/**
* Adapter that wraps both old and new telemetry reporters to implement VS Code's TelemetrySender interface.
* Supports lazy flag evaluation to avoid circular dependencies during service initialization.
*/
class TelemetryReporterAdapter implements TelemetrySender {
… into vijayu/ghTelUpdate
| if (this.useNewTelemetryLib && this.newReporter) { | ||
| // Remove the wrapped-telemetry-event-name marker if present. | ||
| // This marker is used to signal that the event name should not be re-namespaced. | ||
| const unwrappedEventName = unwrapEventNameFromPrefix(eventName); | ||
|
|
There was a problem hiding this comment.
When routing to the new @vscode/extension-telemetry reporter, the event name is only unwrapped but never re-namespaced with the extension name (equivalent of AzureInsightReporter.massageEventName). This changes GitHub telemetry event names (e.g. previously "<extensionName>/<event>" unless wrapped) and breaks the existing wrapEventNameForPrefixRemoval contract used by callers (e.g. completionsTelemetryServiceBridge). Consider passing extensionName into the adapter and applying the same prefixing logic: if the name was wrapped, unwrap and send as-is; otherwise, prefix with ${extensionName}/ unless already prefixed.
| if (this.cachedFlagValue === undefined) { | ||
| this.cachedFlagValue = this.useNewTelemetryLibGetter(); | ||
| } | ||
| return this.cachedFlagValue; | ||
| } |
There was a problem hiding this comment.
This PR adds experiment-gated routing (old Application Insights reporter vs new @vscode/extension-telemetry reporter) plus special handling for wrapped event names. There are existing telemetry unit tests under src/platform/telemetry/test/, but none cover this new adapter logic—please add tests to lock down the routing decision and event-name handling (wrapped vs non-wrapped).
1 participant
Fixes https://github.com/microsoft/vscode-internalbacklog/issues/6549
Adds support for routing GitHub telemetry events through @vscode/extension-telemetry library instead of existing applicationinsights SDK. The routing is controlled by an experiment flag, enabling A/B testing of the new implementation.