refactor(telemetry): slim the export manifest and drop sink health

Self-review follow-ups on the export hardening:

- Drop sink health from the manifest. It was session-cumulative and could
  not be scoped to the export's date range, so it implied an accuracy it
  could not deliver. Removes getSinkHealth, TelemetrySink.health, and the
  sink's drop/flush counters; flush and overflow failures are still logged.
- Make the manifest a required OTLP write option and remove the
  includeManifest flag threaded through the zip pipeline.
- Slim the manifest types and take `format` as a parameter so the builder
  is reusable across formats.
- Trim route-normalization comments and share a writer test helper.

Author: EhabY

src/commands.ts

@@ -359,7 +359,6 @@ export class Commands {
 			this.pathResolver.getTelemetryPath(),
 			this.logger,
 			() => this.telemetryService.flush(),
-			() => this.telemetryService.getSinkHealth(),
 			this.telemetryService.getContext(),
 		);
 	}

src/logging/routeNormalization.ts

@@ -1,9 +1,8 @@
 /**
- * Normalizes request and websocket routes into low-cardinality labels for
- * telemetry. Two safety goals drive this: never leak secrets (query strings
- * and fragments can carry tokens) and never persist an unbounded label (ids
- * collapse and unknown routes bucket), so even a route the extension has
- * never seen stays safe to emit.
+ * Normalizes request and websocket routes into low-cardinality telemetry
+ * labels. Drops the query/fragment (which can carry tokens) and bounds
+ * cardinality by collapsing ids and bucketing unmatched routes, so even an
+ * unseen route is safe to emit.
  */
 
 const UNKNOWN_ROUTE = "<unknown>";
@@ -20,10 +19,9 @@ const UUID_PATTERN =
 const INTEGER_PATTERN = /^\d+$/;
 
 /**
- * Templates refine name segments (usernames, workspace and template names)
- * that generic id collapsing cannot catch. This is a precision aid only:
- * unknown routes are already bounded by id collapsing and bucketing, so a
- * missing rule never risks correctness or cardinality.
+ * Templates refine name segments (usernames, workspace/template names) that
+ * id collapsing misses. Precision only: a missing rule never risks
+ * cardinality, since unmatched routes still collapse ids and bucket.
  */
 const ROUTE_NORMALIZATION_RULES: ReadonlyArray<readonly string[]> = [
 	"api/v2/users/{name}/workspace/{name}",
@@ -109,9 +107,8 @@ function normalizeByRule(
 }
 
 /**
- * Bucket for routes with no matching template: keeps a short resource-path
- * prefix and collapses the rest, so unseen routes stay bounded even when
- * their variable segments are names rather than ids.
+ * Bucket for unmatched routes: keep a short prefix and collapse the rest, so
+ * unseen routes stay bounded even when their variable segments are names.
  */
 function bucketUnmatchedRoute(segments: readonly string[]): string {
 	if (segments.length <= UNMATCHED_PREFIX_SEGMENTS) {

src/telemetry/event.ts

@@ -27,21 +27,6 @@ export type CallerMeasurements = Record<string, number> & {
 	durationMs?: never;
 };
 
-/**
- * In-memory health counters a sink may expose for export diagnostics. All
- * counters are cumulative for the life of the sink and never reset.
- */
-export interface SinkHealth {
-	/** Events dropped because the in-memory buffer overflowed. */
-	readonly droppedBufferOverflow: number;
-	/** Events dropped because a flush write failed. */
-	readonly droppedWriteFailure: number;
-	/** Flushes that threw. */
-	readonly failedFlushes: number;
-	/** Category (errno code or error name) of the most recent flush error. */
-	readonly lastErrorCategory: string | null;
-}
-
 /**
  * Sink for telemetry events. `write` is sync and must buffer in memory; I/O
  * happens in `flush`/`dispose`. The service filters by `minLevel`; sinks can
@@ -53,8 +38,6 @@ export interface TelemetrySink {
 	write(event: TelemetryEvent): void;
 	flush(): Promise<void>;
 	dispose(): Promise<void>;
-	/** Optional in-memory health counters for export diagnostics. */
-	health?(): SinkHealth;
 }
 
 /** Build session attributes from the extension version and ambient host data. */

src/telemetry/export/command.ts

@@ -19,7 +19,7 @@ import { writeJsonArrayExport } from "./writers/json";
 import { writeOtlpZipExport } from "./writers/otlp/writer";
 
 import type { Logger } from "../../logging/logger";
-import type { SinkHealth, TelemetryContext } from "../event";
+import type { TelemetryContext } from "../event";
 import type { FlushStatus } from "../service";
 
 interface FormatPick extends vscode.QuickPickItem {
@@ -69,7 +69,6 @@ export async function runExportTelemetryCommand(
 	telemetryDir: string,
 	logger: Logger,
 	flushTelemetry: () => Promise<FlushStatus>,
-	getSinkHealth: () => SinkHealth,
 	context: TelemetryContext,
 ): Promise<void> {
 	const range = await promptDateRange();
@@ -144,7 +143,6 @@ export async function runExportTelemetryCommand(
 								endMs: range.endMs,
 							},
 							sourceFiles: filePaths.length,
-							sinkHealth: getSinkHealth(),
 						},
 					},
 				);

src/telemetry/export/writers/otlp/manifest.ts

@@ -1,90 +1,73 @@
 import { CURRENT_TELEMETRY_SCHEMA_VERSION } from "../../../wireFormat";
 
-import type { SinkHealth, TelemetryContext } from "../../../event";
+import type { TelemetryContext } from "../../../event";
 
-/** File name of the manifest packed alongside the OTLP envelopes. */
+/** File name of the manifest packed alongside the export envelopes. */
 export const MANIFEST_FILE = "manifest.json";
 
-/**
- * Version of the manifest document's own shape, separate from the telemetry
- * row version (`CURRENT_TELEMETRY_SCHEMA_VERSION`) it reports. Bump when the
- * manifest structure changes.
- */
+/** Manifest document format version; bump when this shape changes. */
 export const MANIFEST_SCHEMA_VERSION = 1;
 
-/** Range portion of the manifest input, mirroring `TelemetryDateRange`. */
-export interface ManifestRangeInput {
+/** Date range the export covers. */
+export interface ManifestRange {
 	readonly label: string;
 	readonly startMs?: number;
 	readonly endMs?: number;
 }
 
-/** Caller-supplied inputs the writer cannot derive from the event stream. */
-export interface ManifestInput {
-	readonly range: ManifestRangeInput;
-	readonly sourceFiles: number;
-	readonly sinkHealth: SinkHealth;
-}
-
-/** OTLP record counts per signal (records written, not source events). */
+/** Per-signal record counts (records written, not source events). */
 export interface RecordCounts {
 	readonly logs: number;
 	readonly traces: number;
 	readonly metrics: number;
 }
 
+/** Caller-supplied inputs the writer cannot derive from the event stream. */
+export interface ManifestInput {
+	readonly range: ManifestRange;
+	readonly sourceFiles: number;
+}
+
+/** Self-describing metadata written alongside an export. */
 export interface ExportManifest {
-	/** Manifest document format version. */
 	readonly schemaVersion: number;
-	/** Schema version of the exported telemetry rows. */
 	readonly telemetrySchemaVersion: number;
 	readonly exportedAt: string;
 	readonly extensionVersion: string;
+	readonly format: string;
 	readonly range: {
 		readonly label: string;
 		readonly start: string | null;
 		readonly end: string | null;
 	};
-	readonly format: "otlp-json";
 	readonly sourceFiles: number;
 	readonly sourceEvents: number;
 	readonly records: RecordCounts;
-	readonly sinkHealth: {
-		readonly droppedEvents: number;
-		readonly failedFlushes: number;
-		readonly lastErrorCategory: string | null;
-	};
 }
 
 export function buildManifest(args: {
-	readonly input: ManifestInput;
+	readonly format: string;
 	readonly context: TelemetryContext;
+	readonly input: ManifestInput;
 	readonly sourceEvents: number;
 	readonly records: RecordCounts;
 	readonly exportedAt: string;
 }): ExportManifest {
-	const { input, context, sourceEvents, records, exportedAt } = args;
-	const { sinkHealth } = input;
+	const { format, context, input, sourceEvents, records, exportedAt } = args;
 	return {
 		schemaVersion: MANIFEST_SCHEMA_VERSION,
 		telemetrySchemaVersion: CURRENT_TELEMETRY_SCHEMA_VERSION,
 		exportedAt,
 		extensionVersion: context.extensionVersion,
+		format,
 		range: {
 			label: input.range.label,
 			start: toIso(input.range.startMs),
 			end: toIso(input.range.endMs),
 		},
-		format: "otlp-json",
 		sourceFiles: input.sourceFiles,
 		sourceEvents,
 		records,
-		sinkHealth: {
-			droppedEvents:
-				sinkHealth.droppedBufferOverflow + sinkHealth.droppedWriteFailure,
-			failedFlushes: sinkHealth.failedFlushes,
-			lastErrorCategory: sinkHealth.lastErrorCategory,
-		},
 	};
 }
 

src/telemetry/export/writers/otlp/writer.ts

@@ -44,14 +44,17 @@ export interface OtlpExportCounts {
 }
 
 export interface OtlpWriteOptions {
+	/** Manifest metadata, written as `manifest.json` inside the zip. */
+	readonly manifest: ManifestInput;
 	readonly signal?: AbortSignal;
 	readonly onTempCleanupError?: (err: unknown, tempPath: string) => void;
 	/** Fires on either success or failure path so cleanup errors never mask the export outcome. */
 	readonly onStagingCleanupError?: (err: unknown, dir: string) => void;
-	/** When provided, a `manifest.json` describing the export is added to the zip. */
-	readonly manifest?: ManifestInput;
 }
 
+/** OTLP/JSON format tag recorded in the manifest. */
+const OTLP_FORMAT = "otlp-json";
+
 interface Channel {
 	file: EnvelopeFile;
 	/** Source events routed to this signal. */
@@ -74,7 +77,7 @@ export async function writeOtlpZipExport(
 	outputPath: string,
 	events: AsyncIterable<TelemetryEvent>,
 	context: TelemetryContext,
-	options: OtlpWriteOptions = {},
+	options: OtlpWriteOptions,
 ): Promise<OtlpExportCounts> {
 	throwIfAborted(options.signal);
 	return writeAtomically(
@@ -92,12 +95,7 @@ export async function writeOtlpZipExport(
 					options.signal,
 					options.manifest,
 				);
-				await packZip(
-					zipPath,
-					stagingDir,
-					options.signal,
-					options.manifest !== undefined,
-				);
+				await packZip(zipPath, stagingDir, options.signal);
 			} catch (err) {
 				await safeRemove(stagingDir, options.onStagingCleanupError);
 				throw err;
@@ -129,7 +127,7 @@ async function writeStagedFiles(
 	events: AsyncIterable<TelemetryEvent>,
 	context: TelemetryContext,
 	signal: AbortSignal | undefined,
-	manifestInput: ManifestInput | undefined,
+	manifestInput: ManifestInput,
 ): Promise<OtlpExportCounts> {
 	const resource = JSON.stringify(otlpResource(context));
 	const scope = JSON.stringify(otlpScope(context.extensionVersion));
@@ -155,11 +153,7 @@ async function writeStagedFiles(
 		traces: channels.traces.count,
 		metrics: channels.metrics.count,
 	};
-
-	if (manifestInput) {
-		await writeManifest(dir, manifestInput, context, channels);
-	}
-
+	await writeManifest(dir, manifestInput, context, channels);
 	return counts;
 }
 
@@ -177,6 +171,7 @@ async function writeManifest(
 	const sourceEvents =
 		channels.logs.count + channels.traces.count + channels.metrics.count;
 	const manifest = buildManifest({
+		format: OTLP_FORMAT,
 		input,
 		context,
 		sourceEvents,
@@ -283,11 +278,10 @@ async function packZip(
 	outputPath: string,
 	sourceDir: string,
 	signal: AbortSignal | undefined,
-	includeManifest: boolean,
 ): Promise<void> {
 	const outStream = createWriteStream(outputPath);
 	try {
-		await streamEnvelopesIntoZip(outStream, sourceDir, signal, includeManifest);
+		await streamEnvelopesIntoZip(outStream, sourceDir, signal);
 	} catch (err) {
 		outStream.destroy();
 		if (isAbortError(err)) {
@@ -302,7 +296,6 @@ function streamEnvelopesIntoZip(
 	outStream: WriteStream,
 	sourceDir: string,
 	signal: AbortSignal | undefined,
-	includeManifest: boolean,
 ): Promise<void> {
 	return new Promise<void>((resolve, reject) => {
 		const fail = (err: unknown): void => reject(toError(err));
@@ -331,13 +324,7 @@ function streamEnvelopesIntoZip(
 			}
 		});
 
-		void pumpEnvelopes(
-			zip,
-			sourceDir,
-			signal,
-			waitForDrain,
-			includeManifest,
-		).catch((err) => {
+		void pumpEnvelopes(zip, sourceDir, signal, waitForDrain).catch((err) => {
 			zip.terminate();
 			fail(err);
 		});
@@ -349,14 +336,11 @@ async function pumpEnvelopes(
 	sourceDir: string,
 	signal: AbortSignal | undefined,
 	waitForDrain: () => Promise<void>,
-	includeManifest: boolean,
 ): Promise<void> {
-	const names: string[] = Object.values(ENVELOPES).map(
-		(envelope) => envelope.file,
-	);
-	if (includeManifest) {
-		names.push(MANIFEST_FILE);
-	}
+	const names = [
+		...Object.values(ENVELOPES).map((envelope) => envelope.file),
+		MANIFEST_FILE,
+	];
 	for (const name of names) {
 		throwIfAborted(signal);
 		await streamFileIntoZip(

src/telemetry/service.ts

@@ -13,7 +13,6 @@ import {
 	type CallerProperties,
 	type CallerPropertyValue,
 	type SessionContext,
-	type SinkHealth,
 	type TelemetryContext,
 	type TelemetryEvent,
 	type TelemetryLevel,
@@ -67,13 +66,6 @@ export interface FlushStatus {
 	readonly sinks: readonly SinkFlushResult[];
 }
 
-const EMPTY_SINK_HEALTH: SinkHealth = {
-	droppedBufferOverflow: 0,
-	droppedWriteFailure: 0,
-	failedFlushes: 0,
-	lastErrorCategory: null,
-};
-
 /**
  * Emits structured telemetry events to a fan-out of sinks. Sinks are filtered
  * by `minLevel` and may self-gate. `dispose` flushes are best-effort since
@@ -189,24 +181,6 @@ export class TelemetryService implements vscode.Disposable, TelemetryReporter {
 		return { ok: sinks.every((s) => s.ok), sinks };
 	}
 
-	/** Aggregate health across sinks that report it, for export diagnostics. */
-	public getSinkHealth(): SinkHealth {
-		return this.sinks.reduce<SinkHealth>((acc, sink) => {
-			const health = sink.health?.();
-			if (!health) {
-				return acc;
-			}
-			return {
-				droppedBufferOverflow:
-					acc.droppedBufferOverflow + health.droppedBufferOverflow,
-				droppedWriteFailure:
-					acc.droppedWriteFailure + health.droppedWriteFailure,
-				failedFlushes: acc.failedFlushes + health.failedFlushes,
-				lastErrorCategory: health.lastErrorCategory ?? acc.lastErrorCategory,
-			};
-		}, EMPTY_SINK_HEALTH);
-	}
-
 	public async dispose(): Promise<void> {
 		this.#configWatcher.dispose();
 		await this.flush();