From 5031ae9a47c2bbca651754c2261aa26eeb56807e Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Mon, 27 Jul 2026 18:48:58 +0000 Subject: [PATCH 1/5] feat(telemetry): setup client with logging sink --- src/index.ts | 37 +++++++-- src/telemetry/client.test.tsx | 151 ++++++++++++++++++++++++++++++++++ src/telemetry/client.tsx | 115 ++++++++++++++++++++++++++ src/telemetry/index.tsx | 3 + src/telemetry/loggingSink.tsx | 31 +++++++ src/telemetry/recorder.tsx | 27 ++++++ src/telemetry/shapes.tsx | 31 +++++++ src/telemetry/types.tsx | 52 ++++++++++++ 8 files changed, 438 insertions(+), 9 deletions(-) create mode 100644 src/telemetry/client.test.tsx create mode 100644 src/telemetry/client.tsx create mode 100644 src/telemetry/index.tsx create mode 100644 src/telemetry/loggingSink.tsx create mode 100644 src/telemetry/recorder.tsx create mode 100644 src/telemetry/shapes.tsx create mode 100644 src/telemetry/types.tsx diff --git a/src/index.ts b/src/index.ts index 348fb81ca..d86a96202 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,11 +14,12 @@ import { FsReadWriteJson } from "./io"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; import { DefaultGlobalConfigAccessor } from "./globalConfig"; +import { DefaultTelemetryClient, TelemetryAttributesRecorder } from "./telemetry"; process.exit( await runWithExitCode(async (argv: string[]) => { + const startTime = Date.now(); // generate a unique identifier corresponding to this process of this CLI. (ex. one command invoke, one TUI session) - // TODO: wire this id into telemetry as well const cliSessionId = crypto.randomUUID(); const rootLogger = createFileLogger({ @@ -33,15 +34,25 @@ process.exit( stderr: process.stderr, }; - try { - const globalConfigAccessor = new DefaultGlobalConfigAccessor({ - logger: rootLogger.child({ module: "globalConfigAccessor" }), - filePath: join(homedir(), ".agentcore", "config.json"), - json: new FsReadWriteJson({ - logger: rootLogger.child({ module: "jsonDataSource" }), - }), - }); + const globalConfigAccessor = new DefaultGlobalConfigAccessor({ + logger: rootLogger.child({ module: "globalConfigAccessor" }), + filePath: join(homedir(), ".agentcore", "config.json"), + json: new FsReadWriteJson({ + logger: rootLogger.child({ module: "jsonDataSource" }), + }), + }); + + const telemetryClient = new DefaultTelemetryClient({ + logger: rootLogger.child({ module: "telemetry" }), + sessionId: cliSessionId, + globalConfigAccessor, + }); + + const commandRunTelemetryRecorder = new TelemetryAttributesRecorder("cli.command_run", { + exit_reason: "success", + }); + try { rootLogger.info(`running CLI`); // factories (rather than instances) lets CoreClient build one client per @@ -69,8 +80,16 @@ process.exit( rootLogger .child({ errorName: error.name, errorMessage: error.message, stack: error.stack ?? "" }) .error(); + // TODO: add error details to telemetry recorder; + commandRunTelemetryRecorder.record({ exit_reason: "failure" }); throw e; } finally { + await telemetryClient.emit( + "cli.command_run", + Date.now() - startTime, + commandRunTelemetryRecorder.getAttributes(), + ); + await telemetryClient.shutdown(); await rootLogger.end(); } }), diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx new file mode 100644 index 000000000..fdaf95662 --- /dev/null +++ b/src/telemetry/client.test.tsx @@ -0,0 +1,151 @@ +import { test, describe, beforeEach, afterEach, expect } from "bun:test"; +import { join } from "node:path"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { DefaultTelemetryClient } from "./client"; +import { TelemetryAttributesRecorder } from "./recorder"; +import { createFileLogger, type Logger } from "../logging"; +import { LOG_LEVEL } from "../logging"; +import { assertLogsMatch, TestGlobalConfigAccessor } from "../testing"; +import type { MetricSink } from "./types"; +import { LoggingSink } from "./loggingSink"; + +describe("DefaultTelemetryClient", () => { + let tempDir: string; + let logger: Logger; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "telemetry-client-test-")); + logger = createFileLogger({ + filePath: join(tempDir, "output"), + logLevel: LOG_LEVEL.DEBUG, + }); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + }); + + test("emits metrics with resource and command attributes to configured sinks", async () => { + const sink = new LoggingSink({ logger: logger.child({ module: "loggingSink" }) }); + const sessionId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + const globalConfigAccessor = new TestGlobalConfigAccessor(); + const client = new DefaultTelemetryClient({ + logger, + sessionId, + globalConfigAccessor, + metricSinks: [sink], + }); + + const recorder = new TelemetryAttributesRecorder("cli.command_run", { exit_reason: "success" }); + + recorder.record({ exit_reason: "failure" }); + + await client.emit("cli.command_run", 123, recorder.getAttributes()); + + expect(sink.name).toBe("LoggingSink"); + await sink.shutdown(); + await client.shutdown(); + + const { installationId } = await globalConfigAccessor.get(); + await assertLogsMatch(tempDir, [ + { + filter: (log: any) => + log.metricName === "cli.command_run" && + log.metricValue === 123 && + log.metricAttributes?.["exit_reason"] === "failure" && + log.metricAttributes?.["service.name"] === "agentcore-cli" && + log.metricAttributes?.["agentcore-cli.session_id"] === sessionId && + log.metricAttributes?.["agentcore-cli.installation_id"] === installationId, + expectedCount: 1, + }, + ]); + }); + + test("rejects malformed payloads before the sinks sees data", async () => { + const recordedCalls: Array<{ metricName: string; value: number; attributes: any }> = []; + const spySink: MetricSink = { + name: "SpySink", + send: (metricName, value, attributes) => { + recordedCalls.push({ metricName, value, attributes }); + }, + shutdown: async () => {}, + }; + + const client = new DefaultTelemetryClient({ + logger, + sessionId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + globalConfigAccessor: new TestGlobalConfigAccessor(), + metricSinks: [spySink], + }); + + // Negative metric value violates z.number().min(0) + await client.emit("cli.command_run", -1, { exit_reason: "success" }); + + // The sink should never have been called — validation rejects the payload + expect(recordedCalls).toHaveLength(0); + + await client.shutdown(); + + await assertLogsMatch(tempDir, [ + { + filter: (log: any) => + log.msg === "failed to emit telemetry" && log.errorName === "ZodError", + expectedCount: 1, + }, + ]); + }); + + test("handles sink errors gracefully without throwing", async () => { + const recordedMetrics: string[] = []; + const goodSink: MetricSink = { + name: "GoodSink", + send: (metricName) => { + recordedMetrics.push(metricName); + }, + shutdown: async () => {}, + }; + + const badSink: MetricSink = { + name: "BadSink", + send: () => { + throw new Error("record exploded"); + }, + shutdown: async () => { + throw new Error("shutdown exploded"); + }, + }; + + const client = new DefaultTelemetryClient({ + logger, + sessionId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + globalConfigAccessor: new TestGlobalConfigAccessor(), + metricSinks: [badSink, goodSink], + }); + + // emit should not throw even though the sink's record() throws + await client.emit("cli.command_run", 100, { exit_reason: "success" }); + // shutdown should not throw even though the sink's shutdown() rejects + await client.shutdown(); + + // GoodSink still receives data despite BadSink throwing + expect(recordedMetrics).toEqual(["cli.command_run"]); + + await assertLogsMatch(tempDir, [ + { + filter: (log: any) => + log.msg === "failed to record to sink 'BadSink'" && + log.errorName === "Error" && + log.errorMessage === "record exploded", + expectedCount: 1, + }, + { + filter: (log: any) => + log.msg === "failed to shutdown metric sink with name 'BadSink'" && + log.errorName === "Error" && + log.errorMessage === "shutdown exploded", + expectedCount: 1, + }, + ]); + }); +}); diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx new file mode 100644 index 000000000..d3cb7d4b9 --- /dev/null +++ b/src/telemetry/client.tsx @@ -0,0 +1,115 @@ +import type { Logger } from "../logging"; +import { LoggingSink } from "./loggingSink"; +import { resourceAttributesSchema, type ResourceAttributes } from "./shapes"; +import os from "os"; +import { + type AttributesOf, + type MetricSink, + type TelemetryClient, + type ValueOf, + METRICS, + type MetricName, +} from "./types"; +import type { GlobalConfigAccessor } from "../globalConfig"; + +export type DefaultTelemetryClientConfig = { + logger: Logger; + globalConfigAccessor: GlobalConfigAccessor; + sessionId: string; + metricSinks?: MetricSink[]; +}; + +/** + * Implements {@link TelemetryClient} by validating and fanning out metrics to a list of {@link MetricSink} implementations. + */ +export class DefaultTelemetryClient implements TelemetryClient { + private logger: Logger; + private readonly sessionId: string; + private globalConfigAccessor: GlobalConfigAccessor; + private resourceAttributes: ResourceAttributes | undefined; + private metricSinks: MetricSink[] | undefined; + + constructor(config: DefaultTelemetryClientConfig) { + this.logger = config.logger; + this.sessionId = config.sessionId; + this.globalConfigAccessor = config.globalConfigAccessor; + this.resourceAttributes = undefined; + this.metricSinks = config.metricSinks; + } + + async emit( + metricName: TMetricName, + metricValue: ValueOf, + metricAttributes: Partial>, + ): Promise { + try { + const metricSinks = this.getMetricSinks(); + const resourceAttributes = await this.getResourceAttributes(); + // merge in resource attributes with metric attributes before sending to sink. + const attributes = { + ...resourceAttributes, + ...METRICS[metricName]["attributeSchema"].parse(metricAttributes), + }; + + const validatedMetricValue = METRICS[metricName]["valueSchema"].parse(metricValue); + + metricSinks.forEach((sink) => { + try { + sink.send(metricName, validatedMetricValue, attributes); + } catch (e) { + const error = e instanceof Error ? e : new Error(String(e)); + this.logger + .child({ errorName: error.name, errorMessage: error.message }) + .warn(`failed to record to sink '${sink.name}'`); + // do not allow a single sink failure to fail other sinks. + } + }); + } catch (e) { + const error = e instanceof Error ? e : new Error(String(e)); + this.logger + .child({ errorName: error.name, errorMessage: error.message }) + .warn(`failed to emit telemetry`); + // telemetry is best-effort, don't throw. + } + } + + async shutdown(): Promise { + const metricSinks = this.getMetricSinks(); + + const promises = metricSinks.map(async (sink) => { + return sink.shutdown().catch((e) => { + const error = e instanceof Error ? e : new Error(String(e)); + this.logger + .child({ errorName: error.name, errorMessage: error.message }) + .warn(`failed to shutdown metric sink with name '${sink.name}'`); + }); + }); + await Promise.all(promises); + } + + private getMetricSinks(): MetricSink[] { + if (this.metricSinks !== undefined) return this.metricSinks; + + this.metricSinks = [new LoggingSink({ logger: this.logger.child({ module: "loggingSink" }) })]; + + return this.metricSinks; + } + + private async getResourceAttributes(): Promise { + if (this.resourceAttributes !== undefined) return this.resourceAttributes; + + const globalConfig = await this.globalConfigAccessor.get(); + this.resourceAttributes = resourceAttributesSchema.parse({ + "service.name": "agentcore-cli", + // TODO: wire up real package version. + "service.version": "0.0.0", + "agentcore-cli.installation_id": globalConfig.installationId, + "agentcore-cli.session_id": this.sessionId, + "os.type": os.type(), + "os.version": os.release(), + "host.arch": os.arch(), + "node.version": process.version, + }); + return this.resourceAttributes; + } +} diff --git a/src/telemetry/index.tsx b/src/telemetry/index.tsx new file mode 100644 index 000000000..103df657c --- /dev/null +++ b/src/telemetry/index.tsx @@ -0,0 +1,3 @@ +export { DefaultTelemetryClient } from "./client"; +export { TelemetryAttributesRecorder } from "./recorder"; +export { type AttributesOf } from "./types"; diff --git a/src/telemetry/loggingSink.tsx b/src/telemetry/loggingSink.tsx new file mode 100644 index 000000000..a9afa18ea --- /dev/null +++ b/src/telemetry/loggingSink.tsx @@ -0,0 +1,31 @@ +import type { Logger } from "../logging"; +import type { MetricSink } from "./types"; + +type LoggingSinkConfig = { + logger: Logger; +}; + +/** + * An implementation of {@link MetricSink} that logs metrics using the given logger + */ +export class LoggingSink implements MetricSink { + private logger: Logger; + + constructor(config: LoggingSinkConfig) { + this.logger = config.logger; + } + + send( + metricName: string, + metricValue: number, + metricAttributes: Record, + ): void { + this.logger.child({ metricName, metricValue, metricAttributes }).info("recording telemetry"); + } + + async shutdown(): Promise {} + + get name() { + return "LoggingSink"; + } +} diff --git a/src/telemetry/recorder.tsx b/src/telemetry/recorder.tsx new file mode 100644 index 000000000..dd3f5b044 --- /dev/null +++ b/src/telemetry/recorder.tsx @@ -0,0 +1,27 @@ +import type { MetricName, AttributesOf } from "./types"; + +/** + * A strongly typed recorder for accumulating metric attributes, bound to a specific metric's schema. + */ +export class TelemetryAttributesRecorder { + private attributes: Partial>; + + constructor( + _metricName: TMetricName, + initialAttributes: Partial> = {}, + ) { + this.attributes = initialAttributes; + } + + getAttributes(): Partial> { + return this.attributes; + } + + record(data: Partial>): Partial> { + this.attributes = { + ...this.attributes, + ...data, + }; + return this.attributes; + } +} diff --git a/src/telemetry/shapes.tsx b/src/telemetry/shapes.tsx new file mode 100644 index 000000000..7141aba99 --- /dev/null +++ b/src/telemetry/shapes.tsx @@ -0,0 +1,31 @@ +import z from "zod"; + +const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +const SEMVER_PATTERN = /^\d+\.\d+\.\d+$/; +const NODE_VERSION_PATTERN = /^v\d+\.\d+\.\d+$/; +const MAX_ATTR_LENGTH = 64; + +/** + * Resource attributes that are attached to every metric datapoint. + * Set once per session, not metric. + */ +export const resourceAttributesSchema = z.object({ + "service.name": z.literal("agentcore-cli"), + "service.version": z.string().regex(SEMVER_PATTERN), + "agentcore-cli.installation_id": z.string().regex(UUID_PATTERN), + "agentcore-cli.session_id": z.string().regex(UUID_PATTERN), + "os.type": z.string().min(1).max(MAX_ATTR_LENGTH), + "os.version": z.string().min(1).max(MAX_ATTR_LENGTH), + "host.arch": z.string().min(1).max(MAX_ATTR_LENGTH), + "node.version": z.string().regex(NODE_VERSION_PATTERN), +}); + +/** + * Type derived from {@link resourceAttributesSchema} + */ +export type ResourceAttributes = z.infer; + +// TODO: expand this definition. +export const commandRunSchema = z.object({ + exit_reason: z.enum(["success", "failure"]), +}); diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx new file mode 100644 index 000000000..15ebf989b --- /dev/null +++ b/src/telemetry/types.tsx @@ -0,0 +1,52 @@ +import z from "zod"; +import { commandRunSchema } from "./shapes"; + +/** + * The primary interface for telemetry that orchestrates the emitting of metrics + */ +export interface TelemetryClient { + emit( + metricName: TMetricName, + metricValue: ValueOf, + attributes: Partial>, + ): Promise; + shutdown(): Promise; +} + +/** + * A destination to send metric data. + */ +export interface MetricSink { + /** Send data to the given metric sink **/ + send(metricName: string, value: number, attributes: Record): void; + /** Flush and close the given metric sink **/ + shutdown(): Promise; + name: string; +} + +/** + * Static definition of all metrics the CLI emits. + */ +export const METRICS = { + "cli.command_run": { + attributeSchema: commandRunSchema, + // value describes duration (ms) of the command + valueSchema: z.number().min(0), + }, +} satisfies Record; + +export type MetricName = keyof typeof METRICS & string; + +/** + * Describes the value type for the given {@link Metric} + */ +export type ValueOf = z.infer< + (typeof METRICS)[TMetricName]["valueSchema"] +>; + +/** + * Describes the attributes type for the given {@link Metric} + */ +export type AttributesOf = z.infer< + (typeof METRICS)[TMetricName]["attributeSchema"] +>; From 7c03c412689ba21e32b3856cfec8e30e2ccecb26 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Tue, 28 Jul 2026 20:57:14 +0000 Subject: [PATCH 2/5] refactor(telemetry): move validation to the recorder --- src/index.ts | 13 ++++++++----- src/telemetry/client.test.tsx | 34 ---------------------------------- src/telemetry/client.tsx | 4 ++-- src/telemetry/recorder.tsx | 16 ++++++++++++---- src/telemetry/types.tsx | 4 ++-- 5 files changed, 24 insertions(+), 47 deletions(-) diff --git a/src/index.ts b/src/index.ts index 8314b5312..6fd6b3e73 100644 --- a/src/index.ts +++ b/src/index.ts @@ -84,11 +84,14 @@ process.exit( throw error; } finally { - await telemetryClient.emit( - "cli.command_run", - Date.now() - startTime, - commandRunTelemetryRecorder.getAttributes(), - ); + try { + const attributes = commandRunTelemetryRecorder.getAttributes(); + await telemetryClient.emit("cli.command_run", Date.now() - startTime, attributes); + } catch (e) { + const error = AgentCoreCLIError.fromError(e); + rootLogger.child({ error: error.json() }).warn("failed to emit telemetry"); + // telemetry is best-effort + } await telemetryClient.shutdown(); await rootLogger.end(); } diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index fdaf95662..61f2d3c80 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -62,40 +62,6 @@ describe("DefaultTelemetryClient", () => { ]); }); - test("rejects malformed payloads before the sinks sees data", async () => { - const recordedCalls: Array<{ metricName: string; value: number; attributes: any }> = []; - const spySink: MetricSink = { - name: "SpySink", - send: (metricName, value, attributes) => { - recordedCalls.push({ metricName, value, attributes }); - }, - shutdown: async () => {}, - }; - - const client = new DefaultTelemetryClient({ - logger, - sessionId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", - globalConfigAccessor: new TestGlobalConfigAccessor(), - metricSinks: [spySink], - }); - - // Negative metric value violates z.number().min(0) - await client.emit("cli.command_run", -1, { exit_reason: "success" }); - - // The sink should never have been called — validation rejects the payload - expect(recordedCalls).toHaveLength(0); - - await client.shutdown(); - - await assertLogsMatch(tempDir, [ - { - filter: (log: any) => - log.msg === "failed to emit telemetry" && log.errorName === "ZodError", - expectedCount: 1, - }, - ]); - }); - test("handles sink errors gracefully without throwing", async () => { const recordedMetrics: string[] = []; const goodSink: MetricSink = { diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx index d3cb7d4b9..90be2e9c4 100644 --- a/src/telemetry/client.tsx +++ b/src/telemetry/client.tsx @@ -40,7 +40,7 @@ export class DefaultTelemetryClient implements TelemetryClient { async emit( metricName: TMetricName, metricValue: ValueOf, - metricAttributes: Partial>, + metricAttributes: AttributesOf, ): Promise { try { const metricSinks = this.getMetricSinks(); @@ -48,7 +48,7 @@ export class DefaultTelemetryClient implements TelemetryClient { // merge in resource attributes with metric attributes before sending to sink. const attributes = { ...resourceAttributes, - ...METRICS[metricName]["attributeSchema"].parse(metricAttributes), + ...metricAttributes, }; const validatedMetricValue = METRICS[metricName]["valueSchema"].parse(metricValue); diff --git a/src/telemetry/recorder.tsx b/src/telemetry/recorder.tsx index dd3f5b044..ba83df103 100644 --- a/src/telemetry/recorder.tsx +++ b/src/telemetry/recorder.tsx @@ -1,4 +1,4 @@ -import type { MetricName, AttributesOf } from "./types"; +import { type MetricName, type AttributesOf, METRICS } from "./types"; /** * A strongly typed recorder for accumulating metric attributes, bound to a specific metric's schema. @@ -7,16 +7,24 @@ export class TelemetryAttributesRecorder { private attributes: Partial>; constructor( - _metricName: TMetricName, + private readonly metricName: TMetricName, initialAttributes: Partial> = {}, ) { this.attributes = initialAttributes; } - getAttributes(): Partial> { - return this.attributes; + /** + * Retrieves the underlying attributes and validates them against the metric schema. + * Throws if metric shape is invalid. + */ + getAttributes(): AttributesOf { + const attributes = METRICS[this.metricName]["attributeSchema"].parse(this.attributes); + return attributes as AttributesOf; } + /** + * Add attributes that overwrite existing values if already set. + */ record(data: Partial>): Partial> { this.attributes = { ...this.attributes, diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index 15ebf989b..db20846aa 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -8,7 +8,7 @@ export interface TelemetryClient { emit( metricName: TMetricName, metricValue: ValueOf, - attributes: Partial>, + attributes: AttributesOf, ): Promise; shutdown(): Promise; } @@ -35,7 +35,7 @@ export const METRICS = { }, } satisfies Record; -export type MetricName = keyof typeof METRICS & string; +export type MetricName = keyof typeof METRICS; /** * Describes the value type for the given {@link Metric} From d4bffc914b56d7ca53bc3332d1dd5fa975beed5e Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Tue, 28 Jul 2026 20:58:14 +0000 Subject: [PATCH 3/5] refactor(telemetry): swap to input for schema to support defaults --- src/telemetry/types.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index db20846aa..bf3f3f6ea 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -40,13 +40,13 @@ export type MetricName = keyof typeof METRICS; /** * Describes the value type for the given {@link Metric} */ -export type ValueOf = z.infer< +export type ValueOf = z.input< (typeof METRICS)[TMetricName]["valueSchema"] >; /** * Describes the attributes type for the given {@link Metric} */ -export type AttributesOf = z.infer< +export type AttributesOf = z.input< (typeof METRICS)[TMetricName]["attributeSchema"] >; From 69ca5c4d955e7c6335f673d97dd58c95b3bde2ee Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Tue, 28 Jul 2026 21:02:20 +0000 Subject: [PATCH 4/5] refactor(telemetry): swap to getter for name attribute --- src/telemetry/client.test.tsx | 6 +++--- src/telemetry/client.tsx | 4 ++-- src/telemetry/loggingSink.tsx | 2 +- src/telemetry/types.tsx | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index 61f2d3c80..ae09295d7 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -43,7 +43,7 @@ describe("DefaultTelemetryClient", () => { await client.emit("cli.command_run", 123, recorder.getAttributes()); - expect(sink.name).toBe("LoggingSink"); + expect(sink.getName()).toBe("LoggingSink"); await sink.shutdown(); await client.shutdown(); @@ -65,7 +65,7 @@ describe("DefaultTelemetryClient", () => { test("handles sink errors gracefully without throwing", async () => { const recordedMetrics: string[] = []; const goodSink: MetricSink = { - name: "GoodSink", + getName: () => "GoodSink", send: (metricName) => { recordedMetrics.push(metricName); }, @@ -73,7 +73,7 @@ describe("DefaultTelemetryClient", () => { }; const badSink: MetricSink = { - name: "BadSink", + getName: () => "BadSink", send: () => { throw new Error("record exploded"); }, diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx index 90be2e9c4..40775efb6 100644 --- a/src/telemetry/client.tsx +++ b/src/telemetry/client.tsx @@ -60,7 +60,7 @@ export class DefaultTelemetryClient implements TelemetryClient { const error = e instanceof Error ? e : new Error(String(e)); this.logger .child({ errorName: error.name, errorMessage: error.message }) - .warn(`failed to record to sink '${sink.name}'`); + .warn(`failed to record to sink '${sink.getName()}'`); // do not allow a single sink failure to fail other sinks. } }); @@ -81,7 +81,7 @@ export class DefaultTelemetryClient implements TelemetryClient { const error = e instanceof Error ? e : new Error(String(e)); this.logger .child({ errorName: error.name, errorMessage: error.message }) - .warn(`failed to shutdown metric sink with name '${sink.name}'`); + .warn(`failed to shutdown metric sink with name '${sink.getName()}'`); }); }); await Promise.all(promises); diff --git a/src/telemetry/loggingSink.tsx b/src/telemetry/loggingSink.tsx index a9afa18ea..4074f03f6 100644 --- a/src/telemetry/loggingSink.tsx +++ b/src/telemetry/loggingSink.tsx @@ -25,7 +25,7 @@ export class LoggingSink implements MetricSink { async shutdown(): Promise {} - get name() { + getName() { return "LoggingSink"; } } diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index bf3f3f6ea..f29e01f3d 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -21,7 +21,7 @@ export interface MetricSink { send(metricName: string, value: number, attributes: Record): void; /** Flush and close the given metric sink **/ shutdown(): Promise; - name: string; + getName(): string; } /** From e561a2fa56ca65b310ee38d162496a92123a9414 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Tue, 28 Jul 2026 21:06:24 +0000 Subject: [PATCH 5/5] test(telemetry): add back simple test for validation --- src/telemetry/client.test.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index ae09295d7..e9288fca2 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -62,6 +62,20 @@ describe("DefaultTelemetryClient", () => { ]); }); + test("throws when recorder has incomplete attributes", async () => { + const client = new DefaultTelemetryClient({ + logger, + sessionId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + globalConfigAccessor: new TestGlobalConfigAccessor(), + metricSinks: [], + }); + + const recorder = new TelemetryAttributesRecorder("cli.command_run"); + + expect(() => client.emit("cli.command_run", 100, recorder.getAttributes())).toThrow(); + await client.shutdown(); + }); + test("handles sink errors gracefully without throwing", async () => { const recordedMetrics: string[] = []; const goodSink: MetricSink = {