Skip to content

Commit c9314a5

Browse files
authored
feat: add telemetry for CLI and remote setup (#993)
Adds structured, bounded traces for the CLI and remote-setup flows, following the instrumentation conventions (snake_case keys, subject-first phases, typed unions, error.type / abort_stage via recordError/recordAborted). Traces: - cli.resolve: parent trace over cache_lookup, version_check, lock_wait, and fallback_to_existing_binary phases; surfaces outcome, cache_source, download_reason/action, error.type, and fallback_reason on the parent. - cli.download: reason + downloaded_bytes, with a verify child phase. - cli.configure: silent, credential_source, error.type, abort_stage. - remote.setup: child phases for each setup step, with a typed outcome for non-throwing early exits. - websocket reconnect: adds attempts, max_backoff_ms, total_duration_ms. Implementation: - Telemetry threaded explicitly via a trace/span arg; public methods are thin wrappers over private business logic. - cliManager unit and telemetry tests share a setupCliManager() harness. Closes #985
1 parent 7f29568 commit c9314a5

13 files changed

Lines changed: 1442 additions & 632 deletions

src/core/cliManager.ts

Lines changed: 240 additions & 86 deletions
Large diffs are not rendered by default.

src/instrumentation/cli.ts

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
import { CredentialFileError } from "./credentials";
2+
import { recordAborted, recordError } from "./outcomes";
3+
4+
import type { CallerPropertyValue } from "../telemetry/event";
5+
import type { TelemetryService } from "../telemetry/service";
6+
import type { Span } from "../telemetry/span";
7+
8+
export type CliCacheSource = "file-path" | "directory" | "not-found";
9+
export type CliDownloadReason = "missing" | "version_mismatch" | "unreadable";
10+
export type CliDownloadAction = "download" | "fallback" | "blocked";
11+
export type CliCredentialSource = "session_token" | "empty_token";
12+
export type CliResolveOutcome =
13+
| "cache_hit"
14+
| "downloaded"
15+
| "lock_wait_cache_hit"
16+
| "download_disabled_fallback"
17+
| "fallback_to_existing_binary";
18+
export type CliVersionCheckOutcome =
19+
| "missing"
20+
| "match"
21+
| "mismatch"
22+
| "unreadable";
23+
export type CliConfigureErrorType =
24+
| "filesystem"
25+
| "credential_store"
26+
| "unknown";
27+
export type CliResolveErrorType =
28+
| "downloads_disabled"
29+
| "download"
30+
| "fallback_declined";
31+
32+
interface CliConfigureOptions {
33+
readonly silent: boolean;
34+
readonly credentialSource: CliCredentialSource;
35+
}
36+
37+
export class CliDownloadsDisabledError extends Error {
38+
public constructor() {
39+
super("Unable to download CLI because downloads are disabled");
40+
this.name = "CliDownloadsDisabledError";
41+
}
42+
}
43+
44+
export class CliFallbackDeclinedError extends Error {
45+
public constructor(cause: unknown) {
46+
super(
47+
cause instanceof Error ? cause.message : "CLI binary fallback declined",
48+
{
49+
cause,
50+
},
51+
);
52+
this.name = "CliFallbackDeclinedError";
53+
}
54+
}
55+
56+
export class CliTelemetry {
57+
public constructor(private readonly telemetry: TelemetryService) {}
58+
59+
public traceResolve<T>(
60+
fn: (trace: CliResolveTrace) => Promise<T>,
61+
): Promise<T> {
62+
return this.telemetry.trace("cli.resolve", (span) =>
63+
fn(new CliResolveTrace(span)),
64+
);
65+
}
66+
67+
public traceDownload<T>(
68+
reason: CliDownloadReason,
69+
fn: (span: Span) => Promise<T>,
70+
): Promise<T> {
71+
return this.telemetry.trace("cli.download", fn, { reason });
72+
}
73+
74+
public traceConfigure<T>(
75+
options: CliConfigureOptions,
76+
fn: (trace: CliConfigureTrace) => Promise<T>,
77+
): Promise<T> {
78+
return this.telemetry.trace("cli.configure", (span) => {
79+
span.setProperty("silent", options.silent);
80+
span.setProperty("credential_source", options.credentialSource);
81+
return fn(new CliConfigureTrace(span));
82+
});
83+
}
84+
}
85+
86+
export class CliResolveTrace {
87+
public constructor(private readonly span: Span) {}
88+
89+
public setOutcome(outcome: CliResolveOutcome): void {
90+
this.span.setProperty("outcome", outcome);
91+
}
92+
93+
public error(category: CliResolveErrorType | "unknown"): void {
94+
recordError(this.span, category);
95+
}
96+
97+
public cacheLookup<T extends { readonly source: CliCacheSource }>(
98+
fn: () => Promise<T>,
99+
): Promise<T> {
100+
return this.tracedPhase("cache_lookup", fn, (r) => r.source, {
101+
child: "source",
102+
parent: "cache_source",
103+
});
104+
}
105+
106+
public versionCheck<T extends { readonly outcome: CliVersionCheckOutcome }>(
107+
fn: () => Promise<T>,
108+
): Promise<T> {
109+
return this.tracedPhase("version_check", fn, (r) => r.outcome, {
110+
child: "outcome",
111+
parent: "version_check",
112+
});
113+
}
114+
115+
public lockWait<T extends { readonly waited: boolean }>(
116+
fn: () => Promise<T>,
117+
): Promise<T> {
118+
return this.tracedPhase("lock_wait", fn, (r) => r.waited, {
119+
child: "waited",
120+
});
121+
}
122+
123+
public lockRecheck<T extends { readonly outcome: CliVersionCheckOutcome }>(
124+
fn: () => Promise<T>,
125+
): Promise<T> {
126+
return this.tracedPhase("lock_wait_recheck", fn, (r) => r.outcome, {
127+
child: "outcome",
128+
});
129+
}
130+
131+
public setDownloadDecision(
132+
reason: CliDownloadReason,
133+
action: CliDownloadAction,
134+
): void {
135+
this.span.setProperty("download_reason", reason);
136+
this.span.setProperty("download_action", action);
137+
}
138+
139+
public async fallback<T>(error: unknown, fn: () => Promise<T>): Promise<T> {
140+
// Why we fell back, recorded even when the fallback succeeds.
141+
this.span.setProperty("fallback_reason", categorizeResolveError(error));
142+
const result = await this.span.phase(
143+
"fallback_to_existing_binary",
144+
async (child) => {
145+
try {
146+
return await fn();
147+
} catch (fallbackError) {
148+
// Fallback failed too: tag the phase and parent with its error.type.
149+
const category = categorizeResolveError(fallbackError);
150+
recordError(child, category);
151+
this.error(category);
152+
throw fallbackError;
153+
}
154+
},
155+
);
156+
this.setOutcome("fallback_to_existing_binary");
157+
return result;
158+
}
159+
160+
/**
161+
* Run `fn` as a child phase tagged with `select(result)`, mirroring it onto
162+
* the parent when `keys.parent` is given.
163+
*/
164+
private async tracedPhase<T>(
165+
name: string,
166+
fn: () => Promise<T>,
167+
select: (result: T) => CallerPropertyValue,
168+
keys: { readonly child: string; readonly parent?: string },
169+
): Promise<T> {
170+
const result = await this.span.phase(name, async (child) => {
171+
const value = await fn();
172+
child.setProperty(keys.child, select(value));
173+
return value;
174+
});
175+
if (keys.parent) {
176+
this.span.setProperty(keys.parent, select(result));
177+
}
178+
return result;
179+
}
180+
}
181+
182+
export class CliConfigureTrace {
183+
public constructor(private readonly span: Span) {}
184+
185+
public abort(): void {
186+
recordAborted(this.span, "credential_store");
187+
}
188+
189+
public error(error: unknown): void {
190+
recordError(this.span, categorizeConfigureError(error));
191+
}
192+
}
193+
194+
function categorizeConfigureError(error: unknown): CliConfigureErrorType {
195+
// A CredentialFileError is a file-write failure; anything else is keyring/CLI.
196+
if (error instanceof CredentialFileError) {
197+
return "filesystem";
198+
}
199+
return error instanceof Error ? "credential_store" : "unknown";
200+
}
201+
202+
function categorizeResolveError(error: unknown): CliResolveErrorType {
203+
if (error instanceof CliDownloadsDisabledError) {
204+
return "downloads_disabled";
205+
}
206+
if (error instanceof CliFallbackDeclinedError) {
207+
return "fallback_declined";
208+
}
209+
return "download";
210+
}

src/instrumentation/remoteSetup.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,23 @@
11
import type { TelemetryService } from "../telemetry/service";
22

33
export type RemoteSetupPhase =
4+
| "cli_resolve"
5+
| "cli_configure"
6+
| "compatibility_check"
47
| "workspace_lookup"
8+
| "workspace_monitor_setup"
59
| "workspace_ready"
6-
| "resolve_agent"
7-
| "ssh_config_write";
10+
| "agent_resolve"
11+
| "ssh_config_write"
12+
| "ssh_monitor_setup"
13+
| "connection_handoff";
814

915
/** Reason for a non-throwing early exit from `remote.setup`. */
1016
export type RemoteSetupOutcome = "workspace_not_found" | "incompatible_server";
1117

1218
/** Helpers scoped to the remote.setup trace's lifetime. */
1319
export interface RemoteSetupTracer {
20+
/** Emit a typed child phase of `remote.setup`. */
1421
phase<T>(name: RemoteSetupPhase, fn: () => T | PromiseLike<T>): Promise<T>;
1522
/** Mark this setup as aborted with a typed reason; emits as `outcome` on the parent event. */
1623
markAborted(reason: RemoteSetupOutcome): void;

src/instrumentation/websocket.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ interface ReconnectCycle {
3939
readonly startMs: number;
4040
readonly reason: ConnectionStateReason;
4141
attempts: number;
42+
maxBackoffMs: number;
4243
}
4344

4445
interface DropOptions {
@@ -85,7 +86,7 @@ export class WebSocketTelemetry {
8586
this.#telemetry.log(
8687
"connection.opened",
8788
{ route: normalizeRoute(route) },
88-
{ connectDurationMs: now - start },
89+
{ connect_duration_ms: now - start },
8990
);
9091
this.#finishReconnect({ result: "success" });
9192
}
@@ -104,10 +105,10 @@ export class WebSocketTelemetry {
104105

105106
const properties: CallerProperties = { cause };
106107
if (closeCode !== undefined) {
107-
properties.closeCode = closeCode;
108+
properties.close_code = closeCode;
108109
}
109110
const measurements = {
110-
connectionDurationMs: performance.now() - openedAtMs,
111+
connection_duration_ms: performance.now() - openedAtMs,
111112
};
112113
if (error === undefined) {
113114
this.#telemetry.log("connection.dropped", properties, measurements);
@@ -136,6 +137,7 @@ export class WebSocketTelemetry {
136137
startMs: performance.now(),
137138
reason,
138139
attempts: 0,
140+
maxBackoffMs: 0,
139141
};
140142
}
141143

@@ -146,9 +148,17 @@ export class WebSocketTelemetry {
146148
}
147149

148150
/** Drop and (re)open a reconnect cycle. */
149-
public retrying(reason: ConnectionStateReason, options: DropOptions): void {
151+
public retrying(
152+
reason: ConnectionStateReason,
153+
options: DropOptions,
154+
backoffMs: number,
155+
): void {
150156
this.dropped(options.cause, options.code, options.error);
151157
this.reconnectStarted(reason);
158+
const cycle = this.#reconnectCycle;
159+
if (cycle) {
160+
cycle.maxBackoffMs = Math.max(cycle.maxBackoffMs, backoffMs);
161+
}
152162
}
153163

154164
#finishReconnect(outcome: ReconnectOutcome): void {
@@ -163,11 +173,12 @@ export class WebSocketTelemetry {
163173
reason: cycle.reason,
164174
};
165175
if (outcome.result === "error") {
166-
properties.terminationReason = outcome.terminationReason;
176+
properties.termination_reason = outcome.terminationReason;
167177
}
168178
this.#telemetry.log("connection.reconnect_resolved", properties, {
169179
attempts: cycle.attempts,
170-
totalDurationMs: performance.now() - cycle.startMs,
180+
max_backoff_ms: cycle.maxBackoffMs,
181+
total_duration_ms: performance.now() - cycle.startMs,
171182
});
172183
}
173184
}

0 commit comments

Comments
 (0)