From 4b8c05a505174dca611b98b9b64bf4c5c82d4c63 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 8 Jun 2026 11:15:35 +0000 Subject: [PATCH 1/6] feat: add workflow telemetry traces --- src/commands.ts | 643 ++++++++++++-------- src/instrumentation/commands.ts | 486 +++++++++++++++ src/instrumentation/workspace.ts | 23 +- src/remote/workspaceStateMachine.ts | 26 +- src/telemetry/export/command.ts | 26 +- src/uri/uriHandler.ts | 1 + test/unit/instrumentation/commands.test.ts | 208 +++++++ test/unit/instrumentation/workspace.test.ts | 42 +- test/unit/telemetry/export/command.test.ts | 37 +- test/unit/uri/uriHandler.test.ts | 4 + 10 files changed, 1219 insertions(+), 277 deletions(-) create mode 100644 src/instrumentation/commands.ts create mode 100644 test/unit/instrumentation/commands.test.ts diff --git a/src/commands.ts b/src/commands.ts index e0aee6936..816e7f8b0 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -19,6 +19,15 @@ import { type AuthLoginOutcome, type AuthLogoutOutcome, } from "./instrumentation/auth"; +import { + CommandInstrumentation, + type DevcontainerMode, + type WorkspaceOpenSource, + type WorkspaceOpenTrace, + type WorkspacePickerFailureCategory, + type WorkspacePickerResult, + type WorkspacePickerSource, +} from "./instrumentation/commands"; import { reportElapsedProgress, withCancellableProgress, @@ -34,7 +43,6 @@ import { appendVsCodeLogs } from "./supportBundle/appendVsCodeLogs"; import { runExportTelemetryCommand } from "./telemetry/export/command"; import { openInBrowser, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; -import { parseSpeedtestResult } from "./webviews/speedtest/types"; import { AgentTreeItem, type OpenableTreeItem, @@ -68,6 +76,7 @@ interface OpenOptions { agentName?: string; folderPath?: string; openRecent?: boolean; + source?: WorkspaceOpenSource; /** When false, an absent folderPath opens a bare remote window instead of * falling back to the agent's expanded_directory. Defaults to true. */ useDefaultDirectory?: boolean; @@ -78,6 +87,18 @@ interface LoginArgs { readonly autoLogin?: boolean; } +type WorkspaceResolution = + | { + readonly status: "selected"; + readonly client: CoderApi; + readonly workspaceId: string; + } + | { readonly status: "cancelled" } + | { + readonly status: "failed"; + readonly category: WorkspacePickerFailureCategory; + }; + const openDefaults = { openRecent: false, useDefaultDirectory: true, @@ -94,6 +115,7 @@ export class Commands { private readonly speedtestPanelFactory: SpeedtestPanelFactory; private readonly telemetryService: TelemetryService; private readonly authTelemetry: AuthTelemetry; + private readonly traces: CommandInstrumentation; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -112,6 +134,7 @@ export class Commands { private readonly deploymentManager: DeploymentManager, ) { this.telemetryService = serviceContainer.getTelemetryService(); + this.traces = new CommandInstrumentation(this.telemetryService); this.logger = serviceContainer.getLogger(); this.pathResolver = serviceContainer.getPathResolver(); this.mementoManager = serviceContainer.getMementoManager(); @@ -223,138 +246,178 @@ export class Commands { * editor document. Can be triggered from the sidebar or command palette. */ public async speedTest(item?: OpenableTreeItem): Promise { - const resolved = await this.resolveClientAndWorkspace(item); - if (!resolved) { - return; - } + await this.traces.diagnostic("coder.speedTest", async (telemetry) => { + const resolved = await this.resolveClientAndWorkspace(item); + if (resolved.status === "cancelled") { + telemetry.cancel("workspace_picker"); + return; + } + if (resolved.status === "failed") { + telemetry.fail(undefined, resolved.category); + return; + } - const { client, workspaceId } = resolved; + const { client, workspaceId } = resolved; - const input = await vscode.window.showInputBox({ - title: "Speed Test Duration", - prompt: "Duration in seconds", - value: "5", - validateInput: (value) => { - const n = Number(value.trim()); - if (!value.trim() || !Number.isFinite(n) || n <= 0) { - return "Please enter a positive number"; - } - return undefined; - }, - }); - if (input === undefined) { - return; - } - const seconds = Number(input.trim()); - - const result = await withCancellableProgress( - async ({ signal, progress }) => { - progress.report({ message: "Connecting..." }); - const env = await this.resolveCliEnv(client); + const input = await vscode.window.showInputBox({ + title: "Speed Test Duration", + prompt: "Duration in seconds", + value: "5", + validateInput: (value) => { + const n = Number(value.trim()); + if (!value.trim() || !Number.isFinite(n) || n <= 0) { + return "Please enter a positive number"; + } + return undefined; + }, + }); + if (input === undefined) { + telemetry.cancel("input"); + return; + } + const seconds = Number(input.trim()); + telemetry.speedtestRequestedDuration(seconds); + + const result = await withCancellableProgress( + async ({ signal, progress }) => { + progress.report({ message: "Connecting..." }); + const env = await this.resolveCliEnv(client); + + const stopProgress = reportElapsedProgress({ + progress, + totalMs: seconds * 1000, + format: (pct, elapsedMs) => + pct >= 100 + ? "Collecting results..." + : `${Math.floor(elapsedMs / 1000)}s / ${seconds}s`, + }); + try { + return await cliExec.speedtest( + env, + workspaceId, + `${seconds}s`, + signal, + ); + } finally { + stopProgress(); + } + }, + { + location: vscode.ProgressLocation.Notification, + title: `Running speed test for ${workspaceId}`, + cancellable: true, + }, + ); - const stopProgress = reportElapsedProgress({ - progress, - totalMs: seconds * 1000, - format: (pct, elapsedMs) => - pct >= 100 - ? "Collecting results..." - : `${Math.floor(elapsedMs / 1000)}s / ${seconds}s`, - }); - try { - return await cliExec.speedtest( - env, - workspaceId, - `${seconds}s`, - signal, + if (!telemetry.progressResult(result)) { + if (!result.cancelled) { + this.logger.error("Speed test failed", result.error); + vscode.window.showErrorMessage( + `Speed test failed: ${toError(result.error).message}`, ); - } finally { - stopProgress(); } - }, - { - location: vscode.ProgressLocation.Notification, - title: `Running speed test for ${workspaceId}`, - cancellable: true, - }, - ); + return; + } - if (result.ok) { try { - const parsed = parseSpeedtestResult(result.value); + const parsed = telemetry.speedtestSuccess(result.value); this.speedtestPanelFactory.show({ result: parsed, rawJson: result.value, workspaceId, }); } catch (err) { - this.logger.error("Failed to parse speedtest output", err); - const message = - err instanceof ZodError - ? "Speed test output did not match the expected format. Check `Output > Coder` for details." - : `Speed test returned unexpected output: ${toError(err).message}`; - vscode.window.showErrorMessage(message); + if (err instanceof ZodError || err instanceof SyntaxError) { + telemetry.fail(err, "parse_error"); + this.logger.error("Failed to parse speedtest output", err); + vscode.window.showErrorMessage( + "Speed test output did not match the expected format. Check `Output > Coder` for details.", + ); + return; + } + telemetry.fail(err); + this.logger.error("Failed to display speedtest results", err); + vscode.window.showErrorMessage( + `Speed test returned unexpected output: ${toError(err).message}`, + ); } - return; - } - - if (result.cancelled) { - return; - } - - this.logger.error("Speed test failed", result.error); - vscode.window.showErrorMessage( - `Speed test failed: ${toError(result.error).message}`, - ); + }); } public async supportBundle(item?: OpenableTreeItem): Promise { - const resolved = await this.resolveClientAndWorkspace(item); - if (!resolved) { - return; - } + await this.traces.diagnostic("coder.supportBundle", async (telemetry) => { + const resolved = await this.resolveClientAndWorkspace(item); + if (resolved.status === "cancelled") { + telemetry.cancel("workspace_picker"); + return; + } + if (resolved.status === "failed") { + telemetry.fail(undefined, resolved.category); + return; + } - const { client, workspaceId } = resolved; + const { client, workspaceId } = resolved; - const outputUri = await this.promptSupportBundlePath(); - if (!outputUri) { - return; - } + const outputUri = await this.promptSupportBundlePath(); + if (!outputUri) { + telemetry.cancel("save_dialog"); + return; + } - const result = await withCancellableProgress( - async ({ signal, progress }) => { - progress.report({ message: "Resolving CLI..." }); - const env = await this.resolveCliEnv(client); - if (!env.featureSet.supportBundle) { - throw new Error( - "Support bundles require Coder CLI v2.10.0 or later. Please update your Coder deployment.", + const result = await withCancellableProgress( + async ({ signal, progress }) => { + progress.report({ message: "Resolving CLI..." }); + const env = await this.resolveCliEnv(client); + if (!env.featureSet.supportBundle) { + throw new Error( + "Support bundles require Coder CLI v2.10.0 or later. Please update your Coder deployment.", + ); + } + + progress.report({ message: "Collecting diagnostics..." }); + await cliExec.supportBundle( + env, + workspaceId, + outputUri.fsPath, + signal, ); - } - progress.report({ message: "Collecting diagnostics..." }); - await cliExec.supportBundle(env, workspaceId, outputUri.fsPath, signal); + progress.report({ message: "Adding VS Code logs..." }); + await appendVsCodeLogs( + outputUri.fsPath, + { + activeProxyLogPath: this.workspaceLogPath, + proxyLogDir: this.pathResolver.getProxyLogPath(), + extensionLogDir: this.pathResolver.getCodeLogDir(), + telemetryDir: this.pathResolver.getTelemetryPath(), + }, + this.logger, + ); - progress.report({ message: "Adding VS Code logs..." }); - await appendVsCodeLogs( - outputUri.fsPath, - { - activeProxyLogPath: this.workspaceLogPath, - proxyLogDir: this.pathResolver.getProxyLogPath(), - extensionLogDir: this.pathResolver.getCodeLogDir(), - telemetryDir: this.pathResolver.getTelemetryPath(), - }, - this.logger, - ); + return outputUri; + }, + { + location: vscode.ProgressLocation.Notification, + title: `Creating support bundle for ${workspaceId}`, + cancellable: true, + }, + ); - return outputUri; - }, - { - location: vscode.ProgressLocation.Notification, - title: `Creating support bundle for ${workspaceId}`, - cancellable: true, - }, - ); + if (!telemetry.progressResult(result)) { + if (!result.cancelled) { + if ( + toError(result.error).message.startsWith("Support bundles require") + ) { + telemetry.fail(result.error, "unsupported_cli"); + } + this.logger.error("Support bundle failed", result.error); + vscode.window.showErrorMessage( + `Support bundle failed: ${toError(result.error).message}`, + ); + } + return; + } - if (result.ok) { const action = await vscode.window.showInformationMessage( `Support bundle saved to ${result.value.fsPath}`, "Reveal in File Explorer", @@ -362,17 +425,7 @@ export class Commands { if (action === "Reveal in File Explorer") { await vscode.commands.executeCommand("revealFileInOS", result.value); } - return; - } - - if (result.cancelled) { - return; - } - - this.logger.error("Support bundle failed", result.error); - vscode.window.showErrorMessage( - `Support bundle failed: ${toError(result.error).message}`, - ); + }); } private promptSupportBundlePath(): Thenable { @@ -385,12 +438,25 @@ export class Commands { } public async exportTelemetry(): Promise { - await runExportTelemetryCommand( - this.pathResolver.getTelemetryPath(), - this.logger, - () => this.telemetryService.flush(), - this.telemetryService.getContext(), - ); + await this.traces.diagnostic("coder.exportTelemetry", async (telemetry) => { + const outcome = await runExportTelemetryCommand( + this.pathResolver.getTelemetryPath(), + this.logger, + () => this.telemetryService.flush(), + this.telemetryService.getContext(), + ); + switch (outcome.status) { + case "cancelled": + telemetry.cancel(outcome.stage); + return; + case "failed": + telemetry.fail(outcome.error); + return; + case "success": + telemetry.exportSuccess(outcome.format, outcome.eventCount); + return; + } + }); } /** @@ -690,26 +756,45 @@ export class Commands { throw new Error("You are not logged in"); } if (item instanceof AgentTreeItem) { - await this.openWorkspace(baseUrl, item.workspace, item.agent, { - openRecent: true, - }); + await this.traces.workspaceOpen( + "sidebar.agent", + { workspace: item.workspace, agent: item.agent }, + (telemetry) => { + return this.openWorkspace(baseUrl, item.workspace, item.agent, { + openRecent: true, + telemetry, + }); + }, + ); } else if (item instanceof WorkspaceTreeItem) { - const agents = await this.extractAgentsWithFallback(item.workspace); - const agent = await maybeAskAgent(agents); - if (!agent) { - // User declined to pick an agent. - return; - } - await this.openWorkspace(baseUrl, item.workspace, agent, { - openRecent: true, - }); + await this.traces.workspaceOpen( + "sidebar.workspace", + { workspace: item.workspace }, + async (telemetry) => { + const agents = await this.extractAgentsWithFallback(item.workspace); + const agent = await maybeAskAgent(agents); + if (!agent) { + return telemetry.cancel("agent_picker", { + workspace: item.workspace, + }); + } + telemetry.select({ + workspace: item.workspace, + agent, + }); + return this.openWorkspace(baseUrl, item.workspace, agent, { + openRecent: true, + telemetry, + }); + }, + ); } else { throw new TypeError("Unable to open unknown sidebar item"); } } else { // If there is no tree item, then the user manually ran this command. // Default to the regular open instead. - await this.open(); + await this.open({ source: "sidebar.fallback" }); } } @@ -756,39 +841,46 @@ export class Commands { agentName, folderPath, openRecent, + source = workspaceOwner && workspaceName ? "uri" : "command", useDefaultDirectory, } = { ...openDefaults, ...options }; - const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; - if (!baseUrl) { - throw new Error("You are not logged in"); - } + return this.traces.workspaceOpen(source, undefined, async (telemetry) => { + const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; + if (!baseUrl) { + throw new Error("You are not logged in"); + } - let workspace: Workspace | undefined; - if (workspaceOwner && workspaceName) { - workspace = await this.extensionClient.getWorkspaceByOwnerAndName( - workspaceOwner, - workspaceName, - ); - } else { - workspace = await this.pickWorkspace(); - if (!workspace) { - // User declined to pick a workspace. - return false; + let workspace: Workspace; + if (workspaceOwner && workspaceName) { + workspace = await this.extensionClient.getWorkspaceByOwnerAndName( + workspaceOwner, + workspaceName, + ); + } else { + const pick = await this.pickWorkspace("workspace.open"); + if (pick.status === "cancelled") { + return telemetry.cancel("workspace_picker"); + } + if (pick.status === "failed") { + return telemetry.fail(pick.category); + } + workspace = pick.workspace; } - } - const agents = await this.extractAgentsWithFallback(workspace); - const agent = await maybeAskAgent(agents, agentName); - if (!agent) { - // User declined to pick an agent. - return false; - } + const agents = await this.extractAgentsWithFallback(workspace); + const agent = await maybeAskAgent(agents, agentName); + if (!agent) { + return telemetry.cancel("agent_picker", { workspace }); + } + telemetry.select({ workspace, agent }); - return this.openWorkspace(baseUrl, workspace, agent, { - folderPath, - openRecent, - useDefaultDirectory, + return this.openWorkspace(baseUrl, workspace, agent, { + folderPath, + openRecent, + useDefaultDirectory, + telemetry, + }); }); } @@ -806,55 +898,62 @@ export class Commands { localWorkspaceFolder = "", localConfigFile = "", ): Promise { - const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; - if (!baseUrl) { - throw new Error("You are not logged in"); - } + const mode: DevcontainerMode = localWorkspaceFolder + ? "dev_container" + : "attached_container"; + await this.traces.devcontainerOpen(mode, async () => { + const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; + if (!baseUrl) { + throw new Error("You are not logged in"); + } - const remoteAuthority = toRemoteAuthority( - baseUrl, - workspaceOwner, - workspaceName, - workspaceAgent, - ); + const remoteAuthority = toRemoteAuthority( + baseUrl, + workspaceOwner, + workspaceName, + workspaceAgent, + ); - const hostPath = localWorkspaceFolder || undefined; - const configFile = - hostPath && localConfigFile - ? { - path: localConfigFile, - scheme: "vscode-fileHost", - } - : undefined; - const devContainer = Buffer.from( - JSON.stringify({ - containerName: devContainerName, - hostPath, - configFile, - localDocker: false, - }), - "utf-8", - ).toString("hex"); + const hostPath = localWorkspaceFolder || undefined; + const configFile = + hostPath && localConfigFile + ? { + path: localConfigFile, + scheme: "vscode-fileHost", + } + : undefined; + const devContainer = Buffer.from( + JSON.stringify({ + containerName: devContainerName, + hostPath, + configFile, + localDocker: false, + }), + "utf-8", + ).toString("hex"); - const type = localWorkspaceFolder ? "dev-container" : "attached-container"; - const devContainerAuthority = `${type}+${devContainer}@${remoteAuthority}`; + const type = localWorkspaceFolder + ? "dev-container" + : "attached-container"; + const devContainerAuthority = `${type}+${devContainer}@${remoteAuthority}`; - let newWindow = true; - if (!vscode.workspace.workspaceFolders?.length) { - newWindow = false; - } + let newWindow = true; + if (!vscode.workspace.workspaceFolders?.length) { + newWindow = false; + } - // Only set the memento when opening a new folder - await this.mementoManager.setStartupMode("start"); - await vscode.commands.executeCommand( - "vscode.openFolder", - vscode.Uri.from({ - scheme: "vscode-remote", - authority: devContainerAuthority, - path: devContainerFolder, - }), - newWindow, - ); + // Only set the memento when opening a new folder + await this.mementoManager.setStartupMode("start"); + await vscode.commands.executeCommand( + "vscode.openFolder", + vscode.Uri.from({ + scheme: "vscode-remote", + authority: devContainerAuthority, + path: devContainerFolder, + }), + newWindow, + ); + }); } /** @@ -905,7 +1004,7 @@ export class Commands { public async pingWorkspace(item?: OpenableTreeItem): Promise { const resolved = await this.resolveClientAndWorkspace(item); - if (!resolved) { + if (resolved.status !== "selected") { return; } @@ -927,36 +1026,39 @@ export class Commands { /** * Resolve the API client and workspace identifier from a sidebar item, * the currently connected workspace, or by prompting the user to pick one. - * Returns undefined if the user cancels the picker. + * Returns cancelled/failed if the user cancels the picker or the picker cannot load. */ private async resolveClientAndWorkspace( item?: OpenableTreeItem, - ): Promise<{ client: CoderApi; workspaceId: string } | undefined> { + ): Promise { if (item) { return { + status: "selected", client: this.extensionClient, workspaceId: createWorkspaceIdentifier(item.workspace), }; } if (this.workspace && this.remoteWorkspaceClient) { return { + status: "selected", client: this.remoteWorkspaceClient, workspaceId: createWorkspaceIdentifier(this.workspace), }; } - const workspace = await this.pickWorkspace({ + const pick = await this.pickWorkspace("diagnostic", { title: "Select a running workspace", initialValue: "owner:me status:running ", placeholder: "Search running workspaces...", filter: (w) => w.latest_build.status === "running", }); - if (!workspace) { - return undefined; + if (pick.status === "selected") { + return { + status: "selected", + client: this.extensionClient, + workspaceId: createWorkspaceIdentifier(pick.workspace), + }; } - return { - client: this.extensionClient, - workspaceId: createWorkspaceIdentifier(workspace), - }; + return pick; } /** Resolve a CliEnv, preferring a locally cached binary over a network fetch. */ @@ -991,19 +1093,24 @@ export class Commands { /** * Ask the user to select a workspace. Return undefined if canceled. */ - private async pickWorkspace(options?: { - title?: string; - initialValue?: string; - placeholder?: string; - filter?: (w: Workspace) => boolean; - }): Promise { + private async pickWorkspace( + source: WorkspacePickerSource, + options?: { + title?: string; + initialValue?: string; + placeholder?: string; + filter?: (w: Workspace) => boolean; + }, + ): Promise { const quickPick = vscode.window.createQuickPick(); quickPick.value = options?.initialValue ?? "owner:me "; quickPick.placeholder = options?.placeholder ?? "owner:me template:go"; quickPick.title = options?.title ?? "Connect to a workspace"; const filter = options?.filter; - let lastWorkspaces: readonly Workspace[]; + let lastWorkspaces: readonly Workspace[] = []; + let settled = false; + let fetchFailureCategory: WorkspacePickerFailureCategory | undefined; const disposables: vscode.Disposable[] = []; disposables.push( quickPick.onDidChangeValue((value) => { @@ -1013,6 +1120,7 @@ export class Commands { q: value, }) .then((workspaces) => { + fetchFailureCategory = undefined; const filtered = filter ? workspaces.workspaces.filter(filter) : workspaces.workspaces; @@ -1042,6 +1150,7 @@ export class Commands { } }) .catch((ex) => { + fetchFailureCategory = "fetch_failed"; this.logger.error("Failed to fetch workspaces", ex); if (ex instanceof CertificateError) { void ex.showNotification(); @@ -1058,26 +1167,58 @@ export class Commands { ); quickPick.show(); - return new Promise((resolve) => { - disposables.push( - quickPick.onDidHide(() => { - resolve(undefined); - }), - quickPick.onDidChangeSelection((selected) => { - if (selected.length < 1) { - return resolve(undefined); - } - const workspace = - lastWorkspaces[quickPick.items.indexOf(selected[0])]; - resolve(workspace); - }), - ); - }).finally(() => { - for (const d of disposables) { - d.dispose(); - } - quickPick.dispose(); - }); + return this.traces + .workspacePicker( + source, + async (telemetry) => + new Promise((resolve) => { + const finish = (result: WorkspacePickerResult) => { + if (settled) { + return; + } + settled = true; + if (result.status === "selected") { + telemetry.selected(result.workspace, lastWorkspaces.length); + } else if (result.status === "failed") { + telemetry.failed(result.category, lastWorkspaces.length); + } else { + telemetry.cancelled(lastWorkspaces.length); + } + resolve(result); + }; + disposables.push( + quickPick.onDidHide(() => { + if (fetchFailureCategory) { + finish({ + status: "failed", + category: fetchFailureCategory, + }); + return; + } + finish({ status: "cancelled" }); + }), + quickPick.onDidChangeSelection((selected) => { + if (selected.length < 1) { + finish({ status: "cancelled" }); + return; + } + const workspace = + lastWorkspaces[quickPick.items.indexOf(selected[0])]; + if (!workspace) { + finish({ status: "cancelled" }); + return; + } + finish({ status: "selected", workspace }); + }), + ); + }), + ) + .finally(() => { + for (const d of disposables) { + d.dispose(); + } + quickPick.dispose(); + }); } /** @@ -1120,12 +1261,13 @@ export class Commands { options: Pick< OpenOptions, "folderPath" | "openRecent" | "useDefaultDirectory" - > = {}, + > & { telemetry?: WorkspaceOpenTrace } = {}, ): Promise { const { openRecent, useDefaultDirectory } = { ...openDefaults, ...options, }; + const { telemetry } = options; let { folderPath } = options; const remoteAuthority = toRemoteAuthority( baseUrl, @@ -1169,7 +1311,12 @@ export class Commands { }); if (!folderPath) { // User aborted. - return false; + return ( + telemetry?.cancel("recent_folder_picker", { + workspace, + agent, + }) ?? false + ); } } } @@ -1191,6 +1338,7 @@ export class Commands { }); if (folderPath) { + telemetry?.handoff("folder"); await vscode.commands.executeCommand( "vscode.openFolder", vscode.Uri.from({ @@ -1205,6 +1353,7 @@ export class Commands { } // This opens the workspace without an active folder opened. + telemetry?.handoff("empty_window"); await vscode.commands.executeCommand("vscode.newWindow", { remoteAuthority: remoteAuthority, reuseWindow: !newWindow, diff --git a/src/instrumentation/commands.ts b/src/instrumentation/commands.ts new file mode 100644 index 000000000..b8481e778 --- /dev/null +++ b/src/instrumentation/commands.ts @@ -0,0 +1,486 @@ +import { extractAgents } from "../api/api-helper"; +import { isAbortError } from "../error/errorUtils"; +import { parseSpeedtestResult } from "../webviews/speedtest/types"; + +import type { + Workspace, + WorkspaceAgent, +} from "coder/site/src/api/typesGenerated"; + +import type { SpeedtestResult } from "@repo/shared"; + +import type { ProgressResult } from "../progress"; +import type { TelemetryReporter } from "../telemetry/reporter"; +import type { Span } from "../telemetry/span"; + +export type WorkspaceOpenSource = + | "command" + | "sidebar.agent" + | "sidebar.workspace" + | "sidebar.fallback" + | "uri"; + +export type WorkspacePickerSource = "workspace.open" | "diagnostic"; +export type WorkspacePickerFailureCategory = "fetch_failed"; +export type WorkspaceOpenFailureCategory = + | WorkspacePickerFailureCategory + | "aborted" + | "error"; +export type WorkspacePickerResult = + | { readonly status: "selected"; readonly workspace: Workspace } + | { readonly status: "cancelled" } + | { + readonly status: "failed"; + readonly category: WorkspacePickerFailureCategory; + }; +export type DiagnosticCommandId = + | "coder.speedTest" + | "coder.supportBundle" + | "coder.exportTelemetry"; +export type DiagnosticFailureCategory = + | WorkspacePickerFailureCategory + | "parse_error" + | "unsupported_cli" + | "aborted" + | "error"; +export type DevcontainerMode = "dev_container" | "attached_container"; +export type DevcontainerFailureCategory = "aborted" | "error"; +export type WorkspaceOpenCancelStage = + | "workspace_picker" + | "agent_picker" + | "recent_folder_picker"; +export type DiagnosticCancelStage = + | "workspace_picker" + | "input" + | "prompt" + | "save_dialog" + | "progress"; + +type WorkspaceStateBucket = + | "running" + | "stopped" + | "failed" + | "starting" + | "stopping" + | "pending" + | "deleting" + | "deleted" + | "canceled" + | "canceling" + | "unknown"; + +type AgentStatusBucket = + | "connected" + | "connecting" + | "disconnected" + | "timeout" + | "unknown"; + +type AgentLifecycleBucket = + | "ready" + | "starting" + | "created" + | "start_error" + | "start_timeout" + | "shutting_down" + | "off" + | "shutdown_error" + | "shutdown_timeout" + | "unknown"; + +export interface WorkspaceOpenSelection { + readonly workspace: Workspace; + readonly agent?: WorkspaceAgent; +} + +export interface WorkspacePickerTrace { + selected(workspace: Workspace, resultCount: number): void; + cancelled(resultCount: number): void; + failed(category: WorkspacePickerFailureCategory, resultCount: number): void; +} + +export interface WorkspaceOpenTrace { + select(selection: WorkspaceOpenSelection): void; + cancel( + stage: WorkspaceOpenCancelStage, + selection?: WorkspaceOpenSelection, + ): false; + fail(category: WorkspaceOpenFailureCategory): false; + handoff(kind: "folder" | "empty_window"): void; +} + +export interface DiagnosticTrace { + cancel(stage: DiagnosticCancelStage): void; + fail(error: unknown, category?: DiagnosticFailureCategory): void; + progressResult( + result: ProgressResult, + ): result is { ok: true; value: T }; + speedtestRequestedDuration(seconds: number): void; + speedtestSuccess(rawJson: string): SpeedtestResult; + exportSuccess(format: string, eventCount: number): void; +} + +export interface DevcontainerTrace { + fail(error: unknown): void; +} + +export class CommandInstrumentation { + public constructor(private readonly telemetry: TelemetryReporter) {} + + public diagnostic( + commandId: DiagnosticCommandId, + fn: (trace: DiagnosticTrace) => Promise, + ): Promise { + return this.telemetry.trace( + "command.diagnostic.completed", + (span) => fn(new SpanDiagnosticTrace(span)), + { commandId }, + ); + } + + public async workspaceOpen( + source: WorkspaceOpenSource, + selection: WorkspaceOpenSelection | undefined, + fn: (trace: WorkspaceOpenTrace) => Promise, + ): Promise { + let deferredError: unknown; + let failed = false; + const opened = await this.telemetry.trace( + "workspace.open", + async (span) => { + const trace = new SpanWorkspaceOpenTrace(span); + if (selection) { + trace.select(selection); + } + try { + const result = await fn(trace); + if (!result) { + span.markAborted(); + } + return result; + } catch (error) { + failed = true; + deferredError = error; + trace.fail(categorizeWorkspaceOpenFailure(error)); + return false; + } + }, + { source }, + ); + if (failed) { + throw deferredError; + } + return opened; + } + + public workspacePicker( + source: WorkspacePickerSource, + fn: (trace: WorkspacePickerTrace) => Promise, + ): Promise { + return this.telemetry.trace( + "workspace.picker.prompted", + (span) => fn(new SpanWorkspacePickerTrace(span)), + { source }, + ); + } + + public async devcontainerOpen( + mode: DevcontainerMode, + fn: (trace: DevcontainerTrace) => Promise, + ): Promise { + let deferredError: unknown; + let failed = false; + await this.telemetry.trace( + "workspace.devcontainer.open", + async (span) => { + const trace = new SpanDevcontainerTrace(span); + try { + await fn(trace); + } catch (error) { + failed = true; + deferredError = error; + trace.fail(error); + } + }, + { mode }, + ); + if (failed) { + throw deferredError; + } + } +} + +class SpanWorkspacePickerTrace implements WorkspacePickerTrace { + public constructor(private readonly span: Span) {} + + public selected(workspace: Workspace, resultCount: number): void { + setWorkspacePickerResult(this.span, workspace, resultCount); + } + + public cancelled(resultCount: number): void { + setWorkspacePickerResult(this.span, undefined, resultCount); + } + + public failed( + category: WorkspacePickerFailureCategory, + resultCount: number, + ): void { + setWorkspacePickerFailure(this.span, category, resultCount); + } +} + +class SpanWorkspaceOpenTrace implements WorkspaceOpenTrace { + public constructor(private readonly span: Span) {} + + public select(selection: WorkspaceOpenSelection): void { + setWorkspaceOpenSelection(this.span, selection); + } + + public cancel( + stage: WorkspaceOpenCancelStage, + selection?: WorkspaceOpenSelection, + ): false { + return markWorkspaceOpenCancelled(this.span, stage, selection); + } + + public fail(category: WorkspaceOpenFailureCategory): false { + return markWorkspaceOpenFailure(this.span, category); + } + + public handoff(kind: "folder" | "empty_window"): void { + this.span.setProperty("handoff", kind); + } +} + +class SpanDiagnosticTrace implements DiagnosticTrace { + public constructor(private readonly span: Span) {} + + public cancel(stage: DiagnosticCancelStage): void { + markDiagnosticCancelled(this.span, stage); + } + + public fail( + error: unknown, + category: DiagnosticFailureCategory = categorizeFailure(error), + ): void { + markDiagnosticFailure(this.span, error, category); + } + + public progressResult( + result: ProgressResult, + ): result is { ok: true; value: T } { + return setDiagnosticProgressResult(this.span, result); + } + + public speedtestRequestedDuration(seconds: number): void { + this.span.setMeasurement("requestedDurationSec", seconds); + } + + public speedtestSuccess(rawJson: string): SpeedtestResult { + return setSpeedtestSuccess(this.span, rawJson); + } + + public exportSuccess(format: string, eventCount: number): void { + this.span.setProperty("format", format); + this.span.setMeasurement("eventCount", eventCount); + } +} + +class SpanDevcontainerTrace implements DevcontainerTrace { + public constructor(private readonly span: Span) {} + + public fail(error: unknown): void { + this.span.setProperty( + "failure.category", + categorizeDevcontainerFailure(error), + ); + this.span.markFailure(); + } +} + +function setWorkspaceProperties( + span: Span, + workspace: Workspace, + agent?: WorkspaceAgent, +): void { + const agents = extractAgents(workspace.latest_build.resources); + span.setProperty("workspace.status", bucketWorkspaceStatus(workspace)); + span.setProperty("workspace.outdated", workspace.outdated); + span.setMeasurement("agentCount", agents.length); + span.setMeasurement( + "connectedAgentCount", + agents.filter((candidate) => candidate.status === "connected").length, + ); + if (!agent) { + return; + } + span.setProperty("agent.status", bucketAgentStatus(agent)); + span.setProperty("agent.lifecycle_state", bucketAgentLifecycle(agent)); +} + +function setWorkspacePickerResult( + span: Span, + workspace: Workspace | undefined, + resultCount: number, +): void { + span.setMeasurement("workspaceCount", resultCount); + if (!workspace) { + span.markAborted(); + return; + } + setWorkspaceProperties(span, workspace); +} + +function setWorkspacePickerFailure( + span: Span, + category: WorkspacePickerFailureCategory, + resultCount: number, +): void { + span.setMeasurement("workspaceCount", resultCount); + span.setProperty("failure.category", category); + span.markFailure(); +} + +function setWorkspaceOpenSelection( + span: Span, + selection: WorkspaceOpenSelection, +): void { + setWorkspaceProperties(span, selection.workspace, selection.agent); +} + +function markWorkspaceOpenCancelled( + span: Span, + stage: WorkspaceOpenCancelStage, + selection?: WorkspaceOpenSelection, +): false { + span.setProperty("cancel.stage", stage); + if (selection) { + setWorkspaceOpenSelection(span, selection); + } + span.markAborted(); + return false; +} + +function markWorkspaceOpenFailure( + span: Span, + category: WorkspaceOpenFailureCategory, + selection?: WorkspaceOpenSelection, +): false { + span.setProperty("failure.category", category); + if (selection) { + setWorkspaceOpenSelection(span, selection); + } + span.markFailure(); + return false; +} + +function setDiagnosticProgressResult( + span: Span, + result: ProgressResult, +): result is { ok: true; value: T } { + if (result.ok) { + return true; + } + if (result.cancelled) { + markDiagnosticCancelled(span, "progress"); + } else { + markDiagnosticFailure(span, result.error); + } + return false; +} + +function markDiagnosticCancelled( + span: Span, + stage: DiagnosticCancelStage, +): void { + span.setProperty("cancel.stage", stage); + span.markAborted(); +} + +function markDiagnosticFailure( + span: Span, + error: unknown, + category: DiagnosticFailureCategory = categorizeFailure(error), +): void { + span.setProperty("failure.category", category); + span.markFailure(); +} + +function setSpeedtestSuccess(span: Span, rawJson: string): SpeedtestResult { + const parsed = parseSpeedtestResult(rawJson); + span.setMeasurement("intervalCount", parsed.intervals.length); + span.setMeasurement("throughputMbits", parsed.overall.throughput_mbits); + return parsed; +} + +function categorizeFailure(error: unknown): DiagnosticFailureCategory { + if (isAbortError(error)) { + return "aborted"; + } + return "error"; +} + +function categorizeWorkspaceOpenFailure( + error: unknown, +): WorkspaceOpenFailureCategory { + if (isAbortError(error)) { + return "aborted"; + } + return "error"; +} + +function bucketWorkspaceStatus(workspace: Workspace): WorkspaceStateBucket { + switch (workspace.latest_build.status) { + case "running": + case "stopped": + case "failed": + case "starting": + case "stopping": + case "pending": + case "deleting": + case "deleted": + case "canceled": + case "canceling": + return workspace.latest_build.status; + default: + return "unknown"; + } +} + +function bucketAgentStatus(agent: WorkspaceAgent): AgentStatusBucket { + switch (agent.status) { + case "connected": + case "connecting": + case "disconnected": + case "timeout": + return agent.status; + default: + return "unknown"; + } +} + +function bucketAgentLifecycle(agent: WorkspaceAgent): AgentLifecycleBucket { + switch (agent.lifecycle_state) { + case "ready": + case "starting": + case "created": + case "start_error": + case "start_timeout": + case "shutting_down": + case "off": + case "shutdown_error": + case "shutdown_timeout": + return agent.lifecycle_state; + default: + return "unknown"; + } +} + +function categorizeDevcontainerFailure( + error: unknown, +): DevcontainerFailureCategory { + if (isAbortError(error)) { + return "aborted"; + } + return "error"; +} diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index 010deec5c..515db0df4 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -24,6 +24,8 @@ const PROVISIONING_STATUSES: ReadonlySet = new Set([ "deleting", ]); +export type WorkspacePromptAction = "start" | "update"; + interface ObservedWorkspaceState { readonly status: WorkspaceStatus; readonly buildTransition: WorkspaceBuild["transition"]; @@ -174,6 +176,25 @@ export class WorkspaceOperationTelemetry { }); } + public async traceStartPrompted( + outdated: boolean, + fn: () => Promise, + ): Promise { + return this.telemetry.trace( + "workspace.start.prompted", + async (span) => { + const action = await fn(); + if (!action) { + span.markAborted(); + return undefined; + } + span.setProperty("action", action); + return action; + }, + { "update.offered": outdated }, + ); + } + /** * Records dismissal as `result: "aborted"`. The framework treats any throw * as `result: "error"`, so we return inside the span and rethrow outside. @@ -196,7 +217,7 @@ export class WorkspaceOperationTelemetry { throw error; } }, - { workspaceName: this.workspaceName }, + { prompt: "parameters" }, ); if (cancel) throw cancel; return parameters; diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index 4ce44e0fa..8cd7217f9 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -337,18 +337,20 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspaceName: string, outdated: boolean, ): Promise<"start" | "update" | undefined> { - const buttons = outdated ? ["Start", "Update and Start"] : ["Start"]; - const action = await vscodeProposed.window.showInformationMessage( - `The workspace ${workspaceName} is not running. How would you like to proceed?`, - { - useCustom: true, - modal: true, - }, - ...buttons, - ); - if (action === "Start") return "start"; - if (action === "Update and Start") return "update"; - return undefined; + return this.operationTelemetry.traceStartPrompted(outdated, async () => { + const buttons = outdated ? ["Start", "Update and Start"] : ["Start"]; + const action = await vscodeProposed.window.showInformationMessage( + `The workspace ${workspaceName} is not running. How would you like to proceed?`, + { + useCustom: true, + modal: true, + }, + ...buttons, + ); + if (action === "Start") return "start"; + if (action === "Update and Start") return "update"; + return undefined; + }); } public getAgentId(): string | undefined { diff --git a/src/telemetry/export/command.ts b/src/telemetry/export/command.ts index 1186e1bb9..ef0575858 100644 --- a/src/telemetry/export/command.ts +++ b/src/telemetry/export/command.ts @@ -19,6 +19,8 @@ import type { Logger } from "../../logging/logger"; import type { TelemetryContext } from "../event"; import type { FlushStatus } from "../service"; +import type { ExportFormat } from "./writers/types"; + const REVEAL_ACTION = "Reveal in File Explorer"; const PROGRESS_OPTIONS = { @@ -27,15 +29,24 @@ const PROGRESS_OPTIONS = { cancellable: true, } as const; +export type ExportTelemetryOutcome = + | { readonly status: "cancelled"; readonly stage: "prompt" | "progress" } + | { readonly status: "failed"; readonly error: unknown } + | { + readonly status: "success"; + readonly eventCount: number; + readonly format: ExportFormat; + }; + export async function runExportTelemetryCommand( telemetryDir: string, logger: Logger, flushTelemetry: () => Promise, context: TelemetryContext, -): Promise { +): Promise { const choice = await promptForExport(); if (!choice) { - return; + return { status: "cancelled", stage: "prompt" }; } const request: ExportRequest = { @@ -53,7 +64,7 @@ export async function runExportTelemetryCommand( PROGRESS_OPTIONS, ); - await reportOutcome(result, choice, logger); + return reportOutcome(result, choice, logger); } /** Wires the pipeline's host hooks to the progress UI and the logger. */ @@ -81,16 +92,16 @@ async function reportOutcome( result: ProgressResult, choice: ExportChoice, logger: Logger, -): Promise { +): Promise { if (!result.ok) { if (result.cancelled) { - return; + return { status: "cancelled", stage: "progress" }; } logger.error("Telemetry export failed", result.error); void vscode.window.showErrorMessage( `Telemetry export failed: ${toError(result.error).message}`, ); - return; + return { status: "failed", error: result.error }; } const eventCount = result.value; @@ -98,9 +109,10 @@ async function reportOutcome( void vscode.window.showInformationMessage( `No telemetry events found for ${choice.range.label}.`, ); - return; + return { status: "success", eventCount, format: choice.format }; } await notifyExportSucceeded(choice.outputPath, eventCount, logger); + return { status: "success", eventCount, format: choice.format }; } async function notifyExportSucceeded( diff --git a/src/uri/uriHandler.ts b/src/uri/uriHandler.ts index 861eeff24..d6772cda8 100644 --- a/src/uri/uriHandler.ts +++ b/src/uri/uriHandler.ts @@ -86,6 +86,7 @@ async function handleOpen(ctx: UriRouteContext): Promise { agentName: agent ?? undefined, folderPath: folder ?? undefined, openRecent, + source: "uri", useDefaultDirectory: false, }); } diff --git a/test/unit/instrumentation/commands.test.ts b/test/unit/instrumentation/commands.test.ts new file mode 100644 index 000000000..96d78b635 --- /dev/null +++ b/test/unit/instrumentation/commands.test.ts @@ -0,0 +1,208 @@ +import { describe, expect, it } from "vitest"; + +import { CommandInstrumentation } from "@/instrumentation/commands"; + +import { agent, resource, workspace } from "@repo/mocks"; + +import { createTelemetryHarness } from "../../mocks/telemetry"; + +function workspaceWithAgents() { + const connected = agent({ + status: "connected", + lifecycle_state: "ready", + }); + const disconnected = agent({ + id: "agent-2", + name: "secondary", + status: "disconnected", + lifecycle_state: "off", + }); + return { + connected, + disconnected, + workspace: workspace({ + outdated: true, + latest_build: { + status: "running", + resources: [resource({ agents: [connected, disconnected] })], + }, + }), + }; +} + +describe("command instrumentation helpers", () => { + it("records workspace selection without workspace or agent names", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + const selection = workspaceWithAgents(); + + await traces.workspaceOpen( + "command", + { workspace: selection.workspace, agent: selection.connected }, + () => Promise.resolve(true), + ); + + const event = sink.expectOne("workspace.open"); + expect(event.properties).toMatchObject({ + "agent.lifecycle_state": "ready", + "agent.status": "connected", + "workspace.outdated": "true", + "workspace.status": "running", + result: "success", + }); + expect(event.measurements).toMatchObject({ + agentCount: 2, + connectedAgentCount: 1, + }); + expect(event.properties.workspaceName).toBeUndefined(); + expect(event.properties.agentName).toBeUndefined(); + }); + + it("records workspace picker cancellation and failure distinctly", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + + await traces.workspacePicker("workspace.open", (telemetry) => { + telemetry.cancelled(3); + return Promise.resolve({ status: "cancelled" }); + }); + await traces.workspacePicker("workspace.open", (telemetry) => { + telemetry.failed("fetch_failed", 0); + return Promise.resolve({ status: "failed", category: "fetch_failed" }); + }); + + const [cancelled, failed] = sink.eventsNamed("workspace.picker.prompted"); + expect(cancelled.properties).toMatchObject({ result: "aborted" }); + expect(cancelled.measurements.workspaceCount).toBe(3); + expect(failed.properties).toMatchObject({ + "failure.category": "fetch_failed", + result: "error", + }); + expect(failed.measurements.workspaceCount).toBe(0); + }); + + it("records workspace open cancellation and handled failure distinctly", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + const selection = workspaceWithAgents(); + + await traces.workspaceOpen("command", undefined, (telemetry) => + Promise.resolve( + telemetry.cancel("agent_picker", { workspace: selection.workspace }), + ), + ); + await traces.workspaceOpen("command", undefined, (telemetry) => + Promise.resolve(telemetry.fail("fetch_failed")), + ); + + const [cancelled, failed] = sink.eventsNamed("workspace.open"); + expect(cancelled.properties).toMatchObject({ + "cancel.stage": "agent_picker", + "workspace.status": "running", + result: "aborted", + }); + expect(failed.properties).toMatchObject({ + "failure.category": "fetch_failed", + result: "error", + }); + }); + + it("records thrown workspace open failures without raw error details", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + + await expect( + traces.workspaceOpen("command", undefined, () => + Promise.reject(new Error("secret path /tmp/workspace")), + ), + ).rejects.toThrow("secret path /tmp/workspace"); + + const event = sink.expectOne("workspace.open"); + expect(event.properties).toMatchObject({ + "failure.category": "error", + result: "error", + }); + expect(event.error).toBeUndefined(); + }); + + it("records diagnostic cancellation and failure categories", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + const failure = new Error("boom"); + + await traces.diagnostic("coder.supportBundle", (telemetry) => { + telemetry.cancel("save_dialog"); + return Promise.resolve(); + }); + await traces.diagnostic("coder.supportBundle", (telemetry) => { + telemetry.fail(failure, "unsupported_cli"); + return Promise.resolve(); + }); + + const [cancelled, failed] = sink.eventsNamed( + "command.diagnostic.completed", + ); + expect(cancelled.properties).toMatchObject({ + "cancel.stage": "save_dialog", + result: "aborted", + }); + expect(failed.properties).toMatchObject({ + "failure.category": "unsupported_cli", + result: "error", + }); + expect(failed.error).toBeUndefined(); + }); + + it("records bounded speed test measurements", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + + await traces.diagnostic("coder.speedTest", (telemetry) => { + const parsed = telemetry.speedtestSuccess( + JSON.stringify({ + overall: { + start_time_seconds: 0, + end_time_seconds: 5, + throughput_mbits: 42, + }, + intervals: [ + { + start_time_seconds: 0, + end_time_seconds: 5, + throughput_mbits: 42, + }, + ], + }), + ); + expect(parsed.overall.throughput_mbits).toBe(42); + return Promise.resolve(); + }); + + expect(sink.expectOne("command.diagnostic.completed")).toMatchObject({ + measurements: { + intervalCount: 1, + throughputMbits: 42, + }, + properties: { result: "success" }, + }); + }); + + it("records thrown devcontainer failures without raw error details", async () => { + const { sink, service } = createTelemetryHarness(); + const traces = new CommandInstrumentation(service); + + await expect( + traces.devcontainerOpen("dev_container", () => + Promise.reject(new Error("secret local path /tmp/workspace")), + ), + ).rejects.toThrow("secret local path /tmp/workspace"); + + const event = sink.expectOne("workspace.devcontainer.open"); + expect(event.properties).toMatchObject({ + "failure.category": "error", + mode: "dev_container", + result: "error", + }); + expect(event.error).toBeUndefined(); + }); +}); diff --git a/test/unit/instrumentation/workspace.test.ts b/test/unit/instrumentation/workspace.test.ts index f219fa1c8..674e66353 100644 --- a/test/unit/instrumentation/workspace.test.ts +++ b/test/unit/instrumentation/workspace.test.ts @@ -70,6 +70,41 @@ describe("WorkspaceOperationTelemetry", () => { }); }); + describe("traceStartPrompted", () => { + it("emits result=success with accepted action", async () => { + const { sink, instance: ops } = setup(newOps); + + const result = await ops.traceStartPrompted(true, () => + Promise.resolve("update"), + ); + + expect(result).toBe("update"); + expect(sink.expectOne("workspace.start.prompted")).toMatchObject({ + properties: { + "update.offered": "true", + action: "update", + result: "success", + }, + }); + }); + + it("emits result=aborted when dismissed", async () => { + const { sink, instance: ops } = setup(newOps); + + const result = await ops.traceStartPrompted(false, () => + Promise.resolve(undefined), + ); + + expect(result).toBeUndefined(); + expect(sink.expectOne("workspace.start.prompted")).toMatchObject({ + properties: { + "update.offered": "false", + result: "aborted", + }, + }); + }); + }); + describe("traceUpdatePrompted", () => { it("returns the collected parameters and emits result=success", async () => { const { sink, instance: ops } = setup(newOps); @@ -80,9 +115,12 @@ describe("WorkspaceOperationTelemetry", () => { ); expect(result).toEqual(collected); - expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ - properties: { workspaceName: WORKSPACE_NAME, result: "success" }, + const event = sink.expectOne("workspace.update.prompted"); + expect(event.properties).toMatchObject({ + prompt: "parameters", + result: "success", }); + expect(event.properties.workspaceName).toBeUndefined(); }); it("emits result=aborted (no error block) and rethrows on cancellation", async () => { diff --git a/test/unit/telemetry/export/command.test.ts b/test/unit/telemetry/export/command.test.ts index c660fb22e..922a9fcf5 100644 --- a/test/unit/telemetry/export/command.test.ts +++ b/test/unit/telemetry/export/command.test.ts @@ -72,7 +72,10 @@ describe("runExportTelemetryCommand", () => { const { run } = setup(); vi.mocked(promptForExport).mockResolvedValue(undefined); - await run(); + await expect(run()).resolves.toEqual({ + status: "cancelled", + stage: "prompt", + }); expect(collectTelemetryExport).not.toHaveBeenCalled(); expect(vscode.window.withProgress).not.toHaveBeenCalled(); @@ -121,7 +124,11 @@ describe("runExportTelemetryCommand", () => { ])("notifies with a pluralized %i-event count", async (count, message) => { const { run } = setup({ outcome: { count } }); - await run(); + await expect(run()).resolves.toEqual({ + status: "success", + eventCount: count, + format: "json", + }); expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( `${message} ${OUTPUT_PATH}.`, @@ -132,7 +139,10 @@ describe("runExportTelemetryCommand", () => { it("reveals the file when the user clicks the action", async () => { const { run } = setup({ revealChoice: REVEAL_ACTION }); - await run(); + await expect(run()).resolves.toMatchObject({ + status: "success", + eventCount: 2, + }); expect(vscode.commands.executeCommand).toHaveBeenCalledWith( "revealFileInOS", @@ -154,7 +164,10 @@ describe("runExportTelemetryCommand", () => { new Error("no command"), ); - await expect(run()).resolves.toBeUndefined(); + await expect(run()).resolves.toMatchObject({ + status: "success", + eventCount: 2, + }); expect(vscode.window.showErrorMessage).not.toHaveBeenCalled(); }); @@ -164,7 +177,11 @@ describe("runExportTelemetryCommand", () => { it("reports that no events were found", async () => { const { run } = setup({ outcome: { count: 0 } }); - await run(); + await expect(run()).resolves.toEqual({ + status: "success", + eventCount: 0, + format: "json", + }); expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( "No telemetry events found for Last 24 hours.", @@ -174,9 +191,10 @@ describe("runExportTelemetryCommand", () => { describe("failure", () => { it("shows an error notification without throwing", async () => { - const { run } = setup({ outcome: { error: new Error("disk full") } }); + const error = new Error("disk full"); + const { run } = setup({ outcome: { error } }); - await expect(run()).resolves.toBeUndefined(); + await expect(run()).resolves.toEqual({ status: "failed", error }); expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( "Telemetry export failed: disk full", @@ -190,7 +208,10 @@ describe("runExportTelemetryCommand", () => { }); const { run } = setup({ outcome: { error: aborted } }); - await run(); + await expect(run()).resolves.toEqual({ + status: "cancelled", + stage: "progress", + }); expect(vscode.window.showErrorMessage).not.toHaveBeenCalled(); expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); diff --git a/test/unit/uri/uriHandler.test.ts b/test/unit/uri/uriHandler.test.ts index 4c87ae72f..9c86547db 100644 --- a/test/unit/uri/uriHandler.test.ts +++ b/test/unit/uri/uriHandler.test.ts @@ -142,6 +142,7 @@ describe("uriHandler", () => { agentName: "a", folderPath: "/f", openRecent: true, + source: "uri", useDefaultDirectory: false, }); }); @@ -161,6 +162,7 @@ describe("uriHandler", () => { agentName: undefined, folderPath: undefined, openRecent: expected, + source: "uri", useDefaultDirectory: false, }); }); @@ -178,6 +180,7 @@ describe("uriHandler", () => { agentName: undefined, folderPath: undefined, openRecent: false, + source: "uri", useDefaultDirectory: false, }); expect(showErrorMessage).not.toHaveBeenCalled(); @@ -194,6 +197,7 @@ describe("uriHandler", () => { agentName: undefined, folderPath: undefined, openRecent: false, + source: "uri", useDefaultDirectory: false, }); expect(showErrorMessage).not.toHaveBeenCalled(); From a54b6a9f952546855751319690a5b785e9eac552 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 9 Jun 2026 09:37:05 +0000 Subject: [PATCH 2/6] refactor: align telemetry naming conventions --- src/commands.ts | 281 ++++++++++---------- src/instrumentation/commands.ts | 60 ++--- src/instrumentation/workspace.ts | 2 +- test/unit/instrumentation/commands.test.ts | 64 ++--- test/unit/instrumentation/workspace.test.ts | 4 +- 5 files changed, 212 insertions(+), 199 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 816e7f8b0..173c26d60 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -20,7 +20,7 @@ import { type AuthLogoutOutcome, } from "./instrumentation/auth"; import { - CommandInstrumentation, + CommandTelemetry, type DevcontainerMode, type WorkspaceOpenSource, type WorkspaceOpenTrace, @@ -115,7 +115,7 @@ export class Commands { private readonly speedtestPanelFactory: SpeedtestPanelFactory; private readonly telemetryService: TelemetryService; private readonly authTelemetry: AuthTelemetry; - private readonly traces: CommandInstrumentation; + private readonly commandTelemetry: CommandTelemetry; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -134,7 +134,7 @@ export class Commands { private readonly deploymentManager: DeploymentManager, ) { this.telemetryService = serviceContainer.getTelemetryService(); - this.traces = new CommandInstrumentation(this.telemetryService); + this.commandTelemetry = new CommandTelemetry(this.telemetryService); this.logger = serviceContainer.getLogger(); this.pathResolver = serviceContainer.getPathResolver(); this.mementoManager = serviceContainer.getMementoManager(); @@ -246,7 +246,7 @@ export class Commands { * editor document. Can be triggered from the sidebar or command palette. */ public async speedTest(item?: OpenableTreeItem): Promise { - await this.traces.diagnostic("coder.speedTest", async (telemetry) => { + await this.commandTelemetry.diagnostic("speed_test", async (telemetry) => { const resolved = await this.resolveClientAndWorkspace(item); if (resolved.status === "cancelled") { telemetry.cancel("workspace_picker"); @@ -345,87 +345,92 @@ export class Commands { } public async supportBundle(item?: OpenableTreeItem): Promise { - await this.traces.diagnostic("coder.supportBundle", async (telemetry) => { - const resolved = await this.resolveClientAndWorkspace(item); - if (resolved.status === "cancelled") { - telemetry.cancel("workspace_picker"); - return; - } - if (resolved.status === "failed") { - telemetry.fail(undefined, resolved.category); - return; - } + await this.commandTelemetry.diagnostic( + "support_bundle", + async (telemetry) => { + const resolved = await this.resolveClientAndWorkspace(item); + if (resolved.status === "cancelled") { + telemetry.cancel("workspace_picker"); + return; + } + if (resolved.status === "failed") { + telemetry.fail(undefined, resolved.category); + return; + } - const { client, workspaceId } = resolved; + const { client, workspaceId } = resolved; - const outputUri = await this.promptSupportBundlePath(); - if (!outputUri) { - telemetry.cancel("save_dialog"); - return; - } + const outputUri = await this.promptSupportBundlePath(); + if (!outputUri) { + telemetry.cancel("save_dialog"); + return; + } - const result = await withCancellableProgress( - async ({ signal, progress }) => { - progress.report({ message: "Resolving CLI..." }); - const env = await this.resolveCliEnv(client); - if (!env.featureSet.supportBundle) { - throw new Error( - "Support bundles require Coder CLI v2.10.0 or later. Please update your Coder deployment.", - ); - } + const result = await withCancellableProgress( + async ({ signal, progress }) => { + progress.report({ message: "Resolving CLI..." }); + const env = await this.resolveCliEnv(client); + if (!env.featureSet.supportBundle) { + throw new Error( + "Support bundles require Coder CLI v2.10.0 or later. Please update your Coder deployment.", + ); + } - progress.report({ message: "Collecting diagnostics..." }); - await cliExec.supportBundle( - env, - workspaceId, - outputUri.fsPath, - signal, - ); + progress.report({ message: "Collecting diagnostics..." }); + await cliExec.supportBundle( + env, + workspaceId, + outputUri.fsPath, + signal, + ); - progress.report({ message: "Adding VS Code logs..." }); - await appendVsCodeLogs( - outputUri.fsPath, - { - activeProxyLogPath: this.workspaceLogPath, - proxyLogDir: this.pathResolver.getProxyLogPath(), - extensionLogDir: this.pathResolver.getCodeLogDir(), - telemetryDir: this.pathResolver.getTelemetryPath(), - }, - this.logger, - ); + progress.report({ message: "Adding VS Code logs..." }); + await appendVsCodeLogs( + outputUri.fsPath, + { + activeProxyLogPath: this.workspaceLogPath, + proxyLogDir: this.pathResolver.getProxyLogPath(), + extensionLogDir: this.pathResolver.getCodeLogDir(), + telemetryDir: this.pathResolver.getTelemetryPath(), + }, + this.logger, + ); - return outputUri; - }, - { - location: vscode.ProgressLocation.Notification, - title: `Creating support bundle for ${workspaceId}`, - cancellable: true, - }, - ); + return outputUri; + }, + { + location: vscode.ProgressLocation.Notification, + title: `Creating support bundle for ${workspaceId}`, + cancellable: true, + }, + ); - if (!telemetry.progressResult(result)) { - if (!result.cancelled) { - if ( - toError(result.error).message.startsWith("Support bundles require") - ) { - telemetry.fail(result.error, "unsupported_cli"); + if (!telemetry.progressResult(result)) { + if (!result.cancelled) { + if ( + toError(result.error).message.startsWith( + "Support bundles require", + ) + ) { + telemetry.fail(result.error, "unsupported_cli"); + } + this.logger.error("Support bundle failed", result.error); + vscode.window.showErrorMessage( + `Support bundle failed: ${toError(result.error).message}`, + ); } - this.logger.error("Support bundle failed", result.error); - vscode.window.showErrorMessage( - `Support bundle failed: ${toError(result.error).message}`, - ); + return; } - return; - } - const action = await vscode.window.showInformationMessage( - `Support bundle saved to ${result.value.fsPath}`, - "Reveal in File Explorer", - ); - if (action === "Reveal in File Explorer") { - await vscode.commands.executeCommand("revealFileInOS", result.value); - } - }); + const action = await vscode.window.showInformationMessage( + `Support bundle saved to ${result.value.fsPath}`, + "Reveal in File Explorer", + ); + if (action === "Reveal in File Explorer") { + await vscode.commands.executeCommand("revealFileInOS", result.value); + } + }, + ); } private promptSupportBundlePath(): Thenable { @@ -438,25 +443,28 @@ export class Commands { } public async exportTelemetry(): Promise { - await this.traces.diagnostic("coder.exportTelemetry", async (telemetry) => { - const outcome = await runExportTelemetryCommand( - this.pathResolver.getTelemetryPath(), - this.logger, - () => this.telemetryService.flush(), - this.telemetryService.getContext(), - ); - switch (outcome.status) { - case "cancelled": - telemetry.cancel(outcome.stage); - return; - case "failed": - telemetry.fail(outcome.error); - return; - case "success": - telemetry.exportSuccess(outcome.format, outcome.eventCount); - return; - } - }); + await this.commandTelemetry.diagnostic( + "export_telemetry", + async (telemetry) => { + const outcome = await runExportTelemetryCommand( + this.pathResolver.getTelemetryPath(), + this.logger, + () => this.telemetryService.flush(), + this.telemetryService.getContext(), + ); + switch (outcome.status) { + case "cancelled": + telemetry.cancel(outcome.stage); + return; + case "failed": + telemetry.fail(outcome.error); + return; + case "success": + telemetry.exportSuccess(outcome.format, outcome.eventCount); + return; + } + }, + ); } /** @@ -756,8 +764,8 @@ export class Commands { throw new Error("You are not logged in"); } if (item instanceof AgentTreeItem) { - await this.traces.workspaceOpen( - "sidebar.agent", + await this.commandTelemetry.workspaceOpen( + "sidebar_agent", { workspace: item.workspace, agent: item.agent }, (telemetry) => { return this.openWorkspace(baseUrl, item.workspace, item.agent, { @@ -767,8 +775,8 @@ export class Commands { }, ); } else if (item instanceof WorkspaceTreeItem) { - await this.traces.workspaceOpen( - "sidebar.workspace", + await this.commandTelemetry.workspaceOpen( + "sidebar_workspace", { workspace: item.workspace }, async (telemetry) => { const agents = await this.extractAgentsWithFallback(item.workspace); @@ -794,7 +802,7 @@ export class Commands { } else { // If there is no tree item, then the user manually ran this command. // Default to the regular open instead. - await this.open({ source: "sidebar.fallback" }); + await this.open({ source: "sidebar_fallback" }); } } @@ -845,43 +853,48 @@ export class Commands { useDefaultDirectory, } = { ...openDefaults, ...options }; - return this.traces.workspaceOpen(source, undefined, async (telemetry) => { - const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; - if (!baseUrl) { - throw new Error("You are not logged in"); - } - - let workspace: Workspace; - if (workspaceOwner && workspaceName) { - workspace = await this.extensionClient.getWorkspaceByOwnerAndName( - workspaceOwner, - workspaceName, - ); - } else { - const pick = await this.pickWorkspace("workspace.open"); - if (pick.status === "cancelled") { - return telemetry.cancel("workspace_picker"); + return this.commandTelemetry.workspaceOpen( + source, + undefined, + async (telemetry) => { + const baseUrl = + this.extensionClient.getAxiosInstance().defaults.baseURL; + if (!baseUrl) { + throw new Error("You are not logged in"); } - if (pick.status === "failed") { - return telemetry.fail(pick.category); + + let workspace: Workspace; + if (workspaceOwner && workspaceName) { + workspace = await this.extensionClient.getWorkspaceByOwnerAndName( + workspaceOwner, + workspaceName, + ); + } else { + const pick = await this.pickWorkspace("workspace_open"); + if (pick.status === "cancelled") { + return telemetry.cancel("workspace_picker"); + } + if (pick.status === "failed") { + return telemetry.fail(pick.category); + } + workspace = pick.workspace; } - workspace = pick.workspace; - } - const agents = await this.extractAgentsWithFallback(workspace); - const agent = await maybeAskAgent(agents, agentName); - if (!agent) { - return telemetry.cancel("agent_picker", { workspace }); - } - telemetry.select({ workspace, agent }); + const agents = await this.extractAgentsWithFallback(workspace); + const agent = await maybeAskAgent(agents, agentName); + if (!agent) { + return telemetry.cancel("agent_picker", { workspace }); + } + telemetry.select({ workspace, agent }); - return this.openWorkspace(baseUrl, workspace, agent, { - folderPath, - openRecent, - useDefaultDirectory, - telemetry, - }); - }); + return this.openWorkspace(baseUrl, workspace, agent, { + folderPath, + openRecent, + useDefaultDirectory, + telemetry, + }); + }, + ); } /** @@ -901,7 +914,7 @@ export class Commands { const mode: DevcontainerMode = localWorkspaceFolder ? "dev_container" : "attached_container"; - await this.traces.devcontainerOpen(mode, async () => { + await this.commandTelemetry.devcontainerOpen(mode, async () => { const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; if (!baseUrl) { throw new Error("You are not logged in"); @@ -1167,7 +1180,7 @@ export class Commands { ); quickPick.show(); - return this.traces + return this.commandTelemetry .workspacePicker( source, async (telemetry) => diff --git a/src/instrumentation/commands.ts b/src/instrumentation/commands.ts index b8481e778..3eb158b0b 100644 --- a/src/instrumentation/commands.ts +++ b/src/instrumentation/commands.ts @@ -15,12 +15,12 @@ import type { Span } from "../telemetry/span"; export type WorkspaceOpenSource = | "command" - | "sidebar.agent" - | "sidebar.workspace" - | "sidebar.fallback" + | "sidebar_agent" + | "sidebar_workspace" + | "sidebar_fallback" | "uri"; -export type WorkspacePickerSource = "workspace.open" | "diagnostic"; +export type WorkspacePickerSource = "workspace_open" | "diagnostic"; export type WorkspacePickerFailureCategory = "fetch_failed"; export type WorkspaceOpenFailureCategory = | WorkspacePickerFailureCategory @@ -33,10 +33,10 @@ export type WorkspacePickerResult = readonly status: "failed"; readonly category: WorkspacePickerFailureCategory; }; -export type DiagnosticCommandId = - | "coder.speedTest" - | "coder.supportBundle" - | "coder.exportTelemetry"; +export type DiagnosticCommand = + | "speed_test" + | "support_bundle" + | "export_telemetry"; export type DiagnosticFailureCategory = | WorkspacePickerFailureCategory | "parse_error" @@ -124,17 +124,17 @@ export interface DevcontainerTrace { fail(error: unknown): void; } -export class CommandInstrumentation { +export class CommandTelemetry { public constructor(private readonly telemetry: TelemetryReporter) {} public diagnostic( - commandId: DiagnosticCommandId, + command: DiagnosticCommand, fn: (trace: DiagnosticTrace) => Promise, ): Promise { return this.telemetry.trace( "command.diagnostic.completed", (span) => fn(new SpanDiagnosticTrace(span)), - { commandId }, + { command }, ); } @@ -191,7 +191,7 @@ export class CommandInstrumentation { let deferredError: unknown; let failed = false; await this.telemetry.trace( - "workspace.devcontainer.open", + "workspace.dev_container.open", async (span) => { const trace = new SpanDevcontainerTrace(span); try { @@ -273,7 +273,7 @@ class SpanDiagnosticTrace implements DiagnosticTrace { } public speedtestRequestedDuration(seconds: number): void { - this.span.setMeasurement("requestedDurationSec", seconds); + this.span.setMeasurement("requested_duration_seconds", seconds); } public speedtestSuccess(rawJson: string): SpeedtestResult { @@ -282,7 +282,7 @@ class SpanDiagnosticTrace implements DiagnosticTrace { public exportSuccess(format: string, eventCount: number): void { this.span.setProperty("format", format); - this.span.setMeasurement("eventCount", eventCount); + this.span.setMeasurement("event_count", eventCount); } } @@ -291,7 +291,7 @@ class SpanDevcontainerTrace implements DevcontainerTrace { public fail(error: unknown): void { this.span.setProperty( - "failure.category", + "failure_category", categorizeDevcontainerFailure(error), ); this.span.markFailure(); @@ -304,18 +304,18 @@ function setWorkspaceProperties( agent?: WorkspaceAgent, ): void { const agents = extractAgents(workspace.latest_build.resources); - span.setProperty("workspace.status", bucketWorkspaceStatus(workspace)); - span.setProperty("workspace.outdated", workspace.outdated); - span.setMeasurement("agentCount", agents.length); + span.setProperty("workspace_status", bucketWorkspaceStatus(workspace)); + span.setProperty("workspace_outdated", workspace.outdated); + span.setMeasurement("agent_count", agents.length); span.setMeasurement( - "connectedAgentCount", + "connected_agent_count", agents.filter((candidate) => candidate.status === "connected").length, ); if (!agent) { return; } - span.setProperty("agent.status", bucketAgentStatus(agent)); - span.setProperty("agent.lifecycle_state", bucketAgentLifecycle(agent)); + span.setProperty("agent_status", bucketAgentStatus(agent)); + span.setProperty("agent_lifecycle_state", bucketAgentLifecycle(agent)); } function setWorkspacePickerResult( @@ -323,7 +323,7 @@ function setWorkspacePickerResult( workspace: Workspace | undefined, resultCount: number, ): void { - span.setMeasurement("workspaceCount", resultCount); + span.setMeasurement("workspace_count", resultCount); if (!workspace) { span.markAborted(); return; @@ -336,8 +336,8 @@ function setWorkspacePickerFailure( category: WorkspacePickerFailureCategory, resultCount: number, ): void { - span.setMeasurement("workspaceCount", resultCount); - span.setProperty("failure.category", category); + span.setMeasurement("workspace_count", resultCount); + span.setProperty("failure_category", category); span.markFailure(); } @@ -353,7 +353,7 @@ function markWorkspaceOpenCancelled( stage: WorkspaceOpenCancelStage, selection?: WorkspaceOpenSelection, ): false { - span.setProperty("cancel.stage", stage); + span.setProperty("cancel_stage", stage); if (selection) { setWorkspaceOpenSelection(span, selection); } @@ -366,7 +366,7 @@ function markWorkspaceOpenFailure( category: WorkspaceOpenFailureCategory, selection?: WorkspaceOpenSelection, ): false { - span.setProperty("failure.category", category); + span.setProperty("failure_category", category); if (selection) { setWorkspaceOpenSelection(span, selection); } @@ -393,7 +393,7 @@ function markDiagnosticCancelled( span: Span, stage: DiagnosticCancelStage, ): void { - span.setProperty("cancel.stage", stage); + span.setProperty("cancel_stage", stage); span.markAborted(); } @@ -402,14 +402,14 @@ function markDiagnosticFailure( error: unknown, category: DiagnosticFailureCategory = categorizeFailure(error), ): void { - span.setProperty("failure.category", category); + span.setProperty("failure_category", category); span.markFailure(); } function setSpeedtestSuccess(span: Span, rawJson: string): SpeedtestResult { const parsed = parseSpeedtestResult(rawJson); - span.setMeasurement("intervalCount", parsed.intervals.length); - span.setMeasurement("throughputMbits", parsed.overall.throughput_mbits); + span.setMeasurement("interval_count", parsed.intervals.length); + span.setMeasurement("throughput_mbits", parsed.overall.throughput_mbits); return parsed; } diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index 515db0df4..2ac3d6ad1 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -191,7 +191,7 @@ export class WorkspaceOperationTelemetry { span.setProperty("action", action); return action; }, - { "update.offered": outdated }, + { update_offered: outdated }, ); } diff --git a/test/unit/instrumentation/commands.test.ts b/test/unit/instrumentation/commands.test.ts index 96d78b635..6361d6a47 100644 --- a/test/unit/instrumentation/commands.test.ts +++ b/test/unit/instrumentation/commands.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { CommandInstrumentation } from "@/instrumentation/commands"; +import { CommandTelemetry } from "@/instrumentation/commands"; import { agent, resource, workspace } from "@repo/mocks"; @@ -33,7 +33,7 @@ function workspaceWithAgents() { describe("command instrumentation helpers", () => { it("records workspace selection without workspace or agent names", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); const selection = workspaceWithAgents(); await traces.workspaceOpen( @@ -44,15 +44,15 @@ describe("command instrumentation helpers", () => { const event = sink.expectOne("workspace.open"); expect(event.properties).toMatchObject({ - "agent.lifecycle_state": "ready", - "agent.status": "connected", - "workspace.outdated": "true", - "workspace.status": "running", + agent_lifecycle_state: "ready", + agent_status: "connected", + workspace_outdated: "true", + workspace_status: "running", result: "success", }); expect(event.measurements).toMatchObject({ - agentCount: 2, - connectedAgentCount: 1, + agent_count: 2, + connected_agent_count: 1, }); expect(event.properties.workspaceName).toBeUndefined(); expect(event.properties.agentName).toBeUndefined(); @@ -60,30 +60,30 @@ describe("command instrumentation helpers", () => { it("records workspace picker cancellation and failure distinctly", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); - await traces.workspacePicker("workspace.open", (telemetry) => { + await traces.workspacePicker("workspace_open", (telemetry) => { telemetry.cancelled(3); return Promise.resolve({ status: "cancelled" }); }); - await traces.workspacePicker("workspace.open", (telemetry) => { + await traces.workspacePicker("workspace_open", (telemetry) => { telemetry.failed("fetch_failed", 0); return Promise.resolve({ status: "failed", category: "fetch_failed" }); }); const [cancelled, failed] = sink.eventsNamed("workspace.picker.prompted"); expect(cancelled.properties).toMatchObject({ result: "aborted" }); - expect(cancelled.measurements.workspaceCount).toBe(3); + expect(cancelled.measurements.workspace_count).toBe(3); expect(failed.properties).toMatchObject({ - "failure.category": "fetch_failed", + failure_category: "fetch_failed", result: "error", }); - expect(failed.measurements.workspaceCount).toBe(0); + expect(failed.measurements.workspace_count).toBe(0); }); it("records workspace open cancellation and handled failure distinctly", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); const selection = workspaceWithAgents(); await traces.workspaceOpen("command", undefined, (telemetry) => @@ -97,19 +97,19 @@ describe("command instrumentation helpers", () => { const [cancelled, failed] = sink.eventsNamed("workspace.open"); expect(cancelled.properties).toMatchObject({ - "cancel.stage": "agent_picker", - "workspace.status": "running", + cancel_stage: "agent_picker", + workspace_status: "running", result: "aborted", }); expect(failed.properties).toMatchObject({ - "failure.category": "fetch_failed", + failure_category: "fetch_failed", result: "error", }); }); it("records thrown workspace open failures without raw error details", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); await expect( traces.workspaceOpen("command", undefined, () => @@ -119,7 +119,7 @@ describe("command instrumentation helpers", () => { const event = sink.expectOne("workspace.open"); expect(event.properties).toMatchObject({ - "failure.category": "error", + failure_category: "error", result: "error", }); expect(event.error).toBeUndefined(); @@ -127,14 +127,14 @@ describe("command instrumentation helpers", () => { it("records diagnostic cancellation and failure categories", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); const failure = new Error("boom"); - await traces.diagnostic("coder.supportBundle", (telemetry) => { + await traces.diagnostic("support_bundle", (telemetry) => { telemetry.cancel("save_dialog"); return Promise.resolve(); }); - await traces.diagnostic("coder.supportBundle", (telemetry) => { + await traces.diagnostic("support_bundle", (telemetry) => { telemetry.fail(failure, "unsupported_cli"); return Promise.resolve(); }); @@ -143,11 +143,11 @@ describe("command instrumentation helpers", () => { "command.diagnostic.completed", ); expect(cancelled.properties).toMatchObject({ - "cancel.stage": "save_dialog", + cancel_stage: "save_dialog", result: "aborted", }); expect(failed.properties).toMatchObject({ - "failure.category": "unsupported_cli", + failure_category: "unsupported_cli", result: "error", }); expect(failed.error).toBeUndefined(); @@ -155,9 +155,9 @@ describe("command instrumentation helpers", () => { it("records bounded speed test measurements", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); - await traces.diagnostic("coder.speedTest", (telemetry) => { + await traces.diagnostic("speed_test", (telemetry) => { const parsed = telemetry.speedtestSuccess( JSON.stringify({ overall: { @@ -180,8 +180,8 @@ describe("command instrumentation helpers", () => { expect(sink.expectOne("command.diagnostic.completed")).toMatchObject({ measurements: { - intervalCount: 1, - throughputMbits: 42, + interval_count: 1, + throughput_mbits: 42, }, properties: { result: "success" }, }); @@ -189,7 +189,7 @@ describe("command instrumentation helpers", () => { it("records thrown devcontainer failures without raw error details", async () => { const { sink, service } = createTelemetryHarness(); - const traces = new CommandInstrumentation(service); + const traces = new CommandTelemetry(service); await expect( traces.devcontainerOpen("dev_container", () => @@ -197,9 +197,9 @@ describe("command instrumentation helpers", () => { ), ).rejects.toThrow("secret local path /tmp/workspace"); - const event = sink.expectOne("workspace.devcontainer.open"); + const event = sink.expectOne("workspace.dev_container.open"); expect(event.properties).toMatchObject({ - "failure.category": "error", + failure_category: "error", mode: "dev_container", result: "error", }); diff --git a/test/unit/instrumentation/workspace.test.ts b/test/unit/instrumentation/workspace.test.ts index 674e66353..fd16629ef 100644 --- a/test/unit/instrumentation/workspace.test.ts +++ b/test/unit/instrumentation/workspace.test.ts @@ -81,7 +81,7 @@ describe("WorkspaceOperationTelemetry", () => { expect(result).toBe("update"); expect(sink.expectOne("workspace.start.prompted")).toMatchObject({ properties: { - "update.offered": "true", + update_offered: "true", action: "update", result: "success", }, @@ -98,7 +98,7 @@ describe("WorkspaceOperationTelemetry", () => { expect(result).toBeUndefined(); expect(sink.expectOne("workspace.start.prompted")).toMatchObject({ properties: { - "update.offered": "false", + update_offered: "false", result: "aborted", }, }); From 8a332e0d54a694a243ace2093623b831c3abd643 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 9 Jun 2026 14:44:04 +0000 Subject: [PATCH 3/6] refactor: simplify command telemetry traces --- src/commands.ts | 535 +++++++++++--------- src/instrumentation/commands.ts | 287 ++--------- src/instrumentation/workspace.ts | 56 +- src/telemetry/export/command.ts | 34 +- test/mocks/vscode.runtime.ts | 15 + test/unit/command/updateWorkspace.test.ts | 82 +++ test/unit/instrumentation/commands.test.ts | 53 +- test/unit/instrumentation/workspace.test.ts | 35 ++ test/unit/telemetry/export/command.test.ts | 67 +-- 9 files changed, 605 insertions(+), 559 deletions(-) create mode 100644 test/unit/command/updateWorkspace.test.ts diff --git a/src/commands.ts b/src/commands.ts index 173c26d60..11fe0cfb2 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -22,12 +22,14 @@ import { import { CommandTelemetry, type DevcontainerMode, + type DiagnosticTrace, type WorkspaceOpenSource, type WorkspaceOpenTrace, type WorkspacePickerFailureCategory, type WorkspacePickerResult, type WorkspacePickerSource, } from "./instrumentation/commands"; +import { WorkspaceOperationTelemetry } from "./instrumentation/workspace"; import { reportElapsedProgress, withCancellableProgress, @@ -40,9 +42,13 @@ import { } from "./remote/sshOverrides"; import { resolveCliAuth } from "./settings/cli"; import { appendVsCodeLogs } from "./supportBundle/appendVsCodeLogs"; -import { runExportTelemetryCommand } from "./telemetry/export/command"; +import { + runExportTelemetryCommand, + type ExportTelemetryObserver, +} from "./telemetry/export/command"; import { openInBrowser, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; +import { parseSpeedtestResult } from "./webviews/speedtest/types"; import { AgentTreeItem, type OpenableTreeItem, @@ -99,6 +105,20 @@ type WorkspaceResolution = readonly category: WorkspacePickerFailureCategory; }; +type OpenWorkspaceResult = + | { readonly status: "opened"; readonly handoff: "folder" | "empty_window" } + | { readonly status: "cancelled"; readonly stage: "recent_folder_picker" }; + +class SupportBundleUnsupportedCliError extends Error { + public constructor() { + super( + "Support bundles require Coder CLI v2.10.0 or later. Please update your Coder deployment.", + ); + } +} + +const UPDATE_AND_RESTART_ACTION = "Update and Restart"; + const openDefaults = { openRecent: false, useDefaultDirectory: true, @@ -246,193 +266,200 @@ export class Commands { * editor document. Can be triggered from the sidebar or command palette. */ public async speedTest(item?: OpenableTreeItem): Promise { - await this.commandTelemetry.diagnostic("speed_test", async (telemetry) => { - const resolved = await this.resolveClientAndWorkspace(item); - if (resolved.status === "cancelled") { - telemetry.cancel("workspace_picker"); - return; - } - if (resolved.status === "failed") { - telemetry.fail(undefined, resolved.category); - return; - } + await this.commandTelemetry.diagnostic("speed_test", (telemetry) => + this.runSpeedTest(item, telemetry), + ); + } - const { client, workspaceId } = resolved; + private async runSpeedTest( + item: OpenableTreeItem | undefined, + telemetry: DiagnosticTrace, + ): Promise { + const resolved = await this.resolveClientAndWorkspace(item); + if (resolved.status === "cancelled") { + telemetry.cancel("workspace_picker"); + return; + } + if (resolved.status === "failed") { + telemetry.fail(resolved.category); + return; + } - const input = await vscode.window.showInputBox({ - title: "Speed Test Duration", - prompt: "Duration in seconds", - value: "5", - validateInput: (value) => { - const n = Number(value.trim()); - if (!value.trim() || !Number.isFinite(n) || n <= 0) { - return "Please enter a positive number"; - } - return undefined; - }, - }); - if (input === undefined) { - telemetry.cancel("input"); - return; - } - const seconds = Number(input.trim()); - telemetry.speedtestRequestedDuration(seconds); - - const result = await withCancellableProgress( - async ({ signal, progress }) => { - progress.report({ message: "Connecting..." }); - const env = await this.resolveCliEnv(client); - - const stopProgress = reportElapsedProgress({ - progress, - totalMs: seconds * 1000, - format: (pct, elapsedMs) => - pct >= 100 - ? "Collecting results..." - : `${Math.floor(elapsedMs / 1000)}s / ${seconds}s`, - }); - try { - return await cliExec.speedtest( - env, - workspaceId, - `${seconds}s`, - signal, - ); - } finally { - stopProgress(); - } - }, - { - location: vscode.ProgressLocation.Notification, - title: `Running speed test for ${workspaceId}`, - cancellable: true, - }, - ); + const { client, workspaceId } = resolved; - if (!telemetry.progressResult(result)) { - if (!result.cancelled) { - this.logger.error("Speed test failed", result.error); - vscode.window.showErrorMessage( - `Speed test failed: ${toError(result.error).message}`, - ); + const input = await vscode.window.showInputBox({ + title: "Speed Test Duration", + prompt: "Duration in seconds", + value: "5", + validateInput: (value) => { + const n = Number(value.trim()); + if (!value.trim() || !Number.isFinite(n) || n <= 0) { + return "Please enter a positive number"; } - return; - } + return undefined; + }, + }); + if (input === undefined) { + telemetry.cancel("input"); + return; + } + const seconds = Number(input.trim()); + telemetry.speedtestRequestedDuration(seconds); + + const result = await withCancellableProgress( + async ({ signal, progress }) => { + progress.report({ message: "Connecting..." }); + const env = await this.resolveCliEnv(client); - try { - const parsed = telemetry.speedtestSuccess(result.value); - this.speedtestPanelFactory.show({ - result: parsed, - rawJson: result.value, - workspaceId, + const stopProgress = reportElapsedProgress({ + progress, + totalMs: seconds * 1000, + format: (pct, elapsedMs) => + pct >= 100 + ? "Collecting results..." + : `${Math.floor(elapsedMs / 1000)}s / ${seconds}s`, }); - } catch (err) { - if (err instanceof ZodError || err instanceof SyntaxError) { - telemetry.fail(err, "parse_error"); - this.logger.error("Failed to parse speedtest output", err); - vscode.window.showErrorMessage( - "Speed test output did not match the expected format. Check `Output > Coder` for details.", + try { + return await cliExec.speedtest( + env, + workspaceId, + `${seconds}s`, + signal, ); - return; + } finally { + stopProgress(); } - telemetry.fail(err); - this.logger.error("Failed to display speedtest results", err); + }, + { + location: vscode.ProgressLocation.Notification, + title: `Running speed test for ${workspaceId}`, + cancellable: true, + }, + ); + + if (!result.ok) { + if (result.cancelled) { + telemetry.cancel("progress"); + return; + } + telemetry.fail(); + this.logger.error("Speed test failed", result.error); + vscode.window.showErrorMessage( + `Speed test failed: ${toError(result.error).message}`, + ); + return; + } + + try { + const parsed = parseSpeedtestResult(result.value); + telemetry.speedtestSuccess(parsed); + this.speedtestPanelFactory.show({ + result: parsed, + rawJson: result.value, + workspaceId, + }); + } catch (err) { + if (err instanceof ZodError || err instanceof SyntaxError) { + telemetry.fail("parse_error"); + this.logger.error("Failed to parse speedtest output", err); vscode.window.showErrorMessage( - `Speed test returned unexpected output: ${toError(err).message}`, + "Speed test output did not match the expected format. Check `Output > Coder` for details.", ); + return; } - }); + telemetry.fail(); + this.logger.error("Failed to display speedtest results", err); + vscode.window.showErrorMessage( + `Speed test returned unexpected output: ${toError(err).message}`, + ); + } } public async supportBundle(item?: OpenableTreeItem): Promise { - await this.commandTelemetry.diagnostic( - "support_bundle", - async (telemetry) => { - const resolved = await this.resolveClientAndWorkspace(item); - if (resolved.status === "cancelled") { - telemetry.cancel("workspace_picker"); - return; - } - if (resolved.status === "failed") { - telemetry.fail(undefined, resolved.category); - return; - } + await this.commandTelemetry.diagnostic("support_bundle", (telemetry) => + this.runSupportBundle(item, telemetry), + ); + } - const { client, workspaceId } = resolved; + private async runSupportBundle( + item: OpenableTreeItem | undefined, + telemetry: DiagnosticTrace, + ): Promise { + const resolved = await this.resolveClientAndWorkspace(item); + if (resolved.status === "cancelled") { + telemetry.cancel("workspace_picker"); + return; + } + if (resolved.status === "failed") { + telemetry.fail(resolved.category); + return; + } - const outputUri = await this.promptSupportBundlePath(); - if (!outputUri) { - telemetry.cancel("save_dialog"); - return; - } + const { client, workspaceId } = resolved; - const result = await withCancellableProgress( - async ({ signal, progress }) => { - progress.report({ message: "Resolving CLI..." }); - const env = await this.resolveCliEnv(client); - if (!env.featureSet.supportBundle) { - throw new Error( - "Support bundles require Coder CLI v2.10.0 or later. Please update your Coder deployment.", - ); - } + const outputUri = await this.promptSupportBundlePath(); + if (!outputUri) { + telemetry.cancel("save_dialog"); + return; + } - progress.report({ message: "Collecting diagnostics..." }); - await cliExec.supportBundle( - env, - workspaceId, - outputUri.fsPath, - signal, - ); + const result = await withCancellableProgress( + async ({ signal, progress }) => { + progress.report({ message: "Resolving CLI..." }); + const env = await this.resolveCliEnv(client); + if (!env.featureSet.supportBundle) { + throw new SupportBundleUnsupportedCliError(); + } - progress.report({ message: "Adding VS Code logs..." }); - await appendVsCodeLogs( - outputUri.fsPath, - { - activeProxyLogPath: this.workspaceLogPath, - proxyLogDir: this.pathResolver.getProxyLogPath(), - extensionLogDir: this.pathResolver.getCodeLogDir(), - telemetryDir: this.pathResolver.getTelemetryPath(), - }, - this.logger, - ); + progress.report({ message: "Collecting diagnostics..." }); + await cliExec.supportBundle(env, workspaceId, outputUri.fsPath, signal); - return outputUri; - }, + progress.report({ message: "Adding VS Code logs..." }); + await appendVsCodeLogs( + outputUri.fsPath, { - location: vscode.ProgressLocation.Notification, - title: `Creating support bundle for ${workspaceId}`, - cancellable: true, + activeProxyLogPath: this.workspaceLogPath, + proxyLogDir: this.pathResolver.getProxyLogPath(), + extensionLogDir: this.pathResolver.getCodeLogDir(), + telemetryDir: this.pathResolver.getTelemetryPath(), }, + this.logger, ); - if (!telemetry.progressResult(result)) { - if (!result.cancelled) { - if ( - toError(result.error).message.startsWith( - "Support bundles require", - ) - ) { - telemetry.fail(result.error, "unsupported_cli"); - } - this.logger.error("Support bundle failed", result.error); - vscode.window.showErrorMessage( - `Support bundle failed: ${toError(result.error).message}`, - ); - } - return; - } - - const action = await vscode.window.showInformationMessage( - `Support bundle saved to ${result.value.fsPath}`, - "Reveal in File Explorer", - ); - if (action === "Reveal in File Explorer") { - await vscode.commands.executeCommand("revealFileInOS", result.value); - } + return outputUri; + }, + { + location: vscode.ProgressLocation.Notification, + title: `Creating support bundle for ${workspaceId}`, + cancellable: true, }, ); - } + if (!result.ok) { + if (result.cancelled) { + telemetry.cancel("progress"); + return; + } + telemetry.fail( + result.error instanceof SupportBundleUnsupportedCliError + ? "unsupported_cli" + : undefined, + ); + this.logger.error("Support bundle failed", result.error); + vscode.window.showErrorMessage( + `Support bundle failed: ${toError(result.error).message}`, + ); + return; + } + + const action = await vscode.window.showInformationMessage( + `Support bundle saved to ${result.value.fsPath}`, + "Reveal in File Explorer", + ); + if (action === "Reveal in File Explorer") { + await vscode.commands.executeCommand("revealFileInOS", result.value); + } + } private promptSupportBundlePath(): Thenable { const defaultName = `coder-support-${Math.floor(Date.now() / 1000)}.zip`; return vscode.window.showSaveDialog({ @@ -443,27 +470,24 @@ export class Commands { } public async exportTelemetry(): Promise { - await this.commandTelemetry.diagnostic( - "export_telemetry", - async (telemetry) => { - const outcome = await runExportTelemetryCommand( - this.pathResolver.getTelemetryPath(), - this.logger, - () => this.telemetryService.flush(), - this.telemetryService.getContext(), - ); - switch (outcome.status) { - case "cancelled": - telemetry.cancel(outcome.stage); - return; - case "failed": - telemetry.fail(outcome.error); - return; - case "success": - telemetry.exportSuccess(outcome.format, outcome.eventCount); - return; - } - }, + await this.commandTelemetry.diagnostic("export_telemetry", (telemetry) => + this.runExportTelemetry(telemetry), + ); + } + + private async runExportTelemetry(telemetry: DiagnosticTrace): Promise { + const observer: ExportTelemetryObserver = { + cancelled: (stage) => telemetry.cancel(stage), + failed: () => telemetry.fail(), + succeeded: (format, eventCount) => + telemetry.exportSuccess(format, eventCount), + }; + await runExportTelemetryCommand( + this.pathResolver.getTelemetryPath(), + this.logger, + () => this.telemetryService.flush(), + this.telemetryService.getContext(), + observer, ); } @@ -767,11 +791,18 @@ export class Commands { await this.commandTelemetry.workspaceOpen( "sidebar_agent", { workspace: item.workspace, agent: item.agent }, - (telemetry) => { - return this.openWorkspace(baseUrl, item.workspace, item.agent, { - openRecent: true, + async (telemetry) => { + const result = await this.openWorkspaceInternal( + baseUrl, + item.workspace, + item.agent, + { openRecent: true }, + ); + return recordWorkspaceOpenResult( telemetry, - }); + { workspace: item.workspace, agent: item.agent }, + result, + ); }, ); } else if (item instanceof WorkspaceTreeItem) { @@ -782,18 +813,20 @@ export class Commands { const agents = await this.extractAgentsWithFallback(item.workspace); const agent = await maybeAskAgent(agents); if (!agent) { - return telemetry.cancel("agent_picker", { + telemetry.cancel("agent_picker", { workspace: item.workspace, }); + return false; } - telemetry.select({ - workspace: item.workspace, + const selection = { workspace: item.workspace, agent }; + telemetry.select(selection); + const result = await this.openWorkspaceInternal( + baseUrl, + item.workspace, agent, - }); - return this.openWorkspace(baseUrl, item.workspace, agent, { - openRecent: true, - telemetry, - }); + { openRecent: true }, + ); + return recordWorkspaceOpenResult(telemetry, selection, result); }, ); } else { @@ -872,10 +905,12 @@ export class Commands { } else { const pick = await this.pickWorkspace("workspace_open"); if (pick.status === "cancelled") { - return telemetry.cancel("workspace_picker"); + telemetry.cancel("workspace_picker"); + return false; } if (pick.status === "failed") { - return telemetry.fail(pick.category); + telemetry.fail(pick.category); + return false; } workspace = pick.workspace; } @@ -883,16 +918,23 @@ export class Commands { const agents = await this.extractAgentsWithFallback(workspace); const agent = await maybeAskAgent(agents, agentName); if (!agent) { - return telemetry.cancel("agent_picker", { workspace }); + telemetry.cancel("agent_picker", { workspace }); + return false; } - telemetry.select({ workspace, agent }); + const selection = { workspace, agent }; + telemetry.select(selection); - return this.openWorkspace(baseUrl, workspace, agent, { - folderPath, - openRecent, - useDefaultDirectory, - telemetry, - }); + const result = await this.openWorkspaceInternal( + baseUrl, + workspace, + agent, + { + folderPath, + openRecent, + useDefaultDirectory, + }, + ); + return recordWorkspaceOpenResult(telemetry, selection, result); }, ); } @@ -986,16 +1028,24 @@ export class Commands { showUpToDate(); return; } - const action = await vscodeProposed.window.showWarningMessage( - "Update Workspace", - { - useCustom: true, - modal: true, - detail: `Update ${createWorkspaceIdentifier(this.workspace)} to the latest version?\n\nUpdating will restart your workspace which stops any running processes and may result in the loss of unsaved work.`, - }, - "Update and Restart", + const workspaceName = createWorkspaceIdentifier(this.workspace); + const operationTelemetry = new WorkspaceOperationTelemetry( + this.telemetryService, + workspaceName, + ); + const action = await operationTelemetry.traceUpdateConfirmationPrompted( + async () => + vscodeProposed.window.showWarningMessage( + "Update Workspace", + { + useCustom: true, + modal: true, + detail: `Update ${workspaceName} to the latest version?\n\nUpdating will restart your workspace which stops any running processes and may result in the loss of unsaved work.`, + }, + UPDATE_AND_RESTART_ACTION, + ), ); - if (action !== "Update and Restart") { + if (action !== UPDATE_AND_RESTART_ACTION) { return; } @@ -1008,9 +1058,7 @@ export class Commands { return; } - this.logger.info( - `Updating workspace ${createWorkspaceIdentifier(this.workspace)}`, - ); + this.logger.info(`Updating workspace ${workspaceName}`); await this.mementoManager.setStartupMode("update"); await vscode.commands.executeCommand("workbench.action.reloadWindow"); } @@ -1190,13 +1238,7 @@ export class Commands { return; } settled = true; - if (result.status === "selected") { - telemetry.selected(result.workspace, lastWorkspaces.length); - } else if (result.status === "failed") { - telemetry.failed(result.category, lastWorkspaces.length); - } else { - telemetry.cancelled(lastWorkspaces.length); - } + telemetry.finish(result, lastWorkspaces.length); resolve(result); }; disposables.push( @@ -1274,13 +1316,30 @@ export class Commands { options: Pick< OpenOptions, "folderPath" | "openRecent" | "useDefaultDirectory" - > & { telemetry?: WorkspaceOpenTrace } = {}, + > = {}, ): Promise { + const result = await this.openWorkspaceInternal( + baseUrl, + workspace, + agent, + options, + ); + return result.status === "opened"; + } + + private async openWorkspaceInternal( + baseUrl: string, + workspace: Workspace, + agent: WorkspaceAgent, + options: Pick< + OpenOptions, + "folderPath" | "openRecent" | "useDefaultDirectory" + > = {}, + ): Promise { const { openRecent, useDefaultDirectory } = { ...openDefaults, ...options, }; - const { telemetry } = options; let { folderPath } = options; const remoteAuthority = toRemoteAuthority( baseUrl, @@ -1324,12 +1383,7 @@ export class Commands { }); if (!folderPath) { // User aborted. - return ( - telemetry?.cancel("recent_folder_picker", { - workspace, - agent, - }) ?? false - ); + return { status: "cancelled", stage: "recent_folder_picker" }; } } } @@ -1351,7 +1405,6 @@ export class Commands { }); if (folderPath) { - telemetry?.handoff("folder"); await vscode.commands.executeCommand( "vscode.openFolder", vscode.Uri.from({ @@ -1362,16 +1415,15 @@ export class Commands { // Open this in a new window! newWindow, ); - return true; + return { status: "opened", handoff: "folder" }; } // This opens the workspace without an active folder opened. - telemetry?.handoff("empty_window"); await vscode.commands.executeCommand("vscode.newWindow", { remoteAuthority: remoteAuthority, reuseWindow: !newWindow, }); - return true; + return { status: "opened", handoff: "empty_window" }; } // VS Code may dismiss a non-modal info message without resolving the @@ -1407,6 +1459,19 @@ export class Commands { } } +function recordWorkspaceOpenResult( + telemetry: WorkspaceOpenTrace, + selection: { readonly workspace: Workspace; readonly agent: WorkspaceAgent }, + result: OpenWorkspaceResult, +): boolean { + if (result.status === "cancelled") { + telemetry.cancel(result.stage, selection); + return false; + } + telemetry.handoff(result.handoff); + return true; +} + async function openFile(filePath: string): Promise { const uri = vscode.Uri.file(filePath); await vscode.window.showTextDocument(uri); diff --git a/src/instrumentation/commands.ts b/src/instrumentation/commands.ts index 3eb158b0b..53b7bcf0d 100644 --- a/src/instrumentation/commands.ts +++ b/src/instrumentation/commands.ts @@ -1,6 +1,5 @@ import { extractAgents } from "../api/api-helper"; import { isAbortError } from "../error/errorUtils"; -import { parseSpeedtestResult } from "../webviews/speedtest/types"; import type { Workspace, @@ -9,7 +8,6 @@ import type { import type { SpeedtestResult } from "@repo/shared"; -import type { ProgressResult } from "../progress"; import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; @@ -22,10 +20,10 @@ export type WorkspaceOpenSource = export type WorkspacePickerSource = "workspace_open" | "diagnostic"; export type WorkspacePickerFailureCategory = "fetch_failed"; +type AbortableFailureCategory = "aborted" | "error"; export type WorkspaceOpenFailureCategory = | WorkspacePickerFailureCategory - | "aborted" - | "error"; + | AbortableFailureCategory; export type WorkspacePickerResult = | { readonly status: "selected"; readonly workspace: Workspace } | { readonly status: "cancelled" } @@ -41,10 +39,9 @@ export type DiagnosticFailureCategory = | WorkspacePickerFailureCategory | "parse_error" | "unsupported_cli" - | "aborted" | "error"; export type DevcontainerMode = "dev_container" | "attached_container"; -export type DevcontainerFailureCategory = "aborted" | "error"; +export type DevcontainerFailureCategory = AbortableFailureCategory; export type WorkspaceOpenCancelStage = | "workspace_picker" | "agent_picker" @@ -56,47 +53,13 @@ export type DiagnosticCancelStage = | "save_dialog" | "progress"; -type WorkspaceStateBucket = - | "running" - | "stopped" - | "failed" - | "starting" - | "stopping" - | "pending" - | "deleting" - | "deleted" - | "canceled" - | "canceling" - | "unknown"; - -type AgentStatusBucket = - | "connected" - | "connecting" - | "disconnected" - | "timeout" - | "unknown"; - -type AgentLifecycleBucket = - | "ready" - | "starting" - | "created" - | "start_error" - | "start_timeout" - | "shutting_down" - | "off" - | "shutdown_error" - | "shutdown_timeout" - | "unknown"; - export interface WorkspaceOpenSelection { readonly workspace: Workspace; readonly agent?: WorkspaceAgent; } export interface WorkspacePickerTrace { - selected(workspace: Workspace, resultCount: number): void; - cancelled(resultCount: number): void; - failed(category: WorkspacePickerFailureCategory, resultCount: number): void; + finish(result: WorkspacePickerResult, resultCount: number): void; } export interface WorkspaceOpenTrace { @@ -104,26 +67,19 @@ export interface WorkspaceOpenTrace { cancel( stage: WorkspaceOpenCancelStage, selection?: WorkspaceOpenSelection, - ): false; - fail(category: WorkspaceOpenFailureCategory): false; + ): void; + fail(category: WorkspaceOpenFailureCategory): void; handoff(kind: "folder" | "empty_window"): void; } export interface DiagnosticTrace { cancel(stage: DiagnosticCancelStage): void; - fail(error: unknown, category?: DiagnosticFailureCategory): void; - progressResult( - result: ProgressResult, - ): result is { ok: true; value: T }; + fail(category?: DiagnosticFailureCategory): void; speedtestRequestedDuration(seconds: number): void; - speedtestSuccess(rawJson: string): SpeedtestResult; + speedtestSuccess(result: SpeedtestResult): void; exportSuccess(format: string, eventCount: number): void; } -export interface DevcontainerTrace { - fail(error: unknown): void; -} - export class CommandTelemetry { public constructor(private readonly telemetry: TelemetryReporter) {} @@ -161,7 +117,7 @@ export class CommandTelemetry { } catch (error) { failed = true; deferredError = error; - trace.fail(categorizeWorkspaceOpenFailure(error)); + trace.fail(categorizeAbortableFailure(error)); return false; } }, @@ -186,20 +142,19 @@ export class CommandTelemetry { public async devcontainerOpen( mode: DevcontainerMode, - fn: (trace: DevcontainerTrace) => Promise, + fn: () => Promise, ): Promise { let deferredError: unknown; let failed = false; await this.telemetry.trace( "workspace.dev_container.open", async (span) => { - const trace = new SpanDevcontainerTrace(span); try { - await fn(trace); + await fn(); } catch (error) { failed = true; deferredError = error; - trace.fail(error); + recordFailure(span, categorizeAbortableFailure(error)); } }, { mode }, @@ -213,19 +168,8 @@ export class CommandTelemetry { class SpanWorkspacePickerTrace implements WorkspacePickerTrace { public constructor(private readonly span: Span) {} - public selected(workspace: Workspace, resultCount: number): void { - setWorkspacePickerResult(this.span, workspace, resultCount); - } - - public cancelled(resultCount: number): void { - setWorkspacePickerResult(this.span, undefined, resultCount); - } - - public failed( - category: WorkspacePickerFailureCategory, - resultCount: number, - ): void { - setWorkspacePickerFailure(this.span, category, resultCount); + public finish(result: WorkspacePickerResult, resultCount: number): void { + recordWorkspacePickerResult(this.span, result, resultCount); } } @@ -233,18 +177,18 @@ class SpanWorkspaceOpenTrace implements WorkspaceOpenTrace { public constructor(private readonly span: Span) {} public select(selection: WorkspaceOpenSelection): void { - setWorkspaceOpenSelection(this.span, selection); + recordWorkspaceContext(this.span, selection.workspace, selection.agent); } public cancel( stage: WorkspaceOpenCancelStage, selection?: WorkspaceOpenSelection, - ): false { - return markWorkspaceOpenCancelled(this.span, stage, selection); + ): void { + recordWorkspaceOpenCancelled(this.span, stage, selection); } - public fail(category: WorkspaceOpenFailureCategory): false { - return markWorkspaceOpenFailure(this.span, category); + public fail(category: WorkspaceOpenFailureCategory): void { + recordFailure(this.span, category); } public handoff(kind: "folder" | "empty_window"): void { @@ -256,28 +200,19 @@ class SpanDiagnosticTrace implements DiagnosticTrace { public constructor(private readonly span: Span) {} public cancel(stage: DiagnosticCancelStage): void { - markDiagnosticCancelled(this.span, stage); + recordCancelled(this.span, stage); } - public fail( - error: unknown, - category: DiagnosticFailureCategory = categorizeFailure(error), - ): void { - markDiagnosticFailure(this.span, error, category); - } - - public progressResult( - result: ProgressResult, - ): result is { ok: true; value: T } { - return setDiagnosticProgressResult(this.span, result); + public fail(category: DiagnosticFailureCategory = "error"): void { + recordFailure(this.span, category); } public speedtestRequestedDuration(seconds: number): void { this.span.setMeasurement("requested_duration_seconds", seconds); } - public speedtestSuccess(rawJson: string): SpeedtestResult { - return setSpeedtestSuccess(this.span, rawJson); + public speedtestSuccess(result: SpeedtestResult): void { + recordSpeedtestResult(this.span, result); } public exportSuccess(format: string, eventCount: number): void { @@ -286,25 +221,13 @@ class SpanDiagnosticTrace implements DiagnosticTrace { } } -class SpanDevcontainerTrace implements DevcontainerTrace { - public constructor(private readonly span: Span) {} - - public fail(error: unknown): void { - this.span.setProperty( - "failure_category", - categorizeDevcontainerFailure(error), - ); - this.span.markFailure(); - } -} - -function setWorkspaceProperties( +function recordWorkspaceContext( span: Span, workspace: Workspace, agent?: WorkspaceAgent, ): void { const agents = extractAgents(workspace.latest_build.resources); - span.setProperty("workspace_status", bucketWorkspaceStatus(workspace)); + span.setProperty("workspace_status", workspace.latest_build.status); span.setProperty("workspace_outdated", workspace.outdated); span.setMeasurement("agent_count", agents.length); span.setMeasurement( @@ -314,171 +237,61 @@ function setWorkspaceProperties( if (!agent) { return; } - span.setProperty("agent_status", bucketAgentStatus(agent)); - span.setProperty("agent_lifecycle_state", bucketAgentLifecycle(agent)); + span.setProperty("agent_status", agent.status); + span.setProperty("agent_lifecycle_state", agent.lifecycle_state); } -function setWorkspacePickerResult( +function recordWorkspacePickerResult( span: Span, - workspace: Workspace | undefined, + result: WorkspacePickerResult, resultCount: number, ): void { span.setMeasurement("workspace_count", resultCount); - if (!workspace) { - span.markAborted(); + if (result.status === "selected") { + recordWorkspaceContext(span, result.workspace); return; } - setWorkspaceProperties(span, workspace); -} - -function setWorkspacePickerFailure( - span: Span, - category: WorkspacePickerFailureCategory, - resultCount: number, -): void { - span.setMeasurement("workspace_count", resultCount); - span.setProperty("failure_category", category); - span.markFailure(); -} - -function setWorkspaceOpenSelection( - span: Span, - selection: WorkspaceOpenSelection, -): void { - setWorkspaceProperties(span, selection.workspace, selection.agent); + if (result.status === "failed") { + recordFailure(span, result.category); + return; + } + span.markAborted(); } -function markWorkspaceOpenCancelled( +function recordWorkspaceOpenCancelled( span: Span, stage: WorkspaceOpenCancelStage, selection?: WorkspaceOpenSelection, -): false { +): void { span.setProperty("cancel_stage", stage); if (selection) { - setWorkspaceOpenSelection(span, selection); + recordWorkspaceContext(span, selection.workspace, selection.agent); } span.markAborted(); - return false; } -function markWorkspaceOpenFailure( - span: Span, - category: WorkspaceOpenFailureCategory, - selection?: WorkspaceOpenSelection, -): false { - span.setProperty("failure_category", category); - if (selection) { - setWorkspaceOpenSelection(span, selection); - } - span.markFailure(); - return false; -} - -function setDiagnosticProgressResult( - span: Span, - result: ProgressResult, -): result is { ok: true; value: T } { - if (result.ok) { - return true; - } - if (result.cancelled) { - markDiagnosticCancelled(span, "progress"); - } else { - markDiagnosticFailure(span, result.error); - } - return false; -} - -function markDiagnosticCancelled( - span: Span, - stage: DiagnosticCancelStage, -): void { +function recordCancelled(span: Span, stage: DiagnosticCancelStage): void { span.setProperty("cancel_stage", stage); span.markAborted(); } -function markDiagnosticFailure( +function recordFailure( span: Span, - error: unknown, - category: DiagnosticFailureCategory = categorizeFailure(error), + category: + | DiagnosticFailureCategory + | WorkspaceOpenFailureCategory + | DevcontainerFailureCategory, ): void { span.setProperty("failure_category", category); span.markFailure(); } -function setSpeedtestSuccess(span: Span, rawJson: string): SpeedtestResult { - const parsed = parseSpeedtestResult(rawJson); - span.setMeasurement("interval_count", parsed.intervals.length); - span.setMeasurement("throughput_mbits", parsed.overall.throughput_mbits); - return parsed; -} - -function categorizeFailure(error: unknown): DiagnosticFailureCategory { - if (isAbortError(error)) { - return "aborted"; - } - return "error"; -} - -function categorizeWorkspaceOpenFailure( - error: unknown, -): WorkspaceOpenFailureCategory { - if (isAbortError(error)) { - return "aborted"; - } - return "error"; -} - -function bucketWorkspaceStatus(workspace: Workspace): WorkspaceStateBucket { - switch (workspace.latest_build.status) { - case "running": - case "stopped": - case "failed": - case "starting": - case "stopping": - case "pending": - case "deleting": - case "deleted": - case "canceled": - case "canceling": - return workspace.latest_build.status; - default: - return "unknown"; - } -} - -function bucketAgentStatus(agent: WorkspaceAgent): AgentStatusBucket { - switch (agent.status) { - case "connected": - case "connecting": - case "disconnected": - case "timeout": - return agent.status; - default: - return "unknown"; - } -} - -function bucketAgentLifecycle(agent: WorkspaceAgent): AgentLifecycleBucket { - switch (agent.lifecycle_state) { - case "ready": - case "starting": - case "created": - case "start_error": - case "start_timeout": - case "shutting_down": - case "off": - case "shutdown_error": - case "shutdown_timeout": - return agent.lifecycle_state; - default: - return "unknown"; - } +function recordSpeedtestResult(span: Span, result: SpeedtestResult): void { + span.setMeasurement("interval_count", result.intervals.length); + span.setMeasurement("throughput_mbits", result.overall.throughput_mbits); } -function categorizeDevcontainerFailure( - error: unknown, -): DevcontainerFailureCategory { +function categorizeAbortableFailure(error: unknown): AbortableFailureCategory { if (isAbortError(error)) { return "aborted"; } diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index 2ac3d6ad1..3b3db5b61 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -11,6 +11,7 @@ import type { } from "coder/site/src/api/typesGenerated"; import type { TelemetryReporter } from "../telemetry/reporter"; +import type { Span } from "../telemetry/span"; /** Sentinel for `from*` before any state is observed. `"unknown"` is a real server-reported value, so avoid it. */ const INITIAL_STATE = "none"; @@ -25,6 +26,7 @@ const PROVISIONING_STATUSES: ReadonlySet = new Set([ ]); export type WorkspacePromptAction = "start" | "update"; +export type WorkspaceUpdatePrompt = "parameters" | "confirmation"; interface ObservedWorkspaceState { readonly status: WorkspaceStatus; @@ -199,27 +201,61 @@ export class WorkspaceOperationTelemetry { * Records dismissal as `result: "aborted"`. The framework treats any throw * as `result: "error"`, so we return inside the span and rethrow outside. */ - public async traceUpdatePrompted( + public traceUpdatePrompted( fn: () => Promise, ): Promise { - let cancel: WorkspaceUpdateCancelledError | undefined; - const parameters = await this.telemetry.trace( + return this.traceUpdatePrompt("parameters", fn, { + isCancelled: (error) => error instanceof WorkspaceUpdateCancelledError, + cancelledValue: () => [], + }); + } + + public traceUpdateConfirmationPrompted( + fn: () => Promise, + ): Promise { + return this.traceUpdatePrompt("confirmation", fn, { + isCancelled: (value) => value === undefined, + recordAccepted: (span) => span.setProperty("action", "update"), + }); + } + + private async traceUpdatePrompt( + prompt: WorkspaceUpdatePrompt, + fn: () => Promise, + options: { + readonly isCancelled: (value: unknown) => boolean; + readonly cancelledValue?: () => T; + readonly recordAccepted?: (span: Span, value: T) => void; + }, + ): Promise { + let cancelledError: Error | undefined; + const cancelledValue = options.cancelledValue; + const result = await this.telemetry.trace( "workspace.update.prompted", async (span) => { try { - return await fn(); + const value = await fn(); + if (options.isCancelled(value)) { + span.markAborted(); + } else { + options.recordAccepted?.(span, value); + } + return value; } catch (error) { - if (error instanceof WorkspaceUpdateCancelledError) { + if (error instanceof Error && options.isCancelled(error)) { span.markAborted(); - cancel = error; - return []; + cancelledError = error; + if (!cancelledValue) { + throw error; + } + return cancelledValue(); } throw error; } }, - { prompt: "parameters" }, + { prompt }, ); - if (cancel) throw cancel; - return parameters; + if (cancelledError !== undefined) throw cancelledError; + return result; } } diff --git a/src/telemetry/export/command.ts b/src/telemetry/export/command.ts index ef0575858..833138288 100644 --- a/src/telemetry/export/command.ts +++ b/src/telemetry/export/command.ts @@ -29,24 +29,23 @@ const PROGRESS_OPTIONS = { cancellable: true, } as const; -export type ExportTelemetryOutcome = - | { readonly status: "cancelled"; readonly stage: "prompt" | "progress" } - | { readonly status: "failed"; readonly error: unknown } - | { - readonly status: "success"; - readonly eventCount: number; - readonly format: ExportFormat; - }; +export interface ExportTelemetryObserver { + cancelled(stage: "prompt" | "progress"): void; + failed(): void; + succeeded(format: ExportFormat, eventCount: number): void; +} export async function runExportTelemetryCommand( telemetryDir: string, logger: Logger, flushTelemetry: () => Promise, context: TelemetryContext, -): Promise { + observer?: ExportTelemetryObserver, +): Promise { const choice = await promptForExport(); if (!choice) { - return { status: "cancelled", stage: "prompt" }; + observer?.cancelled("prompt"); + return; } const request: ExportRequest = { @@ -64,7 +63,7 @@ export async function runExportTelemetryCommand( PROGRESS_OPTIONS, ); - return reportOutcome(result, choice, logger); + await reportOutcome(result, choice, logger, observer); } /** Wires the pipeline's host hooks to the progress UI and the logger. */ @@ -92,27 +91,30 @@ async function reportOutcome( result: ProgressResult, choice: ExportChoice, logger: Logger, -): Promise { + observer?: ExportTelemetryObserver, +): Promise { if (!result.ok) { if (result.cancelled) { - return { status: "cancelled", stage: "progress" }; + observer?.cancelled("progress"); + return; } + observer?.failed(); logger.error("Telemetry export failed", result.error); void vscode.window.showErrorMessage( `Telemetry export failed: ${toError(result.error).message}`, ); - return { status: "failed", error: result.error }; + return; } const eventCount = result.value; + observer?.succeeded(choice.format, eventCount); if (eventCount === 0) { void vscode.window.showInformationMessage( `No telemetry events found for ${choice.range.label}.`, ); - return { status: "success", eventCount, format: choice.format }; + return; } await notifyExportSucceeded(choice.outputPath, eventCount, logger); - return { status: "success", eventCount, format: choice.format }; } async function notifyExportSucceeded( diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index a111a5c42..8eee24521 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -65,6 +65,20 @@ export class ThemeColor { } } +export class TreeItem { + id?: string; + contextValue?: string; + description?: string; + tooltip?: string; + command?: unknown; + iconPath?: unknown; + + constructor( + public label: string, + public collapsibleState?: number, + ) {} +} + export class Uri { constructor( public scheme: string, @@ -213,6 +227,7 @@ const vscode = { EventEmitter, MarkdownString, ThemeColor, + TreeItem, window, commands, workspace, diff --git a/test/unit/command/updateWorkspace.test.ts b/test/unit/command/updateWorkspace.test.ts new file mode 100644 index 000000000..3374a85fe --- /dev/null +++ b/test/unit/command/updateWorkspace.test.ts @@ -0,0 +1,82 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; + +import { Commands } from "@/commands"; +import { MementoManager } from "@/core/mementoManager"; + +import { workspace } from "@repo/mocks"; + +import { createTelemetryHarness } from "../../mocks/telemetry"; +import { createMockLogger, InMemoryMemento } from "../../mocks/testHelpers"; + +import type { CoderApi } from "@/api/coderApi"; +import type { ServiceContainer } from "@/core/container"; +import type { DeploymentManager } from "@/deployment/deploymentManager"; + +const UPDATE_ACTION = "Update and Restart"; + +function setup() { + const { sink, service } = createTelemetryHarness(); + const mementoManager = new MementoManager(new InMemoryMemento()); + const logger = createMockLogger(); + const container = { + getTelemetryService: () => service, + getLogger: () => logger, + getPathResolver: () => ({}), + getMementoManager: () => mementoManager, + getSecretsManager: () => ({}), + getCliManager: () => ({}), + getLoginCoordinator: () => ({}), + getDuplicateWorkspaceIpc: () => ({}), + getSpeedtestPanelFactory: () => ({}), + } as unknown as ServiceContainer; + const commands = new Commands( + container, + {} as CoderApi, + {} as DeploymentManager, + ); + commands.workspace = workspace({ outdated: true }); + commands.remoteWorkspaceClient = {} as CoderApi; + return { commands, sink }; +} + +describe("Commands.updateWorkspace", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("records an aborted update confirmation when the prompt is dismissed", async () => { + const { commands, sink } = setup(); + vi.mocked(vscode.window.showWarningMessage).mockResolvedValue(undefined); + + await commands.updateWorkspace(); + + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { + prompt: "confirmation", + result: "aborted", + }, + }); + expect(vscode.commands.executeCommand).not.toHaveBeenCalled(); + }); + + it("records success and reloads when the update confirmation is accepted", async () => { + const { commands, sink } = setup(); + vi.mocked(vscode.window.showWarningMessage).mockResolvedValue( + UPDATE_ACTION as never, + ); + + await commands.updateWorkspace(); + + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { + action: "update", + prompt: "confirmation", + result: "success", + }, + }); + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "workbench.action.reloadWindow", + ); + }); +}); diff --git a/test/unit/instrumentation/commands.test.ts b/test/unit/instrumentation/commands.test.ts index 6361d6a47..a927af636 100644 --- a/test/unit/instrumentation/commands.test.ts +++ b/test/unit/instrumentation/commands.test.ts @@ -63,12 +63,14 @@ describe("command instrumentation helpers", () => { const traces = new CommandTelemetry(service); await traces.workspacePicker("workspace_open", (telemetry) => { - telemetry.cancelled(3); - return Promise.resolve({ status: "cancelled" }); + const result = { status: "cancelled" } as const; + telemetry.finish(result, 3); + return Promise.resolve(result); }); await traces.workspacePicker("workspace_open", (telemetry) => { - telemetry.failed("fetch_failed", 0); - return Promise.resolve({ status: "failed", category: "fetch_failed" }); + const result = { status: "failed", category: "fetch_failed" } as const; + telemetry.finish(result, 0); + return Promise.resolve(result); }); const [cancelled, failed] = sink.eventsNamed("workspace.picker.prompted"); @@ -86,14 +88,14 @@ describe("command instrumentation helpers", () => { const traces = new CommandTelemetry(service); const selection = workspaceWithAgents(); - await traces.workspaceOpen("command", undefined, (telemetry) => - Promise.resolve( - telemetry.cancel("agent_picker", { workspace: selection.workspace }), - ), - ); - await traces.workspaceOpen("command", undefined, (telemetry) => - Promise.resolve(telemetry.fail("fetch_failed")), - ); + await traces.workspaceOpen("command", undefined, (telemetry) => { + telemetry.cancel("agent_picker", { workspace: selection.workspace }); + return Promise.resolve(false); + }); + await traces.workspaceOpen("command", undefined, (telemetry) => { + telemetry.fail("fetch_failed"); + return Promise.resolve(false); + }); const [cancelled, failed] = sink.eventsNamed("workspace.open"); expect(cancelled.properties).toMatchObject({ @@ -128,14 +130,12 @@ describe("command instrumentation helpers", () => { it("records diagnostic cancellation and failure categories", async () => { const { sink, service } = createTelemetryHarness(); const traces = new CommandTelemetry(service); - const failure = new Error("boom"); - await traces.diagnostic("support_bundle", (telemetry) => { telemetry.cancel("save_dialog"); return Promise.resolve(); }); await traces.diagnostic("support_bundle", (telemetry) => { - telemetry.fail(failure, "unsupported_cli"); + telemetry.fail("unsupported_cli"); return Promise.resolve(); }); @@ -158,23 +158,20 @@ describe("command instrumentation helpers", () => { const traces = new CommandTelemetry(service); await traces.diagnostic("speed_test", (telemetry) => { - const parsed = telemetry.speedtestSuccess( - JSON.stringify({ - overall: { + telemetry.speedtestSuccess({ + overall: { + start_time_seconds: 0, + end_time_seconds: 5, + throughput_mbits: 42, + }, + intervals: [ + { start_time_seconds: 0, end_time_seconds: 5, throughput_mbits: 42, }, - intervals: [ - { - start_time_seconds: 0, - end_time_seconds: 5, - throughput_mbits: 42, - }, - ], - }), - ); - expect(parsed.overall.throughput_mbits).toBe(42); + ], + }); return Promise.resolve(); }); diff --git a/test/unit/instrumentation/workspace.test.ts b/test/unit/instrumentation/workspace.test.ts index fd16629ef..1f01cce12 100644 --- a/test/unit/instrumentation/workspace.test.ts +++ b/test/unit/instrumentation/workspace.test.ts @@ -150,6 +150,41 @@ describe("WorkspaceOperationTelemetry", () => { }); }); }); + + describe("traceUpdateConfirmationPrompted", () => { + it("emits result=success with accepted action", async () => { + const { sink, instance: ops } = setup(newOps); + + const result = await ops.traceUpdateConfirmationPrompted(() => + Promise.resolve("Update and Restart"), + ); + + expect(result).toBe("Update and Restart"); + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { + action: "update", + prompt: "confirmation", + result: "success", + }, + }); + }); + + it("emits result=aborted when dismissed", async () => { + const { sink, instance: ops } = setup(newOps); + + const result = await ops.traceUpdateConfirmationPrompted(() => + Promise.resolve(undefined), + ); + + expect(result).toBeUndefined(); + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { + prompt: "confirmation", + result: "aborted", + }, + }); + }); + }); }); describe("WorkspaceStateTelemetry.observe", () => { diff --git a/test/unit/telemetry/export/command.test.ts b/test/unit/telemetry/export/command.test.ts index 922a9fcf5..91cba3f74 100644 --- a/test/unit/telemetry/export/command.test.ts +++ b/test/unit/telemetry/export/command.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; -import { runExportTelemetryCommand } from "@/telemetry/export/command"; +import { + runExportTelemetryCommand, + type ExportTelemetryObserver, +} from "@/telemetry/export/command"; import { collectTelemetryExport } from "@/telemetry/export/pipeline"; import { promptForExport, type ExportChoice } from "@/telemetry/export/prompts"; @@ -56,26 +59,33 @@ function setup( vi.mocked(collectTelemetryExport).mockResolvedValue(outcome.count); } + const observer: ExportTelemetryObserver = { + cancelled: vi.fn(), + failed: vi.fn(), + succeeded: vi.fn(), + }; + return { + observer, run: () => runExportTelemetryCommand( TELEMETRY_DIR, createMockLogger(), vi.fn(() => Promise.resolve(OK_FLUSH)), context, + observer, ), }; } describe("runExportTelemetryCommand", () => { it("does nothing when the user cancels the prompts", async () => { - const { run } = setup(); + const { observer, run } = setup(); vi.mocked(promptForExport).mockResolvedValue(undefined); - await expect(run()).resolves.toEqual({ - status: "cancelled", - stage: "prompt", - }); + await run(); + + expect(observer.cancelled).toHaveBeenCalledWith("prompt"); expect(collectTelemetryExport).not.toHaveBeenCalled(); expect(vscode.window.withProgress).not.toHaveBeenCalled(); @@ -122,13 +132,11 @@ describe("runExportTelemetryCommand", () => { [1, "Exported 1 telemetry event to"], [3, "Exported 3 telemetry events to"], ])("notifies with a pluralized %i-event count", async (count, message) => { - const { run } = setup({ outcome: { count } }); + const { observer, run } = setup({ outcome: { count } }); - await expect(run()).resolves.toEqual({ - status: "success", - eventCount: count, - format: "json", - }); + await run(); + + expect(observer.succeeded).toHaveBeenCalledWith("json", count); expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( `${message} ${OUTPUT_PATH}.`, @@ -139,10 +147,7 @@ describe("runExportTelemetryCommand", () => { it("reveals the file when the user clicks the action", async () => { const { run } = setup({ revealChoice: REVEAL_ACTION }); - await expect(run()).resolves.toMatchObject({ - status: "success", - eventCount: 2, - }); + await run(); expect(vscode.commands.executeCommand).toHaveBeenCalledWith( "revealFileInOS", @@ -164,10 +169,7 @@ describe("runExportTelemetryCommand", () => { new Error("no command"), ); - await expect(run()).resolves.toMatchObject({ - status: "success", - eventCount: 2, - }); + await run(); expect(vscode.window.showErrorMessage).not.toHaveBeenCalled(); }); @@ -175,13 +177,11 @@ describe("runExportTelemetryCommand", () => { describe("nothing to export", () => { it("reports that no events were found", async () => { - const { run } = setup({ outcome: { count: 0 } }); + const { observer, run } = setup({ outcome: { count: 0 } }); - await expect(run()).resolves.toEqual({ - status: "success", - eventCount: 0, - format: "json", - }); + await run(); + + expect(observer.succeeded).toHaveBeenCalledWith("json", 0); expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( "No telemetry events found for Last 24 hours.", @@ -192,9 +192,11 @@ describe("runExportTelemetryCommand", () => { describe("failure", () => { it("shows an error notification without throwing", async () => { const error = new Error("disk full"); - const { run } = setup({ outcome: { error } }); + const { observer, run } = setup({ outcome: { error } }); - await expect(run()).resolves.toEqual({ status: "failed", error }); + await run(); + + expect(observer.failed).toHaveBeenCalledOnce(); expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( "Telemetry export failed: disk full", @@ -206,12 +208,11 @@ describe("runExportTelemetryCommand", () => { const aborted = Object.assign(new Error("Aborted"), { name: "AbortError", }); - const { run } = setup({ outcome: { error: aborted } }); + const { observer, run } = setup({ outcome: { error: aborted } }); - await expect(run()).resolves.toEqual({ - status: "cancelled", - stage: "progress", - }); + await run(); + + expect(observer.cancelled).toHaveBeenCalledWith("progress"); expect(vscode.window.showErrorMessage).not.toHaveBeenCalled(); expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); From 35593a2be022a1723c93de4d7c21ad7001b2ebbc Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 10 Jun 2026 13:28:53 +0300 Subject: [PATCH 4/6] refactor: split command instrumentation and tighten telemetry API Split instrumentation/commands.ts into workspaceOpen.ts, diagnostics.ts, and shared outcomes.ts to mirror a future commands.ts split. Unify trace methods on the cancel/fail/succeed* trio, shorten trace names, and make ExportTelemetryObserver a required structural subset of DiagnosticTrace so callers pass their trace directly. Commands now keep business bodies in named run* methods instead of inline closures, the open source defaults to "command" rather than being inferred from argument presence, and the dead openWorkspace wrapper is gone. Restore workspaceName on workspace.update.prompted and workspace.start.prompted to match the other operation traces. Rename the telemetry-only test to updateWorkspace.telemetry.test.ts. --- src/commands.ts | 371 +++++++++--------- src/instrumentation/commands.ts | 299 -------------- src/instrumentation/diagnostics.ts | 77 ++++ src/instrumentation/outcomes.ts | 23 ++ src/instrumentation/workspace.ts | 89 ++--- src/instrumentation/workspaceOpen.ts | 208 ++++++++++ src/remote/workspaceStateMachine.ts | 10 +- src/telemetry/export/command.ts | 22 +- ...t.ts => updateWorkspace.telemetry.test.ts} | 0 test/unit/instrumentation/diagnostics.test.ts | 68 ++++ test/unit/instrumentation/workspace.test.ts | 41 +- ...commands.test.ts => workspaceOpen.test.ts} | 103 ++--- test/unit/telemetry/export/command.test.ts | 16 +- 13 files changed, 668 insertions(+), 659 deletions(-) delete mode 100644 src/instrumentation/commands.ts create mode 100644 src/instrumentation/diagnostics.ts create mode 100644 src/instrumentation/outcomes.ts create mode 100644 src/instrumentation/workspaceOpen.ts rename test/unit/command/{updateWorkspace.test.ts => updateWorkspace.telemetry.test.ts} (100%) create mode 100644 test/unit/instrumentation/diagnostics.test.ts rename test/unit/instrumentation/{commands.test.ts => workspaceOpen.test.ts} (55%) diff --git a/src/commands.ts b/src/commands.ts index 11fe0cfb2..42997101a 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -20,16 +20,19 @@ import { type AuthLogoutOutcome, } from "./instrumentation/auth"; import { - CommandTelemetry, - type DevcontainerMode, + DiagnosticTelemetry, type DiagnosticTrace, +} from "./instrumentation/diagnostics"; +import { WorkspaceOperationTelemetry } from "./instrumentation/workspace"; +import { + type DevcontainerMode, type WorkspaceOpenSource, + WorkspaceOpenTelemetry, type WorkspaceOpenTrace, type WorkspacePickerFailureCategory, type WorkspacePickerResult, type WorkspacePickerSource, -} from "./instrumentation/commands"; -import { WorkspaceOperationTelemetry } from "./instrumentation/workspace"; +} from "./instrumentation/workspaceOpen"; import { reportElapsedProgress, withCancellableProgress, @@ -42,10 +45,7 @@ import { } from "./remote/sshOverrides"; import { resolveCliAuth } from "./settings/cli"; import { appendVsCodeLogs } from "./supportBundle/appendVsCodeLogs"; -import { - runExportTelemetryCommand, - type ExportTelemetryObserver, -} from "./telemetry/export/command"; +import { runExportTelemetryCommand } from "./telemetry/export/command"; import { openInBrowser, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; import { parseSpeedtestResult } from "./webviews/speedtest/types"; @@ -135,7 +135,8 @@ export class Commands { private readonly speedtestPanelFactory: SpeedtestPanelFactory; private readonly telemetryService: TelemetryService; private readonly authTelemetry: AuthTelemetry; - private readonly commandTelemetry: CommandTelemetry; + private readonly diagnosticTelemetry: DiagnosticTelemetry; + private readonly openTelemetry: WorkspaceOpenTelemetry; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -154,7 +155,8 @@ export class Commands { private readonly deploymentManager: DeploymentManager, ) { this.telemetryService = serviceContainer.getTelemetryService(); - this.commandTelemetry = new CommandTelemetry(this.telemetryService); + this.diagnosticTelemetry = new DiagnosticTelemetry(this.telemetryService); + this.openTelemetry = new WorkspaceOpenTelemetry(this.telemetryService); this.logger = serviceContainer.getLogger(); this.pathResolver = serviceContainer.getPathResolver(); this.mementoManager = serviceContainer.getMementoManager(); @@ -266,7 +268,7 @@ export class Commands { * editor document. Can be triggered from the sidebar or command palette. */ public async speedTest(item?: OpenableTreeItem): Promise { - await this.commandTelemetry.diagnostic("speed_test", (telemetry) => + await this.diagnosticTelemetry.trace("speed_test", (telemetry) => this.runSpeedTest(item, telemetry), ); } @@ -304,7 +306,7 @@ export class Commands { return; } const seconds = Number(input.trim()); - telemetry.speedtestRequestedDuration(seconds); + telemetry.setRequestedDuration(seconds); const result = await withCancellableProgress( async ({ signal, progress }) => { @@ -352,7 +354,7 @@ export class Commands { try { const parsed = parseSpeedtestResult(result.value); - telemetry.speedtestSuccess(parsed); + telemetry.succeedSpeedtest(parsed); this.speedtestPanelFactory.show({ result: parsed, rawJson: result.value, @@ -376,7 +378,7 @@ export class Commands { } public async supportBundle(item?: OpenableTreeItem): Promise { - await this.commandTelemetry.diagnostic("support_bundle", (telemetry) => + await this.diagnosticTelemetry.trace("support_bundle", (telemetry) => this.runSupportBundle(item, telemetry), ); } @@ -470,24 +472,18 @@ export class Commands { } public async exportTelemetry(): Promise { - await this.commandTelemetry.diagnostic("export_telemetry", (telemetry) => + await this.diagnosticTelemetry.trace("export_telemetry", (telemetry) => this.runExportTelemetry(telemetry), ); } private async runExportTelemetry(telemetry: DiagnosticTrace): Promise { - const observer: ExportTelemetryObserver = { - cancelled: (stage) => telemetry.cancel(stage), - failed: () => telemetry.fail(), - succeeded: (format, eventCount) => - telemetry.exportSuccess(format, eventCount), - }; await runExportTelemetryCommand( this.pathResolver.getTelemetryPath(), this.logger, () => this.telemetryService.flush(), this.telemetryService.getContext(), - observer, + telemetry, ); } @@ -783,51 +779,18 @@ export class Commands { */ public async openFromSidebar(item: OpenableTreeItem): Promise { if (item) { - const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; - if (!baseUrl) { - throw new Error("You are not logged in"); - } + const baseUrl = this.requireExtensionBaseUrl(); if (item instanceof AgentTreeItem) { - await this.commandTelemetry.workspaceOpen( + await this.openTelemetry.traceOpen( "sidebar_agent", { workspace: item.workspace, agent: item.agent }, - async (telemetry) => { - const result = await this.openWorkspaceInternal( - baseUrl, - item.workspace, - item.agent, - { openRecent: true }, - ); - return recordWorkspaceOpenResult( - telemetry, - { workspace: item.workspace, agent: item.agent }, - result, - ); - }, + (telemetry) => this.runOpenAgentItem(baseUrl, item, telemetry), ); } else if (item instanceof WorkspaceTreeItem) { - await this.commandTelemetry.workspaceOpen( + await this.openTelemetry.traceOpen( "sidebar_workspace", { workspace: item.workspace }, - async (telemetry) => { - const agents = await this.extractAgentsWithFallback(item.workspace); - const agent = await maybeAskAgent(agents); - if (!agent) { - telemetry.cancel("agent_picker", { - workspace: item.workspace, - }); - return false; - } - const selection = { workspace: item.workspace, agent }; - telemetry.select(selection); - const result = await this.openWorkspaceInternal( - baseUrl, - item.workspace, - agent, - { openRecent: true }, - ); - return recordWorkspaceOpenResult(telemetry, selection, result); - }, + (telemetry) => this.runOpenWorkspaceItem(baseUrl, item, telemetry), ); } else { throw new TypeError("Unable to open unknown sidebar item"); @@ -839,6 +802,45 @@ export class Commands { } } + private async runOpenAgentItem( + baseUrl: string, + item: AgentTreeItem, + telemetry: WorkspaceOpenTrace, + ): Promise { + const result = await this.openWorkspace( + baseUrl, + item.workspace, + item.agent, + { + openRecent: true, + }, + ); + return recordOpenResult( + telemetry, + { workspace: item.workspace, agent: item.agent }, + result, + ); + } + + private async runOpenWorkspaceItem( + baseUrl: string, + item: WorkspaceTreeItem, + telemetry: WorkspaceOpenTrace, + ): Promise { + const agents = await this.extractAgentsWithFallback(item.workspace); + const agent = await maybeAskAgent(agents); + if (!agent) { + telemetry.cancel("agent_picker", { workspace: item.workspace }); + return false; + } + const selection = { workspace: item.workspace, agent }; + telemetry.select(selection); + const result = await this.openWorkspace(baseUrl, item.workspace, agent, { + openRecent: true, + }); + return recordOpenResult(telemetry, selection, result); + } + public async openAppStatus(app: { name?: string; url?: string; @@ -876,67 +878,60 @@ export class Commands { * cannot be found. */ public async open(options: OpenOptions = {}): Promise { + const { source = "command", ...rest } = { ...openDefaults, ...options }; + return this.openTelemetry.traceOpen(source, undefined, (telemetry) => + this.runOpen(rest, telemetry), + ); + } + + private async runOpen( + options: Omit, + telemetry: WorkspaceOpenTrace, + ): Promise { const { workspaceOwner, workspaceName, agentName, folderPath, openRecent, - source = workspaceOwner && workspaceName ? "uri" : "command", useDefaultDirectory, - } = { ...openDefaults, ...options }; - - return this.commandTelemetry.workspaceOpen( - source, - undefined, - async (telemetry) => { - const baseUrl = - this.extensionClient.getAxiosInstance().defaults.baseURL; - if (!baseUrl) { - throw new Error("You are not logged in"); - } + } = options; + const baseUrl = this.requireExtensionBaseUrl(); - let workspace: Workspace; - if (workspaceOwner && workspaceName) { - workspace = await this.extensionClient.getWorkspaceByOwnerAndName( - workspaceOwner, - workspaceName, - ); - } else { - const pick = await this.pickWorkspace("workspace_open"); - if (pick.status === "cancelled") { - telemetry.cancel("workspace_picker"); - return false; - } - if (pick.status === "failed") { - telemetry.fail(pick.category); - return false; - } - workspace = pick.workspace; - } + let workspace: Workspace; + if (workspaceOwner && workspaceName) { + workspace = await this.extensionClient.getWorkspaceByOwnerAndName( + workspaceOwner, + workspaceName, + ); + } else { + const pick = await this.pickWorkspace("workspace_open"); + if (pick.status === "cancelled") { + telemetry.cancel("workspace_picker"); + return false; + } + if (pick.status === "failed") { + telemetry.fail(pick.category); + return false; + } + workspace = pick.workspace; + } - const agents = await this.extractAgentsWithFallback(workspace); - const agent = await maybeAskAgent(agents, agentName); - if (!agent) { - telemetry.cancel("agent_picker", { workspace }); - return false; - } - const selection = { workspace, agent }; - telemetry.select(selection); + const agents = await this.extractAgentsWithFallback(workspace); + const agent = await maybeAskAgent(agents, agentName); + if (!agent) { + telemetry.cancel("agent_picker", { workspace }); + return false; + } + const selection = { workspace, agent }; + telemetry.select(selection); - const result = await this.openWorkspaceInternal( - baseUrl, - workspace, - agent, - { - folderPath, - openRecent, - useDefaultDirectory, - }, - ); - return recordWorkspaceOpenResult(telemetry, selection, result); - }, - ); + const result = await this.openWorkspace(baseUrl, workspace, agent, { + folderPath, + openRecent, + useDefaultDirectory, + }); + return recordOpenResult(telemetry, selection, result); } /** @@ -956,59 +951,74 @@ export class Commands { const mode: DevcontainerMode = localWorkspaceFolder ? "dev_container" : "attached_container"; - await this.commandTelemetry.devcontainerOpen(mode, async () => { - const baseUrl = this.extensionClient.getAxiosInstance().defaults.baseURL; - if (!baseUrl) { - throw new Error("You are not logged in"); - } - - const remoteAuthority = toRemoteAuthority( - baseUrl, + await this.openTelemetry.traceDevcontainer(mode, () => + this.runOpenDevContainer( workspaceOwner, workspaceName, workspaceAgent, - ); + devContainerName, + devContainerFolder, + localWorkspaceFolder, + localConfigFile, + ), + ); + } - const hostPath = localWorkspaceFolder || undefined; - const configFile = - hostPath && localConfigFile - ? { - path: localConfigFile, - scheme: "vscode-fileHost", - } - : undefined; - const devContainer = Buffer.from( - JSON.stringify({ - containerName: devContainerName, - hostPath, - configFile, - localDocker: false, - }), - "utf-8", - ).toString("hex"); + private async runOpenDevContainer( + workspaceOwner: string, + workspaceName: string, + workspaceAgent: string, + devContainerName: string, + devContainerFolder: string, + localWorkspaceFolder: string, + localConfigFile: string, + ): Promise { + const baseUrl = this.requireExtensionBaseUrl(); - const type = localWorkspaceFolder - ? "dev-container" - : "attached-container"; - const devContainerAuthority = `${type}+${devContainer}@${remoteAuthority}`; + const remoteAuthority = toRemoteAuthority( + baseUrl, + workspaceOwner, + workspaceName, + workspaceAgent, + ); - let newWindow = true; - if (!vscode.workspace.workspaceFolders?.length) { - newWindow = false; - } + const hostPath = localWorkspaceFolder || undefined; + const configFile = + hostPath && localConfigFile + ? { + path: localConfigFile, + scheme: "vscode-fileHost", + } + : undefined; + const devContainer = Buffer.from( + JSON.stringify({ + containerName: devContainerName, + hostPath, + configFile, + localDocker: false, + }), + "utf-8", + ).toString("hex"); - // Only set the memento when opening a new folder - await this.mementoManager.setStartupMode("start"); - await vscode.commands.executeCommand( - "vscode.openFolder", - vscode.Uri.from({ - scheme: "vscode-remote", - authority: devContainerAuthority, - path: devContainerFolder, - }), - newWindow, - ); - }); + const type = localWorkspaceFolder ? "dev-container" : "attached-container"; + const devContainerAuthority = `${type}+${devContainer}@${remoteAuthority}`; + + let newWindow = true; + if (!vscode.workspace.workspaceFolders?.length) { + newWindow = false; + } + + // Only set the memento when opening a new folder + await this.mementoManager.setStartupMode("start"); + await vscode.commands.executeCommand( + "vscode.openFolder", + vscode.Uri.from({ + scheme: "vscode-remote", + authority: devContainerAuthority, + path: devContainerFolder, + }), + newWindow, + ); } /** @@ -1033,17 +1043,16 @@ export class Commands { this.telemetryService, workspaceName, ); - const action = await operationTelemetry.traceUpdateConfirmationPrompted( - async () => - vscodeProposed.window.showWarningMessage( - "Update Workspace", - { - useCustom: true, - modal: true, - detail: `Update ${workspaceName} to the latest version?\n\nUpdating will restart your workspace which stops any running processes and may result in the loss of unsaved work.`, - }, - UPDATE_AND_RESTART_ACTION, - ), + const action = await operationTelemetry.traceConfirmationPrompt(async () => + vscodeProposed.window.showWarningMessage( + "Update Workspace", + { + useCustom: true, + modal: true, + detail: `Update ${workspaceName} to the latest version?\n\nUpdating will restart your workspace which stops any running processes and may result in the loss of unsaved work.`, + }, + UPDATE_AND_RESTART_ACTION, + ), ); if (action !== UPDATE_AND_RESTART_ACTION) { return; @@ -1228,8 +1237,8 @@ export class Commands { ); quickPick.show(); - return this.commandTelemetry - .workspacePicker( + return this.openTelemetry + .tracePicker( source, async (telemetry) => new Promise((resolve) => { @@ -1309,25 +1318,7 @@ export class Commands { * If provided, folderPath is always used, otherwise expanded_directory from * the agent is used. */ - async openWorkspace( - baseUrl: string, - workspace: Workspace, - agent: WorkspaceAgent, - options: Pick< - OpenOptions, - "folderPath" | "openRecent" | "useDefaultDirectory" - > = {}, - ): Promise { - const result = await this.openWorkspaceInternal( - baseUrl, - workspace, - agent, - options, - ); - return result.status === "opened"; - } - - private async openWorkspaceInternal( + private async openWorkspace( baseUrl: string, workspace: Workspace, agent: WorkspaceAgent, @@ -1459,7 +1450,7 @@ export class Commands { } } -function recordWorkspaceOpenResult( +function recordOpenResult( telemetry: WorkspaceOpenTrace, selection: { readonly workspace: Workspace; readonly agent: WorkspaceAgent }, result: OpenWorkspaceResult, diff --git a/src/instrumentation/commands.ts b/src/instrumentation/commands.ts deleted file mode 100644 index 53b7bcf0d..000000000 --- a/src/instrumentation/commands.ts +++ /dev/null @@ -1,299 +0,0 @@ -import { extractAgents } from "../api/api-helper"; -import { isAbortError } from "../error/errorUtils"; - -import type { - Workspace, - WorkspaceAgent, -} from "coder/site/src/api/typesGenerated"; - -import type { SpeedtestResult } from "@repo/shared"; - -import type { TelemetryReporter } from "../telemetry/reporter"; -import type { Span } from "../telemetry/span"; - -export type WorkspaceOpenSource = - | "command" - | "sidebar_agent" - | "sidebar_workspace" - | "sidebar_fallback" - | "uri"; - -export type WorkspacePickerSource = "workspace_open" | "diagnostic"; -export type WorkspacePickerFailureCategory = "fetch_failed"; -type AbortableFailureCategory = "aborted" | "error"; -export type WorkspaceOpenFailureCategory = - | WorkspacePickerFailureCategory - | AbortableFailureCategory; -export type WorkspacePickerResult = - | { readonly status: "selected"; readonly workspace: Workspace } - | { readonly status: "cancelled" } - | { - readonly status: "failed"; - readonly category: WorkspacePickerFailureCategory; - }; -export type DiagnosticCommand = - | "speed_test" - | "support_bundle" - | "export_telemetry"; -export type DiagnosticFailureCategory = - | WorkspacePickerFailureCategory - | "parse_error" - | "unsupported_cli" - | "error"; -export type DevcontainerMode = "dev_container" | "attached_container"; -export type DevcontainerFailureCategory = AbortableFailureCategory; -export type WorkspaceOpenCancelStage = - | "workspace_picker" - | "agent_picker" - | "recent_folder_picker"; -export type DiagnosticCancelStage = - | "workspace_picker" - | "input" - | "prompt" - | "save_dialog" - | "progress"; - -export interface WorkspaceOpenSelection { - readonly workspace: Workspace; - readonly agent?: WorkspaceAgent; -} - -export interface WorkspacePickerTrace { - finish(result: WorkspacePickerResult, resultCount: number): void; -} - -export interface WorkspaceOpenTrace { - select(selection: WorkspaceOpenSelection): void; - cancel( - stage: WorkspaceOpenCancelStage, - selection?: WorkspaceOpenSelection, - ): void; - fail(category: WorkspaceOpenFailureCategory): void; - handoff(kind: "folder" | "empty_window"): void; -} - -export interface DiagnosticTrace { - cancel(stage: DiagnosticCancelStage): void; - fail(category?: DiagnosticFailureCategory): void; - speedtestRequestedDuration(seconds: number): void; - speedtestSuccess(result: SpeedtestResult): void; - exportSuccess(format: string, eventCount: number): void; -} - -export class CommandTelemetry { - public constructor(private readonly telemetry: TelemetryReporter) {} - - public diagnostic( - command: DiagnosticCommand, - fn: (trace: DiagnosticTrace) => Promise, - ): Promise { - return this.telemetry.trace( - "command.diagnostic.completed", - (span) => fn(new SpanDiagnosticTrace(span)), - { command }, - ); - } - - public async workspaceOpen( - source: WorkspaceOpenSource, - selection: WorkspaceOpenSelection | undefined, - fn: (trace: WorkspaceOpenTrace) => Promise, - ): Promise { - let deferredError: unknown; - let failed = false; - const opened = await this.telemetry.trace( - "workspace.open", - async (span) => { - const trace = new SpanWorkspaceOpenTrace(span); - if (selection) { - trace.select(selection); - } - try { - const result = await fn(trace); - if (!result) { - span.markAborted(); - } - return result; - } catch (error) { - failed = true; - deferredError = error; - trace.fail(categorizeAbortableFailure(error)); - return false; - } - }, - { source }, - ); - if (failed) { - throw deferredError; - } - return opened; - } - - public workspacePicker( - source: WorkspacePickerSource, - fn: (trace: WorkspacePickerTrace) => Promise, - ): Promise { - return this.telemetry.trace( - "workspace.picker.prompted", - (span) => fn(new SpanWorkspacePickerTrace(span)), - { source }, - ); - } - - public async devcontainerOpen( - mode: DevcontainerMode, - fn: () => Promise, - ): Promise { - let deferredError: unknown; - let failed = false; - await this.telemetry.trace( - "workspace.dev_container.open", - async (span) => { - try { - await fn(); - } catch (error) { - failed = true; - deferredError = error; - recordFailure(span, categorizeAbortableFailure(error)); - } - }, - { mode }, - ); - if (failed) { - throw deferredError; - } - } -} - -class SpanWorkspacePickerTrace implements WorkspacePickerTrace { - public constructor(private readonly span: Span) {} - - public finish(result: WorkspacePickerResult, resultCount: number): void { - recordWorkspacePickerResult(this.span, result, resultCount); - } -} - -class SpanWorkspaceOpenTrace implements WorkspaceOpenTrace { - public constructor(private readonly span: Span) {} - - public select(selection: WorkspaceOpenSelection): void { - recordWorkspaceContext(this.span, selection.workspace, selection.agent); - } - - public cancel( - stage: WorkspaceOpenCancelStage, - selection?: WorkspaceOpenSelection, - ): void { - recordWorkspaceOpenCancelled(this.span, stage, selection); - } - - public fail(category: WorkspaceOpenFailureCategory): void { - recordFailure(this.span, category); - } - - public handoff(kind: "folder" | "empty_window"): void { - this.span.setProperty("handoff", kind); - } -} - -class SpanDiagnosticTrace implements DiagnosticTrace { - public constructor(private readonly span: Span) {} - - public cancel(stage: DiagnosticCancelStage): void { - recordCancelled(this.span, stage); - } - - public fail(category: DiagnosticFailureCategory = "error"): void { - recordFailure(this.span, category); - } - - public speedtestRequestedDuration(seconds: number): void { - this.span.setMeasurement("requested_duration_seconds", seconds); - } - - public speedtestSuccess(result: SpeedtestResult): void { - recordSpeedtestResult(this.span, result); - } - - public exportSuccess(format: string, eventCount: number): void { - this.span.setProperty("format", format); - this.span.setMeasurement("event_count", eventCount); - } -} - -function recordWorkspaceContext( - span: Span, - workspace: Workspace, - agent?: WorkspaceAgent, -): void { - const agents = extractAgents(workspace.latest_build.resources); - span.setProperty("workspace_status", workspace.latest_build.status); - span.setProperty("workspace_outdated", workspace.outdated); - span.setMeasurement("agent_count", agents.length); - span.setMeasurement( - "connected_agent_count", - agents.filter((candidate) => candidate.status === "connected").length, - ); - if (!agent) { - return; - } - span.setProperty("agent_status", agent.status); - span.setProperty("agent_lifecycle_state", agent.lifecycle_state); -} - -function recordWorkspacePickerResult( - span: Span, - result: WorkspacePickerResult, - resultCount: number, -): void { - span.setMeasurement("workspace_count", resultCount); - if (result.status === "selected") { - recordWorkspaceContext(span, result.workspace); - return; - } - if (result.status === "failed") { - recordFailure(span, result.category); - return; - } - span.markAborted(); -} - -function recordWorkspaceOpenCancelled( - span: Span, - stage: WorkspaceOpenCancelStage, - selection?: WorkspaceOpenSelection, -): void { - span.setProperty("cancel_stage", stage); - if (selection) { - recordWorkspaceContext(span, selection.workspace, selection.agent); - } - span.markAborted(); -} - -function recordCancelled(span: Span, stage: DiagnosticCancelStage): void { - span.setProperty("cancel_stage", stage); - span.markAborted(); -} - -function recordFailure( - span: Span, - category: - | DiagnosticFailureCategory - | WorkspaceOpenFailureCategory - | DevcontainerFailureCategory, -): void { - span.setProperty("failure_category", category); - span.markFailure(); -} - -function recordSpeedtestResult(span: Span, result: SpeedtestResult): void { - span.setMeasurement("interval_count", result.intervals.length); - span.setMeasurement("throughput_mbits", result.overall.throughput_mbits); -} - -function categorizeAbortableFailure(error: unknown): AbortableFailureCategory { - if (isAbortError(error)) { - return "aborted"; - } - return "error"; -} diff --git a/src/instrumentation/diagnostics.ts b/src/instrumentation/diagnostics.ts new file mode 100644 index 000000000..11fe2f3b7 --- /dev/null +++ b/src/instrumentation/diagnostics.ts @@ -0,0 +1,77 @@ +import { recordCancelled, recordFailure } from "./outcomes"; + +import type { SpeedtestResult } from "@repo/shared"; + +import type { TelemetryReporter } from "../telemetry/reporter"; +import type { Span } from "../telemetry/span"; + +import type { WorkspacePickerFailureCategory } from "./workspaceOpen"; + +export type DiagnosticCommand = + | "speed_test" + | "support_bundle" + | "export_telemetry"; +export type DiagnosticFailureCategory = + | WorkspacePickerFailureCategory + | "parse_error" + | "unsupported_cli" + | "error"; +export type DiagnosticCancelStage = + | "workspace_picker" + | "input" + | "prompt" + | "save_dialog" + | "progress"; + +export interface DiagnosticTrace { + cancel(stage: DiagnosticCancelStage): void; + fail(category?: DiagnosticFailureCategory): void; + setRequestedDuration(seconds: number): void; + succeedSpeedtest(result: SpeedtestResult): void; + succeedExport(format: string, eventCount: number): void; +} + +/** Emits `command.diagnostic.completed` around each diagnostic command. */ +export class DiagnosticTelemetry { + public constructor(private readonly telemetry: TelemetryReporter) {} + + public trace( + command: DiagnosticCommand, + fn: (trace: DiagnosticTrace) => Promise, + ): Promise { + return this.telemetry.trace( + "command.diagnostic.completed", + (span) => fn(new SpanDiagnosticTrace(span)), + { command }, + ); + } +} + +class SpanDiagnosticTrace implements DiagnosticTrace { + public constructor(private readonly span: Span) {} + + public cancel(stage: DiagnosticCancelStage): void { + recordCancelled(this.span, stage); + } + + public fail(category: DiagnosticFailureCategory = "error"): void { + recordFailure(this.span, category); + } + + public setRequestedDuration(seconds: number): void { + this.span.setMeasurement("requested_duration_seconds", seconds); + } + + public succeedSpeedtest(result: SpeedtestResult): void { + this.span.setMeasurement("interval_count", result.intervals.length); + this.span.setMeasurement( + "throughput_mbits", + result.overall.throughput_mbits, + ); + } + + public succeedExport(format: string, eventCount: number): void { + this.span.setProperty("format", format); + this.span.setMeasurement("event_count", eventCount); + } +} diff --git a/src/instrumentation/outcomes.ts b/src/instrumentation/outcomes.ts new file mode 100644 index 000000000..5568e61b0 --- /dev/null +++ b/src/instrumentation/outcomes.ts @@ -0,0 +1,23 @@ +import { isAbortError } from "../error/errorUtils"; + +import type { Span } from "../telemetry/span"; + +export type AbortableFailureCategory = "aborted" | "error"; + +/** Records a categorized failure without attaching raw error details. */ +export function recordFailure(span: Span, category: string): void { + span.setProperty("failure_category", category); + span.markFailure(); +} + +/** Records the stage at which the user backed out and aborts the span. */ +export function recordCancelled(span: Span, stage: string): void { + span.setProperty("cancel_stage", stage); + span.markAborted(); +} + +export function categorizeAbortableFailure( + error: unknown, +): AbortableFailureCategory { + return isAbortError(error) ? "aborted" : "error"; +} diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index 3b3db5b61..5bc375039 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -166,19 +166,19 @@ export class WorkspaceOperationTelemetry { private readonly workspaceName: string, ) {} - public traceUpdateTriggered(fn: () => Promise): Promise { + public traceUpdate(fn: () => Promise): Promise { return this.telemetry.trace("workspace.update.triggered", fn, { workspaceName: this.workspaceName, }); } - public traceStartTriggered(fn: () => Promise): Promise { + public traceStart(fn: () => Promise): Promise { return this.telemetry.trace("workspace.start.triggered", fn, { workspaceName: this.workspaceName, }); } - public async traceStartPrompted( + public async traceStartPrompt( outdated: boolean, fn: () => Promise, ): Promise { @@ -193,7 +193,7 @@ export class WorkspaceOperationTelemetry { span.setProperty("action", action); return action; }, - { update_offered: outdated }, + { workspaceName: this.workspaceName, update_offered: outdated }, ); } @@ -201,61 +201,52 @@ export class WorkspaceOperationTelemetry { * Records dismissal as `result: "aborted"`. The framework treats any throw * as `result: "error"`, so we return inside the span and rethrow outside. */ - public traceUpdatePrompted( + public async traceParametersPrompt( fn: () => Promise, ): Promise { - return this.traceUpdatePrompt("parameters", fn, { - isCancelled: (error) => error instanceof WorkspaceUpdateCancelledError, - cancelledValue: () => [], - }); - } - - public traceUpdateConfirmationPrompted( - fn: () => Promise, - ): Promise { - return this.traceUpdatePrompt("confirmation", fn, { - isCancelled: (value) => value === undefined, - recordAccepted: (span) => span.setProperty("action", "update"), - }); - } - - private async traceUpdatePrompt( - prompt: WorkspaceUpdatePrompt, - fn: () => Promise, - options: { - readonly isCancelled: (value: unknown) => boolean; - readonly cancelledValue?: () => T; - readonly recordAccepted?: (span: Span, value: T) => void; - }, - ): Promise { - let cancelledError: Error | undefined; - const cancelledValue = options.cancelledValue; - const result = await this.telemetry.trace( - "workspace.update.prompted", + let cancelled: WorkspaceUpdateCancelledError | undefined; + const parameters = await this.traceUpdatePrompt( + "parameters", async (span) => { try { - const value = await fn(); - if (options.isCancelled(value)) { - span.markAborted(); - } else { - options.recordAccepted?.(span, value); - } - return value; + return await fn(); } catch (error) { - if (error instanceof Error && options.isCancelled(error)) { + if (error instanceof WorkspaceUpdateCancelledError) { span.markAborted(); - cancelledError = error; - if (!cancelledValue) { - throw error; - } - return cancelledValue(); + cancelled = error; + return []; } throw error; } }, - { prompt }, ); - if (cancelledError !== undefined) throw cancelledError; - return result; + if (cancelled) { + throw cancelled; + } + return parameters; + } + + public traceConfirmationPrompt( + fn: () => Promise, + ): Promise { + return this.traceUpdatePrompt("confirmation", async (span) => { + const value = await fn(); + if (value === undefined) { + span.markAborted(); + return undefined; + } + span.setProperty("action", "update"); + return value; + }); + } + + private traceUpdatePrompt( + prompt: WorkspaceUpdatePrompt, + fn: (span: Span) => Promise, + ): Promise { + return this.telemetry.trace("workspace.update.prompted", fn, { + prompt, + workspaceName: this.workspaceName, + }); } } diff --git a/src/instrumentation/workspaceOpen.ts b/src/instrumentation/workspaceOpen.ts new file mode 100644 index 000000000..b6fcaa2b8 --- /dev/null +++ b/src/instrumentation/workspaceOpen.ts @@ -0,0 +1,208 @@ +import { extractAgents } from "../api/api-helper"; + +import { + type AbortableFailureCategory, + categorizeAbortableFailure, + recordCancelled, + recordFailure, +} from "./outcomes"; + +import type { + Workspace, + WorkspaceAgent, +} from "coder/site/src/api/typesGenerated"; + +import type { CallerProperties } from "../telemetry/event"; +import type { TelemetryReporter } from "../telemetry/reporter"; +import type { Span } from "../telemetry/span"; + +export type WorkspaceOpenSource = + | "command" + | "sidebar_agent" + | "sidebar_workspace" + | "sidebar_fallback" + | "uri"; + +export type WorkspacePickerSource = "workspace_open" | "diagnostic"; +export type WorkspacePickerFailureCategory = "fetch_failed"; +export type WorkspaceOpenFailureCategory = + | WorkspacePickerFailureCategory + | AbortableFailureCategory; +export type WorkspacePickerResult = + | { readonly status: "selected"; readonly workspace: Workspace } + | { readonly status: "cancelled" } + | { + readonly status: "failed"; + readonly category: WorkspacePickerFailureCategory; + }; +export type DevcontainerMode = "dev_container" | "attached_container"; +export type WorkspaceOpenCancelStage = + | "workspace_picker" + | "agent_picker" + | "recent_folder_picker"; + +export interface WorkspaceOpenSelection { + readonly workspace: Workspace; + readonly agent?: WorkspaceAgent; +} + +export interface WorkspacePickerTrace { + finish(result: WorkspacePickerResult, resultCount: number): void; +} + +export interface WorkspaceOpenTrace { + select(selection: WorkspaceOpenSelection): void; + cancel( + stage: WorkspaceOpenCancelStage, + selection?: WorkspaceOpenSelection, + ): void; + fail(category: WorkspaceOpenFailureCategory): void; + handoff(kind: "folder" | "empty_window"): void; +} + +/** + * Emits the spans around opening a workspace: `workspace.open`, + * `workspace.picker.prompted`, and `workspace.dev_container.open`. + */ +export class WorkspaceOpenTelemetry { + public constructor(private readonly telemetry: TelemetryReporter) {} + + public traceOpen( + source: WorkspaceOpenSource, + selection: WorkspaceOpenSelection | undefined, + fn: (trace: WorkspaceOpenTrace) => Promise, + ): Promise { + return this.traceRethrowing( + "workspace.open", + { source }, + false, + async (span) => { + const trace = new SpanWorkspaceOpenTrace(span); + if (selection) { + trace.select(selection); + } + const opened = await fn(trace); + if (!opened) { + span.markAborted(); + } + return opened; + }, + ); + } + + public tracePicker( + source: WorkspacePickerSource, + fn: (trace: WorkspacePickerTrace) => Promise, + ): Promise { + return this.telemetry.trace( + "workspace.picker.prompted", + (span) => fn(new SpanWorkspacePickerTrace(span)), + { source }, + ); + } + + public async traceDevcontainer( + mode: DevcontainerMode, + fn: () => Promise, + ): Promise { + await this.traceRethrowing( + "workspace.dev_container.open", + { mode }, + undefined, + fn, + ); + } + + /** + * Runs `fn` inside the span, recording a thrown error as a categorized + * failure without its raw details, then rethrows outside the span. + */ + private async traceRethrowing( + eventName: string, + properties: CallerProperties, + fallback: T, + fn: (span: Span) => Promise, + ): Promise { + let thrown: { readonly error: unknown } | undefined; + const result = await this.telemetry.trace( + eventName, + async (span) => { + try { + return await fn(span); + } catch (error) { + thrown = { error }; + recordFailure(span, categorizeAbortableFailure(error)); + return fallback; + } + }, + properties, + ); + if (thrown) { + throw thrown.error; + } + return result; + } +} + +class SpanWorkspacePickerTrace implements WorkspacePickerTrace { + public constructor(private readonly span: Span) {} + + public finish(result: WorkspacePickerResult, resultCount: number): void { + this.span.setMeasurement("workspace_count", resultCount); + if (result.status === "selected") { + recordWorkspaceContext(this.span, result.workspace); + return; + } + if (result.status === "failed") { + recordFailure(this.span, result.category); + return; + } + this.span.markAborted(); + } +} + +class SpanWorkspaceOpenTrace implements WorkspaceOpenTrace { + public constructor(private readonly span: Span) {} + + public select(selection: WorkspaceOpenSelection): void { + recordWorkspaceContext(this.span, selection.workspace, selection.agent); + } + + public cancel( + stage: WorkspaceOpenCancelStage, + selection?: WorkspaceOpenSelection, + ): void { + if (selection) { + recordWorkspaceContext(this.span, selection.workspace, selection.agent); + } + recordCancelled(this.span, stage); + } + + public fail(category: WorkspaceOpenFailureCategory): void { + recordFailure(this.span, category); + } + + public handoff(kind: "folder" | "empty_window"): void { + this.span.setProperty("handoff", kind); + } +} + +function recordWorkspaceContext( + span: Span, + workspace: Workspace, + agent?: WorkspaceAgent, +): void { + const agents = extractAgents(workspace.latest_build.resources); + span.setProperty("workspace_status", workspace.latest_build.status); + span.setProperty("workspace_outdated", workspace.outdated); + span.setMeasurement("agent_count", agents.length); + span.setMeasurement( + "connected_agent_count", + agents.filter((candidate) => candidate.status === "connected").length, + ); + if (!agent) { + return; + } + span.setProperty("agent_status", agent.status); + span.setProperty("agent_lifecycle_state", agent.lifecycle_state); +} diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index 8cd7217f9..5713fd44b 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -289,7 +289,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { mode: this.startupMode, status: workspace.latest_build.status, }); - await this.operationTelemetry.traceStartTriggered(() => + await this.operationTelemetry.traceStart(() => startWorkspace(this.buildCliContext(workspace)), ); this.logger.info(`${workspaceName} start initiated`); @@ -309,10 +309,10 @@ export class WorkspaceStateMachine implements vscode.Disposable { status: workspace.latest_build.status, }); try { - const parameters = await this.operationTelemetry.traceUpdatePrompted(() => - collectUpdateParameters(this.workspaceClient, workspace), + const parameters = await this.operationTelemetry.traceParametersPrompt( + () => collectUpdateParameters(this.workspaceClient, workspace), ); - this.workspace = await this.operationTelemetry.traceUpdateTriggered(() => + this.workspace = await this.operationTelemetry.traceUpdate(() => updateWorkspace(this.buildCliContext(workspace), parameters), ); this.logger.info(`${workspaceName} update initiated`); @@ -337,7 +337,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspaceName: string, outdated: boolean, ): Promise<"start" | "update" | undefined> { - return this.operationTelemetry.traceStartPrompted(outdated, async () => { + return this.operationTelemetry.traceStartPrompt(outdated, async () => { const buttons = outdated ? ["Start", "Update and Start"] : ["Start"]; const action = await vscodeProposed.window.showInformationMessage( `The workspace ${workspaceName} is not running. How would you like to proceed?`, diff --git a/src/telemetry/export/command.ts b/src/telemetry/export/command.ts index 833138288..7ff4dcc91 100644 --- a/src/telemetry/export/command.ts +++ b/src/telemetry/export/command.ts @@ -29,10 +29,14 @@ const PROGRESS_OPTIONS = { cancellable: true, } as const; +/** + * Outcome hooks for the caller's telemetry span. `DiagnosticTrace` satisfies + * this shape, so command callers can pass their trace directly. + */ export interface ExportTelemetryObserver { - cancelled(stage: "prompt" | "progress"): void; - failed(): void; - succeeded(format: ExportFormat, eventCount: number): void; + cancel(stage: "prompt" | "progress"): void; + fail(): void; + succeedExport(format: ExportFormat, eventCount: number): void; } export async function runExportTelemetryCommand( @@ -40,11 +44,11 @@ export async function runExportTelemetryCommand( logger: Logger, flushTelemetry: () => Promise, context: TelemetryContext, - observer?: ExportTelemetryObserver, + observer: ExportTelemetryObserver, ): Promise { const choice = await promptForExport(); if (!choice) { - observer?.cancelled("prompt"); + observer.cancel("prompt"); return; } @@ -91,14 +95,14 @@ async function reportOutcome( result: ProgressResult, choice: ExportChoice, logger: Logger, - observer?: ExportTelemetryObserver, + observer: ExportTelemetryObserver, ): Promise { if (!result.ok) { if (result.cancelled) { - observer?.cancelled("progress"); + observer.cancel("progress"); return; } - observer?.failed(); + observer.fail(); logger.error("Telemetry export failed", result.error); void vscode.window.showErrorMessage( `Telemetry export failed: ${toError(result.error).message}`, @@ -107,7 +111,7 @@ async function reportOutcome( } const eventCount = result.value; - observer?.succeeded(choice.format, eventCount); + observer.succeedExport(choice.format, eventCount); if (eventCount === 0) { void vscode.window.showInformationMessage( `No telemetry events found for ${choice.range.label}.`, diff --git a/test/unit/command/updateWorkspace.test.ts b/test/unit/command/updateWorkspace.telemetry.test.ts similarity index 100% rename from test/unit/command/updateWorkspace.test.ts rename to test/unit/command/updateWorkspace.telemetry.test.ts diff --git a/test/unit/instrumentation/diagnostics.test.ts b/test/unit/instrumentation/diagnostics.test.ts new file mode 100644 index 000000000..81cf9dcd6 --- /dev/null +++ b/test/unit/instrumentation/diagnostics.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; + +import { DiagnosticTelemetry } from "@/instrumentation/diagnostics"; + +import { createTelemetryHarness } from "../../mocks/telemetry"; + +function setup() { + const { sink, service } = createTelemetryHarness(); + return { sink, telemetry: new DiagnosticTelemetry(service) }; +} + +describe("DiagnosticTelemetry", () => { + it("records diagnostic cancellation and failure categories", async () => { + const { sink, telemetry } = setup(); + + await telemetry.trace("support_bundle", (trace) => { + trace.cancel("save_dialog"); + return Promise.resolve(); + }); + await telemetry.trace("support_bundle", (trace) => { + trace.fail("unsupported_cli"); + return Promise.resolve(); + }); + + const [cancelled, failed] = sink.eventsNamed( + "command.diagnostic.completed", + ); + expect(cancelled.properties).toMatchObject({ + cancel_stage: "save_dialog", + result: "aborted", + }); + expect(failed.properties).toMatchObject({ + failure_category: "unsupported_cli", + result: "error", + }); + expect(failed.error).toBeUndefined(); + }); + + it("records bounded speed test measurements", async () => { + const { sink, telemetry } = setup(); + + await telemetry.trace("speed_test", (trace) => { + trace.succeedSpeedtest({ + overall: { + start_time_seconds: 0, + end_time_seconds: 5, + throughput_mbits: 42, + }, + intervals: [ + { + start_time_seconds: 0, + end_time_seconds: 5, + throughput_mbits: 42, + }, + ], + }); + return Promise.resolve(); + }); + + expect(sink.expectOne("command.diagnostic.completed")).toMatchObject({ + measurements: { + interval_count: 1, + throughput_mbits: 42, + }, + properties: { result: "success" }, + }); + }); +}); diff --git a/test/unit/instrumentation/workspace.test.ts b/test/unit/instrumentation/workspace.test.ts index 1f01cce12..03bbdca3e 100644 --- a/test/unit/instrumentation/workspace.test.ts +++ b/test/unit/instrumentation/workspace.test.ts @@ -33,11 +33,11 @@ const newAgentTelemetry = (svc: TelemetryService, name: string) => describe("WorkspaceOperationTelemetry", () => { it.each([ { - method: "traceStartTriggered" as const, + method: "traceStart" as const, event: "workspace.start.triggered", }, { - method: "traceUpdateTriggered" as const, + method: "traceUpdate" as const, event: "workspace.update.triggered", }, ])("$method emits $event with result=success", async ({ method, event }) => { @@ -52,11 +52,11 @@ describe("WorkspaceOperationTelemetry", () => { it.each([ { - method: "traceStartTriggered" as const, + method: "traceStart" as const, event: "workspace.start.triggered", }, { - method: "traceUpdateTriggered" as const, + method: "traceUpdate" as const, event: "workspace.update.triggered", }, ])("$method emits result=error and rethrows", async ({ method, event }) => { @@ -70,17 +70,18 @@ describe("WorkspaceOperationTelemetry", () => { }); }); - describe("traceStartPrompted", () => { + describe("traceStartPrompt", () => { it("emits result=success with accepted action", async () => { const { sink, instance: ops } = setup(newOps); - const result = await ops.traceStartPrompted(true, () => + const result = await ops.traceStartPrompt(true, () => Promise.resolve("update"), ); expect(result).toBe("update"); expect(sink.expectOne("workspace.start.prompted")).toMatchObject({ properties: { + workspaceName: WORKSPACE_NAME, update_offered: "true", action: "update", result: "success", @@ -91,7 +92,7 @@ describe("WorkspaceOperationTelemetry", () => { it("emits result=aborted when dismissed", async () => { const { sink, instance: ops } = setup(newOps); - const result = await ops.traceStartPrompted(false, () => + const result = await ops.traceStartPrompt(false, () => Promise.resolve(undefined), ); @@ -105,22 +106,23 @@ describe("WorkspaceOperationTelemetry", () => { }); }); - describe("traceUpdatePrompted", () => { + describe("traceParametersPrompt", () => { it("returns the collected parameters and emits result=success", async () => { const { sink, instance: ops } = setup(newOps); const collected = [{ name: "region", value: "us-east" }]; - const result = await ops.traceUpdatePrompted(() => + const result = await ops.traceParametersPrompt(() => Promise.resolve(collected), ); expect(result).toEqual(collected); - const event = sink.expectOne("workspace.update.prompted"); - expect(event.properties).toMatchObject({ - prompt: "parameters", - result: "success", + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { + workspaceName: WORKSPACE_NAME, + prompt: "parameters", + result: "success", + }, }); - expect(event.properties.workspaceName).toBeUndefined(); }); it("emits result=aborted (no error block) and rethrows on cancellation", async () => { @@ -128,7 +130,7 @@ describe("WorkspaceOperationTelemetry", () => { const cancel = new WorkspaceUpdateCancelledError(); await expect( - ops.traceUpdatePrompted(() => Promise.reject(cancel)), + ops.traceParametersPrompt(() => Promise.reject(cancel)), ).rejects.toBe(cancel); const event = sink.expectOne("workspace.update.prompted"); @@ -141,7 +143,7 @@ describe("WorkspaceOperationTelemetry", () => { const boom = new Error("rest call failed"); await expect( - ops.traceUpdatePrompted(() => Promise.reject(boom)), + ops.traceParametersPrompt(() => Promise.reject(boom)), ).rejects.toBe(boom); expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ @@ -151,17 +153,18 @@ describe("WorkspaceOperationTelemetry", () => { }); }); - describe("traceUpdateConfirmationPrompted", () => { + describe("traceConfirmationPrompt", () => { it("emits result=success with accepted action", async () => { const { sink, instance: ops } = setup(newOps); - const result = await ops.traceUpdateConfirmationPrompted(() => + const result = await ops.traceConfirmationPrompt(() => Promise.resolve("Update and Restart"), ); expect(result).toBe("Update and Restart"); expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ properties: { + workspaceName: WORKSPACE_NAME, action: "update", prompt: "confirmation", result: "success", @@ -172,7 +175,7 @@ describe("WorkspaceOperationTelemetry", () => { it("emits result=aborted when dismissed", async () => { const { sink, instance: ops } = setup(newOps); - const result = await ops.traceUpdateConfirmationPrompted(() => + const result = await ops.traceConfirmationPrompt(() => Promise.resolve(undefined), ); diff --git a/test/unit/instrumentation/commands.test.ts b/test/unit/instrumentation/workspaceOpen.test.ts similarity index 55% rename from test/unit/instrumentation/commands.test.ts rename to test/unit/instrumentation/workspaceOpen.test.ts index a927af636..cdf89356f 100644 --- a/test/unit/instrumentation/commands.test.ts +++ b/test/unit/instrumentation/workspaceOpen.test.ts @@ -1,11 +1,16 @@ import { describe, expect, it } from "vitest"; -import { CommandTelemetry } from "@/instrumentation/commands"; +import { WorkspaceOpenTelemetry } from "@/instrumentation/workspaceOpen"; import { agent, resource, workspace } from "@repo/mocks"; import { createTelemetryHarness } from "../../mocks/telemetry"; +function setup() { + const { sink, service } = createTelemetryHarness(); + return { sink, telemetry: new WorkspaceOpenTelemetry(service) }; +} + function workspaceWithAgents() { const connected = agent({ status: "connected", @@ -30,13 +35,12 @@ function workspaceWithAgents() { }; } -describe("command instrumentation helpers", () => { +describe("WorkspaceOpenTelemetry", () => { it("records workspace selection without workspace or agent names", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); + const { sink, telemetry } = setup(); const selection = workspaceWithAgents(); - await traces.workspaceOpen( + await telemetry.traceOpen( "command", { workspace: selection.workspace, agent: selection.connected }, () => Promise.resolve(true), @@ -59,17 +63,16 @@ describe("command instrumentation helpers", () => { }); it("records workspace picker cancellation and failure distinctly", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); + const { sink, telemetry } = setup(); - await traces.workspacePicker("workspace_open", (telemetry) => { + await telemetry.tracePicker("workspace_open", (trace) => { const result = { status: "cancelled" } as const; - telemetry.finish(result, 3); + trace.finish(result, 3); return Promise.resolve(result); }); - await traces.workspacePicker("workspace_open", (telemetry) => { + await telemetry.tracePicker("workspace_open", (trace) => { const result = { status: "failed", category: "fetch_failed" } as const; - telemetry.finish(result, 0); + trace.finish(result, 0); return Promise.resolve(result); }); @@ -84,16 +87,15 @@ describe("command instrumentation helpers", () => { }); it("records workspace open cancellation and handled failure distinctly", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); + const { sink, telemetry } = setup(); const selection = workspaceWithAgents(); - await traces.workspaceOpen("command", undefined, (telemetry) => { - telemetry.cancel("agent_picker", { workspace: selection.workspace }); + await telemetry.traceOpen("command", undefined, (trace) => { + trace.cancel("agent_picker", { workspace: selection.workspace }); return Promise.resolve(false); }); - await traces.workspaceOpen("command", undefined, (telemetry) => { - telemetry.fail("fetch_failed"); + await telemetry.traceOpen("command", undefined, (trace) => { + trace.fail("fetch_failed"); return Promise.resolve(false); }); @@ -110,11 +112,10 @@ describe("command instrumentation helpers", () => { }); it("records thrown workspace open failures without raw error details", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); + const { sink, telemetry } = setup(); await expect( - traces.workspaceOpen("command", undefined, () => + telemetry.traceOpen("command", undefined, () => Promise.reject(new Error("secret path /tmp/workspace")), ), ).rejects.toThrow("secret path /tmp/workspace"); @@ -127,69 +128,11 @@ describe("command instrumentation helpers", () => { expect(event.error).toBeUndefined(); }); - it("records diagnostic cancellation and failure categories", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); - await traces.diagnostic("support_bundle", (telemetry) => { - telemetry.cancel("save_dialog"); - return Promise.resolve(); - }); - await traces.diagnostic("support_bundle", (telemetry) => { - telemetry.fail("unsupported_cli"); - return Promise.resolve(); - }); - - const [cancelled, failed] = sink.eventsNamed( - "command.diagnostic.completed", - ); - expect(cancelled.properties).toMatchObject({ - cancel_stage: "save_dialog", - result: "aborted", - }); - expect(failed.properties).toMatchObject({ - failure_category: "unsupported_cli", - result: "error", - }); - expect(failed.error).toBeUndefined(); - }); - - it("records bounded speed test measurements", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); - - await traces.diagnostic("speed_test", (telemetry) => { - telemetry.speedtestSuccess({ - overall: { - start_time_seconds: 0, - end_time_seconds: 5, - throughput_mbits: 42, - }, - intervals: [ - { - start_time_seconds: 0, - end_time_seconds: 5, - throughput_mbits: 42, - }, - ], - }); - return Promise.resolve(); - }); - - expect(sink.expectOne("command.diagnostic.completed")).toMatchObject({ - measurements: { - interval_count: 1, - throughput_mbits: 42, - }, - properties: { result: "success" }, - }); - }); - it("records thrown devcontainer failures without raw error details", async () => { - const { sink, service } = createTelemetryHarness(); - const traces = new CommandTelemetry(service); + const { sink, telemetry } = setup(); await expect( - traces.devcontainerOpen("dev_container", () => + telemetry.traceDevcontainer("dev_container", () => Promise.reject(new Error("secret local path /tmp/workspace")), ), ).rejects.toThrow("secret local path /tmp/workspace"); diff --git a/test/unit/telemetry/export/command.test.ts b/test/unit/telemetry/export/command.test.ts index 91cba3f74..0cfefa98d 100644 --- a/test/unit/telemetry/export/command.test.ts +++ b/test/unit/telemetry/export/command.test.ts @@ -60,9 +60,9 @@ function setup( } const observer: ExportTelemetryObserver = { - cancelled: vi.fn(), - failed: vi.fn(), - succeeded: vi.fn(), + cancel: vi.fn(), + fail: vi.fn(), + succeedExport: vi.fn(), }; return { @@ -85,7 +85,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.cancelled).toHaveBeenCalledWith("prompt"); + expect(observer.cancel).toHaveBeenCalledWith("prompt"); expect(collectTelemetryExport).not.toHaveBeenCalled(); expect(vscode.window.withProgress).not.toHaveBeenCalled(); @@ -136,7 +136,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.succeeded).toHaveBeenCalledWith("json", count); + expect(observer.succeedExport).toHaveBeenCalledWith("json", count); expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( `${message} ${OUTPUT_PATH}.`, @@ -181,7 +181,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.succeeded).toHaveBeenCalledWith("json", 0); + expect(observer.succeedExport).toHaveBeenCalledWith("json", 0); expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( "No telemetry events found for Last 24 hours.", @@ -196,7 +196,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.failed).toHaveBeenCalledOnce(); + expect(observer.fail).toHaveBeenCalledOnce(); expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( "Telemetry export failed: disk full", @@ -212,7 +212,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.cancelled).toHaveBeenCalledWith("progress"); + expect(observer.cancel).toHaveBeenCalledWith("progress"); expect(vscode.window.showErrorMessage).not.toHaveBeenCalled(); expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); From 6827b2c2cdf24fc7bfc667d3ba5b57c24e0476d3 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 10 Jun 2026 15:13:12 +0300 Subject: [PATCH 5/6] refactor: tidy telemetry naming after rebase on auth telemetry Rename the Commands openTelemetry field to workspaceOpenTelemetry so it mirrors its class like the sibling fields (and stops reading like the OpenTelemetry framework), group the three telemetry helpers together in the constructor, and rename AuthTelemetry.traceAuthRecovery to traceRecovery to drop the qualifier the class already implies. --- src/api/authInterceptor.ts | 2 +- src/commands.ts | 22 +++++++++++++--------- src/instrumentation/auth.ts | 2 +- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/api/authInterceptor.ts b/src/api/authInterceptor.ts index 7f62f278c..4341c6d3b 100644 --- a/src/api/authInterceptor.ts +++ b/src/api/authInterceptor.ts @@ -95,7 +95,7 @@ export class AuthInterceptor implements vscode.Disposable { } this.logger.debug("Received 401 response, attempting recovery"); - return this.authTelemetry.traceAuthRecovery(async (recorder) => { + return this.authTelemetry.traceRecovery(async (recorder) => { recorder.logReceived(); // 1) OAuth refresh path. diff --git a/src/commands.ts b/src/commands.ts index 42997101a..26b761459 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -136,7 +136,7 @@ export class Commands { private readonly telemetryService: TelemetryService; private readonly authTelemetry: AuthTelemetry; private readonly diagnosticTelemetry: DiagnosticTelemetry; - private readonly openTelemetry: WorkspaceOpenTelemetry; + private readonly workspaceOpenTelemetry: WorkspaceOpenTelemetry; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -155,8 +155,11 @@ export class Commands { private readonly deploymentManager: DeploymentManager, ) { this.telemetryService = serviceContainer.getTelemetryService(); + this.authTelemetry = new AuthTelemetry(this.telemetryService); this.diagnosticTelemetry = new DiagnosticTelemetry(this.telemetryService); - this.openTelemetry = new WorkspaceOpenTelemetry(this.telemetryService); + this.workspaceOpenTelemetry = new WorkspaceOpenTelemetry( + this.telemetryService, + ); this.logger = serviceContainer.getLogger(); this.pathResolver = serviceContainer.getPathResolver(); this.mementoManager = serviceContainer.getMementoManager(); @@ -165,7 +168,6 @@ export class Commands { this.loginCoordinator = serviceContainer.getLoginCoordinator(); this.duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc(); this.speedtestPanelFactory = serviceContainer.getSpeedtestPanelFactory(); - this.authTelemetry = new AuthTelemetry(this.telemetryService); } /** @@ -781,13 +783,13 @@ export class Commands { if (item) { const baseUrl = this.requireExtensionBaseUrl(); if (item instanceof AgentTreeItem) { - await this.openTelemetry.traceOpen( + await this.workspaceOpenTelemetry.traceOpen( "sidebar_agent", { workspace: item.workspace, agent: item.agent }, (telemetry) => this.runOpenAgentItem(baseUrl, item, telemetry), ); } else if (item instanceof WorkspaceTreeItem) { - await this.openTelemetry.traceOpen( + await this.workspaceOpenTelemetry.traceOpen( "sidebar_workspace", { workspace: item.workspace }, (telemetry) => this.runOpenWorkspaceItem(baseUrl, item, telemetry), @@ -879,8 +881,10 @@ export class Commands { */ public async open(options: OpenOptions = {}): Promise { const { source = "command", ...rest } = { ...openDefaults, ...options }; - return this.openTelemetry.traceOpen(source, undefined, (telemetry) => - this.runOpen(rest, telemetry), + return this.workspaceOpenTelemetry.traceOpen( + source, + undefined, + (telemetry) => this.runOpen(rest, telemetry), ); } @@ -951,7 +955,7 @@ export class Commands { const mode: DevcontainerMode = localWorkspaceFolder ? "dev_container" : "attached_container"; - await this.openTelemetry.traceDevcontainer(mode, () => + await this.workspaceOpenTelemetry.traceDevcontainer(mode, () => this.runOpenDevContainer( workspaceOwner, workspaceName, @@ -1237,7 +1241,7 @@ export class Commands { ); quickPick.show(); - return this.openTelemetry + return this.workspaceOpenTelemetry .tracePicker( source, async (telemetry) => diff --git a/src/instrumentation/auth.ts b/src/instrumentation/auth.ts index c4d5f2018..f5399c83c 100644 --- a/src/instrumentation/auth.ts +++ b/src/instrumentation/auth.ts @@ -100,7 +100,7 @@ export class AuthTelemetry { * Wraps the auth-recovery path triggered by a 401. Initial properties * cover the throw-before-callback case. */ - public traceAuthRecovery( + public traceRecovery( fn: (recorder: AuthRecoveryRecorder) => Promise, ): Promise { return this.telemetry.trace( From c0057d778bac1ba06b4304315bc1fd1e890e14bd Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 10 Jun 2026 16:48:34 +0300 Subject: [PATCH 6/6] refactor: align telemetry outcomes with OpenTelemetry naming Address review feedback and standardize span outcome vocabulary: - Rename failure -> error (markError, error.type, *ErrorCategory) to match the OTel span status and the error.type semantic convention. - Unify cancellation on "abort" across the telemetry layer; the VS Code cancellation mirrors (CancellationToken, ProgressResult) stay as-is. - OTLP export: map aborted spans to UNSET and backfill error.type on error spans from the exception type/code, falling back to OTel's _OTHER. - DevContainer casing, type-safe start/update button captions, and test cleanups (shared open defaults, multi-connected-agent coverage). --- src/commands.ts | 40 ++++++++-------- src/core/cliCredentialManager.ts | 8 ++-- src/instrumentation/auth.ts | 9 ++-- src/instrumentation/credentials.ts | 11 ++--- src/instrumentation/diagnostics.ts | 22 ++++----- src/instrumentation/outcomes.ts | 18 +++---- src/instrumentation/workspaceOpen.ts | 48 +++++++++---------- src/remote/workspaceStateMachine.ts | 4 +- src/telemetry/export/command.ts | 6 +-- src/telemetry/export/writers/otlp/records.ts | 31 +++++++++--- src/telemetry/service.ts | 6 +-- src/telemetry/span.ts | 6 +-- test/unit/core/cliCredentialManager.test.ts | 10 ++-- test/unit/instrumentation/diagnostics.test.ts | 6 +-- .../instrumentation/workspaceOpen.test.ts | 47 +++++++++++++++--- test/unit/telemetry/export/command.test.ts | 6 +-- .../otlp/__golden__/envelope-traces.json | 6 +++ .../writers/otlp/__golden__/span-records.json | 6 +++ .../export/writers/otlp/records.test.ts | 38 +++++++++++++++ test/unit/telemetry/service.test.ts | 18 +++---- test/unit/uri/uriHandler.test.ts | 15 +++--- 21 files changed, 230 insertions(+), 131 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 26b761459..eb7c795b2 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -25,11 +25,11 @@ import { } from "./instrumentation/diagnostics"; import { WorkspaceOperationTelemetry } from "./instrumentation/workspace"; import { - type DevcontainerMode, + type DevContainerMode, type WorkspaceOpenSource, WorkspaceOpenTelemetry, type WorkspaceOpenTrace, - type WorkspacePickerFailureCategory, + type WorkspacePickerErrorCategory, type WorkspacePickerResult, type WorkspacePickerSource, } from "./instrumentation/workspaceOpen"; @@ -102,7 +102,7 @@ type WorkspaceResolution = | { readonly status: "cancelled" } | { readonly status: "failed"; - readonly category: WorkspacePickerFailureCategory; + readonly category: WorkspacePickerErrorCategory; }; type OpenWorkspaceResult = @@ -281,7 +281,7 @@ export class Commands { ): Promise { const resolved = await this.resolveClientAndWorkspace(item); if (resolved.status === "cancelled") { - telemetry.cancel("workspace_picker"); + telemetry.abort("workspace_picker"); return; } if (resolved.status === "failed") { @@ -304,7 +304,7 @@ export class Commands { }, }); if (input === undefined) { - telemetry.cancel("input"); + telemetry.abort("input"); return; } const seconds = Number(input.trim()); @@ -343,7 +343,7 @@ export class Commands { if (!result.ok) { if (result.cancelled) { - telemetry.cancel("progress"); + telemetry.abort("progress"); return; } telemetry.fail(); @@ -391,7 +391,7 @@ export class Commands { ): Promise { const resolved = await this.resolveClientAndWorkspace(item); if (resolved.status === "cancelled") { - telemetry.cancel("workspace_picker"); + telemetry.abort("workspace_picker"); return; } if (resolved.status === "failed") { @@ -403,7 +403,7 @@ export class Commands { const outputUri = await this.promptSupportBundlePath(); if (!outputUri) { - telemetry.cancel("save_dialog"); + telemetry.abort("save_dialog"); return; } @@ -441,7 +441,7 @@ export class Commands { if (!result.ok) { if (result.cancelled) { - telemetry.cancel("progress"); + telemetry.abort("progress"); return; } telemetry.fail( @@ -832,7 +832,7 @@ export class Commands { const agents = await this.extractAgentsWithFallback(item.workspace); const agent = await maybeAskAgent(agents); if (!agent) { - telemetry.cancel("agent_picker", { workspace: item.workspace }); + telemetry.abort("agent_picker", { workspace: item.workspace }); return false; } const selection = { workspace: item.workspace, agent }; @@ -911,7 +911,7 @@ export class Commands { } else { const pick = await this.pickWorkspace("workspace_open"); if (pick.status === "cancelled") { - telemetry.cancel("workspace_picker"); + telemetry.abort("workspace_picker"); return false; } if (pick.status === "failed") { @@ -924,7 +924,7 @@ export class Commands { const agents = await this.extractAgentsWithFallback(workspace); const agent = await maybeAskAgent(agents, agentName); if (!agent) { - telemetry.cancel("agent_picker", { workspace }); + telemetry.abort("agent_picker", { workspace }); return false; } const selection = { workspace, agent }; @@ -952,10 +952,10 @@ export class Commands { localWorkspaceFolder = "", localConfigFile = "", ): Promise { - const mode: DevcontainerMode = localWorkspaceFolder + const mode: DevContainerMode = localWorkspaceFolder ? "dev_container" : "attached_container"; - await this.workspaceOpenTelemetry.traceDevcontainer(mode, () => + await this.workspaceOpenTelemetry.traceDevContainer(mode, () => this.runOpenDevContainer( workspaceOwner, workspaceName, @@ -1184,7 +1184,7 @@ export class Commands { let lastWorkspaces: readonly Workspace[] = []; let settled = false; - let fetchFailureCategory: WorkspacePickerFailureCategory | undefined; + let fetchErrorCategory: WorkspacePickerErrorCategory | undefined; const disposables: vscode.Disposable[] = []; disposables.push( quickPick.onDidChangeValue((value) => { @@ -1194,7 +1194,7 @@ export class Commands { q: value, }) .then((workspaces) => { - fetchFailureCategory = undefined; + fetchErrorCategory = undefined; const filtered = filter ? workspaces.workspaces.filter(filter) : workspaces.workspaces; @@ -1224,7 +1224,7 @@ export class Commands { } }) .catch((ex) => { - fetchFailureCategory = "fetch_failed"; + fetchErrorCategory = "fetch_failed"; this.logger.error("Failed to fetch workspaces", ex); if (ex instanceof CertificateError) { void ex.showNotification(); @@ -1256,10 +1256,10 @@ export class Commands { }; disposables.push( quickPick.onDidHide(() => { - if (fetchFailureCategory) { + if (fetchErrorCategory) { finish({ status: "failed", - category: fetchFailureCategory, + category: fetchErrorCategory, }); return; } @@ -1460,7 +1460,7 @@ function recordOpenResult( result: OpenWorkspaceResult, ): boolean { if (result.status === "cancelled") { - telemetry.cancel(result.stage, selection); + telemetry.abort(result.stage, selection); return false; } telemetry.handoff(result.handoff); diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index a6399cc1a..245f1a5e1 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -281,8 +281,8 @@ export class CliCredentialManager { binPath = await this.resolveKeyringBinary(url, configs, "keyringAuth"); } catch (error) { this.logger.warn("Could not resolve keyring binary for delete:", error); - span.setProperty("failure_category", "binary"); - span.markFailure(); + span.setProperty("error.type", "binary"); + span.markError(); return; } if (!binPath) { @@ -298,8 +298,8 @@ export class CliCredentialManager { throw error; } this.logger.warn("Failed to delete token via CLI:", error); - span.setProperty("failure_category", "cli"); - span.markFailure(); + span.setProperty("error.type", "cli"); + span.markError(); } } diff --git a/src/instrumentation/auth.ts b/src/instrumentation/auth.ts index f5399c83c..70643e66a 100644 --- a/src/instrumentation/auth.ts +++ b/src/instrumentation/auth.ts @@ -117,9 +117,9 @@ export class AuthTelemetry { } /** - * Records `auth.login_prompted`. `auth_failed` marks the span as failure; + * Records `auth.login_prompted`. `auth_failed` marks the span as error; * other non-success reasons mark it as aborted. The reason is copied to the - * span's `reason` property on failure/abort only. + * span's `reason` property on error/abort only. */ public traceLoginPrompt( trigger: AuthLoginPromptTrigger, @@ -139,11 +139,12 @@ export class AuthTelemetry { } } -/** `auth_failed` is a real failure; user/URL dismissals are intentional aborts. */ +/** `auth_failed` is a real error; user/URL dismissals are intentional aborts. */ function recordReason(span: Span, reason: LoginPromptReason): void { span.setProperty("reason", reason); if (reason === "auth_failed") { - span.markFailure(); + span.setProperty("error.type", reason); + span.markError(); } else { span.markAborted(); } diff --git a/src/instrumentation/credentials.ts b/src/instrumentation/credentials.ts index 754b26e1a..393d6c249 100644 --- a/src/instrumentation/credentials.ts +++ b/src/instrumentation/credentials.ts @@ -6,13 +6,13 @@ import type { WorkspaceConfiguration } from "vscode"; import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; -export type CredentialFailureCategory = "aborted" | "binary" | "cli" | "file"; +export type CredentialErrorCategory = "aborted" | "binary" | "cli" | "file"; type CredentialEvent = "auth.credential.store" | "auth.credential.clear"; /** * Wraps credential store/clear in a span carrying `keyring_enabled`, the - * `category` of storage involved, and a `failure_category` on failure. The + * `category` of storage involved, and an `error.type` on failure. The * traced operation sets `category` on the span and reports failures by * throwing a categorized error (store) or recording on the span (clear, which * is best-effort). Aborts are recorded and re-thrown so callers still unwind. @@ -47,10 +47,7 @@ export class CredentialTelemetry { try { await fn(span); } catch (error) { - span.setProperty( - "failure_category", - categorizeCredentialError(error), - ); + span.setProperty("error.type", categorizeCredentialError(error)); if (isAbortError(error)) { span.markAborted(); aborted = error; @@ -70,7 +67,7 @@ export class CredentialTelemetry { } } -function categorizeCredentialError(error: unknown): CredentialFailureCategory { +function categorizeCredentialError(error: unknown): CredentialErrorCategory { if (isAbortError(error)) { return "aborted"; } diff --git a/src/instrumentation/diagnostics.ts b/src/instrumentation/diagnostics.ts index 11fe2f3b7..0b7dfd5e6 100644 --- a/src/instrumentation/diagnostics.ts +++ b/src/instrumentation/diagnostics.ts @@ -1,22 +1,22 @@ -import { recordCancelled, recordFailure } from "./outcomes"; +import { recordAborted, recordError } from "./outcomes"; import type { SpeedtestResult } from "@repo/shared"; import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; -import type { WorkspacePickerFailureCategory } from "./workspaceOpen"; +import type { WorkspacePickerErrorCategory } from "./workspaceOpen"; export type DiagnosticCommand = | "speed_test" | "support_bundle" | "export_telemetry"; -export type DiagnosticFailureCategory = - | WorkspacePickerFailureCategory +export type DiagnosticErrorCategory = + | WorkspacePickerErrorCategory | "parse_error" | "unsupported_cli" | "error"; -export type DiagnosticCancelStage = +export type DiagnosticAbortStage = | "workspace_picker" | "input" | "prompt" @@ -24,8 +24,8 @@ export type DiagnosticCancelStage = | "progress"; export interface DiagnosticTrace { - cancel(stage: DiagnosticCancelStage): void; - fail(category?: DiagnosticFailureCategory): void; + abort(stage: DiagnosticAbortStage): void; + fail(category?: DiagnosticErrorCategory): void; setRequestedDuration(seconds: number): void; succeedSpeedtest(result: SpeedtestResult): void; succeedExport(format: string, eventCount: number): void; @@ -50,12 +50,12 @@ export class DiagnosticTelemetry { class SpanDiagnosticTrace implements DiagnosticTrace { public constructor(private readonly span: Span) {} - public cancel(stage: DiagnosticCancelStage): void { - recordCancelled(this.span, stage); + public abort(stage: DiagnosticAbortStage): void { + recordAborted(this.span, stage); } - public fail(category: DiagnosticFailureCategory = "error"): void { - recordFailure(this.span, category); + public fail(category: DiagnosticErrorCategory = "error"): void { + recordError(this.span, category); } public setRequestedDuration(seconds: number): void { diff --git a/src/instrumentation/outcomes.ts b/src/instrumentation/outcomes.ts index 5568e61b0..9c844d790 100644 --- a/src/instrumentation/outcomes.ts +++ b/src/instrumentation/outcomes.ts @@ -2,22 +2,22 @@ import { isAbortError } from "../error/errorUtils"; import type { Span } from "../telemetry/span"; -export type AbortableFailureCategory = "aborted" | "error"; +export type AbortableErrorCategory = "aborted" | "error"; -/** Records a categorized failure without attaching raw error details. */ -export function recordFailure(span: Span, category: string): void { - span.setProperty("failure_category", category); - span.markFailure(); +/** Records a categorized error without attaching the raw error details. */ +export function recordError(span: Span, category: string): void { + span.setProperty("error.type", category); + span.markError(); } /** Records the stage at which the user backed out and aborts the span. */ -export function recordCancelled(span: Span, stage: string): void { - span.setProperty("cancel_stage", stage); +export function recordAborted(span: Span, stage: string): void { + span.setProperty("abort_stage", stage); span.markAborted(); } -export function categorizeAbortableFailure( +export function categorizeAbortableError( error: unknown, -): AbortableFailureCategory { +): AbortableErrorCategory { return isAbortError(error) ? "aborted" : "error"; } diff --git a/src/instrumentation/workspaceOpen.ts b/src/instrumentation/workspaceOpen.ts index b6fcaa2b8..69e716af4 100644 --- a/src/instrumentation/workspaceOpen.ts +++ b/src/instrumentation/workspaceOpen.ts @@ -1,10 +1,10 @@ import { extractAgents } from "../api/api-helper"; import { - type AbortableFailureCategory, - categorizeAbortableFailure, - recordCancelled, - recordFailure, + type AbortableErrorCategory, + categorizeAbortableError, + recordAborted, + recordError, } from "./outcomes"; import type { @@ -24,19 +24,19 @@ export type WorkspaceOpenSource = | "uri"; export type WorkspacePickerSource = "workspace_open" | "diagnostic"; -export type WorkspacePickerFailureCategory = "fetch_failed"; -export type WorkspaceOpenFailureCategory = - | WorkspacePickerFailureCategory - | AbortableFailureCategory; +export type WorkspacePickerErrorCategory = "fetch_failed"; +export type WorkspaceOpenErrorCategory = + | WorkspacePickerErrorCategory + | AbortableErrorCategory; export type WorkspacePickerResult = | { readonly status: "selected"; readonly workspace: Workspace } | { readonly status: "cancelled" } | { readonly status: "failed"; - readonly category: WorkspacePickerFailureCategory; + readonly category: WorkspacePickerErrorCategory; }; -export type DevcontainerMode = "dev_container" | "attached_container"; -export type WorkspaceOpenCancelStage = +export type DevContainerMode = "dev_container" | "attached_container"; +export type WorkspaceOpenAbortStage = | "workspace_picker" | "agent_picker" | "recent_folder_picker"; @@ -52,11 +52,11 @@ export interface WorkspacePickerTrace { export interface WorkspaceOpenTrace { select(selection: WorkspaceOpenSelection): void; - cancel( - stage: WorkspaceOpenCancelStage, + abort( + stage: WorkspaceOpenAbortStage, selection?: WorkspaceOpenSelection, ): void; - fail(category: WorkspaceOpenFailureCategory): void; + fail(category: WorkspaceOpenErrorCategory): void; handoff(kind: "folder" | "empty_window"): void; } @@ -101,8 +101,8 @@ export class WorkspaceOpenTelemetry { ); } - public async traceDevcontainer( - mode: DevcontainerMode, + public async traceDevContainer( + mode: DevContainerMode, fn: () => Promise, ): Promise { await this.traceRethrowing( @@ -115,7 +115,7 @@ export class WorkspaceOpenTelemetry { /** * Runs `fn` inside the span, recording a thrown error as a categorized - * failure without its raw details, then rethrows outside the span. + * error without its raw details, then rethrows outside the span. */ private async traceRethrowing( eventName: string, @@ -131,7 +131,7 @@ export class WorkspaceOpenTelemetry { return await fn(span); } catch (error) { thrown = { error }; - recordFailure(span, categorizeAbortableFailure(error)); + recordError(span, categorizeAbortableError(error)); return fallback; } }, @@ -154,7 +154,7 @@ class SpanWorkspacePickerTrace implements WorkspacePickerTrace { return; } if (result.status === "failed") { - recordFailure(this.span, result.category); + recordError(this.span, result.category); return; } this.span.markAborted(); @@ -168,18 +168,18 @@ class SpanWorkspaceOpenTrace implements WorkspaceOpenTrace { recordWorkspaceContext(this.span, selection.workspace, selection.agent); } - public cancel( - stage: WorkspaceOpenCancelStage, + public abort( + stage: WorkspaceOpenAbortStage, selection?: WorkspaceOpenSelection, ): void { if (selection) { recordWorkspaceContext(this.span, selection.workspace, selection.agent); } - recordCancelled(this.span, stage); + recordAborted(this.span, stage); } - public fail(category: WorkspaceOpenFailureCategory): void { - recordFailure(this.span, category); + public fail(category: WorkspaceOpenErrorCategory): void { + recordError(this.span, category); } public handoff(kind: "folder" | "empty_window"): void { diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index 5713fd44b..c459aa502 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -338,7 +338,9 @@ export class WorkspaceStateMachine implements vscode.Disposable { outdated: boolean, ): Promise<"start" | "update" | undefined> { return this.operationTelemetry.traceStartPrompt(outdated, async () => { - const buttons = outdated ? ["Start", "Update and Start"] : ["Start"]; + const buttons = outdated + ? (["Start", "Update and Start"] as const) + : (["Start"] as const); const action = await vscodeProposed.window.showInformationMessage( `The workspace ${workspaceName} is not running. How would you like to proceed?`, { diff --git a/src/telemetry/export/command.ts b/src/telemetry/export/command.ts index 7ff4dcc91..5248f3671 100644 --- a/src/telemetry/export/command.ts +++ b/src/telemetry/export/command.ts @@ -34,7 +34,7 @@ const PROGRESS_OPTIONS = { * this shape, so command callers can pass their trace directly. */ export interface ExportTelemetryObserver { - cancel(stage: "prompt" | "progress"): void; + abort(stage: "prompt" | "progress"): void; fail(): void; succeedExport(format: ExportFormat, eventCount: number): void; } @@ -48,7 +48,7 @@ export async function runExportTelemetryCommand( ): Promise { const choice = await promptForExport(); if (!choice) { - observer.cancel("prompt"); + observer.abort("prompt"); return; } @@ -99,7 +99,7 @@ async function reportOutcome( ): Promise { if (!result.ok) { if (result.cancelled) { - observer.cancel("progress"); + observer.abort("progress"); return; } observer.fail(); diff --git a/src/telemetry/export/writers/otlp/records.ts b/src/telemetry/export/writers/otlp/records.ts index 11a4e9386..04c97d7f2 100644 --- a/src/telemetry/export/writers/otlp/records.ts +++ b/src/telemetry/export/writers/otlp/records.ts @@ -52,6 +52,9 @@ const AGGREGATION_TEMPORALITY_CUMULATIVE = 2; // start at 0 (INTERNAL), so shift by 1 when encoding for proto. const OTLP_SPAN_KIND_OFFSET = 1; +// OtlpStatus.code is a wire-format int, so compare against a numeric alias. +const OTLP_STATUS_ERROR: number = SpanStatusCode.ERROR; + export function envelopePrefix( envelope: EnvelopeSpec, resource: string, @@ -141,6 +144,22 @@ export function spanRecord( const startNano = endNano - nanosFromMs(event.measurements.durationMs ?? 0); const endTimeUnixNano = String(endNano); const { durationMs: _, ...measurements } = event.measurements; + const status = spanStatus(event); + const attributes: Record = { + ...event.properties, + ...measurements, + "coder.event_name": event.eventName, + ...eventContextAttributes(event), + }; + // OTel wants every error span to carry a low-cardinality error.type, so + // backfill the exception type or code, falling back to OTel's _OTHER sentinel. + if ( + status.code === OTLP_STATUS_ERROR && + attributes["error.type"] === undefined + ) { + attributes["error.type"] = + event.error?.type ?? event.error?.code ?? "_OTHER"; + } return { traceId: event.traceId, spanId: event.eventId, @@ -151,13 +170,8 @@ export function spanRecord( kind: SpanKind.INTERNAL + OTLP_SPAN_KIND_OFFSET, startTimeUnixNano: String(startNano), endTimeUnixNano, - attributes: keyValues({ - ...event.properties, - ...measurements, - "coder.event_name": event.eventName, - ...eventContextAttributes(event), - }), - status: spanStatus(event), + attributes: keyValues(attributes), + status, ...(event.error && { events: [ { @@ -174,6 +188,9 @@ function spanStatus(event: TelemetryEvent): OtlpStatus { switch (event.properties.result) { case "success": return { code: SpanStatusCode.OK }; + case "aborted": + // OTel treats intentional cancellation as non-error, so leave it unset. + return { code: SpanStatusCode.UNSET }; case "error": return { code: SpanStatusCode.ERROR, diff --git a/src/telemetry/service.ts b/src/telemetry/service.ts index f3c69501e..86bd89361 100644 --- a/src/telemetry/service.ts +++ b/src/telemetry/service.ts @@ -208,7 +208,7 @@ export class TelemetryService implements vscode.Disposable, TelemetryReporter { const spanMeasurements = { ...measurements }; const { traceId, traceLevel } = spanOpts; let completed = false; - // `markFailure` wins over `markAborted` regardless of call order. + // `markError` wins over `markAborted` regardless of call order. let mark: "aborted" | "error" | undefined; const warnPostEmit = (op: string, name: string): void => { this.logger.warn( @@ -298,9 +298,9 @@ export class TelemetryService implements vscode.Disposable, TelemetryReporter { } mark ??= "aborted"; }, - markFailure(): void { + markError(): void { if (completed) { - warnPostEmit("markFailure", ""); + warnPostEmit("markError", ""); return; } mark = "error"; diff --git a/src/telemetry/span.ts b/src/telemetry/span.ts index 89d1284aa..af08c5780 100644 --- a/src/telemetry/span.ts +++ b/src/telemetry/span.ts @@ -41,8 +41,8 @@ export interface Span { setMeasurement(name: string, value: number): void; /** Flip this span's `result` from `success` to `aborted` on normal return. */ markAborted(): void; - /** Flip `result` to `error` for a failure captured in the return value. See `SpanResult`. */ - markFailure(): void; + /** Flip `result` to `error` for an error captured in the return value. See `SpanResult`. */ + markError(): void; } /** No-op `Span` used when telemetry is off. Runs phase fns but emits nothing. */ @@ -56,7 +56,7 @@ export const NOOP_SPAN: Span = { log: () => undefined, logError: () => undefined, markAborted: () => undefined, - markFailure: () => undefined, + markError: () => undefined, setProperty: () => undefined, setMeasurement: () => undefined, }; diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index 200eb8801..91799edf9 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -237,7 +237,7 @@ describe("CliCredentialManager", () => { ).rejects.toThrow("Credential CLI operation failed"); expect(sink.expectOne("auth.credential.store")).toMatchObject({ properties: { - failure_category: "cli", + "error.type": "cli", result: "error", }, }); @@ -298,7 +298,7 @@ describe("CliCredentialManager", () => { ).rejects.toThrow("The operation was aborted"); expect(sink.expectOne("auth.credential.store")).toMatchObject({ properties: { - failure_category: "aborted", + "error.type": "aborted", result: "aborted", }, }); @@ -443,7 +443,7 @@ describe("CliCredentialManager", () => { ).resolves.not.toThrow(); expect(sink.expectOne("auth.credential.clear")).toMatchObject({ properties: { - failure_category: "cli", + "error.type": "cli", result: "error", }, }); @@ -460,7 +460,7 @@ describe("CliCredentialManager", () => { expect(sink.expectOne("auth.credential.clear")).toMatchObject({ properties: { category: "keyring", - failure_category: "binary", + "error.type": "binary", result: "error", }, }); @@ -513,7 +513,7 @@ describe("CliCredentialManager", () => { ).rejects.toThrow("The operation was aborted"); expect(sink.expectOne("auth.credential.clear")).toMatchObject({ properties: { - failure_category: "aborted", + "error.type": "aborted", result: "aborted", }, }); diff --git a/test/unit/instrumentation/diagnostics.test.ts b/test/unit/instrumentation/diagnostics.test.ts index 81cf9dcd6..86aeee7b2 100644 --- a/test/unit/instrumentation/diagnostics.test.ts +++ b/test/unit/instrumentation/diagnostics.test.ts @@ -14,7 +14,7 @@ describe("DiagnosticTelemetry", () => { const { sink, telemetry } = setup(); await telemetry.trace("support_bundle", (trace) => { - trace.cancel("save_dialog"); + trace.abort("save_dialog"); return Promise.resolve(); }); await telemetry.trace("support_bundle", (trace) => { @@ -26,11 +26,11 @@ describe("DiagnosticTelemetry", () => { "command.diagnostic.completed", ); expect(cancelled.properties).toMatchObject({ - cancel_stage: "save_dialog", + abort_stage: "save_dialog", result: "aborted", }); expect(failed.properties).toMatchObject({ - failure_category: "unsupported_cli", + "error.type": "unsupported_cli", result: "error", }); expect(failed.error).toBeUndefined(); diff --git a/test/unit/instrumentation/workspaceOpen.test.ts b/test/unit/instrumentation/workspaceOpen.test.ts index cdf89356f..3d3962266 100644 --- a/test/unit/instrumentation/workspaceOpen.test.ts +++ b/test/unit/instrumentation/workspaceOpen.test.ts @@ -62,6 +62,39 @@ describe("WorkspaceOpenTelemetry", () => { expect(event.properties.agentName).toBeUndefined(); }); + it("counts every connected agent on the workspace", async () => { + const { sink, telemetry } = setup(); + const first = agent({ status: "connected", lifecycle_state: "ready" }); + const second = agent({ + id: "agent-2", + name: "secondary", + status: "connected", + lifecycle_state: "ready", + }); + const offline = agent({ + id: "agent-3", + name: "tertiary", + status: "disconnected", + lifecycle_state: "off", + }); + const selection = workspace({ + latest_build: { + status: "running", + resources: [resource({ agents: [first, second, offline] })], + }, + }); + + await telemetry.traceOpen("command", { workspace: selection }, () => + Promise.resolve(true), + ); + + const event = sink.expectOne("workspace.open"); + expect(event.measurements).toMatchObject({ + agent_count: 3, + connected_agent_count: 2, + }); + }); + it("records workspace picker cancellation and failure distinctly", async () => { const { sink, telemetry } = setup(); @@ -80,7 +113,7 @@ describe("WorkspaceOpenTelemetry", () => { expect(cancelled.properties).toMatchObject({ result: "aborted" }); expect(cancelled.measurements.workspace_count).toBe(3); expect(failed.properties).toMatchObject({ - failure_category: "fetch_failed", + "error.type": "fetch_failed", result: "error", }); expect(failed.measurements.workspace_count).toBe(0); @@ -91,7 +124,7 @@ describe("WorkspaceOpenTelemetry", () => { const selection = workspaceWithAgents(); await telemetry.traceOpen("command", undefined, (trace) => { - trace.cancel("agent_picker", { workspace: selection.workspace }); + trace.abort("agent_picker", { workspace: selection.workspace }); return Promise.resolve(false); }); await telemetry.traceOpen("command", undefined, (trace) => { @@ -101,12 +134,12 @@ describe("WorkspaceOpenTelemetry", () => { const [cancelled, failed] = sink.eventsNamed("workspace.open"); expect(cancelled.properties).toMatchObject({ - cancel_stage: "agent_picker", + abort_stage: "agent_picker", workspace_status: "running", result: "aborted", }); expect(failed.properties).toMatchObject({ - failure_category: "fetch_failed", + "error.type": "fetch_failed", result: "error", }); }); @@ -122,7 +155,7 @@ describe("WorkspaceOpenTelemetry", () => { const event = sink.expectOne("workspace.open"); expect(event.properties).toMatchObject({ - failure_category: "error", + "error.type": "error", result: "error", }); expect(event.error).toBeUndefined(); @@ -132,14 +165,14 @@ describe("WorkspaceOpenTelemetry", () => { const { sink, telemetry } = setup(); await expect( - telemetry.traceDevcontainer("dev_container", () => + telemetry.traceDevContainer("dev_container", () => Promise.reject(new Error("secret local path /tmp/workspace")), ), ).rejects.toThrow("secret local path /tmp/workspace"); const event = sink.expectOne("workspace.dev_container.open"); expect(event.properties).toMatchObject({ - failure_category: "error", + "error.type": "error", mode: "dev_container", result: "error", }); diff --git a/test/unit/telemetry/export/command.test.ts b/test/unit/telemetry/export/command.test.ts index 0cfefa98d..a1e02d72e 100644 --- a/test/unit/telemetry/export/command.test.ts +++ b/test/unit/telemetry/export/command.test.ts @@ -60,7 +60,7 @@ function setup( } const observer: ExportTelemetryObserver = { - cancel: vi.fn(), + abort: vi.fn(), fail: vi.fn(), succeedExport: vi.fn(), }; @@ -85,7 +85,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.cancel).toHaveBeenCalledWith("prompt"); + expect(observer.abort).toHaveBeenCalledWith("prompt"); expect(collectTelemetryExport).not.toHaveBeenCalled(); expect(vscode.window.withProgress).not.toHaveBeenCalled(); @@ -212,7 +212,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.cancel).toHaveBeenCalledWith("progress"); + expect(observer.abort).toHaveBeenCalledWith("progress"); expect(vscode.window.showErrorMessage).not.toHaveBeenCalled(); expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); diff --git a/test/unit/telemetry/export/writers/otlp/__golden__/envelope-traces.json b/test/unit/telemetry/export/writers/otlp/__golden__/envelope-traces.json index 9ffdc4b11..d1e214157 100644 --- a/test/unit/telemetry/export/writers/otlp/__golden__/envelope-traces.json +++ b/test/unit/telemetry/export/writers/otlp/__golden__/envelope-traces.json @@ -173,6 +173,12 @@ "value": { "stringValue": "https://dev.coder.com" } + }, + { + "key": "error.type", + "value": { + "stringValue": "Error" + } } ], "status": { diff --git a/test/unit/telemetry/export/writers/otlp/__golden__/span-records.json b/test/unit/telemetry/export/writers/otlp/__golden__/span-records.json index 9a0f3ee94..d6e61005f 100644 --- a/test/unit/telemetry/export/writers/otlp/__golden__/span-records.json +++ b/test/unit/telemetry/export/writers/otlp/__golden__/span-records.json @@ -98,6 +98,12 @@ "value": { "stringValue": "https://dev.coder.com" } + }, + { + "key": "error.type", + "value": { + "stringValue": "Error" + } } ], "status": { diff --git a/test/unit/telemetry/export/writers/otlp/records.test.ts b/test/unit/telemetry/export/writers/otlp/records.test.ts index 58cdebfa8..4487481fc 100644 --- a/test/unit/telemetry/export/writers/otlp/records.test.ts +++ b/test/unit/telemetry/export/writers/otlp/records.test.ts @@ -196,6 +196,8 @@ describe("spanRecord", () => { it.each([ [{ properties: { result: "success" } }, { code: 1 }], + // Intentional cancellation stays unset per OTel. + [{ properties: { result: "aborted" } }, { code: 0 }], [{ properties: { result: "error" } }, { code: 2 }], [ { properties: { result: "error" }, error: { message: "boom" } }, @@ -208,6 +210,42 @@ describe("spanRecord", () => { expect(span.status).toEqual(expected); }); + it.each([ + // The exception type wins when present. + [ + { + properties: { result: "error" }, + error: { message: "boom", type: "TypeError" }, + }, + "TypeError", + ], + // The error code stands in when there is no type. + [ + { + properties: { result: "error" }, + error: { message: "boom", code: "ECONNREFUSED" }, + }, + "ECONNREFUSED", + ], + // Neither type nor code falls back to OTel's _OTHER sentinel. + [{ properties: { result: "error" } }, "_OTHER"], + // An instrumentation-set category is never overwritten. + [ + { properties: { result: "error", "error.type": "fetch_failed" } }, + "fetch_failed", + ], + ])("backfills error.type on error spans: %j -> %s", (overrides, expected) => { + const span = spanRecord(makeSpanEvent(overrides)); + expect(attrs(span.attributes)["error.type"]).toBe(expected); + }); + + it("never adds error.type to a non-error span", () => { + const span = spanRecord( + makeSpanEvent({ properties: { result: "success" } }), + ); + expect(attrs(span.attributes)).not.toHaveProperty("error.type"); + }); + it("attaches an `exception` event to errored spans", () => { const span = spanRecord( makeSpanEvent({ diff --git a/test/unit/telemetry/service.test.ts b/test/unit/telemetry/service.test.ts index ca5264b56..f18f93f0c 100644 --- a/test/unit/telemetry/service.test.ts +++ b/test/unit/telemetry/service.test.ts @@ -379,7 +379,7 @@ describe("TelemetryService", () => { expect(phase.eventName).toBe("op.bad_name"); }); - it("drops setProperty/setMeasurement/markAborted/markFailure called after emit", async () => { + it("drops setProperty/setMeasurement/markAborted/markError called after emit", async () => { let escapedSpan: Span | undefined; await h.service.trace("op", (span) => { escapedSpan = span; @@ -391,7 +391,7 @@ describe("TelemetryService", () => { escapedSpan?.setProperty("late", "ignored"); escapedSpan?.setMeasurement("lateMs", 99); escapedSpan?.markAborted(); - escapedSpan?.markFailure(); + escapedSpan?.markError(); expect(h.sink.events[0].properties.late).toBeUndefined(); expect(h.sink.events[0].measurements.lateMs).toBeUndefined(); @@ -438,7 +438,7 @@ describe("TelemetryService", () => { escapedSpan?.setProperty("late", "ignored"); escapedSpan?.setMeasurement("lateMs", 99); escapedSpan?.markAborted(); - escapedSpan?.markFailure(); + escapedSpan?.markError(); escapedSpan?.log("late_log"); escapedSpan?.logError("late_log_error", new Error("ignored")); await escapedSpan?.phase("late_phase", () => Promise.resolve()); @@ -474,9 +474,9 @@ describe("TelemetryService", () => { }); }); - it("markFailure flips result to 'error' on normal return without an error block", async () => { + it("markError flips result to 'error' on normal return without an error block", async () => { await h.service.trace("op", (span) => { - span.markFailure(); + span.markError(); return Promise.resolve(); }); @@ -487,21 +487,21 @@ describe("TelemetryService", () => { expect(h.sink.events[0].error).toBeUndefined(); }); - it("markFailure overrides markAborted (failure wins over abort)", async () => { + it("markError overrides markAborted (error wins over abort)", async () => { await h.service.trace("op", (span) => { span.markAborted(); - span.markFailure(); + span.markError(); return Promise.resolve(); }); expect(h.sink.events[0].properties.result).toBe("error"); }); - it("thrown errors take precedence over markFailure (error block is preserved)", async () => { + it("thrown errors take precedence over markError (error block is preserved)", async () => { const boom = new Error("kaboom"); await expect( h.service.trace("op", (span) => { - span.markFailure(); + span.markError(); return Promise.reject(boom); }), ).rejects.toBe(boom); diff --git a/test/unit/uri/uriHandler.test.ts b/test/unit/uri/uriHandler.test.ts index 9c86547db..82d186e57 100644 --- a/test/unit/uri/uriHandler.test.ts +++ b/test/unit/uri/uriHandler.test.ts @@ -126,6 +126,9 @@ describe("uriHandler", () => { }); describe("/open", () => { + // Fields the URI handler always supplies; per-test overrides spread on top. + const OPEN_DEFAULTS = { source: "uri", useDefaultDirectory: false }; + it("opens workspace with parameters", async () => { const { handleUri, commands, deploymentManager } = createTestContext(); await handleUri( @@ -142,8 +145,7 @@ describe("uriHandler", () => { agentName: "a", folderPath: "/f", openRecent: true, - source: "uri", - useDefaultDirectory: false, + ...OPEN_DEFAULTS, }); }); @@ -162,8 +164,7 @@ describe("uriHandler", () => { agentName: undefined, folderPath: undefined, openRecent: expected, - source: "uri", - useDefaultDirectory: false, + ...OPEN_DEFAULTS, }); }); @@ -180,8 +181,7 @@ describe("uriHandler", () => { agentName: undefined, folderPath: undefined, openRecent: false, - source: "uri", - useDefaultDirectory: false, + ...OPEN_DEFAULTS, }); expect(showErrorMessage).not.toHaveBeenCalled(); }); @@ -197,8 +197,7 @@ describe("uriHandler", () => { agentName: undefined, folderPath: undefined, openRecent: false, - source: "uri", - useDefaultDirectory: false, + ...OPEN_DEFAULTS, }); expect(showErrorMessage).not.toHaveBeenCalled(); });