-
Notifications
You must be signed in to change notification settings - Fork 33.2k
track latency from request -> showing terminal completions, indicate first shown for shell type and for window #252007
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
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestTelemetry.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminal/common/terminalStorageKeys.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
This PR adds telemetry to measure latency from a terminal completion request to when suggestions are shown and flags the first time completions are displayed per window and per shell type.
- Introduces
logCompletionLatency
inTerminalSuggestTelemetry
to send latency and first-shown flags. - Updates
SuggestAddon
to timestamp requests, compute/display latency, and track first-shown state. - Adds a new
FirstShown
storage key interminalStorageKeys.ts
.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestTelemetry.ts | Added logCompletionLatency method with its telemetry schema. |
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts | Implemented request timestamping, first-shown determination, and telemetry logging. |
src/vs/workbench/contrib/terminal/common/terminalStorageKeys.ts | Added FirstShown enum entry for storing first-shown state. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestTelemetry.ts:58
- The new
logCompletionLatency
method adds critical telemetry logic but lacks accompanying unit or integration tests. Please add tests to verify the correct payload is sent under various conditions (e.g., first vs. subsequent invocations).
logCompletionLatency(sessionId: string, latency: number, firstShownFor: { window: boolean; shell: boolean }): void {
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminal/common/terminalStorageKeys.ts
Outdated
Show resolved
Hide resolved
…nalSuggestAddon.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Just realized a set for the shell types might be better |
part of #251994