-
Notifications
You must be signed in to change notification settings - Fork 33.3k
Add provider ID to terminal suggest telemetry #252016
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>
@@ -96,11 +98,17 @@ export class TerminalSuggestTelemetry extends Disposable { | |||
purpose: 'FeatureInsight'; | |||
comment: 'The session ID of the terminal session where the completion was accepted'; | |||
}; | |||
providerId: { |
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.
Let's rename this to provider
instead, that matches the property on completion
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.
Renamed providerId
to provider
throughout the codebase to match the property name on the completion object. Commit b7b1df1
} | ||
private _sendTelemetryInfo(fromInterrupt?: boolean, exitCode?: number): void { | ||
const commandLine = this._promptInputModel?.value; | ||
for (const completion of this._acceptedCompletions || []) { | ||
const label = completion?.label; | ||
const kind = completion?.kind; | ||
const providerId = completion?.providerId; | ||
|
||
if (label === undefined || commandLine === undefined || kind === undefined) { |
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.
Let's verify provider is set here
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.
Added provider validation to the undefined check - now verifies that provider
is set before sending telemetry data. Commit b7b1df1
Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
@meganrogge could you verify this before merging?
This change enhances terminal suggest telemetry by including the provider ID in completion acceptance events. This will help understand which completion providers are most helpful and which have issues.
Changes Made
The
terminal.suggest.acceptedCompletion
telemetry event now includes aproviderId
field that identifies which completion provider supplied each accepted completion.Technical Details
_acceptedCompletions
type to includeproviderId: string
acceptCompletion
method to capturecompletion.provider
asproviderId
providerId: string | undefined
to telemetry data structure with proper GDPR classificationExample Telemetry Data
Before:
After:
This enables analysis of provider effectiveness across different shell types and completion scenarios (e.g.,
'builtinPwsh'
,'lsp'
, extension providers, etc.).Fixes #252008.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
electronjs.org
node-gyp
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.