refactor(telemetry): tighten connection telemetry types and flow

Type the connection reason/cause strings (ConnectionStateReason,
ConnectionDropCause), have disconnect/scheduleReconnect own drop
emission so the dedup flag and triple pre-call drops can go away,
add an attempts measurement to the ssh discovery span, and extract
the duplicated test telemetry helper.

Author: EhabY

src/instrumentation/reconnectingWebSocket.ts

@@ -1,9 +1,30 @@
 import { NOOP_SPAN, type Span } from "../telemetry/span";
 
 import type { TelemetryReporter } from "../telemetry/reporter";
-import type { CloseEvent } from "../websocket/eventStreamConnection";
 import type { ConnectionState } from "../websocket/reconnectingWebSocket";
 
+export type ConnectionStateReason =
+	| "initial_connect"
+	| "manual_reconnect"
+	| "scheduled_reconnect"
+	| "open"
+	| "disconnect"
+	| "dispose"
+	| "unrecoverable_close"
+	| "unrecoverable_http"
+	| "certificate_error"
+	| "connection_error"
+	| "unexpected_close";
+
+export type ConnectionDropCause =
+	| "manual_disconnect"
+	| "replaced"
+	| "unrecoverable_close"
+	| "normal_close"
+	| "unexpected_close"
+	| "disposed"
+	| "error";
+
 interface ReconnectTelemetryCycle {
 	readonly startMs: number;
 	readonly resolve: () => void;
@@ -15,23 +36,21 @@ interface ReconnectTelemetryCycle {
 
 export class ReconnectingWebSocketTelemetry {
 	private connectionOpenedAtMs: number | undefined;
-	private connectionDropEmitted = false;
 	private reconnectCycle: ReconnectTelemetryCycle | undefined;
 
 	constructor(private readonly telemetry: TelemetryReporter) {}
 
 	stateTransition(
 		from: ConnectionState,
 		to: ConnectionState,
-		reason: string,
+		reason: ConnectionStateReason,
 	): void {
 		this.telemetry.log("connection.state_transition", { from, to, reason });
 	}
 
 	connectionOpened(route: string, connectStartedAtMs: number): void {
 		const openedAtMs = performance.now();
 		this.connectionOpenedAtMs = openedAtMs;
-		this.connectionDropEmitted = false;
 		this.telemetry.log(
 			"connection.open",
 			{ url: route },
@@ -40,8 +59,12 @@ export class ReconnectingWebSocketTelemetry {
 		this.finishReconnect(true);
 	}
 
-	connectionDropped(cause: string, closeCode?: number, error?: unknown): void {
-		if (this.connectionOpenedAtMs === undefined || this.connectionDropEmitted) {
+	connectionDropped(
+		cause: ConnectionDropCause,
+		closeCode?: number,
+		error?: unknown,
+	): void {
+		if (this.connectionOpenedAtMs === undefined) {
 			return;
 		}
 
@@ -52,25 +75,19 @@ export class ReconnectingWebSocketTelemetry {
 		const measurements = {
 			connectionDurationMs: performance.now() - this.connectionOpenedAtMs,
 		};
+		this.connectionOpenedAtMs = undefined;
 		if (error === undefined) {
 			this.telemetry.log("connection.drop", properties, measurements);
-		} else {
-			this.telemetry.logError(
-				"connection.drop",
-				error,
-				properties,
-				measurements,
-			);
+			return;
 		}
-		this.connectionDropEmitted = true;
+		this.telemetry.logError("connection.drop", error, properties, measurements);
 	}
 
 	clearConnection(): void {
 		this.connectionOpenedAtMs = undefined;
-		this.connectionDropEmitted = false;
 	}
 
-	startReconnect(reason: string): void {
+	startReconnect(reason: ConnectionStateReason): void {
 		if (this.reconnectCycle) {
 			return;
 		}
@@ -137,9 +154,3 @@ export class ReconnectingWebSocketTelemetry {
 		);
 	}
 }
-
-export function closeEventError(event: CloseEvent): Error {
-	return new Error(
-		`WebSocket closed unexpectedly with code ${event.code}: ${event.reason}`,
-	);
-}

src/instrumentation/sshProcess.ts

@@ -25,9 +25,15 @@ export class SshProcessTelemetry {
 	constructor(private readonly telemetry: TelemetryReporter) {}
 
 	async discover(
-		fn: () => Promise<number | undefined>,
+		fn: (recordAttempt: () => void) => Promise<number | undefined>,
 	): Promise<number | undefined> {
-		return this.telemetry.trace("ssh.process.discovered", fn);
+		let attempts = 0;
+		return this.telemetry.trace("ssh.process.discovered", (span) =>
+			fn(() => {
+				attempts += 1;
+				span.setMeasurement("attempts", attempts);
+			}),
+		);
 	}
 
 	processStarted(now = performance.now()): void {
@@ -110,7 +116,7 @@ function toNetworkTelemetrySample(
 	network: NetworkInfo,
 ): NetworkTelemetrySample {
 	return {
-		emittedAtMs: Date.now(),
+		emittedAtMs: performance.now(),
 		p2p: network.p2p,
 		derp: network.preferred_derp,
 		latencyMs: network.latency,

src/remote/sshProcess.ts

@@ -54,6 +54,7 @@ const CLEANUP_NETWORK_MAX_AGE_MS = 60 * 60 * 1000;
 const CLEANUP_LOG_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000;
 // Maximum number of proxy log files to keep during cleanup
 const CLEANUP_MAX_LOG_FILES = 20;
+
 /**
  * Monitors the SSH process for a Coder workspace connection and displays
  * network status in the VS Code status bar.
@@ -221,7 +222,9 @@ export class SshProcessMonitor implements vscode.Disposable {
 	 * Starts monitoring when it finds the process through the port.
 	 */
 	private async searchForProcess(): Promise<void> {
-		const pid = await this.telemetry.discover(() => this.discoverSshProcess());
+		const pid = await this.telemetry.discover((recordAttempt) =>
+			this.discoverSshProcess(recordAttempt),
+		);
 		if (pid === undefined || this.disposed) {
 			return;
 		}
@@ -230,7 +233,9 @@ export class SshProcessMonitor implements vscode.Disposable {
 		this.startMonitoring();
 	}
 
-	private async discoverSshProcess(): Promise<number | undefined> {
+	private async discoverSshProcess(
+		recordAttempt: () => void,
+	): Promise<number | undefined> {
 		const { discoveryPollIntervalMs, maxDiscoveryBackoffMs, logger, sshHost } =
 			this.options;
 		let attempt = 0;
@@ -239,6 +244,7 @@ export class SshProcessMonitor implements vscode.Disposable {
 
 		while (!this.disposed) {
 			attempt++;
+			recordAttempt();
 
 			if (attempt === 1 || attempt % 10 === 0) {
 				logger.debug(
@@ -334,21 +340,21 @@ export class SshProcessMonitor implements vscode.Disposable {
 			return;
 		}
 
-		if (previousPid !== pid) {
-			this.telemetry.processLost("process_changed");
+		if (previousPid === pid) {
+			this.telemetry.processRecovered(now);
+			return;
 		}
-		this.telemetry.processRecovered(now);
 
-		if (previousPid !== pid) {
-			this.telemetry.processStarted(now);
-			this.telemetry.resetNetworkSampling();
-			this.options.logger.info(
-				`SSH process changed from ${previousPid} to ${pid}`,
-			);
-			this.logFilePath = undefined;
-			this._onLogFilePathChange.fire(undefined);
-			this._onPidChange.fire(pid);
-		}
+		this.telemetry.processLost("process_changed");
+		this.telemetry.processRecovered(now);
+		this.telemetry.processStarted(now);
+		this.telemetry.resetNetworkSampling();
+		this.options.logger.info(
+			`SSH process changed from ${previousPid} to ${pid}`,
+		);
+		this.logFilePath = undefined;
+		this._onLogFilePathChange.fire(undefined);
+		this._onPidChange.fire(pid);
 	}
 
 	/**

src/telemetry/span.ts

@@ -35,10 +35,6 @@ export const NOOP_SPAN: Span = {
 	): Promise<T> {
 		return fn(NOOP_SPAN);
 	},
-	setProperty(): void {
-		return undefined;
-	},
-	setMeasurement(): void {
-		return undefined;
-	},
+	setProperty: () => undefined,
+	setMeasurement: () => undefined,
 };