From 5e1a54522aed4bbe95cbb9590de85850383fb06e Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 12 May 2026 14:43:00 +0000 Subject: [PATCH 1/4] feat(telemetry): instrument auth and workspace state --- CHANGELOG.md | 4 +- src/api/authInterceptor.ts | 20 +++ src/core/container.ts | 1 + src/extension.ts | 2 + src/instrumentation/auth.ts | 49 +++++++ src/instrumentation/workspace.ts | 74 ++++++++++ src/login/loginCoordinator.ts | 128 ++++++++++-------- src/oauth/sessionManager.ts | 27 +++- src/remote/remote.ts | 9 +- src/remote/workspaceStateMachine.ts | 57 +++++++- src/workspace/workspaceMonitor.ts | 35 +++++ test/unit/api/authInterceptor.test.ts | 32 ++++- test/unit/login/loginCoordinator.test.ts | 30 +++- test/unit/oauth/sessionManager.test.ts | 66 ++++++++- .../unit/remote/workspaceStateMachine.test.ts | 90 +++++++++++- test/unit/workspace/workspaceMonitor.test.ts | 55 +++++++- 16 files changed, 606 insertions(+), 73 deletions(-) create mode 100644 src/instrumentation/auth.ts create mode 100644 src/instrumentation/workspace.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 00180352..7f02312f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,10 +23,12 @@ local telemetry. - Local telemetry now records `http.requests` rollups for per-route HTTP health without emitting one event per request. -- Connection lifecycle now records local telemetry: SSH process +- Local telemetry now records connection lifecycle: SSH process discovery/loss/recovery with sampled network info, and reconnecting WebSocket open, drop, reconnect, and state transitions, so connection stability is captured alongside other local telemetry. +- Local telemetry now records authentication refresh/recovery prompts plus + workspace and agent state transitions for connection lifecycle diagnostics. ### Fixed diff --git a/src/api/authInterceptor.ts b/src/api/authInterceptor.ts index d362c1d1..f28c6596 100644 --- a/src/api/authInterceptor.ts +++ b/src/api/authInterceptor.ts @@ -1,6 +1,11 @@ import { type AxiosError, isAxiosError } from "axios"; +import { AuthTelemetry } from "../instrumentation/auth"; import { OAuthError } from "../oauth/errors"; +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; import { toSafeHost } from "../util"; import type * as vscode from "vscode"; @@ -28,6 +33,7 @@ export type AuthRequiredHandler = (hostname: string) => Promise; */ export class AuthInterceptor implements vscode.Disposable { private readonly interceptorId: number; + private readonly authTelemetry: AuthTelemetry; private authRequiredPromise: Promise | null = null; constructor( @@ -36,7 +42,9 @@ export class AuthInterceptor implements vscode.Disposable { private readonly oauthSessionManager: OAuthSessionManager, private readonly secretsManager: SecretsManager, private readonly onAuthRequired?: AuthRequiredHandler, + telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ) { + this.authTelemetry = new AuthTelemetry(telemetry); this.interceptorId = this.client .getAxiosInstance() .interceptors.response.use( @@ -54,6 +62,9 @@ export class AuthInterceptor implements vscode.Disposable { if (error.config) { const config = error.config as { _retryAttempted?: boolean }; if (config._retryAttempted) { + if (error.response?.status === 401) { + this.authTelemetry.intercept401("none"); + } throw error; } } @@ -64,6 +75,7 @@ export class AuthInterceptor implements vscode.Disposable { const baseUrl = this.client.getHost(); if (!baseUrl) { + this.authTelemetry.intercept401("none"); throw error; } const hostname = toSafeHost(baseUrl); @@ -77,11 +89,14 @@ export class AuthInterceptor implements vscode.Disposable { ): Promise { this.logger.debug("Received 401 response, attempting recovery"); + let loggedRecovery = false; + if (await this.oauthSessionManager.isLoggedInWithOAuth(hostname)) { try { const newTokens = await this.oauthSessionManager.refreshToken(); this.client.setSessionToken(newTokens.access_token); this.logger.debug("Token refresh successful, retrying request"); + this.authTelemetry.intercept401("refresh_success"); return this.retryRequest(error, newTokens.access_token); } catch (refreshError) { if (refreshError instanceof OAuthError) { @@ -98,6 +113,8 @@ export class AuthInterceptor implements vscode.Disposable { } if (this.onAuthRequired) { + this.authTelemetry.intercept401("login_required"); + loggedRecovery = true; const success = await this.executeAuthRequired(hostname); if (success) { const auth = await this.secretsManager.getSessionAuth(hostname); @@ -108,6 +125,9 @@ export class AuthInterceptor implements vscode.Disposable { } } + if (!loggedRecovery) { + this.authTelemetry.intercept401("none"); + } throw error; } diff --git a/src/core/container.ts b/src/core/container.ts index 042eb81f..9a0f5a27 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -105,6 +105,7 @@ export class ServiceContainer implements vscode.Disposable { this.cliCredentialManager, this.oauthCallback, context.extension.id, + this.telemetryService, ); this.duplicateWorkspaceIpc = new DuplicateWorkspaceIpc( context.secrets, diff --git a/src/extension.ts b/src/extension.ts index 3067014c..ba790173 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -128,6 +128,7 @@ async function doActivate( deployment, serviceContainer, handleAuthFailure, + telemetryService, ); ctx.subscriptions.push(oauthSessionManager); @@ -152,6 +153,7 @@ async function doActivate( await handleAuthFailure(); return false; }, + telemetryService, ); ctx.subscriptions.push(authInterceptor); diff --git a/src/instrumentation/auth.ts b/src/instrumentation/auth.ts new file mode 100644 index 00000000..6d033d2b --- /dev/null +++ b/src/instrumentation/auth.ts @@ -0,0 +1,49 @@ +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; + +export type AuthTokenRefreshTrigger = "background" | "reactive"; +export type AuthIntercept401Recovery = + | "refresh_success" + | "login_required" + | "none"; +export type AuthLoginPromptTrigger = "auth_required" | "missing_session"; + +interface LoginPromptResult { + readonly success: boolean; +} + +export class AuthTelemetry { + public constructor( + private readonly telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, + ) {} + + public traceTokenRefresh( + trigger: AuthTokenRefreshTrigger, + fn: () => Promise, + ): Promise { + return this.telemetry.trace("auth.token_refresh", fn, { trigger }); + } + + public intercept401(recovery: AuthIntercept401Recovery): void { + this.telemetry.log("auth.intercept_401", { recovery }); + } + + public traceLoginPrompt( + trigger: AuthLoginPromptTrigger, + fn: () => Promise, + ): Promise { + return this.telemetry.trace( + "auth.login_prompt", + async (span) => { + const result = await fn(); + if (!result.success) { + span.markAborted(); + } + return result; + }, + { trigger }, + ); + } +} diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts new file mode 100644 index 00000000..bfee8711 --- /dev/null +++ b/src/instrumentation/workspace.ts @@ -0,0 +1,74 @@ +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; + +import type { + BuildReason, + WorkspaceAgentLifecycle, + WorkspaceAgentStatus, + WorkspaceStatus, + WorkspaceTransition, +} from "coder/site/src/api/typesGenerated"; + +export const INITIAL_STATE = "unknown"; + +type InitialState = typeof INITIAL_STATE; + +interface WorkspaceStateTransition { + readonly from: WorkspaceStatus | InitialState; + readonly to: WorkspaceStatus; + readonly transition?: WorkspaceTransition; + readonly reason?: BuildReason; + readonly observedDurationMs?: number; +} + +interface WorkspaceAgentStateTransition { + readonly agentName: string; + readonly fromStatus: WorkspaceAgentStatus | InitialState; + readonly toStatus: WorkspaceAgentStatus; + readonly fromLifecycleState: WorkspaceAgentLifecycle | InitialState; + readonly toLifecycleState: WorkspaceAgentLifecycle; + readonly observedDurationMs?: number; +} + +export class WorkspaceTelemetry { + public constructor( + private readonly telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, + ) {} + + public workspaceStateTransition(transition: WorkspaceStateTransition): void { + this.telemetry.log( + "workspace.state_transitioned", + { + from: transition.from, + to: transition.to, + ...(transition.transition && { transition: transition.transition }), + ...(transition.reason && { reason: transition.reason }), + }, + transition.observedDurationMs === undefined + ? {} + : { observedDurationMs: transition.observedDurationMs }, + ); + } + + public agentStateTransition(transition: WorkspaceAgentStateTransition): void { + this.telemetry.log( + "workspace.agent.state_transitioned", + { + agentName: transition.agentName, + fromStatus: transition.fromStatus, + toStatus: transition.toStatus, + fromLifecycleState: transition.fromLifecycleState, + toLifecycleState: transition.toLifecycleState, + }, + transition.observedDurationMs === undefined + ? {} + : { observedDurationMs: transition.observedDurationMs }, + ); + } + + public traceUpdateTriggered(fn: () => Promise): Promise { + return this.telemetry.trace("workspace.update.triggered", () => fn()); + } +} diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index 018d9558..c457b667 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -5,11 +5,19 @@ import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; import { needToken } from "../api/utils"; import { CertificateError } from "../error/certificateError"; +import { + AuthTelemetry, + type AuthLoginPromptTrigger, +} from "../instrumentation/auth"; import { OAuthAuthorizer } from "../oauth/authorizer"; import { buildOAuthTokenData } from "../oauth/utils"; import { withOptionalProgress } from "../progress"; import { maybeAskAuthMethod, maybeAskUrl } from "../promptUtils"; import { isKeyringEnabled } from "../settings/cli"; +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; import { vscodeProposed } from "../vscodeProposed"; import type { User } from "coder/site/src/api/typesGenerated"; @@ -38,6 +46,7 @@ export interface LoginOptions { export class LoginCoordinator implements vscode.Disposable { private loginQueue: Promise = Promise.resolve(); private readonly oauthAuthorizer: OAuthAuthorizer; + private readonly authTelemetry: AuthTelemetry; constructor( private readonly secretsManager: SecretsManager, @@ -46,7 +55,9 @@ export class LoginCoordinator implements vscode.Disposable { private readonly cliCredentialManager: CliCredentialManager, oauthCallback: OAuthCallback, extensionId: string, + telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ) { + this.authTelemetry = new AuthTelemetry(telemetry); this.oauthAuthorizer = new OAuthAuthorizer( secretsManager, oauthCallback, @@ -80,63 +91,72 @@ export class LoginCoordinator implements vscode.Disposable { * Shows dialog then login - for system-initiated auth (remote, OAuth refresh). */ public async ensureLoggedInWithDialog( - options: LoginOptions & { message?: string; detailPrefix?: string }, + options: LoginOptions & { + message?: string; + detailPrefix?: string; + trigger?: AuthLoginPromptTrigger; + }, ): Promise { const { safeHostname, url, detailPrefix, message } = options; - return this.executeWithGuard(async () => { - // Show dialog promise - const dialogPromise = vscodeProposed.window - .showErrorMessage( - message || "Authentication Required", - { - modal: true, - useCustom: true, - detail: - (detailPrefix || `Authentication needed for ${safeHostname}.`) + - "\n\nIf you've already logged in, you may close this dialog.", - }, - "Login", - ) - .then(async (action) => { - if (action === "Login") { - // Proceed with the login flow, handling logging in from another window - const storedAuth = - await this.secretsManager.getSessionAuth(safeHostname); - const newUrl = await maybeAskUrl( - this.mementoManager, - url, - storedAuth?.url, - ); - if (!newUrl) { - throw new Error("URL must be provided"); - } - - const result = await this.attemptLogin( - { url: newUrl, safeHostname }, - false, - options.token, - ); - - await this.persistSessionAuth(result, safeHostname, newUrl); - - return result; - } else { - // User cancelled - return { success: false } as const; + return this.authTelemetry.traceLoginPrompt( + options.trigger ?? "auth_required", + () => + this.executeWithGuard(async () => { + // Show dialog promise + const dialogPromise = vscodeProposed.window + .showErrorMessage( + message || "Authentication Required", + { + modal: true, + useCustom: true, + detail: + (detailPrefix || + `Authentication needed for ${safeHostname}.`) + + "\n\nIf you've already logged in, you may close this dialog.", + }, + "Login", + ) + .then(async (action) => { + if (action === "Login") { + // Proceed with the login flow, handling logging in from another window + const storedAuth = + await this.secretsManager.getSessionAuth(safeHostname); + const newUrl = await maybeAskUrl( + this.mementoManager, + url, + storedAuth?.url, + ); + if (!newUrl) { + throw new Error("URL must be provided"); + } + + const result = await this.attemptLogin( + { url: newUrl, safeHostname }, + false, + options.token, + ); + + await this.persistSessionAuth(result, safeHostname, newUrl); + + return result; + } else { + // User cancelled + return { success: false } as const; + } + }); + + // Race between user clicking login and cross-window detection + const { + promise: crossWindowPromise, + dispose: disposeCrossWindowListener, + } = this.waitForCrossWindowLogin(safeHostname); + try { + return await Promise.race([dialogPromise, crossWindowPromise]); + } finally { + disposeCrossWindowListener(); } - }); - - // Race between user clicking login and cross-window detection - const { - promise: crossWindowPromise, - dispose: disposeCrossWindowListener, - } = this.waitForCrossWindowLogin(safeHostname); - try { - return await Promise.race([dialogPromise, crossWindowPromise]); - } finally { - disposeCrossWindowListener(); - } - }); + }), + ); } private async persistSessionAuth( diff --git a/src/oauth/sessionManager.ts b/src/oauth/sessionManager.ts index 32ab7e78..3360d79f 100644 --- a/src/oauth/sessionManager.ts +++ b/src/oauth/sessionManager.ts @@ -1,4 +1,12 @@ import { CoderApi } from "../api/coderApi"; +import { + AuthTelemetry, + type AuthTokenRefreshTrigger, +} from "../instrumentation/auth"; +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; import { DEFAULT_OAUTH_SCOPES, REFRESH_GRANT_TYPE } from "./constants"; import { OAuthError, parseOAuthError } from "./errors"; @@ -58,24 +66,31 @@ export class OAuthSessionManager implements vscode.Disposable { deployment: Deployment | null, container: ServiceContainer, onAuthRequired: () => Promise = () => Promise.resolve(), + telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ): OAuthSessionManager { const manager = new OAuthSessionManager( deployment, container.getSecretsManager(), container.getLogger(), onAuthRequired, + telemetry, ); manager.setupTokenListener(); manager.scheduleNextRefresh(); return manager; } + private readonly authTelemetry: AuthTelemetry; + private constructor( private deployment: Deployment | null, private readonly secretsManager: SecretsManager, private readonly logger: Logger, private readonly onAuthRequired: () => Promise, - ) {} + telemetry: TelemetryReporter, + ) { + this.authTelemetry = new AuthTelemetry(telemetry); + } /** * Get current deployment, throwing if not set. @@ -218,7 +233,7 @@ export class OAuthSessionManager implements vscode.Disposable { this.refreshTimer = undefined; - this.refreshToken() + this.refreshToken("background") .then(() => { this.logger.debug("Background token refresh succeeded"); }) @@ -342,7 +357,9 @@ export class OAuthSessionManager implements vscode.Disposable { * Refresh the access token using the stored refresh token. * Uses a shared promise to handle concurrent refresh attempts. */ - public async refreshToken(): Promise { + public async refreshToken( + trigger: AuthTokenRefreshTrigger = "reactive", + ): Promise { if (this.refreshPromise) { this.logger.debug( "Token refresh already in progress, waiting for result", @@ -352,7 +369,9 @@ export class OAuthSessionManager implements vscode.Disposable { const deployment = this.requireDeployment(); // Assign synchronously before any async work to prevent race conditions - this.refreshPromise = this.executeTokenRefresh(deployment); + this.refreshPromise = this.authTelemetry.traceTokenRefresh(trigger, () => + this.executeTokenRefresh(deployment), + ); return this.refreshPromise; } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 29077169..6324847e 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -195,12 +195,14 @@ export class Remote { try { // Create OAuth session manager for this remote deployment + const telemetry = this.serviceContainer.getTelemetryService(); const remoteOAuthManager = OAuthSessionManager.create( { url: baseUrl, safeHostname: parts.safeHostname }, this.serviceContainer, async () => { await this.showSessionExpiredDialog(context); }, + telemetry, ); disposables.push(remoteOAuthManager); @@ -216,7 +218,7 @@ export class Remote { baseUrl, token, this.logger, - this.serviceContainer.getTelemetryService(), + telemetry, ); disposables.push(workspaceClient); @@ -230,6 +232,7 @@ export class Remote { const result = await this.showSessionExpiredDialog(context); return result.success; }, + telemetry, ); disposables.push(authInterceptor); @@ -315,6 +318,7 @@ export class Remote { workspaceClient, this.logger, this.contextManager, + telemetry, ); disposables.push( monitor, @@ -334,6 +338,7 @@ export class Remote { featureSet, this.logger, cliAuth, + telemetry, ); disposables.push(stateMachine); @@ -642,6 +647,7 @@ export class Remote { url: context.baseUrl, message: "Your session expired...", detailPrefix: `You must log in to access ${context.workspaceName}.`, + trigger: "auth_required", }); } @@ -655,6 +661,7 @@ export class Remote { url, message, detailPrefix: `You must log in to access ${context.workspaceName}.`, + trigger: "missing_session", }); // Dispose before retrying since setup will create new disposables. diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index b533ffb8..3ace82c8 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -6,7 +6,12 @@ import { streamAgentLogs, streamBuildLogs, } from "../api/workspace"; +import { WorkspaceTelemetry } from "../instrumentation/workspace"; import { maybeAskAgent } from "../promptUtils"; +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; import { vscodeProposed } from "../vscodeProposed"; import { TerminalOutputChannel } from "./terminalOutputChannel"; @@ -14,7 +19,10 @@ import { TerminalOutputChannel } from "./terminalOutputChannel"; import type { ProvisionerJobLog, Workspace, + WorkspaceAgent, + WorkspaceAgentLifecycle, WorkspaceAgentLog, + WorkspaceAgentStatus, } from "coder/site/src/api/typesGenerated"; import type * as vscode from "vscode"; @@ -29,12 +37,20 @@ import type { AuthorityParts } from "../util"; * Manages workspace and agent state transitions until ready for SSH connection. * Streams build and agent logs, and handles socket lifecycle. */ +interface ObservedAgentState { + readonly status: WorkspaceAgentStatus; + readonly lifecycleState: WorkspaceAgentLifecycle; + readonly observedAtMs: number; +} + export class WorkspaceStateMachine implements vscode.Disposable { private readonly terminal: TerminalOutputChannel; private readonly buildLogStream = new LazyStream(); private readonly agentLogStream = new LazyStream(); + private readonly telemetry: WorkspaceTelemetry; private agent: { id: string; name: string } | undefined; + private observedAgentState: ObservedAgentState | undefined; constructor( private readonly parts: AuthorityParts, @@ -44,8 +60,10 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly featureSet: FeatureSet, private readonly logger: Logger, private readonly cliAuth: CliAuth, + telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ) { this.terminal = new TerminalOutputChannel("Coder: Workspace Build"); + this.telemetry = new WorkspaceTelemetry(telemetry); } /** @@ -64,7 +82,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { if (this.startupMode === "update") { await this.triggerUpdate(workspace, workspaceName, progress); // Agent IDs may have changed after an update. - this.agent = undefined; + this.resetAgent(); } break; @@ -95,7 +113,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { case "starting": case "stopping": { // Clear the agent since its ID could change after a restart - this.agent = undefined; + this.resetAgent(); this.agentLogStream.close(); progress.report({ message: `building ${workspaceName} (${workspace.latest_build.status})...`, @@ -140,6 +158,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { `Agent ${this.agent.name} not found in ${workspaceName} resources`, ); } + this.recordAgentState(agent); switch (agent.status) { case "connecting": @@ -258,7 +277,9 @@ export class WorkspaceStateMachine implements vscode.Disposable { mode: this.startupMode, status: workspace.latest_build.status, }); - await updateWorkspace(this.buildCliContext(workspace)); + await this.telemetry.traceUpdateTriggered(() => + updateWorkspace(this.buildCliContext(workspace)), + ); // Downgrade so subsequent transitions don't re-trigger the update. this.startupMode = "start"; this.logger.info(`${workspaceName} update initiated`); @@ -286,6 +307,36 @@ export class WorkspaceStateMachine implements vscode.Disposable { return this.agent?.id; } + private resetAgent(): void { + this.agent = undefined; + this.observedAgentState = undefined; + } + + private recordAgentState(agent: WorkspaceAgent): void { + const now = performance.now(); + const previous = this.observedAgentState; + if ( + previous?.status === agent.status && + previous.lifecycleState === agent.lifecycle_state + ) { + return; + } + + this.telemetry.agentStateTransition({ + agentName: agent.name, + fromStatus: previous?.status ?? "unknown", + toStatus: agent.status, + fromLifecycleState: previous?.lifecycleState ?? "unknown", + toLifecycleState: agent.lifecycle_state, + ...(previous && { observedDurationMs: now - previous.observedAtMs }), + }); + this.observedAgentState = { + status: agent.status, + lifecycleState: agent.lifecycle_state, + observedAtMs: now, + }; + } + dispose(): void { this.buildLogStream.close(); this.agentLogStream.close(); diff --git a/src/workspace/workspaceMonitor.ts b/src/workspace/workspaceMonitor.ts index 7f37b07a..01c549a7 100644 --- a/src/workspace/workspaceMonitor.ts +++ b/src/workspace/workspaceMonitor.ts @@ -1,15 +1,21 @@ import { type ServerSentEvent, type Workspace, + type WorkspaceStatus, } from "coder/site/src/api/typesGenerated"; import { formatDistanceToNowStrict } from "date-fns"; import * as vscode from "vscode"; import { createWorkspaceIdentifier, errToStr } from "../api/api-helper"; +import { WorkspaceTelemetry } from "../instrumentation/workspace"; import { areNotificationsDisabled, areUpdateNotificationsDisabled, } from "../settings/notifications"; +import { + NOOP_TELEMETRY_REPORTER, + type TelemetryReporter, +} from "../telemetry/reporter"; import { vscodeProposed } from "../vscodeProposed"; import type { CoderApi } from "../api/coderApi"; @@ -42,16 +48,21 @@ export class WorkspaceMonitor implements vscode.Disposable { // For logging. private readonly name: string; + private readonly telemetry: WorkspaceTelemetry; private latestWorkspace: Workspace; + private lastWorkspaceStatus: WorkspaceStatus | undefined; + private lastWorkspaceStatusObservedAtMs: number | undefined; private constructor( workspace: Workspace, private readonly client: CoderApi, private readonly logger: Logger, private readonly contextManager: ContextManager, + telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ) { this.name = createWorkspaceIdentifier(workspace); + this.telemetry = new WorkspaceTelemetry(telemetry); this.latestWorkspace = workspace; const statusBarItem = vscode.window.createStatusBarItem( @@ -77,12 +88,14 @@ export class WorkspaceMonitor implements vscode.Disposable { client: CoderApi, logger: Logger, contextManager: ContextManager, + telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ): Promise { const monitor = new WorkspaceMonitor( workspace, client, logger, contextManager, + telemetry, ); // Initialize websocket connection @@ -134,11 +147,33 @@ export class WorkspaceMonitor implements vscode.Disposable { } private update(workspace: Workspace) { + this.recordWorkspaceStatus(workspace); this.latestWorkspace = workspace; this.updateContext(workspace); this.updateStatusBar(workspace); } + private recordWorkspaceStatus(workspace: Workspace): void { + const status = workspace.latest_build.status; + const now = performance.now(); + const previous = this.lastWorkspaceStatus; + if (previous === status) { + return; + } + + this.telemetry.workspaceStateTransition({ + from: previous ?? "unknown", + to: status, + transition: workspace.latest_build.transition, + reason: workspace.latest_build.reason, + ...(this.lastWorkspaceStatusObservedAtMs !== undefined && { + observedDurationMs: now - this.lastWorkspaceStatusObservedAtMs, + }), + }); + this.lastWorkspaceStatus = status; + this.lastWorkspaceStatusObservedAtMs = now; + } + private maybeNotify(workspace: Workspace) { const cfg = vscode.workspace.getConfiguration(); if (areNotificationsDisabled(cfg)) { diff --git a/test/unit/api/authInterceptor.test.ts b/test/unit/api/authInterceptor.test.ts index e2b8ed3c..9527b8fe 100644 --- a/test/unit/api/authInterceptor.test.ts +++ b/test/unit/api/authInterceptor.test.ts @@ -7,11 +7,13 @@ import { } from "@/api/authInterceptor"; import { SecretsManager } from "@/core/secretsManager"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; import { createAxiosError, createMockLogger, InMemoryMemento, InMemorySecretStorage, + MockConfigurationProvider, MockOAuthSessionManager, } from "../../mocks/testHelpers"; import { @@ -22,6 +24,7 @@ import { import type { CoderApi } from "@/api/coderApi"; import type { OAuthSessionManager } from "@/oauth/sessionManager"; +import type { TelemetryReporter } from "@/telemetry/reporter"; /** * Creates a mock axios instance with controllable interceptors. @@ -84,6 +87,7 @@ const ONE_HOUR_MS = 60 * 60 * 1000; function createTestContext() { vi.resetAllMocks(); + new MockConfigurationProvider().set("coder.telemetry.level", "local"); const secretStorage = new InMemorySecretStorage(); const memento = new InMemoryMemento(); @@ -136,13 +140,17 @@ function createTestContext() { }; /** Creates interceptor with optional callback */ - const createInterceptor = (onAuthRequired?: AuthRequiredHandler) => + const createInterceptor = ( + onAuthRequired?: AuthRequiredHandler, + telemetry?: TelemetryReporter, + ) => new AuthInterceptor( mockCoderApi, logger, mockOAuthManager as unknown as OAuthSessionManager, secretsManager, onAuthRequired, + telemetry, ); return { @@ -182,7 +190,7 @@ describe("AuthInterceptor", () => { }); describe("401 handling with OAuth", () => { - it("refreshes token and retries request", async () => { + it("refreshes token, retries request, and emits telemetry", async () => { const { mockCoderApi, mockOAuthManager, @@ -190,6 +198,7 @@ describe("AuthInterceptor", () => { setupOAuthTokens, createInterceptor, } = createTestContext(); + const sink = new TestSink(); await setupOAuthTokens(); @@ -201,13 +210,19 @@ describe("AuthInterceptor", () => { const retryResponse = { data: "success", status: 200 }; vi.spyOn(axiosInstance, "request").mockResolvedValue(retryResponse); - createInterceptor(); + createInterceptor(undefined, createTestTelemetryService(sink)); const error = createAxiosError(401, "Unauthorized"); const result = await axiosInstance.triggerResponseError(error); expect(mockCoderApi.getSessionToken()).toBe("new-access-token"); expect(result).toEqual(retryResponse); + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "auth.intercept_401", + properties: { recovery: "refresh_success" }, + }), + ); }); it("does not retry if already retried", async () => { @@ -255,16 +270,23 @@ describe("AuthInterceptor", () => { }); describe("401 handling with callback (non-OAuth)", () => { - it("calls onAuthRequired callback on 401", async () => { + it("calls onAuthRequired callback on 401 and emits login-required recovery", async () => { const { axiosInstance, createInterceptor } = createTestContext(); + const sink = new TestSink(); const onAuthRequired = vi.fn().mockResolvedValue(false); - createInterceptor(onAuthRequired); + createInterceptor(onAuthRequired, createTestTelemetryService(sink)); const error = createAxiosError(401, "Unauthorized"); await expect(axiosInstance.triggerResponseError(error)).rejects.toThrow(); expect(onAuthRequired).toHaveBeenCalledWith(TEST_HOSTNAME); + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "auth.intercept_401", + properties: { recovery: "login_required" }, + }), + ); }); it("retries request when callback returns true", async () => { diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index cf376a97..dc77cc6b 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -9,6 +9,7 @@ import { LoginCoordinator } from "@/login/loginCoordinator"; import { OAuthCallback } from "@/oauth/oauthCallback"; import { maybeAskAuthMethod } from "@/promptUtils"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; import { createAxiosError, createMockCliCredentialManager, @@ -21,6 +22,8 @@ import { MockUserInteraction, } from "../../mocks/testHelpers"; +import type { TelemetryReporter } from "@/telemetry/reporter"; + // Hoisted mock adapter implementation const mockAxiosAdapterImpl = vi.hoisted( () => (config: Record) => @@ -98,7 +101,7 @@ const TEST_HOSTNAME = "coder.example.com"; /** * Creates a fresh test context with all dependencies. */ -function createTestContext() { +function createTestContext(telemetry?: TelemetryReporter) { vi.resetAllMocks(); const mockAdapter = (axios as MockedAxios).__mockAdapter; @@ -127,6 +130,7 @@ function createTestContext() { mockCredentialManager, oauthCallback, "coder.coder-remote", + telemetry, ); const mockSuccessfulAuth = (user = createMockUser()) => { @@ -357,6 +361,30 @@ describe("LoginCoordinator", () => { expect(result.success).toBe(false); }); + + it("emits telemetry with trigger and aborted result when dismissed", async () => { + const sink = new TestSink(); + const { userInteraction, coordinator } = createTestContext( + createTestTelemetryService(sink), + ); + userInteraction.setResponse("Authentication Required", undefined); + + await coordinator.ensureLoggedInWithDialog({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + trigger: "missing_session", + }); + + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "auth.login_prompt", + properties: expect.objectContaining({ + trigger: "missing_session", + result: "aborted", + }), + }), + ); + }); }); describe("token fallback order", () => { diff --git a/test/unit/oauth/sessionManager.test.ts b/test/unit/oauth/sessionManager.test.ts index 5cb1af2f..dfaaeaf8 100644 --- a/test/unit/oauth/sessionManager.test.ts +++ b/test/unit/oauth/sessionManager.test.ts @@ -9,8 +9,10 @@ import { type SecretsManager, type SessionAuth } from "@/core/secretsManager"; import { DEFAULT_OAUTH_SCOPES } from "@/oauth/constants"; import { OAuthSessionManager } from "@/oauth/sessionManager"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; import { type createMockLogger, + MockConfigurationProvider, setupAxiosMockRoutes, } from "../../mocks/testHelpers"; @@ -27,6 +29,7 @@ import { import type { ServiceContainer } from "@/core/container"; import type { Deployment } from "@/deployment/types"; +import type { TelemetryReporter } from "@/telemetry/reporter"; vi.mock("axios", async () => { const actual = await vi.importActual("axios"); @@ -102,7 +105,8 @@ function createTestContext(deployment: Deployment = createTestDeployment()) { const createManager = ( d: Deployment = deployment, onAuthRequired: () => Promise = () => Promise.resolve(), - ) => OAuthSessionManager.create(d, container, onAuthRequired); + telemetry?: TelemetryReporter, + ) => OAuthSessionManager.create(d, container, onAuthRequired, telemetry); /** * Sets up a complete OAuth operation test context. @@ -191,6 +195,38 @@ describe("OAuthSessionManager", () => { ); }); + it("emits telemetry for reactive refresh", async () => { + const { createManager, setupForOAuthOperation } = createTestContext(); + const sink = new TestSink(); + new MockConfigurationProvider().set("coder.telemetry.level", "local"); + const manager = createManager( + createTestDeployment(), + () => Promise.resolve(), + createTestTelemetryService(sink), + ); + + await setupForOAuthOperation({ + "/oauth2/token": createMockTokenResponse({ + access_token: "telemetry-token", + }), + }); + + await manager.refreshToken(); + + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "auth.token_refresh", + properties: expect.objectContaining({ + trigger: "reactive", + result: "success", + }), + measurements: expect.objectContaining({ + durationMs: expect.any(Number), + }), + }), + ); + }); + it("refreshes token successfully", async () => { const { secretsManager, mockAdapter, manager, setupOAuthSession } = createTestContext(); @@ -290,6 +326,34 @@ describe("OAuthSessionManager", () => { expect(auth?.token).toBe("background-refreshed-token"); }); + it("emits telemetry with background trigger", async () => { + const { createManager, setupForOAuthOperation } = createTestContext(); + const sink = new TestSink(); + new MockConfigurationProvider().set("coder.telemetry.level", "local"); + const manager = createManager( + createTestDeployment(), + () => Promise.resolve(), + createTestTelemetryService(sink), + ); + + await setupForOAuthOperation({ + "/oauth2/token": createMockTokenResponse({ + access_token: "background-token", + }), + }); + await manager.refreshToken("background"); + + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "auth.token_refresh", + properties: expect.objectContaining({ + trigger: "background", + result: "success", + }), + }), + ); + }); + it("calls onAuthRequired when refresh fails with re-auth error", async () => { const { createManager } = await setupBackgroundRefreshTest( createOAuthAxiosError("invalid_grant"), diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index 1b62a217..e0450aff 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -15,8 +15,10 @@ import { workspace as createWorkspace, } from "@repo/mocks"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; import { createMockLogger, + MockConfigurationProvider, MockProgress, MockTerminalOutputChannel, MockUserInteraction, @@ -30,6 +32,7 @@ import type { import type { CoderApi } from "@/api/coderApi"; import type { StartupMode } from "@/core/mementoManager"; import type { FeatureSet } from "@/featureSet"; +import type { TelemetryReporter } from "@/telemetry/reporter"; import type { AuthorityParts } from "@/util"; vi.mock("@/api/workspace", async (importActual) => { @@ -75,7 +78,11 @@ function runningWorkspace( }); } -function setup(startupMode: StartupMode = "start") { +function setup( + startupMode: StartupMode = "start", + telemetry?: TelemetryReporter, +) { + new MockConfigurationProvider().set("coder.telemetry.level", "local"); const progress = new MockProgress<{ message?: string }>(); const userInteraction = new MockUserInteraction(); const sm = new WorkspaceStateMachine( @@ -86,6 +93,7 @@ function setup(startupMode: StartupMode = "start") { {} as FeatureSet, createMockLogger(), { mode: "url", url: "https://test.coder.com" }, + telemetry, ); return { sm, progress, userInteraction }; } @@ -327,6 +335,86 @@ describe("WorkspaceStateMachine", () => { } }); + describe("telemetry", () => { + it("emits update triggered telemetry with duration", async () => { + const sink = new TestSink(); + const { sm, progress } = setup( + "update", + createTestTelemetryService(sink), + ); + + await sm.processWorkspace(runningWorkspace(), progress); + + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "workspace.update.triggered", + properties: expect.objectContaining({ result: "success" }), + measurements: expect.objectContaining({ + durationMs: expect.any(Number), + }), + }), + ); + }); + + it("emits error result when update fails", async () => { + const sink = new TestSink(); + const { sm, progress } = setup( + "update", + createTestTelemetryService(sink), + ); + vi.mocked(updateWorkspace).mockRejectedValueOnce( + new Error("update failed"), + ); + + await expect( + sm.processWorkspace(runningWorkspace(), progress), + ).rejects.toThrow("update failed"); + + expect(sink.events).toContainEqual( + expect.objectContaining({ + eventName: "workspace.update.triggered", + properties: expect.objectContaining({ result: "error" }), + error: { message: "update failed" }, + }), + ); + }); + + it("emits selected-agent state transitions with observed duration", async () => { + const sink = new TestSink(); + const { sm, progress } = setup("start", createTestTelemetryService(sink)); + + await sm.processWorkspace( + runningWorkspace({ status: "connecting", lifecycle_state: "created" }), + progress, + ); + await sm.processWorkspace(runningWorkspace(), progress); + + const agentEvents = sink.events.filter( + (event) => event.eventName === "workspace.agent.state_transitioned", + ); + expect(agentEvents).toHaveLength(2); + expect(agentEvents[0]).toMatchObject({ + properties: { + agentName: "main", + fromStatus: "unknown", + toStatus: "connecting", + fromLifecycleState: "unknown", + toLifecycleState: "created", + }, + }); + expect(agentEvents[1]).toMatchObject({ + properties: { + agentName: "main", + fromStatus: "connecting", + toStatus: "connected", + fromLifecycleState: "created", + toLifecycleState: "ready", + }, + measurements: { observedDurationMs: expect.any(Number) }, + }); + }); + }); + describe("agent selection", () => { it("throws when user declines agent selection", async () => { vi.mocked(maybeAskAgent).mockResolvedValue(undefined); diff --git a/test/unit/workspace/workspaceMonitor.test.ts b/test/unit/workspace/workspaceMonitor.test.ts index 5ceedcad..5ad3ba33 100644 --- a/test/unit/workspace/workspaceMonitor.test.ts +++ b/test/unit/workspace/workspaceMonitor.test.ts @@ -5,6 +5,7 @@ import { WorkspaceMonitor } from "@/workspace/workspaceMonitor"; import { workspace as createWorkspace } from "@repo/mocks"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; import { MockConfigurationProvider, MockContextManager, @@ -20,6 +21,7 @@ import type { import type { CoderApi } from "@/api/coderApi"; import type { ContextManager } from "@/core/contextManager"; +import type { TelemetryReporter } from "@/telemetry/reporter"; function workspaceEvent( overrides?: Parameters[0], @@ -36,7 +38,11 @@ describe("WorkspaceMonitor", () => { vi.resetAllMocks(); }); - async function setup(stream = new MockEventStream()) { + async function setup( + stream = new MockEventStream(), + telemetry?: TelemetryReporter, + initialWorkspace: Workspace = createWorkspace(), + ) { const config = new MockConfigurationProvider(); const statusBar = new MockStatusBarItem(); const contextManager = new MockContextManager(); @@ -50,14 +56,59 @@ describe("WorkspaceMonitor", () => { }), } as unknown as CoderApi; const monitor = await WorkspaceMonitor.create( - createWorkspace(), + initialWorkspace, client, createMockLogger(), contextManager as unknown as ContextManager, + telemetry, ); return { monitor, client, stream, config, statusBar, contextManager }; } + describe("telemetry", () => { + it("emits initial workspace state and observed transitions", async () => { + const stream = new MockEventStream(); + const sink = new TestSink(); + new MockConfigurationProvider().set("coder.telemetry.level", "local"); + + await setup( + stream, + createTestTelemetryService(sink), + createWorkspace({ latest_build: { status: "running" } }), + ); + stream.pushMessage( + workspaceEvent({ + latest_build: { + status: "stopping", + transition: "stop", + reason: "autostop", + }, + }), + ); + + const events = sink.events.filter( + (event) => event.eventName === "workspace.state_transitioned", + ); + expect(events).toHaveLength(2); + expect(events[0]).toMatchObject({ + properties: { + from: "unknown", + to: "running", + }, + }); + expect(events[0].measurements.observedDurationMs).toBeUndefined(); + expect(events[1]).toMatchObject({ + properties: { + from: "running", + to: "stopping", + transition: "stop", + reason: "autostop", + }, + measurements: { observedDurationMs: expect.any(Number) }, + }); + }); + }); + describe("websocket lifecycle", () => { it("fires onChange when a workspace message arrives", async () => { const { monitor, stream } = await setup(); From e6acbcf7f6a1e20fc5ab9497787b64f38fa27d36 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 12 May 2026 20:47:40 +0000 Subject: [PATCH 2/4] refactor(telemetry): keep lifecycle tracking isolated --- CHANGELOG.md | 7 ++ src/api/authInterceptor.ts | 21 +++-- src/instrumentation/workspace.ts | 71 ++++++++++++++++- src/login/loginCoordinator.ts | 118 ++++++++++++++-------------- src/remote/workspaceStateMachine.ts | 39 +-------- src/workspace/workspaceMonitor.ts | 26 +----- 6 files changed, 149 insertions(+), 133 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f02312f..c8ebd7f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,12 +23,19 @@ local telemetry. - Local telemetry now records `http.requests` rollups for per-route HTTP health without emitting one event per request. +<<<<<<< HEAD - Local telemetry now records connection lifecycle: SSH process discovery/loss/recovery with sampled network info, and reconnecting WebSocket open, drop, reconnect, and state transitions, so connection stability is captured alongside other local telemetry. - Local telemetry now records authentication refresh/recovery prompts plus workspace and agent state transitions for connection lifecycle diagnostics. +======= +- Local telemetry now records authentication refresh/recovery attempts for + connection debugging. +- Local telemetry now records workspace and agent state transitions with observed + durations for connection lifecycle diagnostics. +>>>>>>> c7d49c2 (refactor(telemetry): keep lifecycle tracking isolated) ### Fixed diff --git a/src/api/authInterceptor.ts b/src/api/authInterceptor.ts index f28c6596..fd0e95aa 100644 --- a/src/api/authInterceptor.ts +++ b/src/api/authInterceptor.ts @@ -1,6 +1,9 @@ import { type AxiosError, isAxiosError } from "axios"; -import { AuthTelemetry } from "../instrumentation/auth"; +import { + AuthTelemetry, + type AuthIntercept401Recovery, +} from "../instrumentation/auth"; import { OAuthError } from "../oauth/errors"; import { NOOP_TELEMETRY_REPORTER, @@ -62,9 +65,6 @@ export class AuthInterceptor implements vscode.Disposable { if (error.config) { const config = error.config as { _retryAttempted?: boolean }; if (config._retryAttempted) { - if (error.response?.status === 401) { - this.authTelemetry.intercept401("none"); - } throw error; } } @@ -75,7 +75,6 @@ export class AuthInterceptor implements vscode.Disposable { const baseUrl = this.client.getHost(); if (!baseUrl) { - this.authTelemetry.intercept401("none"); throw error; } const hostname = toSafeHost(baseUrl); @@ -89,14 +88,15 @@ export class AuthInterceptor implements vscode.Disposable { ): Promise { this.logger.debug("Received 401 response, attempting recovery"); - let loggedRecovery = false; + let recovery: AuthIntercept401Recovery = "none"; if (await this.oauthSessionManager.isLoggedInWithOAuth(hostname)) { try { const newTokens = await this.oauthSessionManager.refreshToken(); this.client.setSessionToken(newTokens.access_token); this.logger.debug("Token refresh successful, retrying request"); - this.authTelemetry.intercept401("refresh_success"); + recovery = "refresh_success"; + this.authTelemetry.intercept401(recovery); return this.retryRequest(error, newTokens.access_token); } catch (refreshError) { if (refreshError instanceof OAuthError) { @@ -113,8 +113,7 @@ export class AuthInterceptor implements vscode.Disposable { } if (this.onAuthRequired) { - this.authTelemetry.intercept401("login_required"); - loggedRecovery = true; + recovery = "login_required"; const success = await this.executeAuthRequired(hostname); if (success) { const auth = await this.secretsManager.getSessionAuth(hostname); @@ -125,9 +124,7 @@ export class AuthInterceptor implements vscode.Disposable { } } - if (!loggedRecovery) { - this.authTelemetry.intercept401("none"); - } + this.authTelemetry.intercept401(recovery); throw error; } diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index bfee8711..7d36001f 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -5,6 +5,8 @@ import { import type { BuildReason, + Workspace, + WorkspaceAgent, WorkspaceAgentLifecycle, WorkspaceAgentStatus, WorkspaceStatus, @@ -32,12 +34,75 @@ interface WorkspaceAgentStateTransition { readonly observedDurationMs?: number; } +interface ObservedWorkspaceState { + readonly status: WorkspaceStatus; + readonly observedAtMs: number; +} + +interface ObservedAgentState { + readonly status: WorkspaceAgentStatus; + readonly lifecycleState: WorkspaceAgentLifecycle; + readonly observedAtMs: number; +} + export class WorkspaceTelemetry { + private observedWorkspaceState: ObservedWorkspaceState | undefined; + private observedAgentState: ObservedAgentState | undefined; + public constructor( private readonly telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER, ) {} - public workspaceStateTransition(transition: WorkspaceStateTransition): void { + public observeWorkspace(workspace: Workspace): void { + const status = workspace.latest_build.status; + const now = performance.now(); + const previous = this.observedWorkspaceState; + if (previous?.status === status) { + return; + } + + this.workspaceStateTransition({ + from: previous?.status ?? INITIAL_STATE, + to: status, + transition: workspace.latest_build.transition, + reason: workspace.latest_build.reason, + ...(previous && { + observedDurationMs: now - previous.observedAtMs, + }), + }); + this.observedWorkspaceState = { status, observedAtMs: now }; + } + + public observeAgent(agent: WorkspaceAgent): void { + const now = performance.now(); + const previous = this.observedAgentState; + if ( + previous?.status === agent.status && + previous.lifecycleState === agent.lifecycle_state + ) { + return; + } + + this.agentStateTransition({ + agentName: agent.name, + fromStatus: previous?.status ?? INITIAL_STATE, + toStatus: agent.status, + fromLifecycleState: previous?.lifecycleState ?? INITIAL_STATE, + toLifecycleState: agent.lifecycle_state, + ...(previous && { observedDurationMs: now - previous.observedAtMs }), + }); + this.observedAgentState = { + status: agent.status, + lifecycleState: agent.lifecycle_state, + observedAtMs: now, + }; + } + + public resetAgent(): void { + this.observedAgentState = undefined; + } + + private workspaceStateTransition(transition: WorkspaceStateTransition): void { this.telemetry.log( "workspace.state_transitioned", { @@ -52,7 +117,9 @@ export class WorkspaceTelemetry { ); } - public agentStateTransition(transition: WorkspaceAgentStateTransition): void { + private agentStateTransition( + transition: WorkspaceAgentStateTransition, + ): void { this.telemetry.log( "workspace.agent.state_transitioned", { diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index c457b667..96d2d4b8 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -97,68 +97,72 @@ export class LoginCoordinator implements vscode.Disposable { trigger?: AuthLoginPromptTrigger; }, ): Promise { - const { safeHostname, url, detailPrefix, message } = options; return this.authTelemetry.traceLoginPrompt( options.trigger ?? "auth_required", - () => - this.executeWithGuard(async () => { - // Show dialog promise - const dialogPromise = vscodeProposed.window - .showErrorMessage( - message || "Authentication Required", - { - modal: true, - useCustom: true, - detail: - (detailPrefix || - `Authentication needed for ${safeHostname}.`) + - "\n\nIf you've already logged in, you may close this dialog.", - }, - "Login", - ) - .then(async (action) => { - if (action === "Login") { - // Proceed with the login flow, handling logging in from another window - const storedAuth = - await this.secretsManager.getSessionAuth(safeHostname); - const newUrl = await maybeAskUrl( - this.mementoManager, - url, - storedAuth?.url, - ); - if (!newUrl) { - throw new Error("URL must be provided"); - } - - const result = await this.attemptLogin( - { url: newUrl, safeHostname }, - false, - options.token, - ); - - await this.persistSessionAuth(result, safeHostname, newUrl); - - return result; - } else { - // User cancelled - return { success: false } as const; - } - }); - - // Race between user clicking login and cross-window detection - const { - promise: crossWindowPromise, - dispose: disposeCrossWindowListener, - } = this.waitForCrossWindowLogin(safeHostname); - try { - return await Promise.race([dialogPromise, crossWindowPromise]); - } finally { - disposeCrossWindowListener(); - } - }), + () => this.performLoginDialog(options), ); } + private async performLoginDialog( + options: LoginOptions & { message?: string; detailPrefix?: string }, + ): Promise { + const { safeHostname, url, detailPrefix, message } = options; + return this.executeWithGuard(async () => { + // Show dialog promise + const dialogPromise = vscodeProposed.window + .showErrorMessage( + message || "Authentication Required", + { + modal: true, + useCustom: true, + detail: + (detailPrefix || `Authentication needed for ${safeHostname}.`) + + "\n\nIf you've already logged in, you may close this dialog.", + }, + "Login", + ) + .then(async (action) => { + if (action === "Login") { + // Proceed with the login flow, handling logging in from another window + const storedAuth = + await this.secretsManager.getSessionAuth(safeHostname); + const newUrl = await maybeAskUrl( + this.mementoManager, + url, + storedAuth?.url, + ); + if (!newUrl) { + throw new Error("URL must be provided"); + } + + const result = await this.attemptLogin( + { url: newUrl, safeHostname }, + false, + options.token, + ); + + await this.persistSessionAuth(result, safeHostname, newUrl); + + return result; + } else { + // User cancelled + return { success: false } as const; + } + }); + + // Race between user clicking login and cross-window detection + const { + promise: crossWindowPromise, + dispose: disposeCrossWindowListener, + } = this.waitForCrossWindowLogin(safeHostname); + try { + return await Promise.race([dialogPromise, crossWindowPromise]); + } finally { + disposeCrossWindowListener(); + } + }); + } + private async persistSessionAuth( result: LoginResult, safeHostname: string, diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index 3ace82c8..8b347eb0 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -19,10 +19,7 @@ import { TerminalOutputChannel } from "./terminalOutputChannel"; import type { ProvisionerJobLog, Workspace, - WorkspaceAgent, - WorkspaceAgentLifecycle, WorkspaceAgentLog, - WorkspaceAgentStatus, } from "coder/site/src/api/typesGenerated"; import type * as vscode from "vscode"; @@ -37,12 +34,6 @@ import type { AuthorityParts } from "../util"; * Manages workspace and agent state transitions until ready for SSH connection. * Streams build and agent logs, and handles socket lifecycle. */ -interface ObservedAgentState { - readonly status: WorkspaceAgentStatus; - readonly lifecycleState: WorkspaceAgentLifecycle; - readonly observedAtMs: number; -} - export class WorkspaceStateMachine implements vscode.Disposable { private readonly terminal: TerminalOutputChannel; private readonly buildLogStream = new LazyStream(); @@ -50,7 +41,6 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly telemetry: WorkspaceTelemetry; private agent: { id: string; name: string } | undefined; - private observedAgentState: ObservedAgentState | undefined; constructor( private readonly parts: AuthorityParts, @@ -158,7 +148,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { `Agent ${this.agent.name} not found in ${workspaceName} resources`, ); } - this.recordAgentState(agent); + this.telemetry.observeAgent(agent); switch (agent.status) { case "connecting": @@ -309,32 +299,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { private resetAgent(): void { this.agent = undefined; - this.observedAgentState = undefined; - } - - private recordAgentState(agent: WorkspaceAgent): void { - const now = performance.now(); - const previous = this.observedAgentState; - if ( - previous?.status === agent.status && - previous.lifecycleState === agent.lifecycle_state - ) { - return; - } - - this.telemetry.agentStateTransition({ - agentName: agent.name, - fromStatus: previous?.status ?? "unknown", - toStatus: agent.status, - fromLifecycleState: previous?.lifecycleState ?? "unknown", - toLifecycleState: agent.lifecycle_state, - ...(previous && { observedDurationMs: now - previous.observedAtMs }), - }); - this.observedAgentState = { - status: agent.status, - lifecycleState: agent.lifecycle_state, - observedAtMs: now, - }; + this.telemetry.resetAgent(); } dispose(): void { diff --git a/src/workspace/workspaceMonitor.ts b/src/workspace/workspaceMonitor.ts index 01c549a7..78a90466 100644 --- a/src/workspace/workspaceMonitor.ts +++ b/src/workspace/workspaceMonitor.ts @@ -1,7 +1,6 @@ import { type ServerSentEvent, type Workspace, - type WorkspaceStatus, } from "coder/site/src/api/typesGenerated"; import { formatDistanceToNowStrict } from "date-fns"; import * as vscode from "vscode"; @@ -51,8 +50,6 @@ export class WorkspaceMonitor implements vscode.Disposable { private readonly telemetry: WorkspaceTelemetry; private latestWorkspace: Workspace; - private lastWorkspaceStatus: WorkspaceStatus | undefined; - private lastWorkspaceStatusObservedAtMs: number | undefined; private constructor( workspace: Workspace, @@ -147,33 +144,12 @@ export class WorkspaceMonitor implements vscode.Disposable { } private update(workspace: Workspace) { - this.recordWorkspaceStatus(workspace); + this.telemetry.observeWorkspace(workspace); this.latestWorkspace = workspace; this.updateContext(workspace); this.updateStatusBar(workspace); } - private recordWorkspaceStatus(workspace: Workspace): void { - const status = workspace.latest_build.status; - const now = performance.now(); - const previous = this.lastWorkspaceStatus; - if (previous === status) { - return; - } - - this.telemetry.workspaceStateTransition({ - from: previous ?? "unknown", - to: status, - transition: workspace.latest_build.transition, - reason: workspace.latest_build.reason, - ...(this.lastWorkspaceStatusObservedAtMs !== undefined && { - observedDurationMs: now - this.lastWorkspaceStatusObservedAtMs, - }), - }); - this.lastWorkspaceStatus = status; - this.lastWorkspaceStatusObservedAtMs = now; - } - private maybeNotify(workspace: Workspace) { const cfg = vscode.workspace.getConfiguration(); if (areNotificationsDisabled(cfg)) { From d0b5403904022dd5dc4ee832c3859ff027524a9b Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 13 May 2026 13:27:47 +0300 Subject: [PATCH 3/4] refactor(telemetry): simplify auth/workspace instrumentation - workspace.ts: inline single-use transition helpers, unify on the spread idiom for optional `observedDurationMs`, move `performance.now()` below the dedup early-return, drop the no-op `() => fn()` wrapper in `traceUpdateTriggered`, and make `INITIAL_STATE` private. - auth.ts: replace the `T extends LoginPromptResult` constraint with a `LoginPromptTracer` exposing `markAborted()`, matching the `RemoteSetupTracer` pattern. Callers now drive abort signaling, so telemetry no longer depends on `LoginResult`'s shape. - loginCoordinator: call `tracer.markAborted()` on `!result.success`. --- src/instrumentation/auth.ts | 17 ++--- src/instrumentation/workspace.ts | 104 +++++++++---------------------- src/login/loginCoordinator.ts | 8 ++- 3 files changed, 42 insertions(+), 87 deletions(-) diff --git a/src/instrumentation/auth.ts b/src/instrumentation/auth.ts index 6d033d2b..9fdcb3a8 100644 --- a/src/instrumentation/auth.ts +++ b/src/instrumentation/auth.ts @@ -10,8 +10,9 @@ export type AuthIntercept401Recovery = | "none"; export type AuthLoginPromptTrigger = "auth_required" | "missing_session"; -interface LoginPromptResult { - readonly success: boolean; +/** Helpers scoped to the auth.login_prompt trace's lifetime. */ +export interface LoginPromptTracer { + markAborted(): void; } export class AuthTelemetry { @@ -30,19 +31,13 @@ export class AuthTelemetry { this.telemetry.log("auth.intercept_401", { recovery }); } - public traceLoginPrompt( + public traceLoginPrompt( trigger: AuthLoginPromptTrigger, - fn: () => Promise, + fn: (tracer: LoginPromptTracer) => Promise, ): Promise { return this.telemetry.trace( "auth.login_prompt", - async (span) => { - const result = await fn(); - if (!result.success) { - span.markAborted(); - } - return result; - }, + (span) => fn({ markAborted: () => span.markAborted() }), { trigger }, ); } diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index 7d36001f..77676ae7 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -4,35 +4,14 @@ import { } from "../telemetry/reporter"; import type { - BuildReason, Workspace, WorkspaceAgent, WorkspaceAgentLifecycle, WorkspaceAgentStatus, WorkspaceStatus, - WorkspaceTransition, } from "coder/site/src/api/typesGenerated"; -export const INITIAL_STATE = "unknown"; - -type InitialState = typeof INITIAL_STATE; - -interface WorkspaceStateTransition { - readonly from: WorkspaceStatus | InitialState; - readonly to: WorkspaceStatus; - readonly transition?: WorkspaceTransition; - readonly reason?: BuildReason; - readonly observedDurationMs?: number; -} - -interface WorkspaceAgentStateTransition { - readonly agentName: string; - readonly fromStatus: WorkspaceAgentStatus | InitialState; - readonly toStatus: WorkspaceAgentStatus; - readonly fromLifecycleState: WorkspaceAgentLifecycle | InitialState; - readonly toLifecycleState: WorkspaceAgentLifecycle; - readonly observedDurationMs?: number; -} +const INITIAL_STATE = "unknown"; interface ObservedWorkspaceState { readonly status: WorkspaceStatus; @@ -55,26 +34,30 @@ export class WorkspaceTelemetry { public observeWorkspace(workspace: Workspace): void { const status = workspace.latest_build.status; - const now = performance.now(); const previous = this.observedWorkspaceState; if (previous?.status === status) { return; } + const now = performance.now(); - this.workspaceStateTransition({ - from: previous?.status ?? INITIAL_STATE, - to: status, - transition: workspace.latest_build.transition, - reason: workspace.latest_build.reason, - ...(previous && { - observedDurationMs: now - previous.observedAtMs, - }), - }); + this.telemetry.log( + "workspace.state_transitioned", + { + from: previous?.status ?? INITIAL_STATE, + to: status, + ...(workspace.latest_build.transition && { + transition: workspace.latest_build.transition, + }), + ...(workspace.latest_build.reason && { + reason: workspace.latest_build.reason, + }), + }, + previous ? { observedDurationMs: now - previous.observedAtMs } : {}, + ); this.observedWorkspaceState = { status, observedAtMs: now }; } public observeAgent(agent: WorkspaceAgent): void { - const now = performance.now(); const previous = this.observedAgentState; if ( previous?.status === agent.status && @@ -82,15 +65,19 @@ export class WorkspaceTelemetry { ) { return; } + const now = performance.now(); - this.agentStateTransition({ - agentName: agent.name, - fromStatus: previous?.status ?? INITIAL_STATE, - toStatus: agent.status, - fromLifecycleState: previous?.lifecycleState ?? INITIAL_STATE, - toLifecycleState: agent.lifecycle_state, - ...(previous && { observedDurationMs: now - previous.observedAtMs }), - }); + this.telemetry.log( + "workspace.agent.state_transitioned", + { + agentName: agent.name, + fromStatus: previous?.status ?? INITIAL_STATE, + toStatus: agent.status, + fromLifecycleState: previous?.lifecycleState ?? INITIAL_STATE, + toLifecycleState: agent.lifecycle_state, + }, + previous ? { observedDurationMs: now - previous.observedAtMs } : {}, + ); this.observedAgentState = { status: agent.status, lifecycleState: agent.lifecycle_state, @@ -102,40 +89,7 @@ export class WorkspaceTelemetry { this.observedAgentState = undefined; } - private workspaceStateTransition(transition: WorkspaceStateTransition): void { - this.telemetry.log( - "workspace.state_transitioned", - { - from: transition.from, - to: transition.to, - ...(transition.transition && { transition: transition.transition }), - ...(transition.reason && { reason: transition.reason }), - }, - transition.observedDurationMs === undefined - ? {} - : { observedDurationMs: transition.observedDurationMs }, - ); - } - - private agentStateTransition( - transition: WorkspaceAgentStateTransition, - ): void { - this.telemetry.log( - "workspace.agent.state_transitioned", - { - agentName: transition.agentName, - fromStatus: transition.fromStatus, - toStatus: transition.toStatus, - fromLifecycleState: transition.fromLifecycleState, - toLifecycleState: transition.toLifecycleState, - }, - transition.observedDurationMs === undefined - ? {} - : { observedDurationMs: transition.observedDurationMs }, - ); - } - public traceUpdateTriggered(fn: () => Promise): Promise { - return this.telemetry.trace("workspace.update.triggered", () => fn()); + return this.telemetry.trace("workspace.update.triggered", fn); } } diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index 96d2d4b8..fa032dcb 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -99,7 +99,13 @@ export class LoginCoordinator implements vscode.Disposable { ): Promise { return this.authTelemetry.traceLoginPrompt( options.trigger ?? "auth_required", - () => this.performLoginDialog(options), + async (tracer) => { + const result = await this.performLoginDialog(options); + if (!result.success) { + tracer.markAborted(); + } + return result; + }, ); } From fc035611f6ea8d7ff6bb552f2280ef7348a01cb5 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 13 May 2026 14:39:15 +0300 Subject: [PATCH 4/4] fix changelog --- CHANGELOG.md | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8ebd7f5..d90e461b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,29 +13,20 @@ machine only and never sent anywhere. Configure via the new `coder.telemetry.level` setting (`local` by default, `off` to disable); see `coder.telemetry.local` for tunables. -- Every `coder.*` command now records a `command.invoked` telemetry event with - its duration and outcome, so command latency and failures are captured - alongside other local telemetry. -- Extension activation, remote workspace setup phases (auth retrieval, - workspace lookup, workspace and agent readiness, SSH config write), and CLI - binary download/verify now emit local telemetry events with their duration - and outcome, so startup latency and failures are captured alongside other - local telemetry. +- Local telemetry now records `command.invoked` for each `coder.*` command + with duration and outcome. +- Local telemetry now records extension activation, remote workspace setup + phases (auth retrieval, workspace lookup, workspace and agent readiness, + SSH config write), and CLI binary download/verify with their durations + and outcomes. - Local telemetry now records `http.requests` rollups for per-route HTTP - health without emitting one event per request. -<<<<<<< HEAD + health, without emitting one event per request. - Local telemetry now records connection lifecycle: SSH process discovery/loss/recovery with sampled network info, and reconnecting - WebSocket open, drop, reconnect, and state transitions, so connection - stability is captured alongside other local telemetry. -- Local telemetry now records authentication refresh/recovery prompts plus - workspace and agent state transitions for connection lifecycle diagnostics. -======= -- Local telemetry now records authentication refresh/recovery attempts for - connection debugging. -- Local telemetry now records workspace and agent state transitions with observed - durations for connection lifecycle diagnostics. ->>>>>>> c7d49c2 (refactor(telemetry): keep lifecycle tracking isolated) + WebSocket open/drop/reconnect/state transitions. +- Local telemetry now records authentication refresh and recovery prompts. +- Local telemetry now records workspace and agent state transitions with + observed durations. ### Fixed