Skip to content

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 20, 2025

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 a providerId field that identifies which completion provider supplied each accepted completion.

Technical Details

  1. Data Structure: Updated _acceptedCompletions type to include providerId: string
  2. Data Capture: Modified acceptCompletion method to capture completion.provider as providerId
  3. Telemetry Schema: Added providerId: string | undefined to telemetry data structure with proper GDPR classification
  4. Backward Compatibility: Handles cases where provider ID may be undefined gracefully

Example Telemetry Data

Before:

{
  "kind": "File",
  "outcome": "Accepted", 
  "exitCode": 0,
  "sessionId": "abc123"
}

After:

{
  "kind": "File",
  "outcome": "Accepted",
  "exitCode": 0, 
  "sessionId": "abc123",
  "providerId": "builtinPwsh"
}

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
    • Triggering command: 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.

Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Report provider ID with terminal suggest telemetry Add provider ID to terminal suggest telemetry Jun 20, 2025
@Copilot Copilot AI requested a review from Tyriar June 20, 2025 20:32
Copilot finished work on behalf of Tyriar June 20, 2025 20:32
@@ -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: {
Copy link
Member

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

Copy link
Contributor Author

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) {
Copy link
Member

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

Copy link
Contributor Author

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>
Copy link
Contributor Author

Copilot AI commented Jun 20, 2025

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 addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • electronjs.org
    • Triggering command: node-gyp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Copilot Copilot AI requested a review from Tyriar June 20, 2025 20:45
Copilot finished work on behalf of Tyriar June 20, 2025 20:45
Copy link
Member

@Tyriar Tyriar left a 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report provider ID with terminal suggest telemetry
2 participants