diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b0aca787..66a9ef5ec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -94,6 +94,15 @@ pnpm watch # Rebuild extension and webviews on changes Press F5 to launch the Extension Development Host. Use "Developer: Reload Webviews" to see webview changes. +## Telemetry + +Local telemetry instrumentation follows a shared style: how spans are threaded, +how events and properties are named, and properties vs measurements. Read this +before adding new telemetry so it stays consistent across the codebase. It lives +next to the code: + +**[`src/instrumentation/CONVENTIONS.md`](src/instrumentation/CONVENTIONS.md)** + ## Testing There are a few ways you can test the "Open in VS Code" flow: diff --git a/src/commands.ts b/src/commands.ts index eb7c795b2..0a0903302 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -285,7 +285,7 @@ export class Commands { return; } if (resolved.status === "failed") { - telemetry.fail(resolved.category); + telemetry.error(resolved.category); return; } @@ -346,7 +346,7 @@ export class Commands { telemetry.abort("progress"); return; } - telemetry.fail(); + telemetry.error(); this.logger.error("Speed test failed", result.error); vscode.window.showErrorMessage( `Speed test failed: ${toError(result.error).message}`, @@ -364,14 +364,14 @@ export class Commands { }); } catch (err) { if (err instanceof ZodError || err instanceof SyntaxError) { - telemetry.fail("parse_error"); + telemetry.error("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(); + telemetry.error(); this.logger.error("Failed to display speedtest results", err); vscode.window.showErrorMessage( `Speed test returned unexpected output: ${toError(err).message}`, @@ -395,7 +395,7 @@ export class Commands { return; } if (resolved.status === "failed") { - telemetry.fail(resolved.category); + telemetry.error(resolved.category); return; } @@ -444,7 +444,7 @@ export class Commands { telemetry.abort("progress"); return; } - telemetry.fail( + telemetry.error( result.error instanceof SupportBundleUnsupportedCliError ? "unsupported_cli" : undefined, @@ -915,7 +915,7 @@ export class Commands { return false; } if (pick.status === "failed") { - telemetry.fail(pick.category); + telemetry.error(pick.category); return false; } workspace = pick.workspace; @@ -1224,7 +1224,7 @@ export class Commands { } }) .catch((ex) => { - fetchErrorCategory = "fetch_failed"; + fetchErrorCategory = "fetch_error"; this.logger.error("Failed to fetch workspaces", ex); if (ex instanceof CertificateError) { void ex.showNotification(); diff --git a/src/core/cliManager.ts b/src/core/cliManager.ts index 5d036e2a8..7154640b7 100644 --- a/src/core/cliManager.ts +++ b/src/core/cliManager.ts @@ -43,8 +43,8 @@ import type { CliCredentialManager } from "./cliCredentialManager"; import type { PathResolver } from "./pathResolver"; type ResolvedBinary = - | { binPath: string; stat: Stats; source: "file-path" | "directory" } - | { binPath: string; source: "not-found" }; + | { binPath: string; stat: Stats; source: "file_path" | "directory" } + | { binPath: string; source: "not_found" }; type CliVerifyResult = | { kind: "verified" } @@ -77,7 +77,7 @@ export class CliManager { public async locateBinary(url: string): Promise { const safeHostname = toSafeHost(url); const resolved = await this.resolveBinaryPath(safeHostname); - if (resolved.source === "not-found") { + if (resolved.source === "not_found") { throw new Error(`No CLI binary found at ${resolved.binPath}`); } return resolved.binPath; @@ -86,9 +86,9 @@ export class CliManager { /** * Resolve the CLI binary path from the configured cache path. * - * Returns "file-path" when the cache path is an existing file (checked for + * Returns "file_path" when the cache path is an existing file (checked for * version match and updated if needed), "directory" when a binary was found - * inside the directory, or "not-found" with the platform-specific path for + * inside the directory, or "not_found" with the platform-specific path for * the caller to download into. */ private async resolveBinaryPath( @@ -98,14 +98,14 @@ export class CliManager { const cacheStat = await cliUtils.stat(cachePath); if (cacheStat?.isFile()) { - return { binPath: cachePath, stat: cacheStat, source: "file-path" }; + return { binPath: cachePath, stat: cacheStat, source: "file_path" }; } const fullNamePath = path.join(cachePath, cliUtils.fullName()); // Path does not exist yet; return the platform-specific path to download. if (!cacheStat) { - return { binPath: fullNamePath, source: "not-found" }; + return { binPath: fullNamePath, source: "not_found" }; } // Directory exists; check platform-specific name, then simple name. @@ -120,7 +120,7 @@ export class CliManager { return { binPath: simpleNamePath, stat: simpleStat, source: "directory" }; } - return { binPath: fullNamePath, source: "not-found" }; + return { binPath: fullNamePath, source: "not_found" }; } /** @@ -240,7 +240,7 @@ export class CliManager { ); } - if (resolved.source === "not-found") { + if (resolved.source === "not_found") { this.output.info("No existing binary found, starting download"); return { buildInfo, @@ -442,7 +442,7 @@ export class CliManager { downloadBinPath: string, ): Promise { if ( - resolved.source === "file-path" && + resolved.source === "file_path" && downloadBinPath !== resolved.binPath ) { this.output.info( diff --git a/src/core/commandManager.ts b/src/core/commandManager.ts index 33c003519..abbb73e37 100644 --- a/src/core/commandManager.ts +++ b/src/core/commandManager.ts @@ -62,7 +62,7 @@ export class CommandManager implements vscode.Disposable { } const invoke = handler as (...args: unknown[]) => unknown; - const properties = { commandId: id }; + const properties = { command_id: id }; const wrapped = (...args: unknown[]): Thenable => this.telemetry.trace( COMMAND_INVOKED_EVENT, diff --git a/src/instrumentation/CONVENTIONS.md b/src/instrumentation/CONVENTIONS.md new file mode 100644 index 000000000..2a7569f4f --- /dev/null +++ b/src/instrumentation/CONVENTIONS.md @@ -0,0 +1,162 @@ +# Telemetry conventions + +How to add telemetry so every instrumentation reads the same way. The framework +lives in `src/telemetry`; the per-domain instrumentation business code talks to +lives here in `src/instrumentation`. + +## Checklist + +- One instrumentation class per domain (`FooTelemetry`) wrapping + `TelemetryService`; business code imports that, never a raw span. +- Event name is `domain.snake_case`; point-in-time logs use past tense. +- Event names and attribute keys follow OTel: lowercase, `.` for hierarchy, `_` + to split words, never camelCase. Enumerated values are typed `snake_case` + unions, never bare `string`. +- Numbers go in `measurements` (raw), never pre-bucketed into string properties. +- Set attributes imperatively with `setProperty`/`setMeasurement`; never add a + return value that exists only to be logged. +- No secrets, tokens, query strings, or unbounded values in properties; routes + go through `normalizeRoute`. +- Let the framework set `result`; add a domain `outcome` only when an operation + has several success modes. Errors go to a typed `error.type` union; non-error + early exits call `markAborted`. + +## Layers + +- **Framework** (`src/telemetry`): `TelemetryService` (`trace`/`log`/`logError`) + hands out `Span` handles and owns IDs, timing, `result`, level-gating, and the + wire format. Telemetry-off is handled here (`NOOP_SPAN`), so instrumentation + never checks whether telemetry is enabled. +- **Instrumentation** (`src/instrumentation/*`): one typed class per domain, the + only telemetry surface business code sees. + +## Structure + +- Split instrumentation files along the same boundaries as the business code, + not one catch-all module. +- Shared span helpers (`recordError`, `recordAborted`) live in one shared module, + not duplicated per file. +- Record-error-then-rethrow-outside-the-span logic lives once per class, in a + single private helper, not in every `traceX` method. + +## Threading + +Spans are passed **explicitly** as a callback argument; there is no +ambient/active-span context. Two patterns keep telemetry out of business logic: + +1. **Imperative attributes** — `span.setProperty("outcome", "cache_hit")` at the + point the value is known. This is the standard OpenTelemetry model. +2. **Typed phases** — wrap an async step in `span.phase(...)` and read one + property off its _natural_ return value, e.g. + `trace.versionCheck(() => this.checkBinary(...))`. Extraction stays out of + the business function. + +Never return a value purely so a caller can log it; that couples the return type +to observability. Returning is fine when the business uses the value too. + +## Callers + +- Declare telemetry dimensions explicitly at the call site; pass `source: "uri"` + rather than inferring it from which arguments happen to be set. +- Keep business bodies in named private `runX(args, trace)` methods; the public + method just opens the span and wraps them. Small diffs, named telemetry seam. +- When sibling events share a correlating property, emit it on every event in the + family; don't drop it from new ones. + +## Spans, phases, logs + +- `trace(name, fn, props?, meas?)` — a span with framework-set `result` and + `durationMs`. Use for an operation with a start and end. +- `span.phase(name, fn, ...)` — the same, nested (composed as `parent.child`); + child names cannot contain `.`. +- `span.log(name, ...)` / `span.logError(name, error, ...)` — point-in-time + events under a span, no `result`/`durationMs`. Use for discrete signals. + +## Naming + +| Thing | Convention | Examples | +| -------------------------- | ------------------------------------------- | --------------------------------------------------------- | +| Event name | `domain.snake_case` | `cli.resolve`, `remote.setup`, `connection.dropped` | +| Point-in-time log | past tense | `connection.dropped`, `ssh.process.lost` | +| Child phase | bare `snake_case` | `cache_lookup`, `version_check`, `connection_handoff` | +| Property / measurement key | lowercase; `_` splits words, `.` for groups | `cache_source`, `error.type`, `status.from` | +| Enumerated value | typed `snake_case` union | `"cache_hit"`, `"session_token"`, `"unrecoverable_close"` | + +This is the [OTel attribute convention](https://opentelemetry.io/docs/specs/semconv/general/naming/): +`.` is the namespace delimiter, `_` joins words within a segment, never +camelCase. Default to a flat `snake_case` key; use `.` only to group genuinely +related attributes (a `status.from` / `status.to` pair). Keep phase names +subject-first within a domain (`agent_resolve`, not `resolve_agent`). + +**Methods.** Span-wrapper methods are `trace` (`traceOpen`, +`traceConfirmationPrompt`); don't echo the event's past-tense suffix +(`traceUpdateConfirmationPrompted`), and drop qualifiers the class implies. + +**Grouping.** Group related events under a shared dotted namespace so a prefix +query returns the whole family: `workspace.update.triggered` and +`workspace.update.prompted` both sit under `workspace.update.*`, as do +`auth.token_refresh.completed` / `.deduped` under `auth.token_refresh.*`. Keep +the namespace node a pure prefix; don't also emit an event with that exact name, +since dotted names otherwise read as span phases (`parent.child`). This is +[OTel namespacing](https://opentelemetry.io/docs/specs/semconv/general/naming/), +which exists precisely so related signals query together. + +**Namespacing.** OTel suggests prefixing custom attributes (`com.acme.*`) to +avoid clashing with future semconv. We don't namespace event-level attributes: +each is already scoped by its (namespaced) event name and only flows into +Coder's own pipeline, so a bare `cache_source` can't collide with a future OTel +`cache.source`. Resource and provenance attributes stay namespaced +(`coder.deployment.url`, `coder.event.*`). + +## Properties vs measurements + +- **Properties** are low-cardinality string dimensions (the framework stringifies + `string | number | boolean`). Use them for what you group or filter by. +- **Measurements** are raw numbers. Don't pre-bucket into string labels: both + export as record attributes, and a query can bucket the raw number at read + time. `result` and `durationMs` are framework-managed and cannot be set. +- **Units.** There is no unit field at emit time, so put the unit in the + measurement key as a `_ms` / `_seconds` / `_mbits` suffix, the same way for + every event. + The OTLP exporter then resolves it per signal: for metric events + (`http.requests`, `ssh.network.sampled`) it moves the suffix into the OTLP + `unit` field and drops it from the metric name (`latency_ms` exports as metric + `latency`, unit `ms`, which Prometheus then suffixes itself); log and span + attributes keep the key as written. You always author `latency_ms`; only the + exported metric name changes. +- **Counts.** Name a count `.count`, singular entity (`agent.count`, + `workspace.count`), per OTel (`system.process.count`) — not flat `agent_count`. + Related counts share the namespace (`agent.count`, `agent.connected_count`); a + count with no entity (`retry_count`) stays flat. + +## Outcomes, errors, aborts + +- The framework sets `result` (`success` / `error` / `aborted`) on every span; + don't duplicate it. +- Add a domain `outcome` property only when an operation has several success + modes worth distinguishing (e.g. `cli.resolve`: `cache_hit`, `downloaded`). +- Classify errors into a typed `error.type` union via a `categorize*Error` helper + rather than emitting raw error strings; the framework captures the error block. +- For a non-error early exit (backed out, not-found), call `span.markAborted()` + rather than throwing, recording its reason in a separate key, not `error.type`. +- A trace exposes one outcome trio — `abort(stage)`, `error(category?)`, + `succeed*(payload)` — over the shared `recordAborted` / `recordError` helpers; + each outcome sets one, never two. +- Prefer **error** over "failure" and **abort** over "cancel" here (`recordError` + / `error.type` / `error()`; `recordAborted` / `markAborted` / `abort()`). + Point-in-time logs keep past tense (`recovery_failed`) — they state what + happened, not a span's `error` result. + +## Safety + +Never put tokens, credentials, full URLs with query strings, or unbounded user +input into properties. Routes go through `normalizeRoute` +(`src/logging/routeNormalization.ts`). Prefer a closed union over a free-form +string for any property a dashboard groups by. + +## Tests + +- Telemetry-only tests of business code are `.telemetry.test.ts`; + instrumentation modules keep `.test.ts` and split when the module does. +- Assert what privacy intends to omit, not only what is present (e.g. + `workspace_name` undefined on command events). diff --git a/src/instrumentation/activation.ts b/src/instrumentation/activation.ts index e4f6acdbe..008543152 100644 --- a/src/instrumentation/activation.ts +++ b/src/instrumentation/activation.ts @@ -20,7 +20,7 @@ export interface ActivationTracer { } /** - * Emits `activation` with `authState`, plus a sibling `activation.deployment_init` + * Emits `activation` with `auth_state`, plus a sibling `activation.deployment_init` * trace (sibling, not child, because deployment init outlives the activation span). */ export class ActivationTelemetry { @@ -28,17 +28,17 @@ export class ActivationTelemetry { public trace(fn: (tracer: ActivationTracer) => Promise): Promise { return this.telemetry.trace("activation", async (span) => { - span.setProperty("authState", "none"); + span.setProperty("auth_state", "none"); return fn({ - setAuthState: (state) => span.setProperty("authState", state), + setAuthState: (state) => span.setProperty("auth_state", state), traceDeploymentInit: (initFn) => this.telemetry.trace( "activation.deployment_init", async (childSpan) => { - childSpan.setProperty("authState", "unknown"); + childSpan.setProperty("auth_state", "unknown"); const success = await initFn(); childSpan.setProperty( - "authState", + "auth_state", success ? "valid_token" : "auth_failed", ); return success; diff --git a/src/instrumentation/auth.ts b/src/instrumentation/auth.ts index 70643e66a..9bcf0e9af 100644 --- a/src/instrumentation/auth.ts +++ b/src/instrumentation/auth.ts @@ -58,7 +58,7 @@ export class AuthTelemetry { } return result; } catch (error) { - span.setProperty("reason", "exception"); + span.setProperty("error.type", "exception"); throw error; } }, @@ -78,17 +78,24 @@ export class AuthTelemetry { } return result; } catch (error) { - span.setProperty("reason", "exception"); + span.setProperty("error.type", "exception"); throw error; } }); } + /** Wraps the secret-storage session read that seeds a remote connection. */ + public traceSessionLookup(fn: () => Promise): Promise { + return this.telemetry.trace("auth.session_lookup", fn); + } + public traceTokenRefresh( trigger: AuthTokenRefreshTrigger, fn: () => Promise, ): Promise { - return this.telemetry.trace("auth.token_refreshed", fn, { trigger }); + return this.telemetry.trace("auth.token_refresh.completed", fn, { + trigger, + }); } /** Logged when a refresh call joins an in-flight refresh and emits no span of its own. */ @@ -110,9 +117,9 @@ export class AuthTelemetry { logReceived: () => span.log("received"), setRecovery: (recovery) => span.setProperty("recovery", recovery), setRefreshAttempted: (attempted) => - span.setProperty("refreshAttempted", attempted), + span.setProperty("refresh_attempted", attempted), }), - { recovery: "none", refreshAttempted: false }, + { recovery: "none", refresh_attempted: false }, ); } @@ -141,11 +148,11 @@ export class AuthTelemetry { /** `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.setProperty("error.type", reason); span.markError(); } else { + span.setProperty("reason", reason); span.markAborted(); } } diff --git a/src/instrumentation/cli.ts b/src/instrumentation/cli.ts index c8c7d0a34..d819c7e6d 100644 --- a/src/instrumentation/cli.ts +++ b/src/instrumentation/cli.ts @@ -5,7 +5,7 @@ import type { CallerPropertyValue } from "../telemetry/event"; import type { TelemetryService } from "../telemetry/service"; import type { Span } from "../telemetry/span"; -export type CliCacheSource = "file-path" | "directory" | "not-found"; +export type CliCacheSource = "file_path" | "directory" | "not_found"; export type CliDownloadReason = "missing" | "version_mismatch" | "unreadable"; export type CliDownloadAction = "download" | "fallback" | "blocked"; export type CliCredentialSource = "session_token" | "empty_token"; diff --git a/src/instrumentation/credentials.ts b/src/instrumentation/credentials.ts index 393d6c249..5d6096d41 100644 --- a/src/instrumentation/credentials.ts +++ b/src/instrumentation/credentials.ts @@ -6,7 +6,7 @@ import type { WorkspaceConfiguration } from "vscode"; import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; -export type CredentialErrorCategory = "aborted" | "binary" | "cli" | "file"; +export type CredentialErrorCategory = "binary" | "cli" | "file"; type CredentialEvent = "auth.credential.store" | "auth.credential.clear"; @@ -47,12 +47,12 @@ export class CredentialTelemetry { try { await fn(span); } catch (error) { - span.setProperty("error.type", categorizeCredentialError(error)); if (isAbortError(error)) { span.markAborted(); aborted = error; return; } + span.setProperty("error.type", categorizeCredentialError(error)); throw error; } }, @@ -68,9 +68,6 @@ export class CredentialTelemetry { } function categorizeCredentialError(error: unknown): CredentialErrorCategory { - if (isAbortError(error)) { - return "aborted"; - } if (error instanceof CredentialFileError) { return "file"; } diff --git a/src/instrumentation/diagnostics.ts b/src/instrumentation/diagnostics.ts index 0b7dfd5e6..f109f2332 100644 --- a/src/instrumentation/diagnostics.ts +++ b/src/instrumentation/diagnostics.ts @@ -2,6 +2,7 @@ import { recordAborted, recordError } from "./outcomes"; import type { SpeedtestResult } from "@repo/shared"; +import type { ExportFormat } from "../telemetry/export/writers/types"; import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; @@ -25,10 +26,10 @@ export type DiagnosticAbortStage = export interface DiagnosticTrace { abort(stage: DiagnosticAbortStage): void; - fail(category?: DiagnosticErrorCategory): void; + error(category?: DiagnosticErrorCategory): void; setRequestedDuration(seconds: number): void; succeedSpeedtest(result: SpeedtestResult): void; - succeedExport(format: string, eventCount: number): void; + succeedExport(format: ExportFormat, eventCount: number): void; } /** Emits `command.diagnostic.completed` around each diagnostic command. */ @@ -54,7 +55,7 @@ class SpanDiagnosticTrace implements DiagnosticTrace { recordAborted(this.span, stage); } - public fail(category: DiagnosticErrorCategory = "error"): void { + public error(category: DiagnosticErrorCategory = "error"): void { recordError(this.span, category); } @@ -63,15 +64,15 @@ class SpanDiagnosticTrace implements DiagnosticTrace { } public succeedSpeedtest(result: SpeedtestResult): void { - this.span.setMeasurement("interval_count", result.intervals.length); + this.span.setMeasurement("interval.count", result.intervals.length); this.span.setMeasurement( "throughput_mbits", result.overall.throughput_mbits, ); } - public succeedExport(format: string, eventCount: number): void { + public succeedExport(format: ExportFormat, eventCount: number): void { this.span.setProperty("format", format); - this.span.setMeasurement("event_count", eventCount); + this.span.setMeasurement("event.count", eventCount); } } diff --git a/src/instrumentation/outcomes.ts b/src/instrumentation/outcomes.ts index 9c844d790..7b759aebd 100644 --- a/src/instrumentation/outcomes.ts +++ b/src/instrumentation/outcomes.ts @@ -2,8 +2,6 @@ import { isAbortError } from "../error/errorUtils"; import type { Span } from "../telemetry/span"; -export type AbortableErrorCategory = "aborted" | "error"; - /** Records a categorized error without attaching the raw error details. */ export function recordError(span: Span, category: string): void { span.setProperty("error.type", category); @@ -16,8 +14,11 @@ export function recordAborted(span: Span, stage: string): void { span.markAborted(); } -export function categorizeAbortableError( - error: unknown, -): AbortableErrorCategory { - return isAbortError(error) ? "aborted" : "error"; +/** Marks a thrown abort as `aborted`; records anything else as a categorized `"error"`. */ +export function recordAbortableError(span: Span, error: unknown): void { + if (isAbortError(error)) { + span.markAborted(); + } else { + recordError(span, "error"); + } } diff --git a/src/instrumentation/ssh.ts b/src/instrumentation/ssh.ts index 3ec3d0200..f92613a2a 100644 --- a/src/instrumentation/ssh.ts +++ b/src/instrumentation/ssh.ts @@ -57,7 +57,7 @@ export class SshTelemetry { this.#telemetry.log( "ssh.process.lost", { cause }, - { uptimeMs: now - this.#processStartedAtMs }, + { uptime_ms: now - this.#processStartedAtMs }, ); } @@ -68,7 +68,7 @@ export class SshTelemetry { this.#telemetry.log( "ssh.process.recovered", {}, - { recoveryDurationMs: performance.now() - this.#processLostAtMs }, + { recovery_duration_ms: performance.now() - this.#processLostAtMs }, ); this.#processLostAtMs = undefined; } @@ -80,14 +80,14 @@ export class SshTelemetry { const now = performance.now(); if (this.#processStartedAtMs !== undefined) { const measurements: Record = { - previousUptimeMs: now - this.#processStartedAtMs, + previous_uptime_ms: now - this.#processStartedAtMs, }; if (this.#processLostAtMs !== undefined) { - measurements.lostDurationMs = now - this.#processLostAtMs; + measurements.lost_duration_ms = now - this.#processLostAtMs; } this.#telemetry.log( "ssh.process.replaced", - { wasLost: this.#processLostAtMs !== undefined }, + { was_lost: this.#processLostAtMs !== undefined }, measurements, ); } @@ -105,8 +105,8 @@ export class SshTelemetry { const now = performance.now(); this.#telemetry.log( "ssh.process.disposed", - { wasLost: this.#processLostAtMs !== undefined }, - { uptimeMs: now - this.#processStartedAtMs }, + { was_lost: this.#processLostAtMs !== undefined }, + { uptime_ms: now - this.#processStartedAtMs }, ); this.#processStartedAtMs = undefined; this.#processLostAtMs = undefined; @@ -130,12 +130,12 @@ export class SshTelemetry { "ssh.network.sampled", { p2p: network.p2p, - preferredDerp: network.preferred_derp, + preferred_derp: network.preferred_derp, }, { - latencyMs: network.latency, - downloadMbits: bytesPerSecondToMbits(network.download_bytes_sec), - uploadMbits: bytesPerSecondToMbits(network.upload_bytes_sec), + latency_ms: network.latency, + download_mbits: bytesPerSecondToMbits(network.download_bytes_sec), + upload_mbits: bytesPerSecondToMbits(network.upload_bytes_sec), }, ); } diff --git a/src/instrumentation/websocket.ts b/src/instrumentation/websocket.ts index d0f3307a3..4fcd45d06 100644 --- a/src/instrumentation/websocket.ts +++ b/src/instrumentation/websocket.ts @@ -29,12 +29,19 @@ export type ConnectionDropCause = | "error"; type ReconnectOutcome = - | { readonly result: "success" } + | { readonly outcome: "success" } | { - readonly result: "error"; + readonly outcome: "error"; readonly terminationReason: ConnectionStateReason; }; +/** `ConnectionState` lowercased into the OTel snake_case attribute value. */ +type ConnectionStateValue = Lowercase<`${ConnectionState}`>; + +function stateValue(state: ConnectionState): ConnectionStateValue { + return state.toLowerCase() as ConnectionStateValue; +} + interface ReconnectCycle { readonly startMs: number; readonly reason: ConnectionStateReason; @@ -64,8 +71,8 @@ export class WebSocketTelemetry { reason: ConnectionStateReason, ): void { this.#telemetry.log("connection.state_transitioned", { - from, - to, + from: stateValue(from), + to: stateValue(to), reason, }); } @@ -88,7 +95,7 @@ export class WebSocketTelemetry { { route: normalizeRoute(route) }, { connect_duration_ms: now - start }, ); - this.#finishReconnect({ result: "success" }); + this.#finishReconnect({ outcome: "success" }); } public dropped( @@ -144,7 +151,7 @@ export class WebSocketTelemetry { /** Drop and end the reconnect cycle as a failure. */ public terminated(reason: ConnectionStateReason, options: DropOptions): void { this.dropped(options.cause, options.code, options.error); - this.#finishReconnect({ result: "error", terminationReason: reason }); + this.#finishReconnect({ outcome: "error", terminationReason: reason }); } /** Drop and (re)open a reconnect cycle. */ @@ -168,11 +175,11 @@ export class WebSocketTelemetry { } this.#reconnectCycle = undefined; - const properties: Record = { - result: outcome.result, + const properties: CallerProperties = { + outcome: outcome.outcome, reason: cycle.reason, }; - if (outcome.result === "error") { + if (outcome.outcome === "error") { properties.termination_reason = outcome.terminationReason; } this.#telemetry.log("connection.reconnect_resolved", properties, { diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index 5bc375039..f98f32eda 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -43,7 +43,7 @@ interface ObservedAgentState { /** * Emits `workspace.state_transitioned` as a workspace progresses through - * statuses, plus `observedBuildDurationMs` when a provisioner run resolves. + * statuses, plus `observed_build_duration_ms` when a provisioner run resolves. * Construct one per workspace; `WorkspaceMonitor` is the sole call site. */ export class WorkspaceStateTelemetry { @@ -73,7 +73,7 @@ export class WorkspaceStateTelemetry { const now = performance.now(); const measurements: Record = previous - ? { observedDurationMs: now - previous.observedAtMs } + ? { observed_duration_ms: now - previous.observedAtMs } : {}; const wasProvisioning = @@ -83,7 +83,7 @@ export class WorkspaceStateTelemetry { this.buildStartedAtMs ??= now; } else { if (wasProvisioning && this.buildStartedAtMs !== undefined) { - measurements.observedBuildDurationMs = now - this.buildStartedAtMs; + measurements.observed_build_duration_ms = now - this.buildStartedAtMs; } this.buildStartedAtMs = undefined; } @@ -91,7 +91,7 @@ export class WorkspaceStateTelemetry { this.telemetry.log( "workspace.state_transitioned", { - workspaceName: this.workspaceName, + workspace_name: this.workspaceName, from: previous?.status ?? INITIAL_STATE, to: status, "build.transition": buildTransition, @@ -135,14 +135,14 @@ export class WorkspaceAgentTelemetry { this.telemetry.log( "workspace.agent.state_transitioned", { - workspaceName: this.workspaceName, - agentName: agent.name, + workspace_name: this.workspaceName, + agent_name: agent.name, "status.from": previous?.status ?? INITIAL_STATE, "status.to": agent.status, "lifecycle_state.from": previous?.lifecycleState ?? INITIAL_STATE, "lifecycle_state.to": agent.lifecycle_state, }, - previous ? { observedDurationMs: now - previous.observedAtMs } : {}, + previous ? { observed_duration_ms: now - previous.observedAtMs } : {}, ); this.observed = { status: agent.status, @@ -168,13 +168,13 @@ export class WorkspaceOperationTelemetry { public traceUpdate(fn: () => Promise): Promise { return this.telemetry.trace("workspace.update.triggered", fn, { - workspaceName: this.workspaceName, + workspace_name: this.workspaceName, }); } public traceStart(fn: () => Promise): Promise { return this.telemetry.trace("workspace.start.triggered", fn, { - workspaceName: this.workspaceName, + workspace_name: this.workspaceName, }); } @@ -193,7 +193,7 @@ export class WorkspaceOperationTelemetry { span.setProperty("action", action); return action; }, - { workspaceName: this.workspaceName, update_offered: outdated }, + { workspace_name: this.workspaceName, update_offered: outdated }, ); } @@ -246,7 +246,7 @@ export class WorkspaceOperationTelemetry { ): Promise { return this.telemetry.trace("workspace.update.prompted", fn, { prompt, - workspaceName: this.workspaceName, + workspace_name: this.workspaceName, }); } } diff --git a/src/instrumentation/workspaceOpen.ts b/src/instrumentation/workspaceOpen.ts index 69e716af4..c3845bd94 100644 --- a/src/instrumentation/workspaceOpen.ts +++ b/src/instrumentation/workspaceOpen.ts @@ -1,11 +1,6 @@ import { extractAgents } from "../api/api-helper"; -import { - type AbortableErrorCategory, - categorizeAbortableError, - recordAborted, - recordError, -} from "./outcomes"; +import { recordAbortableError, recordAborted, recordError } from "./outcomes"; import type { Workspace, @@ -24,10 +19,8 @@ export type WorkspaceOpenSource = | "uri"; export type WorkspacePickerSource = "workspace_open" | "diagnostic"; -export type WorkspacePickerErrorCategory = "fetch_failed"; -export type WorkspaceOpenErrorCategory = - | WorkspacePickerErrorCategory - | AbortableErrorCategory; +export type WorkspacePickerErrorCategory = "fetch_error"; +export type WorkspaceOpenErrorCategory = WorkspacePickerErrorCategory | "error"; export type WorkspacePickerResult = | { readonly status: "selected"; readonly workspace: Workspace } | { readonly status: "cancelled" } @@ -56,7 +49,7 @@ export interface WorkspaceOpenTrace { stage: WorkspaceOpenAbortStage, selection?: WorkspaceOpenSelection, ): void; - fail(category: WorkspaceOpenErrorCategory): void; + error(category: WorkspaceOpenErrorCategory): void; handoff(kind: "folder" | "empty_window"): void; } @@ -114,8 +107,9 @@ export class WorkspaceOpenTelemetry { } /** - * Runs `fn` inside the span, recording a thrown error as a categorized - * error without its raw details, then rethrows outside the span. + * Runs `fn` inside the span, recording a thrown abort as `aborted` and any + * other error as a categorized error without its raw details, then rethrows + * outside the span. */ private async traceRethrowing( eventName: string, @@ -131,7 +125,7 @@ export class WorkspaceOpenTelemetry { return await fn(span); } catch (error) { thrown = { error }; - recordError(span, categorizeAbortableError(error)); + recordAbortableError(span, error); return fallback; } }, @@ -148,7 +142,7 @@ class SpanWorkspacePickerTrace implements WorkspacePickerTrace { public constructor(private readonly span: Span) {} public finish(result: WorkspacePickerResult, resultCount: number): void { - this.span.setMeasurement("workspace_count", resultCount); + this.span.setMeasurement("workspace.count", resultCount); if (result.status === "selected") { recordWorkspaceContext(this.span, result.workspace); return; @@ -178,7 +172,7 @@ class SpanWorkspaceOpenTrace implements WorkspaceOpenTrace { recordAborted(this.span, stage); } - public fail(category: WorkspaceOpenErrorCategory): void { + public error(category: WorkspaceOpenErrorCategory): void { recordError(this.span, category); } @@ -195,9 +189,9 @@ function recordWorkspaceContext( 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("agent.count", agents.length); span.setMeasurement( - "connected_agent_count", + "agent.connected_count", agents.filter((candidate) => candidate.status === "connected").length, ); if (!agent) { diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 129cf3db1..d64c76ede 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -35,6 +35,7 @@ import { type SecretsManager } from "../core/secretsManager"; import { toError } from "../error/errorUtils"; import { featureSetForVersion, type FeatureSet } from "../featureSet"; import { Inbox } from "../inbox"; +import { AuthTelemetry } from "../instrumentation/auth"; import { RemoteSetupTelemetry, type RemoteSetupTracer, @@ -103,6 +104,7 @@ export class Remote { private readonly secretsManager: SecretsManager; private readonly loginCoordinator: LoginCoordinator; private readonly setupTelemetry: RemoteSetupTelemetry; + private readonly authTelemetry: AuthTelemetry; public constructor( private readonly serviceContainer: ServiceContainer, @@ -118,6 +120,9 @@ export class Remote { this.setupTelemetry = new RemoteSetupTelemetry( serviceContainer.getTelemetryService(), ); + this.authTelemetry = new AuthTelemetry( + serviceContainer.getTelemetryService(), + ); } /** @@ -147,7 +152,7 @@ export class Remote { // first-run migration doesn't pollute that signal. await this.migrateToSecretsStorage(parts.safeHostname); const telemetry = this.serviceContainer.getTelemetryService(); - const auth = await telemetry.trace("auth.session_lookup", () => + const auth = await this.authTelemetry.traceSessionLookup(() => this.secretsManager.getSessionAuth(parts.safeHostname), ); if (auth?.url) { diff --git a/src/telemetry/export/command.ts b/src/telemetry/export/command.ts index 5248f3671..2e7344607 100644 --- a/src/telemetry/export/command.ts +++ b/src/telemetry/export/command.ts @@ -35,7 +35,7 @@ const PROGRESS_OPTIONS = { */ export interface ExportTelemetryObserver { abort(stage: "prompt" | "progress"): void; - fail(): void; + error(): void; succeedExport(format: ExportFormat, eventCount: number): void; } @@ -102,7 +102,7 @@ async function reportOutcome( observer.abort("progress"); return; } - observer.fail(); + observer.error(); logger.error("Telemetry export failed", result.error); void vscode.window.showErrorMessage( `Telemetry export failed: ${toError(result.error).message}`, diff --git a/src/telemetry/export/metrics.ts b/src/telemetry/export/metrics.ts index 057e7e449..4b0f747a2 100644 --- a/src/telemetry/export/metrics.ts +++ b/src/telemetry/export/metrics.ts @@ -25,8 +25,8 @@ const METRIC_EVENT_NAMES: ReadonlySet = new Set([ const UNIT_SUFFIXES: ReadonlyArray = [ ["_ms", "ms"], - ["Ms", "ms"], - ["Mbits", "Mbit/s"], + ["_seconds", "s"], + ["_mbits", "Mbit/s"], ]; export function isMetricEvent(event: TelemetryEvent): boolean { @@ -43,12 +43,10 @@ export function describeMetricEvent( return describeHttpRequests(event); } return { - measurements: Object.entries(event.measurements).map(([name, value]) => ({ - name, - value, - kind: "gauge", - unit: measurementUnit(name), - })), + measurements: Object.entries(event.measurements).map(([key, value]) => { + const { name, unit } = splitUnit(key); + return { name, value, kind: "gauge", unit }; + }), }; } @@ -61,17 +59,21 @@ function describeHttpRequests(event: TelemetryEvent): MetricDescriptor { } else if (name.startsWith("count.")) { measurements.push({ name, value, kind: "counter", unit: "{request}" }); } else { - measurements.push({ - name, - value, - kind: "gauge", - unit: measurementUnit(name), - }); + const { name: metricName, unit } = splitUnit(name); + measurements.push({ name: metricName, value, kind: "gauge", unit }); } } return { windowSeconds, measurements }; } -function measurementUnit(name: string): string { - return UNIT_SUFFIXES.find(([suffix]) => name.endsWith(suffix))?.[1] ?? "1"; +/** + * Split a measurement key into its metric name and unit, dropping the unit + * suffix from the name (OTLP carries the unit in a field, not the name): + * `latency_ms` -> `{ name: "latency", unit: "ms" }`. No known suffix -> "1". + */ +function splitUnit(key: string): { name: string; unit: string } { + const match = UNIT_SUFFIXES.find(([suffix]) => key.endsWith(suffix)); + return match + ? { name: key.slice(0, -match[0].length), unit: match[1] } + : { name: key, unit: "1" }; } diff --git a/test/unit/api/authInterceptor.test.ts b/test/unit/api/authInterceptor.test.ts index 219af8a29..9da903cf0 100644 --- a/test/unit/api/authInterceptor.test.ts +++ b/test/unit/api/authInterceptor.test.ts @@ -550,7 +550,7 @@ describe("AuthInterceptor", () => { expectThrow: boolean; expected: { recovery: AuthRecoveryAction; - refreshAttempted: "true" | "false"; + refresh_attempted: "true" | "false"; result: "success" | "error"; }; } @@ -571,7 +571,7 @@ describe("AuthInterceptor", () => { expectThrow: false, expected: { recovery: "refresh_success", - refreshAttempted: "true", + refresh_attempted: "true", result: "success", }, }, @@ -583,7 +583,7 @@ describe("AuthInterceptor", () => { expectThrow: true, expected: { recovery: "login_required", - refreshAttempted: "false", + refresh_attempted: "false", result: "error", }, }, @@ -594,12 +594,12 @@ describe("AuthInterceptor", () => { expectThrow: true, expected: { recovery: "none", - refreshAttempted: "false", + refresh_attempted: "false", result: "error", }, }, { - name: "OAuth refresh fails + callback declines: refreshAttempted=true, recovery=login_required", + name: "OAuth refresh fails + callback declines: refresh_attempted=true, recovery=login_required", arrange: async (ctx) => { await ctx.setupOAuthTokens(); ctx.mockOAuthManager.refreshToken.mockRejectedValue( @@ -611,7 +611,7 @@ describe("AuthInterceptor", () => { expectThrow: true, expected: { recovery: "login_required", - refreshAttempted: "true", + refresh_attempted: "true", result: "error", }, }, diff --git a/test/unit/commands.telemetry.test.ts b/test/unit/commands.telemetry.test.ts index aa0a41637..090339239 100644 --- a/test/unit/commands.telemetry.test.ts +++ b/test/unit/commands.telemetry.test.ts @@ -207,7 +207,7 @@ describe("Commands", () => { properties: { method: "cli_token", result: "error", - reason: "auth_failed", + "error.type": "auth_failed", }, }); expect(mocks.deploymentManager.setDeployment).not.toHaveBeenCalled(); @@ -267,7 +267,7 @@ describe("Commands", () => { await expect(commands.logout()).rejects.toThrow("secret clear failed"); expect(sink.expectOne("auth.logout")).toMatchObject({ - properties: { result: "error", reason: "exception" }, + properties: { result: "error", "error.type": "exception" }, error: { message: "secret clear failed" }, }); }); diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index 854e43ada..9fc2ccfbc 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -302,12 +302,11 @@ describe("CliCredentialManager", () => { signal: AbortSignal.abort(), }), ).rejects.toThrow("The operation was aborted"); - expect(sink.expectOne("auth.credential.store")).toMatchObject({ - properties: { - "error.type": "aborted", - result: "aborted", - }, + const event = sink.expectOne("auth.credential.store"); + expect(event).toMatchObject({ + properties: { result: "aborted" }, }); + expect(event.properties["error.type"]).toBeUndefined(); }); }); @@ -517,12 +516,11 @@ describe("CliCredentialManager", () => { signal: AbortSignal.abort(), }), ).rejects.toThrow("The operation was aborted"); - expect(sink.expectOne("auth.credential.clear")).toMatchObject({ - properties: { - "error.type": "aborted", - result: "aborted", - }, + const event = sink.expectOne("auth.credential.clear"); + expect(event).toMatchObject({ + properties: { result: "aborted" }, }); + expect(event.properties["error.type"]).toBeUndefined(); }); }); }); diff --git a/test/unit/core/commandManager.test.ts b/test/unit/core/commandManager.test.ts index fa48904b6..c95bff179 100644 --- a/test/unit/core/commandManager.test.ts +++ b/test/unit/core/commandManager.test.ts @@ -81,7 +81,7 @@ describe("CommandManager", () => { expect(sink.events[0]).toMatchObject({ eventName: "command.invoked", properties: { - commandId: "coder.refreshWorkspaces", + command_id: "coder.refreshWorkspaces", result: "success", }, }); @@ -100,7 +100,7 @@ describe("CommandManager", () => { expect(sink.events).toHaveLength(1); expect(sink.events[0]).toMatchObject({ eventName: "command.invoked", - properties: { commandId: "coder.login", result: "error" }, + properties: { command_id: "coder.login", result: "error" }, error: { message: "boom", type: "TypeError" }, }); }); diff --git a/test/unit/instrumentation/activation.test.ts b/test/unit/instrumentation/activation.test.ts index 80a64c624..5d28b4616 100644 --- a/test/unit/instrumentation/activation.test.ts +++ b/test/unit/instrumentation/activation.test.ts @@ -18,14 +18,14 @@ const find = (sink: TestSink, name: string) => sink.events.find((e) => e.eventName === name); describe("ActivationTelemetry.trace", () => { - it("emits one 'activation' event with default authState=none", async () => { + it("emits one 'activation' event with default auth_state=none", async () => { const { sink, activation } = makeHarness(); await activation.trace(() => Promise.resolve()); expect(sink.events).toHaveLength(1); expect(sink.events[0]).toMatchObject({ eventName: "activation", - properties: { authState: "none", result: "success" }, + properties: { auth_state: "none", result: "success" }, }); }); @@ -37,10 +37,10 @@ describe("ActivationTelemetry.trace", () => { return Promise.resolve(); }); - expect(sink.events[0].properties.authState).toBe("valid_token"); + expect(sink.events[0].properties.auth_state).toBe("valid_token"); }); - it("rethrows fn errors and emits result=error with the default authState", async () => { + it("rethrows fn errors and emits result=error with the default auth_state", async () => { const { sink, activation } = makeHarness(); const boom = new Error("nope"); @@ -48,7 +48,7 @@ describe("ActivationTelemetry.trace", () => { boom, ); expect(sink.events[0]).toMatchObject({ - properties: { authState: "none", result: "error" }, + properties: { auth_state: "none", result: "error" }, error: { message: "nope" }, }); }); @@ -71,7 +71,7 @@ describe("ActivationTelemetry.traceDeploymentInit", () => { { ret: true, expected: "valid_token" }, { ret: false, expected: "auth_failed" }, ])( - "maps initFn returning $ret to authState=$expected", + "maps initFn returning $ret to auth_state=$expected", async ({ ret, expected }) => { const { sink, activation } = makeHarness(); await activation.trace((tracer) => @@ -79,12 +79,12 @@ describe("ActivationTelemetry.traceDeploymentInit", () => { ); expect(find(sink, "activation.deployment_init")).toMatchObject({ - properties: { authState: expected, result: "success" }, + properties: { auth_state: expected, result: "success" }, }); }, ); - it("records authState=unknown when initFn throws", async () => { + it("records auth_state=unknown when initFn throws", async () => { const { sink, activation } = makeHarness(); const boom = new Error("init failed"); @@ -95,7 +95,7 @@ describe("ActivationTelemetry.traceDeploymentInit", () => { }); expect(find(sink, "activation.deployment_init")).toMatchObject({ - properties: { authState: "unknown", result: "error" }, + properties: { auth_state: "unknown", result: "error" }, error: { message: "init failed" }, }); }); diff --git a/test/unit/instrumentation/auth.test.ts b/test/unit/instrumentation/auth.test.ts index de7a37be0..a840d84eb 100644 --- a/test/unit/instrumentation/auth.test.ts +++ b/test/unit/instrumentation/auth.test.ts @@ -46,7 +46,7 @@ describe("AuthTelemetry", () => { source: "uri", method: "oauth", result: "error", - reason: "auth_failed", + "error.type": "auth_failed", }, }, ])("$name", async ({ outcome, traceMethod, properties }) => { @@ -101,7 +101,7 @@ describe("AuthTelemetry", () => { expect(sink.expectOne("auth.logout")).toMatchObject({ properties: { result: "error", - reason: "exception", + "error.type": "exception", }, error: { message: "clear failed" }, }); diff --git a/test/unit/instrumentation/diagnostics.test.ts b/test/unit/instrumentation/diagnostics.test.ts index 86aeee7b2..f4165daf8 100644 --- a/test/unit/instrumentation/diagnostics.test.ts +++ b/test/unit/instrumentation/diagnostics.test.ts @@ -18,7 +18,7 @@ describe("DiagnosticTelemetry", () => { return Promise.resolve(); }); await telemetry.trace("support_bundle", (trace) => { - trace.fail("unsupported_cli"); + trace.error("unsupported_cli"); return Promise.resolve(); }); @@ -59,7 +59,7 @@ describe("DiagnosticTelemetry", () => { expect(sink.expectOne("command.diagnostic.completed")).toMatchObject({ measurements: { - interval_count: 1, + "interval.count": 1, throughput_mbits: 42, }, properties: { result: "success" }, diff --git a/test/unit/instrumentation/ssh.test.ts b/test/unit/instrumentation/ssh.test.ts index a49912d16..5fab8ed25 100644 --- a/test/unit/instrumentation/ssh.test.ts +++ b/test/unit/instrumentation/ssh.test.ts @@ -56,10 +56,10 @@ describe("SshTelemetry", () => { "ssh.process.replaced", ]); const [replaced] = sink.eventsNamed("ssh.process.replaced"); - expect(replaced.properties).toMatchObject({ wasLost: "true" }); + expect(replaced.properties).toMatchObject({ was_lost: "true" }); expect(replaced.measurements).toMatchObject({ - previousUptimeMs: expect.any(Number), - lostDurationMs: expect.any(Number), + previous_uptime_ms: expect.any(Number), + lost_duration_ms: expect.any(Number), }); }); @@ -71,11 +71,11 @@ describe("SshTelemetry", () => { const replaced = sink.eventsNamed("ssh.process.replaced"); expect(replaced).toHaveLength(1); - expect(replaced[0].properties).toMatchObject({ wasLost: "false" }); + expect(replaced[0].properties).toMatchObject({ was_lost: "false" }); expect(replaced[0].measurements).toMatchObject({ - previousUptimeMs: expect.any(Number), + previous_uptime_ms: expect.any(Number), }); - expect(replaced[0].measurements.lostDurationMs).toBeUndefined(); + expect(replaced[0].measurements.lost_duration_ms).toBeUndefined(); }); it("emits nothing if there was no prior process", () => { @@ -97,7 +97,7 @@ describe("SshTelemetry", () => { expect(sink.events).toHaveLength(0); }); - it("emits ssh.process.recovered with recoveryDurationMs after a loss", () => { + it("emits ssh.process.recovered with recovery_duration_ms after a loss", () => { const { ssh, sink } = setup(); ssh.processStarted(); @@ -105,7 +105,9 @@ describe("SshTelemetry", () => { ssh.processRecovered(); const [event] = sink.eventsNamed("ssh.process.recovered"); - expect(event.measurements.recoveryDurationMs).toEqual(expect.any(Number)); + expect(event.measurements.recovery_duration_ms).toEqual( + expect.any(Number), + ); }); it("does not double-emit when called twice without another loss", () => { @@ -164,8 +166,8 @@ describe("SshTelemetry", () => { ssh.disposed(); const [event] = sink.eventsNamed("ssh.process.disposed"); - expect(event.properties).toMatchObject({ wasLost }); - expect(event.measurements.uptimeMs).toEqual(expect.any(Number)); + expect(event.properties).toMatchObject({ was_lost: wasLost }); + expect(event.measurements.uptime_ms).toEqual(expect.any(Number)); }); }); @@ -243,17 +245,17 @@ describe("SshTelemetry", () => { }, ); - it("includes p2p, preferredDerp, latency, and bandwidth in the emitted sample", () => { + it("includes p2p, preferred_derp, latency, and bandwidth in the emitted sample", () => { const { ssh, sink } = setup(); ssh.networkSampled(makeNetworkInfo({ latency: 25 })); const [sample] = sink.eventsNamed("ssh.network.sampled"); - expect(sample.properties).toEqual({ p2p: "true", preferredDerp: "NYC" }); + expect(sample.properties).toEqual({ p2p: "true", preferred_derp: "NYC" }); expect(sample.measurements).toMatchObject({ - latencyMs: 25, - downloadMbits: expect.any(Number), - uploadMbits: expect.any(Number), + latency_ms: 25, + download_mbits: expect.any(Number), + upload_mbits: expect.any(Number), }); }); }); diff --git a/test/unit/instrumentation/websocket.test.ts b/test/unit/instrumentation/websocket.test.ts index 2809c097e..a44d17201 100644 --- a/test/unit/instrumentation/websocket.test.ts +++ b/test/unit/instrumentation/websocket.test.ts @@ -23,8 +23,8 @@ describe("WebSocketTelemetry", () => { const [event] = sink.eventsNamed("connection.state_transitioned"); expect(event.properties).toEqual({ - from: "IDLE", - to: "CONNECTING", + from: "idle", + to: "connecting", reason: "initial_connect", }); }); @@ -144,7 +144,7 @@ describe("WebSocketTelemetry", () => { const [event] = sink.eventsNamed("connection.reconnect_resolved"); expect(event).toMatchObject({ - properties: { result: "success", reason: "manual_reconnect" }, + properties: { outcome: "success", reason: "manual_reconnect" }, measurements: { attempts: 1, max_backoff_ms: 0, @@ -161,7 +161,7 @@ describe("WebSocketTelemetry", () => { const [event] = sink.eventsNamed("connection.reconnect_resolved"); expect(event.properties).toEqual({ - result: "error", + outcome: "error", reason: "manual_reconnect", termination_reason: "unrecoverable_http", }); diff --git a/test/unit/instrumentation/workspace.test.ts b/test/unit/instrumentation/workspace.test.ts index 03bbdca3e..4b00b6101 100644 --- a/test/unit/instrumentation/workspace.test.ts +++ b/test/unit/instrumentation/workspace.test.ts @@ -46,7 +46,7 @@ describe("WorkspaceOperationTelemetry", () => { await ops[method](() => Promise.resolve()); expect(sink.expectOne(event)).toMatchObject({ - properties: { workspaceName: WORKSPACE_NAME, result: "success" }, + properties: { workspace_name: WORKSPACE_NAME, result: "success" }, }); }); @@ -81,7 +81,7 @@ describe("WorkspaceOperationTelemetry", () => { expect(result).toBe("update"); expect(sink.expectOne("workspace.start.prompted")).toMatchObject({ properties: { - workspaceName: WORKSPACE_NAME, + workspace_name: WORKSPACE_NAME, update_offered: "true", action: "update", result: "success", @@ -118,7 +118,7 @@ describe("WorkspaceOperationTelemetry", () => { expect(result).toEqual(collected); expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ properties: { - workspaceName: WORKSPACE_NAME, + workspace_name: WORKSPACE_NAME, prompt: "parameters", result: "success", }, @@ -164,7 +164,7 @@ describe("WorkspaceOperationTelemetry", () => { expect(result).toBe("Update and Restart"); expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ properties: { - workspaceName: WORKSPACE_NAME, + workspace_name: WORKSPACE_NAME, action: "update", prompt: "confirmation", result: "success", @@ -206,13 +206,13 @@ describe("WorkspaceStateTelemetry.observe", () => { const event = sink.expectOne("workspace.state_transitioned"); expect(event.properties).toMatchObject({ - workspaceName: WORKSPACE_NAME, + workspace_name: WORKSPACE_NAME, from: "none", to: "running", "build.transition": "start", "build.reason": "initiator", }); - expect(event.measurements.observedDurationMs).toBeUndefined(); + expect(event.measurements.observed_duration_ms).toBeUndefined(); }); it("ignores duplicate observations of the same state", () => { @@ -225,7 +225,7 @@ describe("WorkspaceStateTelemetry.observe", () => { expect(sink.eventsNamed("workspace.state_transitioned")).toHaveLength(1); }); - it("records observedDurationMs across transitions and observedBuildDurationMs once a build resolves", () => { + it("records observed_duration_ms across transitions and observed_build_duration_ms once a build resolves", () => { const { sink, instance: state } = setup(newState); state.observe(createWorkspace({ latest_build: { status: "stopped" } })); @@ -235,10 +235,12 @@ describe("WorkspaceStateTelemetry.observe", () => { const [first, second, third] = sink.eventsNamed( "workspace.state_transitioned", ); - expect(first.measurements.observedDurationMs).toBeUndefined(); - expect(second.measurements.observedDurationMs).toEqual(expect.any(Number)); - expect(second.measurements.observedBuildDurationMs).toBeUndefined(); - expect(third.measurements.observedBuildDurationMs).toEqual( + expect(first.measurements.observed_duration_ms).toBeUndefined(); + expect(second.measurements.observed_duration_ms).toEqual( + expect.any(Number), + ); + expect(second.measurements.observed_build_duration_ms).toBeUndefined(); + expect(third.measurements.observed_build_duration_ms).toEqual( expect.any(Number), ); }); @@ -286,14 +288,14 @@ describe("WorkspaceAgentTelemetry.observe", () => { expect(events[1].properties["status.from"]).toBe("none"); }); - it("includes observedDurationMs between transitions", () => { + it("includes observed_duration_ms between transitions", () => { const { sink, instance: agentTelemetry } = setup(newAgentTelemetry); agentTelemetry.observe(createAgent({ status: "connecting" })); agentTelemetry.observe(createAgent({ status: "connected" })); const events = sink.eventsNamed("workspace.agent.state_transitioned"); - expect(events[1].measurements.observedDurationMs).toEqual( + expect(events[1].measurements.observed_duration_ms).toEqual( expect.any(Number), ); }); diff --git a/test/unit/instrumentation/workspaceOpen.test.ts b/test/unit/instrumentation/workspaceOpen.test.ts index 3d3962266..985c0f32a 100644 --- a/test/unit/instrumentation/workspaceOpen.test.ts +++ b/test/unit/instrumentation/workspaceOpen.test.ts @@ -55,11 +55,11 @@ describe("WorkspaceOpenTelemetry", () => { result: "success", }); expect(event.measurements).toMatchObject({ - agent_count: 2, - connected_agent_count: 1, + "agent.count": 2, + "agent.connected_count": 1, }); - expect(event.properties.workspaceName).toBeUndefined(); - expect(event.properties.agentName).toBeUndefined(); + expect(event.properties.workspace_name).toBeUndefined(); + expect(event.properties.agent_name).toBeUndefined(); }); it("counts every connected agent on the workspace", async () => { @@ -90,8 +90,8 @@ describe("WorkspaceOpenTelemetry", () => { const event = sink.expectOne("workspace.open"); expect(event.measurements).toMatchObject({ - agent_count: 3, - connected_agent_count: 2, + "agent.count": 3, + "agent.connected_count": 2, }); }); @@ -104,19 +104,19 @@ describe("WorkspaceOpenTelemetry", () => { return Promise.resolve(result); }); await telemetry.tracePicker("workspace_open", (trace) => { - const result = { status: "failed", category: "fetch_failed" } as const; + const result = { status: "failed", category: "fetch_error" } as const; trace.finish(result, 0); return Promise.resolve(result); }); const [cancelled, failed] = sink.eventsNamed("workspace.picker.prompted"); expect(cancelled.properties).toMatchObject({ result: "aborted" }); - expect(cancelled.measurements.workspace_count).toBe(3); + expect(cancelled.measurements["workspace.count"]).toBe(3); expect(failed.properties).toMatchObject({ - "error.type": "fetch_failed", + "error.type": "fetch_error", result: "error", }); - expect(failed.measurements.workspace_count).toBe(0); + expect(failed.measurements["workspace.count"]).toBe(0); }); it("records workspace open cancellation and handled failure distinctly", async () => { @@ -128,7 +128,7 @@ describe("WorkspaceOpenTelemetry", () => { return Promise.resolve(false); }); await telemetry.traceOpen("command", undefined, (trace) => { - trace.fail("fetch_failed"); + trace.error("fetch_error"); return Promise.resolve(false); }); @@ -139,7 +139,7 @@ describe("WorkspaceOpenTelemetry", () => { result: "aborted", }); expect(failed.properties).toMatchObject({ - "error.type": "fetch_failed", + "error.type": "fetch_error", result: "error", }); }); @@ -161,6 +161,21 @@ describe("WorkspaceOpenTelemetry", () => { expect(event.error).toBeUndefined(); }); + it("records a thrown abort as aborted without an error.type", async () => { + const { sink, telemetry } = setup(); + const abort = Object.assign(new Error("user backed out"), { + name: "AbortError", + }); + + await expect( + telemetry.traceOpen("command", undefined, () => Promise.reject(abort)), + ).rejects.toThrow("user backed out"); + + const event = sink.expectOne("workspace.open"); + expect(event.properties).toMatchObject({ result: "aborted" }); + expect(event.properties["error.type"]).toBeUndefined(); + }); + it("records thrown devcontainer failures without raw error details", async () => { const { sink, telemetry } = setup(); diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index 03031ccb5..b5925b94d 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -597,7 +597,8 @@ describe("LoginCoordinator", () => { trigger: "auth_required" | "missing_session"; expected: { result: "success" | "aborted" | "error"; - reason?: "user_dismissed" | "no_url_provided" | "auth_failed"; + reason?: "user_dismissed" | "no_url_provided"; + "error.type"?: "auth_failed"; }; } @@ -618,7 +619,7 @@ describe("LoginCoordinator", () => { ctx.userInteraction.setResponse("Authentication Required", "Login"); }, trigger: "auth_required", - expected: { result: "error", reason: "auth_failed" }, + expected: { result: "error", "error.type": "auth_failed" }, }, { name: "user cancels URL prompt: aborted + no_url_provided", diff --git a/test/unit/oauth/sessionManager.test.ts b/test/unit/oauth/sessionManager.test.ts index fda3faae2..205e1c822 100644 --- a/test/unit/oauth/sessionManager.test.ts +++ b/test/unit/oauth/sessionManager.test.ts @@ -497,26 +497,26 @@ describe("OAuthSessionManager", () => { }; it.each(["reactive", "background"] as const)( - "emits auth.token_refreshed with trigger=%s", + "emits auth.token_refresh with trigger=%s", async (trigger) => { const { manager, sink } = await setupWithSink(); await manager.refreshToken(trigger); - const event = sink.expectOne("auth.token_refreshed"); + const event = sink.expectOne("auth.token_refresh.completed"); expect(event.properties).toMatchObject({ trigger, result: "success" }); expect(event.measurements.durationMs).toEqual(expect.any(Number)); }, ); - it("emits auth.token_refreshed with result=error when refresh fails", async () => { + it("emits auth.token_refresh with result=error when refresh fails", async () => { const { manager, sink } = await setupWithSink( createOAuthAxiosError("invalid_grant"), ); await expect(manager.refreshToken("reactive")).rejects.toThrow(); - const event = sink.expectOne("auth.token_refreshed"); + const event = sink.expectOne("auth.token_refresh.completed"); expect(event.properties).toMatchObject({ trigger: "reactive", result: "error", @@ -533,7 +533,7 @@ describe("OAuthSessionManager", () => { ]); expect(first).toBe(second); - expect(sink.eventsNamed("auth.token_refreshed")).toHaveLength(1); + expect(sink.eventsNamed("auth.token_refresh.completed")).toHaveLength(1); expect( sink.expectOne("auth.token_refresh.deduped").properties, ).toMatchObject({ diff --git a/test/unit/remote/sshProcess.test.ts b/test/unit/remote/sshProcess.test.ts index 3ab3a61a1..e53f1b761 100644 --- a/test/unit/remote/sshProcess.test.ts +++ b/test/unit/remote/sshProcess.test.ts @@ -373,12 +373,12 @@ describe("SshProcessMonitor", () => { monitor.dispose(); expect(sink.eventsNamed("ssh.process.disposed")[0]).toMatchObject({ - properties: { wasLost: "false" }, - measurements: { uptimeMs: expect.any(Number) }, + properties: { was_lost: "false" }, + measurements: { uptime_ms: expect.any(Number) }, }); }); - it("emits ssh.process.disposed with wasLost=true when dispose follows a loss", async () => { + it("emits ssh.process.disposed with was_lost=true when dispose follows a loss", async () => { const sink = new TestSink(); const telemetry = createTestTelemetryService(sink); vol.fromJSON({ @@ -401,7 +401,7 @@ describe("SshProcessMonitor", () => { const disposed = sink.eventsNamed("ssh.process.disposed"); expect(disposed).toHaveLength(1); - expect(disposed[0].properties).toMatchObject({ wasLost: "true" }); + expect(disposed[0].properties).toMatchObject({ was_lost: "true" }); }); it("emits missing_network_info as the loss cause when reads fail repeatedly", async () => { @@ -459,10 +459,10 @@ describe("SshProcessMonitor", () => { monitor.dispose(); const replaced = sink.eventsNamed("ssh.process.replaced"); - expect(replaced[0].properties).toMatchObject({ wasLost: "true" }); + expect(replaced[0].properties).toMatchObject({ was_lost: "true" }); expect(replaced[0].measurements).toMatchObject({ - previousUptimeMs: expect.any(Number), - lostDurationMs: expect.any(Number), + previous_uptime_ms: expect.any(Number), + lost_duration_ms: expect.any(Number), }); expect(sink.eventsNamed("ssh.process.recovered")).toHaveLength(0); }); @@ -492,10 +492,10 @@ describe("SshProcessMonitor", () => { expect(sink.eventsNamed("ssh.process.lost")[0]).toMatchObject({ properties: { cause: "stale_network_info" }, - measurements: { uptimeMs: expect.any(Number) }, + measurements: { uptime_ms: expect.any(Number) }, }); expect(sink.eventsNamed("ssh.process.recovered")[0]).toMatchObject({ - measurements: { recoveryDurationMs: expect.any(Number) }, + measurements: { recovery_duration_ms: expect.any(Number) }, }); }); @@ -516,8 +516,8 @@ describe("SshProcessMonitor", () => { await waitUntil(() => sink.eventsNamed("ssh.network.sampled").length > 0); expect(sink.eventsNamed("ssh.network.sampled")[0]).toMatchObject({ - properties: { p2p: "true", preferredDerp: "NYC" }, - measurements: { latencyMs: 25 }, + properties: { p2p: "true", preferred_derp: "NYC" }, + measurements: { latency_ms: 25 }, }); }); }); diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index 3c5c42e2d..fab7d1472 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -404,14 +404,14 @@ describe("WorkspaceStateMachine", () => { mock: typeof startWorkspace | typeof updateWorkspace; }>([ { - name: "workspace.start.triggered on stopped workspace", + name: "workspace.start on stopped workspace", eventName: "workspace.start.triggered", mode: "start", workspace: stoppedWorkspace, mock: startWorkspace, }, { - name: "workspace.update.triggered on running workspace in update mode", + name: "workspace.update on running workspace in update mode", eventName: "workspace.update.triggered", mode: "update", workspace: runningWorkspace, @@ -456,7 +456,7 @@ describe("WorkspaceStateMachine", () => { "lifecycle_state.from": "created", "lifecycle_state.to": "ready", }); - expect(events[1].measurements.observedDurationMs).toEqual( + expect(events[1].measurements.observed_duration_ms).toEqual( expect.any(Number), ); }); diff --git a/test/unit/telemetry/export/command.test.ts b/test/unit/telemetry/export/command.test.ts index a1e02d72e..eb9628b39 100644 --- a/test/unit/telemetry/export/command.test.ts +++ b/test/unit/telemetry/export/command.test.ts @@ -61,7 +61,7 @@ function setup( const observer: ExportTelemetryObserver = { abort: vi.fn(), - fail: vi.fn(), + error: vi.fn(), succeedExport: vi.fn(), }; @@ -196,7 +196,7 @@ describe("runExportTelemetryCommand", () => { await run(); - expect(observer.fail).toHaveBeenCalledOnce(); + expect(observer.error).toHaveBeenCalledOnce(); expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( "Telemetry export failed: disk full", diff --git a/test/unit/telemetry/export/metrics.test.ts b/test/unit/telemetry/export/metrics.test.ts index dd9492562..8ffd9f268 100644 --- a/test/unit/telemetry/export/metrics.test.ts +++ b/test/unit/telemetry/export/metrics.test.ts @@ -28,13 +28,13 @@ describe("describeMetricEvent", () => { const descriptor = describeMetricEvent( makeEvent({ eventName: "ssh.network.sampled", - measurements: { latencyMs: 35, downloadMbits: 10, custom: 1 }, + measurements: { latency_ms: 35, download_mbits: 10, custom: 1 }, }), ); expect(descriptor).toEqual({ measurements: [ - { name: "latencyMs", value: 35, kind: "gauge", unit: "ms" }, - { name: "downloadMbits", value: 10, kind: "gauge", unit: "Mbit/s" }, + { name: "latency", value: 35, kind: "gauge", unit: "ms" }, + { name: "download", value: 10, kind: "gauge", unit: "Mbit/s" }, { name: "custom", value: 1, kind: "gauge", unit: "1" }, ], }); @@ -65,7 +65,7 @@ describe("describeMetricEvent", () => { unit: "{request}", }, { - name: "duration.p95_ms", + name: "duration.p95", value: 42, kind: "gauge", unit: "ms", @@ -86,8 +86,9 @@ describe("describeMetricEvent", () => { it.each([ ["latency_ms", "ms"], - ["durationMs", "ms"], - ["downloadMbits", "Mbit/s"], + ["duration_ms", "ms"], + ["wait_seconds", "s"], + ["download_mbits", "Mbit/s"], ["something_else", "1"], ])("derives unit for measurement '%s' -> '%s'", (name, unit) => { const descriptor = describeMetricEvent( diff --git a/test/unit/telemetry/export/writers/otlp/__golden__/envelope-metrics.json b/test/unit/telemetry/export/writers/otlp/__golden__/envelope-metrics.json index 5c2356945..2a8fe67cc 100644 --- a/test/unit/telemetry/export/writers/otlp/__golden__/envelope-metrics.json +++ b/test/unit/telemetry/export/writers/otlp/__golden__/envelope-metrics.json @@ -237,7 +237,7 @@ } }, { - "name": "http.requests.duration.p50_ms", + "name": "http.requests.duration.p50", "description": "http.requests", "unit": "ms", "gauge": { @@ -289,7 +289,7 @@ } }, { - "name": "http.requests.duration.p95_ms", + "name": "http.requests.duration.p95", "description": "http.requests", "unit": "ms", "gauge": { @@ -341,7 +341,7 @@ } }, { - "name": "http.requests.duration.p99_ms", + "name": "http.requests.duration.p99", "description": "http.requests", "unit": "ms", "gauge": { diff --git a/test/unit/telemetry/export/writers/otlp/__golden__/metric-records.json b/test/unit/telemetry/export/writers/otlp/__golden__/metric-records.json index 1f46f5cb4..14a75026f 100644 --- a/test/unit/telemetry/export/writers/otlp/__golden__/metric-records.json +++ b/test/unit/telemetry/export/writers/otlp/__golden__/metric-records.json @@ -162,7 +162,7 @@ } }, { - "name": "http.requests.duration.p50_ms", + "name": "http.requests.duration.p50", "description": "http.requests", "unit": "ms", "gauge": { @@ -214,7 +214,7 @@ } }, { - "name": "http.requests.duration.p95_ms", + "name": "http.requests.duration.p95", "description": "http.requests", "unit": "ms", "gauge": { @@ -266,7 +266,7 @@ } }, { - "name": "http.requests.duration.p99_ms", + "name": "http.requests.duration.p99", "description": "http.requests", "unit": "ms", "gauge": { diff --git a/test/unit/telemetry/export/writers/otlp/records.test.ts b/test/unit/telemetry/export/writers/otlp/records.test.ts index 4487481fc..d1778ee05 100644 --- a/test/unit/telemetry/export/writers/otlp/records.test.ts +++ b/test/unit/telemetry/export/writers/otlp/records.test.ts @@ -231,8 +231,8 @@ describe("spanRecord", () => { [{ properties: { result: "error" } }, "_OTHER"], // An instrumentation-set category is never overwritten. [ - { properties: { result: "error", "error.type": "fetch_failed" } }, - "fetch_failed", + { properties: { result: "error", "error.type": "fetch_error" } }, + "fetch_error", ], ])("backfills error.type on error spans: %j -> %s", (overrides, expected) => { const span = spanRecord(makeSpanEvent(overrides)); @@ -271,16 +271,16 @@ describe("metricRecords", () => { }); const descriptor: MetricDescriptor = { measurements: [ - gauge("latencyMs", 35, "ms"), - gauge("downloadMbits", 10, "Mbit/s"), + gauge("latency", 35, "ms"), + gauge("download", 10, "Mbit/s"), ], }; const records = metricRecords(event, descriptor, newCumulativeState()); expect(records.map((r) => [r.name, r.unit])).toEqual([ - ["ssh.network.sampled.latencyMs", "ms"], - ["ssh.network.sampled.downloadMbits", "Mbit/s"], + ["ssh.network.sampled.latency", "ms"], + ["ssh.network.sampled.download", "Mbit/s"], ]); const point = records[0].gauge!.dataPoints[0]; expect(point).not.toHaveProperty("startTimeUnixNano"); diff --git a/test/unit/websocket/reconnectingWebSocket.test.ts b/test/unit/websocket/reconnectingWebSocket.test.ts index 250332f22..463490a2b 100644 --- a/test/unit/websocket/reconnectingWebSocket.test.ts +++ b/test/unit/websocket/reconnectingWebSocket.test.ts @@ -618,27 +618,27 @@ describe("ReconnectingWebSocket", () => { .eventsNamed("connection.state_transitioned") .map((e) => e.properties), ).toEqual([ - { from: "IDLE", to: "CONNECTING", reason: "initial_connect" }, - { from: "CONNECTING", to: "CONNECTED", reason: "open" }, + { from: "idle", to: "connecting", reason: "initial_connect" }, + { from: "connecting", to: "connected", reason: "open" }, { - from: "CONNECTED", - to: "AWAITING_RETRY", + from: "connected", + to: "awaiting_retry", reason: "unexpected_close", }, { - from: "AWAITING_RETRY", - to: "CONNECTING", + from: "awaiting_retry", + to: "connecting", reason: "scheduled_reconnect", }, - { from: "CONNECTING", to: "CONNECTED", reason: "open" }, - { from: "CONNECTED", to: "DISPOSED", reason: "dispose" }, + { from: "connecting", to: "connected", reason: "open" }, + { from: "connected", to: "disposed", reason: "dispose" }, ]); expect(sink.eventsNamed("connection.opened")).toHaveLength(2); expect(sink.eventsNamed("connection.dropped")).toHaveLength(2); expect(sink.eventsNamed("connection.reconnect_resolved")).toMatchObject([ { - properties: { result: "success", reason: "unexpected_close" }, + properties: { outcome: "success", reason: "unexpected_close" }, measurements: { attempts: 1, max_backoff_ms: expect.any(Number), diff --git a/test/unit/workspace/workspaceMonitor.test.ts b/test/unit/workspace/workspaceMonitor.test.ts index da5667c89..d156f8952 100644 --- a/test/unit/workspace/workspaceMonitor.test.ts +++ b/test/unit/workspace/workspaceMonitor.test.ts @@ -104,7 +104,7 @@ describe("WorkspaceMonitor", () => { from: "none", to: "running", }); - expect(events[0].measurements.observedDurationMs).toBeUndefined(); + expect(events[0].measurements.observed_duration_ms).toBeUndefined(); expect(events[1]).toMatchObject({ properties: { from: "running", @@ -112,7 +112,7 @@ describe("WorkspaceMonitor", () => { "build.transition": "stop", "build.reason": "autostop", }, - measurements: { observedDurationMs: expect.any(Number) }, + measurements: { observed_duration_ms: expect.any(Number) }, }); }); @@ -157,7 +157,7 @@ describe("WorkspaceMonitor", () => { expect(reasons).toEqual(["autostop", "initiator"]); }); - it("emits observedBuildDurationMs on the event that resolves a build run", async () => { + it("emits observed_build_duration_ms on the event that resolves a build run", async () => { const { stream, sink } = buildSinkContext(); await setup( @@ -176,20 +176,20 @@ describe("WorkspaceMonitor", () => { ); const events = sink.eventsNamed("workspace.state_transitioned"); - // pending and starting are intermediate; only running carries observedBuildDurationMs. + // pending and starting are intermediate; only running carries observed_build_duration_ms. expect(events.map((e) => e.properties.to)).toEqual([ "pending", "starting", "running", "stopping", ]); - expect(events[0].measurements.observedBuildDurationMs).toBeUndefined(); - expect(events[1].measurements.observedBuildDurationMs).toBeUndefined(); - expect(events[2].measurements.observedBuildDurationMs).toEqual( + expect(events[0].measurements.observed_build_duration_ms).toBeUndefined(); + expect(events[1].measurements.observed_build_duration_ms).toBeUndefined(); + expect(events[2].measurements.observed_build_duration_ms).toEqual( expect.any(Number), ); // Next build cycle resets; stopping doesn't carry the previous duration. - expect(events[3].measurements.observedBuildDurationMs).toBeUndefined(); + expect(events[3].measurements.observed_build_duration_ms).toBeUndefined(); }); });