From afc0fb031b291946d7e9ea6b13ec254fac9bf974 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 29 Jul 2026 23:47:36 +0000 Subject: [PATCH 01/11] feat(errors): allow callers to override error name on base error --- src/errors/errors.test.tsx | 2 +- src/errors/errors.tsx | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/errors/errors.test.tsx b/src/errors/errors.test.tsx index 168d02e4e3..090bcd4fe1 100644 --- a/src/errors/errors.test.tsx +++ b/src/errors/errors.test.tsx @@ -45,7 +45,7 @@ describe("AgentCoreCLIError", () => { ])("fromError SDK %s → expected source", (_label, err, expectedSource) => { const result = AgentCoreCLIError.fromError(err as Error); expect(result.json()).toMatchObject({ - name: "AgentCoreCLIError", + name: err.name, source: expectedSource, }); }); diff --git a/src/errors/errors.tsx b/src/errors/errors.tsx index a6b7351acc..11841b9c97 100644 --- a/src/errors/errors.tsx +++ b/src/errors/errors.tsx @@ -8,6 +8,8 @@ export interface AgentCoreCLIErrorOptions extends ErrorOptions { meta?: Record; /** Describes the exitCode for the CLI when this error hits the root handler */ exitCode?: number; + /** Describes the name of the underlying error, defaults to AgentCoreCLIError */ + name?: string; } /** Base error for CLI failures, including their source, metadata, and process exit code. */ @@ -18,7 +20,7 @@ export class AgentCoreCLIError extends Error { constructor(message?: string, options?: AgentCoreCLIErrorOptions) { super(message, options); - this.name = new.target.name; + this.name = options?.name ?? new.target.name; this.source = options?.source ?? ERROR_SOURCE.INTERNAL; this.meta = options?.meta ?? {}; this.exitCode = options?.exitCode ?? 1; @@ -48,6 +50,7 @@ export class AgentCoreCLIError extends Error { return new AgentCoreCLIError(error.message, { cause: error, source, + name: error.name, meta: { ...error.$metadata }, }); } From 287b57fd62aeab43c54abb80ce3daebb0f0a746c Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 11:44:00 +0000 Subject: [PATCH 02/11] feat(tel): add command path shapes explicitly --- src/telemetry/client.test.tsx | 27 ++++++--- src/telemetry/index.tsx | 1 + src/telemetry/shapes.tsx | 101 +++++++++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 9 deletions(-) diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index d895a3ef21..943397546e 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -41,7 +41,10 @@ describe("DefaultTelemetryClient", () => { metricSinks: [fileSystemSink], }); - const recorder = new TelemetryAttributesRecorder("cli.command_run", { exit_reason: "success" }); + const recorder = new TelemetryAttributesRecorder("cli.command_run", { + exit_reason: "success", + command_path: "/agentcore", + }); await client.emit("cli.command_run", 123, recorder.getAttributes()); @@ -75,12 +78,12 @@ describe("DefaultTelemetryClient", () => { { metricName: "cli.command_run", value: 123, - attrs: { ...resourceAttributes, exit_reason: "success" }, + attrs: { ...resourceAttributes, exit_reason: "success", command_path: "/agentcore" }, }, { metricName: "cli.command_run", value: 456, - attrs: { ...resourceAttributes, exit_reason: "failure" }, + attrs: { ...resourceAttributes, exit_reason: "failure", command_path: "/agentcore" }, }, ]); }); @@ -117,8 +120,14 @@ describe("DefaultTelemetryClient", () => { auditFilePath, }); - await enabledClient.emit("cli.command_run", 123, { exit_reason: "success" }); - await disabledClient.emit("cli.command_run", 456, { exit_reason: "failure" }); + await enabledClient.emit("cli.command_run", 123, { + exit_reason: "success", + command_path: "/agentcore", + }); + await disabledClient.emit("cli.command_run", 456, { + exit_reason: "failure", + command_path: "/agentcore", + }); await Promise.all([enabledClient.shutdown(), disabledClient.shutdown()]); const auditLines = (await readFile(auditFilePath, "utf8")).trimEnd().split("\n"); @@ -136,6 +145,7 @@ describe("DefaultTelemetryClient", () => { "host.arch": os.arch(), "node.version": process.version, exit_reason: "success", + command_path: "/agentcore", }, }); }); @@ -168,7 +178,7 @@ describe("DefaultTelemetryClient", () => { metricSinks: [sink], }); - await client.emit("cli.command_run", 1, { exit_reason: "success" }); + await client.emit("cli.command_run", 1, { exit_reason: "success", command_path: "/agentcore" }); await client.shutdown(); await assertLogsMatch(tempDir, [ @@ -209,7 +219,10 @@ describe("DefaultTelemetryClient", () => { }); // emit should not throw even though the sink's record() throws - await client.emit("cli.command_run", 100, { exit_reason: "success" }); + await client.emit("cli.command_run", 100, { + exit_reason: "success", + command_path: "/agentcore", + }); // shutdown should not throw even though the sink's shutdown() rejects await client.shutdown(); diff --git a/src/telemetry/index.tsx b/src/telemetry/index.tsx index 103df657c6..a5eea48703 100644 --- a/src/telemetry/index.tsx +++ b/src/telemetry/index.tsx @@ -1,3 +1,4 @@ export { DefaultTelemetryClient } from "./client"; export { TelemetryAttributesRecorder } from "./recorder"; export { type AttributesOf } from "./types"; +export { type CommandPath } from "./shapes"; diff --git a/src/telemetry/shapes.tsx b/src/telemetry/shapes.tsx index 7141aba991..6d08bd0fb7 100644 --- a/src/telemetry/shapes.tsx +++ b/src/telemetry/shapes.tsx @@ -1,4 +1,5 @@ import z from "zod"; +import { ERROR_SOURCE } from "../errors"; 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+$/; @@ -25,7 +26,103 @@ export const resourceAttributesSchema = z.object({ */ export type ResourceAttributes = z.infer; -// TODO: expand this definition. -export const commandRunSchema = z.object({ +/** + * List of all invocable command paths in the CLI that should emit telemetry. + */ +export const COMMAND_PATHS = [ + // root + "/agentcore", + + // harness + "/agentcore/harness", + "/agentcore/harness/create", + "/agentcore/harness/get", + "/agentcore/harness/list", + "/agentcore/harness/update", + "/agentcore/harness/delete", + "/agentcore/harness/invoke", + "/agentcore/harness/exec", + "/agentcore/harness/endpoint", + "/agentcore/harness/endpoint/create", + "/agentcore/harness/endpoint/get", + "/agentcore/harness/endpoint/list", + "/agentcore/harness/endpoint/update", + "/agentcore/harness/endpoint/delete", + "/agentcore/harness/version", + "/agentcore/harness/version/get", + "/agentcore/harness/version/list", + + // identity + "/agentcore/identity", + "/agentcore/identity/api-key-credential-provider", + "/agentcore/identity/api-key-credential-provider/create", + "/agentcore/identity/api-key-credential-provider/get", + "/agentcore/identity/api-key-credential-provider/list", + "/agentcore/identity/api-key-credential-provider/update", + "/agentcore/identity/api-key-credential-provider/delete", + + // runtime + "/agentcore/runtime", + "/agentcore/runtime/get", + "/agentcore/runtime/list", + "/agentcore/runtime/invoke", + "/agentcore/runtime/version", + "/agentcore/runtime/version/get", + "/agentcore/runtime/version/list", + "/agentcore/runtime/endpoint", + "/agentcore/runtime/endpoint/get", + "/agentcore/runtime/endpoint/list", + + // eval + "/agentcore/eval", + "/agentcore/eval/evaluator", + "/agentcore/eval/evaluator/llm-as-a-judge", + "/agentcore/eval/evaluator/llm-as-a-judge/create", + "/agentcore/eval/evaluator/llm-as-a-judge/update", + "/agentcore/eval/evaluator/code-based", + "/agentcore/eval/evaluator/code-based/create", + "/agentcore/eval/evaluator/code-based/update", + "/agentcore/eval/evaluator/get", + "/agentcore/eval/evaluator/list", + "/agentcore/eval/evaluator/delete", + + // config + "/agentcore/config", + + // project + "/agentcore/project/create", +] as const; + +export type CommandPath = (typeof COMMAND_PATHS)[number]; + +const commandPathSchema = z.enum([...COMMAND_PATHS, "unknown"]).catch("unknown"); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type BaseEnumeratedField = z.ZodEnum | z.ZodBoolean | z.ZodNumber | z.ZodLiteral; +type EnumeratedField = + | BaseEnumeratedField + | z.ZodOptional + | z.ZodDefault + | z.ZodCatch; + +/** Adds a compile time check that the given object has no free-form fields to prevent PII leakage **/ +function safeSchema>(shape: T) { + return z.object(shape); +} + +// We make an exception for error names because they are validated in the +// classification. We still apply a reasonable narrowing here to avoid leakage. +const errorNameSchema = z + .string() + .max(64) + .refine((s) => s.endsWith("Error") || s.endsWith("Exception"), { + message: "error_name must end with 'Error' or 'Exception'", + }) + .catch("UnknownError") as unknown as BaseEnumeratedField; + +export const commandRunSchema = safeSchema({ exit_reason: z.enum(["success", "failure"]), + command_path: commandPathSchema, + error_name: errorNameSchema.optional(), + error_source: z.enum(Object.values(ERROR_SOURCE)).optional(), }); From d728cf8fd9deefdd0f4aa6d0f32e97b706ecc4b8 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 11:44:16 +0000 Subject: [PATCH 03/11] feat(tel): route telemetry through the router context --- src/index.ts | 23 +++++++++++++++++------ src/router/index.tsx | 1 + src/router/router.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ src/router/router.tsx | 13 +++++++++++++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index 6fd6b3e736..f93ae7b267 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,6 +16,7 @@ import { runWithExitCode } from "./runnable"; import { DefaultGlobalConfigAccessor } from "./globalConfig"; import { DefaultTelemetryClient, TelemetryAttributesRecorder } from "./telemetry"; import { AgentCoreCLIError } from "./errors"; +import { TelemetryAttributesRecorderKey, ValueContext } from "./router"; process.exit( await runWithExitCode(async (argv: string[]) => { @@ -74,19 +75,29 @@ process.exit( globalConfigAccessor, }); + // we inject the attribute recorder early such that its available after we route the command, but before we parse the flags/args. + const context = ValueContext.EmptyContext().withValue< + TelemetryAttributesRecorder<"cli.command_run"> + >(TelemetryAttributesRecorderKey, commandRunTelemetryRecorder); + // Handle the request - await rootHandler.route(argv); + await rootHandler.route(argv, context); } catch (e) { const error = AgentCoreCLIError.fromError(e); rootLogger.child({ error: error.json() }).error(); - // TODO: add error details to telemetry recorder; - commandRunTelemetryRecorder.record({ exit_reason: "failure" }); - + commandRunTelemetryRecorder.record({ + exit_reason: "failure", + error_name: error.name, + error_source: error.source, + }); throw error; } finally { try { - const attributes = commandRunTelemetryRecorder.getAttributes(); - await telemetryClient.emit("cli.command_run", Date.now() - startTime, attributes); + await telemetryClient.emit( + "cli.command_run", + Date.now() - startTime, + commandRunTelemetryRecorder.getAttributes(), + ); } catch (e) { const error = AgentCoreCLIError.fromError(e); rootLogger.child({ error: error.json() }).warn("failed to emit telemetry"); diff --git a/src/router/index.tsx b/src/router/index.tsx index 3764b10208..ac69091a6b 100644 --- a/src/router/index.tsx +++ b/src/router/index.tsx @@ -5,6 +5,7 @@ export { PathKey, LoggerKey, GlobalConfigAccessorKey, + TelemetryAttributesRecorderKey, ProjectKey, type DefaultHandle, type DefaultHandlerProvider, diff --git a/src/router/router.test.ts b/src/router/router.test.ts index 5d1798b8b1..8e3296f119 100644 --- a/src/router/router.test.ts +++ b/src/router/router.test.ts @@ -4,6 +4,7 @@ import z from "zod"; import { Router, + TelemetryAttributesRecorderKey, ValueContext, argument, compile, @@ -16,6 +17,7 @@ import { type Middleware, } from "./index"; import { InputValidationError } from "../errors"; +import { TelemetryAttributesRecorder } from "../telemetry"; // --- helpers --------------------------------------------------------------- @@ -668,3 +670,40 @@ test("commands without long-form flag help have no Parameter details section", a expect(out).toContain("--id"); expect(out).not.toContain("Parameter details:"); }); + +// --- telemetry: command path recording ------------------------------------- + +test.each([ + { scenario: "flag validation succeeds", idFlag: "abc", shouldThrow: false }, + { scenario: "flag validation fails", idFlag: "toolong", shouldThrow: true }, +])( + "records command_path on the telemetry recorder when $scenario", + async ({ idFlag, shouldThrow }) => { + // we use a real command name here so that telemetry schemas accept the path produced below + const get = createHandler({ + name: "config", + description: "", + flags: [flag("id", "id", z.string().max(3))], + handle: async () => {}, + }); + + const root = new Router("agentcore"); + root.handler(get); + + const recorder = new TelemetryAttributesRecorder(`cli.command_run`); + const ctx = ValueContext.EmptyContext().withValue(TelemetryAttributesRecorderKey, recorder); + const cmd = exitOverrideAll(compile(root, ctx)); + + if (shouldThrow) { + await expect( + cmd.parseAsync(["node", "agentcore", "config", "--id", idFlag]), + ).rejects.toThrow(); + recorder.record({ exit_reason: "failure" }); + } else { + await cmd.parseAsync(["node", "agentcore", "config", "--id", idFlag]); + recorder.record({ exit_reason: "success" }); + } + + expect(recorder.getAttributes()).toMatchObject({ command_path: "/agentcore/config" }); + }, +); diff --git a/src/router/router.tsx b/src/router/router.tsx index aef8e584e8..805cabf7f6 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -8,6 +8,7 @@ import { Command } from "commander"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; import type { Project } from "../handlers/project/types"; +import { TelemetryAttributesRecorder, type CommandPath } from "../telemetry"; // CommandKey exposes the Commander Command for the executing leaf via context. export const CommandKey: ContextKey = contextKey("commander.command"); @@ -16,6 +17,10 @@ export const PathKey: ContextKey = contextKey("path"); export const LoggerKey = contextKey("logger"); +export const TelemetryAttributesRecorderKey = contextKey< + TelemetryAttributesRecorder<"cli.command_run"> +>("telemetryAttributesRecorder"); + export const GlobalConfigAccessorKey: ContextKey = contextKey("globalConfigAccessor"); export const ProjectKey = contextKey("project"); @@ -72,6 +77,14 @@ function attachAction( const command = actionArgs[actionArgs.length - 1] as Command; const merged = command.optsWithGlobals(); + // avoid requiring telemetry to avoid bloating simple tests. + const telemetryAttributesRecorder = ctx.value(TelemetryAttributesRecorderKey); + const commandPath = ctx.require(PathKey); + + telemetryAttributesRecorder?.record({ + command_path: commandPath as CommandPath, + }); + // Inherited group/global flags -> context (typed, read via ctx.value(key)). let leafCtx = ctx.withValue(CommandKey, command); leafCtx = applyGlobalFlags(globals, merged, leafCtx); From ba52e73f5eba73db190ff551a70534e3cf173fdd Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 12:06:02 +0000 Subject: [PATCH 04/11] refactor(tel): switch to shape check for command path --- src/router/router.tsx | 4 +-- src/telemetry/index.tsx | 1 - src/telemetry/shapes.tsx | 77 ++++------------------------------------ 3 files changed, 9 insertions(+), 73 deletions(-) diff --git a/src/router/router.tsx b/src/router/router.tsx index 805cabf7f6..ee24f9b0b5 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -8,7 +8,7 @@ import { Command } from "commander"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; import type { Project } from "../handlers/project/types"; -import { TelemetryAttributesRecorder, type CommandPath } from "../telemetry"; +import { TelemetryAttributesRecorder } from "../telemetry"; // CommandKey exposes the Commander Command for the executing leaf via context. export const CommandKey: ContextKey = contextKey("commander.command"); @@ -82,7 +82,7 @@ function attachAction( const commandPath = ctx.require(PathKey); telemetryAttributesRecorder?.record({ - command_path: commandPath as CommandPath, + command_path: commandPath, }); // Inherited group/global flags -> context (typed, read via ctx.value(key)). diff --git a/src/telemetry/index.tsx b/src/telemetry/index.tsx index a5eea48703..103df657c6 100644 --- a/src/telemetry/index.tsx +++ b/src/telemetry/index.tsx @@ -1,4 +1,3 @@ export { DefaultTelemetryClient } from "./client"; export { TelemetryAttributesRecorder } from "./recorder"; export { type AttributesOf } from "./types"; -export { type CommandPath } from "./shapes"; diff --git a/src/telemetry/shapes.tsx b/src/telemetry/shapes.tsx index 6d08bd0fb7..4195187c5e 100644 --- a/src/telemetry/shapes.tsx +++ b/src/telemetry/shapes.tsx @@ -26,76 +26,13 @@ export const resourceAttributesSchema = z.object({ */ export type ResourceAttributes = z.infer; -/** - * List of all invocable command paths in the CLI that should emit telemetry. - */ -export const COMMAND_PATHS = [ - // root - "/agentcore", - - // harness - "/agentcore/harness", - "/agentcore/harness/create", - "/agentcore/harness/get", - "/agentcore/harness/list", - "/agentcore/harness/update", - "/agentcore/harness/delete", - "/agentcore/harness/invoke", - "/agentcore/harness/exec", - "/agentcore/harness/endpoint", - "/agentcore/harness/endpoint/create", - "/agentcore/harness/endpoint/get", - "/agentcore/harness/endpoint/list", - "/agentcore/harness/endpoint/update", - "/agentcore/harness/endpoint/delete", - "/agentcore/harness/version", - "/agentcore/harness/version/get", - "/agentcore/harness/version/list", - - // identity - "/agentcore/identity", - "/agentcore/identity/api-key-credential-provider", - "/agentcore/identity/api-key-credential-provider/create", - "/agentcore/identity/api-key-credential-provider/get", - "/agentcore/identity/api-key-credential-provider/list", - "/agentcore/identity/api-key-credential-provider/update", - "/agentcore/identity/api-key-credential-provider/delete", - - // runtime - "/agentcore/runtime", - "/agentcore/runtime/get", - "/agentcore/runtime/list", - "/agentcore/runtime/invoke", - "/agentcore/runtime/version", - "/agentcore/runtime/version/get", - "/agentcore/runtime/version/list", - "/agentcore/runtime/endpoint", - "/agentcore/runtime/endpoint/get", - "/agentcore/runtime/endpoint/list", - - // eval - "/agentcore/eval", - "/agentcore/eval/evaluator", - "/agentcore/eval/evaluator/llm-as-a-judge", - "/agentcore/eval/evaluator/llm-as-a-judge/create", - "/agentcore/eval/evaluator/llm-as-a-judge/update", - "/agentcore/eval/evaluator/code-based", - "/agentcore/eval/evaluator/code-based/create", - "/agentcore/eval/evaluator/code-based/update", - "/agentcore/eval/evaluator/get", - "/agentcore/eval/evaluator/list", - "/agentcore/eval/evaluator/delete", - - // config - "/agentcore/config", - - // project - "/agentcore/project/create", -] as const; - -export type CommandPath = (typeof COMMAND_PATHS)[number]; - -const commandPathSchema = z.enum([...COMMAND_PATHS, "unknown"]).catch("unknown"); +// We make an exception for command paths becase they are validated as part of their construction. +// We still apply a reasonable narrowing here to avoid leakage. +const commandPathSchema = z + .string() + .max(128) + .regex(/^\/agentcore(\/[a-z][a-z0-9-]*)*$/, "command_path must be a valid CLI command path") + .catch("unknown") as unknown as BaseEnumeratedField; // eslint-disable-next-line @typescript-eslint/no-explicit-any type BaseEnumeratedField = z.ZodEnum | z.ZodBoolean | z.ZodNumber | z.ZodLiteral; From 15ac83454f1fa607a79e6e8f57c8b4ce17c5a4c6 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 13:54:46 +0000 Subject: [PATCH 05/11] feat(tel): wire up is_tui flag --- src/telemetry/client.test.tsx | 17 +++++++++++++++-- src/telemetry/shapes.tsx | 1 + src/telemetry/types.tsx | 6 +++++- src/tui/index.tsx | 4 ++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index 943397546e..c4f30527fa 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -78,12 +78,22 @@ describe("DefaultTelemetryClient", () => { { metricName: "cli.command_run", value: 123, - attrs: { ...resourceAttributes, exit_reason: "success", command_path: "/agentcore" }, + attrs: { + ...resourceAttributes, + exit_reason: "success", + command_path: "/agentcore", + is_tui: false, + }, }, { metricName: "cli.command_run", value: 456, - attrs: { ...resourceAttributes, exit_reason: "failure", command_path: "/agentcore" }, + attrs: { + ...resourceAttributes, + exit_reason: "failure", + command_path: "/agentcore", + is_tui: false, + }, }, ]); }); @@ -123,10 +133,12 @@ describe("DefaultTelemetryClient", () => { await enabledClient.emit("cli.command_run", 123, { exit_reason: "success", command_path: "/agentcore", + is_tui: true, }); await disabledClient.emit("cli.command_run", 456, { exit_reason: "failure", command_path: "/agentcore", + is_tui: false, }); await Promise.all([enabledClient.shutdown(), disabledClient.shutdown()]); @@ -146,6 +158,7 @@ describe("DefaultTelemetryClient", () => { "node.version": process.version, exit_reason: "success", command_path: "/agentcore", + is_tui: true, }, }); }); diff --git a/src/telemetry/shapes.tsx b/src/telemetry/shapes.tsx index 4195187c5e..74f8c71e98 100644 --- a/src/telemetry/shapes.tsx +++ b/src/telemetry/shapes.tsx @@ -62,4 +62,5 @@ export const commandRunSchema = safeSchema({ command_path: commandPathSchema, error_name: errorNameSchema.optional(), error_source: z.enum(Object.values(ERROR_SOURCE)).optional(), + is_tui: z.boolean().default(false), }); diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index f29e01f3d4..18d94e9760 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -18,7 +18,11 @@ export interface TelemetryClient { */ export interface MetricSink { /** Send data to the given metric sink **/ - send(metricName: string, value: number, attributes: Record): void; + send( + metricName: string, + value: number, + attributes: Record, + ): void; /** Flush and close the given metric sink **/ shutdown(): Promise; getName(): string; diff --git a/src/tui/index.tsx b/src/tui/index.tsx index 6e423d5039..31fad8c129 100644 --- a/src/tui/index.tsx +++ b/src/tui/index.tsx @@ -7,6 +7,7 @@ import { CommandKey, contextKey, type ContextKey, + TelemetryAttributesRecorderKey, } from "../router"; import type { AppIO } from "../io"; import type { Core } from "../handlers/types"; @@ -43,6 +44,9 @@ export async function renderTuiAt( core: Core, io: AppIO, ): Promise { + const telemetryAttributesRecorder = ctx.value(TelemetryAttributesRecorderKey); + telemetryAttributesRecorder?.record({ is_tui: true }); + if (!io.stdin.isTTY || !io.stdout.isTTY) { throw new InvalidEnvironmentError("interactive mode requires a TTY on stdin and stdout"); } From a154f73cdd8c3f66702ebc5574f402282bb542ef Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 18:27:05 +0000 Subject: [PATCH 06/11] docs(tel): remove some noisy comments --- src/index.ts | 1 - src/router/router.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index f93ae7b267..fa719631d7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -75,7 +75,6 @@ process.exit( globalConfigAccessor, }); - // we inject the attribute recorder early such that its available after we route the command, but before we parse the flags/args. const context = ValueContext.EmptyContext().withValue< TelemetryAttributesRecorder<"cli.command_run"> >(TelemetryAttributesRecorderKey, commandRunTelemetryRecorder); diff --git a/src/router/router.tsx b/src/router/router.tsx index ee24f9b0b5..070def4007 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -77,7 +77,6 @@ function attachAction( const command = actionArgs[actionArgs.length - 1] as Command; const merged = command.optsWithGlobals(); - // avoid requiring telemetry to avoid bloating simple tests. const telemetryAttributesRecorder = ctx.value(TelemetryAttributesRecorderKey); const commandPath = ctx.require(PathKey); From b9973eec4a4be2cefe6df1feb4e1da796f83c44d Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 22:02:55 +0000 Subject: [PATCH 07/11] refactor(tel): move parsing into emit by coupling the recorder to the client --- src/index.ts | 13 +++--- src/router/router.test.ts | 11 ++++- src/router/router.tsx | 4 +- src/telemetry/client.test.tsx | 52 +++++++++++++-------- src/telemetry/client.tsx | 87 +++++++++++++++++++++++------------ src/telemetry/index.tsx | 3 +- src/telemetry/recorder.tsx | 35 -------------- src/telemetry/types.tsx | 28 ++++++++++- 8 files changed, 135 insertions(+), 98 deletions(-) delete mode 100644 src/telemetry/recorder.tsx diff --git a/src/index.ts b/src/index.ts index fa719631d7..b7f295f77c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,7 +14,7 @@ import { FsReadWriteJson } from "./io"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; import { DefaultGlobalConfigAccessor } from "./globalConfig"; -import { DefaultTelemetryClient, TelemetryAttributesRecorder } from "./telemetry"; +import { DefaultTelemetryClient } from "./telemetry"; import { AgentCoreCLIError } from "./errors"; import { TelemetryAttributesRecorderKey, ValueContext } from "./router"; @@ -50,7 +50,7 @@ process.exit( globalConfigAccessor, }); - const commandRunTelemetryRecorder = new TelemetryAttributesRecorder("cli.command_run", { + const commandRunTelemetryRecorder = telemetryClient.getAttributesRecorder("cli.command_run", { exit_reason: "success", }); @@ -75,9 +75,10 @@ process.exit( globalConfigAccessor, }); - const context = ValueContext.EmptyContext().withValue< - TelemetryAttributesRecorder<"cli.command_run"> - >(TelemetryAttributesRecorderKey, commandRunTelemetryRecorder); + const context = ValueContext.EmptyContext().withValue( + TelemetryAttributesRecorderKey, + commandRunTelemetryRecorder, + ); // Handle the request await rootHandler.route(argv, context); @@ -95,7 +96,7 @@ process.exit( await telemetryClient.emit( "cli.command_run", Date.now() - startTime, - commandRunTelemetryRecorder.getAttributes(), + commandRunTelemetryRecorder, ); } catch (e) { const error = AgentCoreCLIError.fromError(e); diff --git a/src/router/router.test.ts b/src/router/router.test.ts index 8e3296f119..0628618b14 100644 --- a/src/router/router.test.ts +++ b/src/router/router.test.ts @@ -17,7 +17,8 @@ import { type Middleware, } from "./index"; import { InputValidationError } from "../errors"; -import { TelemetryAttributesRecorder } from "../telemetry"; +import { DefaultTelemetryClient } from "../telemetry"; +import { createSilentLogger, TestGlobalConfigAccessor } from "../testing"; // --- helpers --------------------------------------------------------------- @@ -687,10 +688,16 @@ test.each([ handle: async () => {}, }); + const telemetryClient = new DefaultTelemetryClient({ + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + sessionId: "test-session-id", + }); + const root = new Router("agentcore"); root.handler(get); - const recorder = new TelemetryAttributesRecorder(`cli.command_run`); + const recorder = telemetryClient.getAttributesRecorder("cli.command_run"); const ctx = ValueContext.EmptyContext().withValue(TelemetryAttributesRecorderKey, recorder); const cmd = exitOverrideAll(compile(root, ctx)); diff --git a/src/router/router.tsx b/src/router/router.tsx index 070def4007..cb6c2f3c92 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -8,7 +8,7 @@ import { Command } from "commander"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; import type { Project } from "../handlers/project/types"; -import { TelemetryAttributesRecorder } from "../telemetry"; +import { type AttributesOf, type AttributesRecorder } from "../telemetry"; // CommandKey exposes the Commander Command for the executing leaf via context. export const CommandKey: ContextKey = contextKey("commander.command"); @@ -18,7 +18,7 @@ export const PathKey: ContextKey = contextKey("path"); export const LoggerKey = contextKey("logger"); export const TelemetryAttributesRecorderKey = contextKey< - TelemetryAttributesRecorder<"cli.command_run"> + AttributesRecorder> >("telemetryAttributesRecorder"); export const GlobalConfigAccessorKey: ContextKey = diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index c4f30527fa..1d2b22a12c 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -3,7 +3,6 @@ import { join } from "node:path"; import { mkdtemp, readFile, rm } from "node:fs/promises"; import os, { 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"; @@ -41,15 +40,15 @@ describe("DefaultTelemetryClient", () => { metricSinks: [fileSystemSink], }); - const recorder = new TelemetryAttributesRecorder("cli.command_run", { + const recorder = client.getAttributesRecorder("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); - await client.emit("cli.command_run", 123, recorder.getAttributes()); + await client.emit("cli.command_run", 123, recorder); recorder.record({ exit_reason: "failure" }); - await client.emit("cli.command_run", 456, recorder.getAttributes()); + await client.emit("cli.command_run", 456, recorder); await client.shutdown(); expect(fileSystemSink.getName()).toBe("FileSystemSink"); @@ -130,16 +129,24 @@ describe("DefaultTelemetryClient", () => { auditFilePath, }); - await enabledClient.emit("cli.command_run", 123, { - exit_reason: "success", - command_path: "/agentcore", - is_tui: true, - }); - await disabledClient.emit("cli.command_run", 456, { - exit_reason: "failure", - command_path: "/agentcore", - is_tui: false, - }); + await enabledClient.emit( + "cli.command_run", + 123, + enabledClient.getAttributesRecorder("cli.command_run", { + exit_reason: "success", + command_path: "/agentcore", + is_tui: true, + }), + ); + await disabledClient.emit( + "cli.command_run", + 123, + enabledClient.getAttributesRecorder("cli.command_run", { + exit_reason: "failure", + command_path: "/agentcore", + is_tui: false, + }), + ); await Promise.all([enabledClient.shutdown(), disabledClient.shutdown()]); const auditLines = (await readFile(auditFilePath, "utf8")).trimEnd().split("\n"); @@ -171,9 +178,9 @@ describe("DefaultTelemetryClient", () => { metricSinks: [], }); - const recorder = new TelemetryAttributesRecorder("cli.command_run"); + const recorder = client.getAttributesRecorder("cli.command_run"); - expect(() => client.emit("cli.command_run", 100, recorder.getAttributes())).toThrow(); + expect(() => client.emit("cli.command_run", 100, recorder)).toThrow(); await client.shutdown(); }); @@ -190,8 +197,11 @@ describe("DefaultTelemetryClient", () => { globalConfigAccessor: new TestGlobalConfigAccessor(), metricSinks: [sink], }); - - await client.emit("cli.command_run", 1, { exit_reason: "success", command_path: "/agentcore" }); + const recorder = client.getAttributesRecorder("cli.command_run", { + exit_reason: "success", + command_path: "/agentcore", + }); + await client.emit("cli.command_run", 1, recorder); await client.shutdown(); await assertLogsMatch(tempDir, [ @@ -231,11 +241,13 @@ describe("DefaultTelemetryClient", () => { metricSinks: [badSink, goodSink], }); - // emit should not throw even though the sink's record() throws - await client.emit("cli.command_run", 100, { + const recorder = client.getAttributesRecorder("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); + + // emit should not throw even though the sink's record() throws + await client.emit("cli.command_run", 100, recorder); // shutdown should not throw even though the sink's shutdown() rejects await client.shutdown(); diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx index 63c1f655ad..0f2182f3f9 100644 --- a/src/telemetry/client.tsx +++ b/src/telemetry/client.tsx @@ -8,6 +8,7 @@ import { type ValueOf, METRICS, type MetricName, + type AttributesRecorder, } from "./types"; import type { GlobalConfigAccessor } from "../globalConfig"; import { FileSystemSink } from "./fileSystemSink"; @@ -40,40 +41,43 @@ export class DefaultTelemetryClient implements TelemetryClient { path.join(os.homedir(), ".agentcore", "telemetry", `${this.sessionId}.jsonl`); } + getAttributesRecorder( + _metricName: TMetricName, + initialAttributes: Partial> = {}, + ): AttributesRecorder> { + return new InMemoryAttributesRecorder(initialAttributes); + } + async emit( metricName: TMetricName, metricValue: ValueOf, - metricAttributes: AttributesOf, + attributesRecorder: AttributesRecorder>, ): Promise { - try { - const metricSinks = await this.getMetricSinks(); - const resourceAttributes = await this.getResourceAttributes(); - // merge in resource attributes with metric attributes before sending to sink. - const attributes = { - ...resourceAttributes, - ...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.getName()}'`); - // 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. - } + const metricSinks = await this.getMetricSinks(); + const resourceAttributes = await this.getResourceAttributes(); + + const metricAttributes = METRICS[metricName]["attributeSchema"].parse( + attributesRecorder.getAttributes(), + ); + const validatedMetricValue = METRICS[metricName]["valueSchema"].parse(metricValue); + + // merge in resource attributes with metric attributes before sending to sink. + const attributes = { + ...resourceAttributes, + ...metricAttributes, + }; + + 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.getName()}'`); + // do not allow a single sink failure to fail other sinks. + } + }); } async shutdown(): Promise { @@ -129,3 +133,26 @@ function once(fn: () => Promise): () => Promise { let cachedPromise: Promise | undefined; return () => (cachedPromise ??= fn()); } + +/** An in-memory implementation of {@link AttributesRecorder} that stores the data in memory **/ +class InMemoryAttributesRecorder< + TAttributes extends Record, +> implements AttributesRecorder { + private data: Partial; + + constructor(initialAttributes?: Partial) { + this.data = initialAttributes ?? {}; + } + + record(newData: Partial): Partial { + this.data = { + ...this.data, + ...newData, + }; + return this.data; + } + + getAttributes(): Partial { + return this.data; + } +} diff --git a/src/telemetry/index.tsx b/src/telemetry/index.tsx index 103df657c6..ab5bd3b345 100644 --- a/src/telemetry/index.tsx +++ b/src/telemetry/index.tsx @@ -1,3 +1,2 @@ export { DefaultTelemetryClient } from "./client"; -export { TelemetryAttributesRecorder } from "./recorder"; -export { type AttributesOf } from "./types"; +export { type AttributesOf, type AttributesRecorder } from "./types"; diff --git a/src/telemetry/recorder.tsx b/src/telemetry/recorder.tsx deleted file mode 100644 index ba83df103c..0000000000 --- a/src/telemetry/recorder.tsx +++ /dev/null @@ -1,35 +0,0 @@ -import { type MetricName, type AttributesOf, METRICS } from "./types"; - -/** - * A strongly typed recorder for accumulating metric attributes, bound to a specific metric's schema. - */ -export class TelemetryAttributesRecorder { - private attributes: Partial>; - - constructor( - private readonly metricName: TMetricName, - initialAttributes: Partial> = {}, - ) { - this.attributes = initialAttributes; - } - - /** - * 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, - ...data, - }; - return this.attributes; - } -} diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index 18d94e9760..d8f8a92f1d 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -5,14 +5,40 @@ import { commandRunSchema } from "./shapes"; * The primary interface for telemetry that orchestrates the emitting of metrics */ export interface TelemetryClient { + /** + * Get a recorder for accumulating attributes associated with a specific metric. + */ + getAttributesRecorder( + metricName: TMetricName, + initialAttributes: Partial>, + ): AttributesRecorder>; + + /** + * Emits the given data, attaching the attributes stored in the given attribute recorder + */ emit( metricName: TMetricName, metricValue: ValueOf, - attributes: AttributesOf, + attributesRecorder: AttributesRecorder>, ): Promise; + shutdown(): Promise; } +/** + * A strongly typed recorder for accumulating metric attributes + */ +export interface AttributesRecorder> { + /** + * Retrieves the underlying attributes + */ + getAttributes(): Partial; + /** + * Add attributes that overwrite existing values if already set. + */ + record(data: Partial): Partial; +} + /** * A destination to send metric data. */ From e8f3d7d96a013ee18880e7b3abb08947c9cabeac Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Thu, 30 Jul 2026 22:36:37 +0000 Subject: [PATCH 08/11] fix(tel): ensure commander errors recieve the command path before exiting --- src/router/router.tsx | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/router/router.tsx b/src/router/router.tsx index cb6c2f3c92..714fc2a9dc 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -77,12 +77,7 @@ function attachAction( const command = actionArgs[actionArgs.length - 1] as Command; const merged = command.optsWithGlobals(); - const telemetryAttributesRecorder = ctx.value(TelemetryAttributesRecorderKey); - const commandPath = ctx.require(PathKey); - - telemetryAttributesRecorder?.record({ - command_path: commandPath, - }); + recordCommandPath(ctx); // Inherited group/global flags -> context (typed, read via ctx.value(key)). let leafCtx = ctx.withValue(CommandKey, command); @@ -102,6 +97,11 @@ function globalFlagsOf(node: Handler): GlobalFlag[] { return node.flags().filter((f): f is GlobalFlag => "id" in f); } +/** Add the command path to active command run metric **/ +function recordCommandPath(ctx: Context): void { + ctx.value(TelemetryAttributesRecorderKey)?.record({ command_path: ctx.value(PathKey) }); +} + // compile walks the Handler tree into a Commander Command tree. // // `stack` is the accumulated middleware declared by ancestors. A node's own @@ -120,7 +120,7 @@ export function compile( stack: Middleware[] = [], inheritedGlobals: GlobalFlag[] = [], ): Command { - const c = new Command(node.name()).exitOverride(); + const c = new Command(node.name()); c.description(node.description()); const ownFlags = node.flags(); @@ -141,6 +141,12 @@ export function compile( const newPath = `${path}/${node.name()}`; ctx = ctx.withValue(PathKey, newPath); + // commander may fail on invalid flags before we are able to record on the happy path, so we must record here as well + c.exitOverride((e) => { + recordCommandPath(ctx); + throw e; + }); + const children = node.children(); if (children.length > 0) { // attaching both children and subcommands leads to ambiguity. From da68951baa7ed007499bfe385c68ca8a73468ae6 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Fri, 31 Jul 2026 12:31:28 +0000 Subject: [PATCH 09/11] refactor(tel): move to metric event instead of attributes recorder --- src/index.ts | 18 +++--- src/router/index.tsx | 2 +- src/router/router.test.ts | 93 ++++++++++++++++------------ src/router/router.tsx | 9 ++- src/telemetry/client.test.tsx | 61 +++++++++--------- src/telemetry/client.tsx | 102 +++++++++++++++++-------------- src/telemetry/fileSystemSink.tsx | 8 ++- src/telemetry/index.tsx | 2 +- src/telemetry/types.tsx | 35 ++++------- src/tui/index.tsx | 5 +- 10 files changed, 174 insertions(+), 161 deletions(-) diff --git a/src/index.ts b/src/index.ts index b7f295f77c..9ab15d439d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,7 +16,7 @@ import { runWithExitCode } from "./runnable"; import { DefaultGlobalConfigAccessor } from "./globalConfig"; import { DefaultTelemetryClient } from "./telemetry"; import { AgentCoreCLIError } from "./errors"; -import { TelemetryAttributesRecorderKey, ValueContext } from "./router"; +import { CommandRunMetricEventKey, ValueContext } from "./router"; process.exit( await runWithExitCode(async (argv: string[]) => { @@ -50,7 +50,7 @@ process.exit( globalConfigAccessor, }); - const commandRunTelemetryRecorder = telemetryClient.getAttributesRecorder("cli.command_run", { + const commandRunMetricEvent = telemetryClient.startMetricEvent("cli.command_run", { exit_reason: "success", }); @@ -75,9 +75,9 @@ process.exit( globalConfigAccessor, }); - const context = ValueContext.EmptyContext().withValue( - TelemetryAttributesRecorderKey, - commandRunTelemetryRecorder, + const context = ValueContext.EmptyContext().withValue( + CommandRunMetricEventKey, + commandRunMetricEvent, ); // Handle the request @@ -85,7 +85,7 @@ process.exit( } catch (e) { const error = AgentCoreCLIError.fromError(e); rootLogger.child({ error: error.json() }).error(); - commandRunTelemetryRecorder.record({ + commandRunMetricEvent.setAttributes({ exit_reason: "failure", error_name: error.name, error_source: error.source, @@ -93,11 +93,7 @@ process.exit( throw error; } finally { try { - await telemetryClient.emit( - "cli.command_run", - Date.now() - startTime, - commandRunTelemetryRecorder, - ); + await commandRunMetricEvent.end(Date.now() - startTime); } catch (e) { const error = AgentCoreCLIError.fromError(e); rootLogger.child({ error: error.json() }).warn("failed to emit telemetry"); diff --git a/src/router/index.tsx b/src/router/index.tsx index ac69091a6b..6915212377 100644 --- a/src/router/index.tsx +++ b/src/router/index.tsx @@ -5,7 +5,7 @@ export { PathKey, LoggerKey, GlobalConfigAccessorKey, - TelemetryAttributesRecorderKey, + CommandRunMetricEventKey, ProjectKey, type DefaultHandle, type DefaultHandlerProvider, diff --git a/src/router/router.test.ts b/src/router/router.test.ts index 0628618b14..88ba197762 100644 --- a/src/router/router.test.ts +++ b/src/router/router.test.ts @@ -4,7 +4,7 @@ import z from "zod"; import { Router, - TelemetryAttributesRecorderKey, + CommandRunMetricEventKey, ValueContext, argument, compile, @@ -677,40 +677,57 @@ test("commands without long-form flag help have no Parameter details section", a test.each([ { scenario: "flag validation succeeds", idFlag: "abc", shouldThrow: false }, { scenario: "flag validation fails", idFlag: "toolong", shouldThrow: true }, -])( - "records command_path on the telemetry recorder when $scenario", - async ({ idFlag, shouldThrow }) => { - // we use a real command name here so that telemetry schemas accept the path produced below - const get = createHandler({ - name: "config", - description: "", - flags: [flag("id", "id", z.string().max(3))], - handle: async () => {}, - }); - - const telemetryClient = new DefaultTelemetryClient({ - logger: createSilentLogger(), - globalConfigAccessor: new TestGlobalConfigAccessor(), - sessionId: "test-session-id", - }); - - const root = new Router("agentcore"); - root.handler(get); - - const recorder = telemetryClient.getAttributesRecorder("cli.command_run"); - const ctx = ValueContext.EmptyContext().withValue(TelemetryAttributesRecorderKey, recorder); - const cmd = exitOverrideAll(compile(root, ctx)); - - if (shouldThrow) { - await expect( - cmd.parseAsync(["node", "agentcore", "config", "--id", idFlag]), - ).rejects.toThrow(); - recorder.record({ exit_reason: "failure" }); - } else { - await cmd.parseAsync(["node", "agentcore", "config", "--id", idFlag]); - recorder.record({ exit_reason: "success" }); - } - - expect(recorder.getAttributes()).toMatchObject({ command_path: "/agentcore/config" }); - }, -); +])("records command_path on the metric event when $scenario", async ({ idFlag, shouldThrow }) => { + // we use a real command name here so that telemetry schemas accept the path produced below + const get = createHandler({ + name: "config", + description: "", + flags: [flag("id", "id", z.string().max(3))], + handle: async () => {}, + }); + + const recordedMetrics: { metricName: string; value: number; attributes: Record }[] = + []; + const inMemorySink = { + getName: () => "InMemorySink", + send: (metricName: string, value: number, attributes: Record) => { + recordedMetrics.push({ metricName, value, attributes }); + }, + shutdown: async () => {}, + }; + + const telemetryClient = new DefaultTelemetryClient({ + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + sessionId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + metricSinks: [inMemorySink], + }); + + const root = new Router("agentcore"); + root.handler(get); + + const commandRunMetricEvent = telemetryClient.startMetricEvent("cli.command_run"); + const ctx = ValueContext.EmptyContext().withValue( + CommandRunMetricEventKey, + commandRunMetricEvent, + ); + const cmd = compile(root, ctx); + + if (shouldThrow) { + await expect(cmd.parseAsync(["node", "agentcore", "config", "--id", idFlag])).rejects.toThrow(); + commandRunMetricEvent.setAttributes({ exit_reason: "failure" }); + } else { + await cmd.parseAsync(["node", "agentcore", "config", "--id", idFlag]); + commandRunMetricEvent.setAttributes({ exit_reason: "success" }); + } + + await commandRunMetricEvent.end(100); + + expect(recordedMetrics).toHaveLength(1); + expect(recordedMetrics[0]!.metricName).toBe("cli.command_run"); + expect(recordedMetrics[0]!.value).toBe(100); + expect(recordedMetrics[0]!.attributes).toMatchObject({ + command_path: "/agentcore/config", + exit_reason: shouldThrow ? "failure" : "success", + }); +}); diff --git a/src/router/router.tsx b/src/router/router.tsx index 714fc2a9dc..492665e792 100644 --- a/src/router/router.tsx +++ b/src/router/router.tsx @@ -8,7 +8,7 @@ import { Command } from "commander"; import type { Logger } from "../logging"; import type { GlobalConfigAccessor } from "../globalConfig"; import type { Project } from "../handlers/project/types"; -import { type AttributesOf, type AttributesRecorder } from "../telemetry"; +import { type MetricEvent } from "../telemetry"; // CommandKey exposes the Commander Command for the executing leaf via context. export const CommandKey: ContextKey = contextKey("commander.command"); @@ -17,9 +17,8 @@ export const PathKey: ContextKey = contextKey("path"); export const LoggerKey = contextKey("logger"); -export const TelemetryAttributesRecorderKey = contextKey< - AttributesRecorder> ->("telemetryAttributesRecorder"); +export const CommandRunMetricEventKey = + contextKey>("commandRunMetricEvent"); export const GlobalConfigAccessorKey: ContextKey = contextKey("globalConfigAccessor"); @@ -99,7 +98,7 @@ function globalFlagsOf(node: Handler): GlobalFlag[] { /** Add the command path to active command run metric **/ function recordCommandPath(ctx: Context): void { - ctx.value(TelemetryAttributesRecorderKey)?.record({ command_path: ctx.value(PathKey) }); + ctx.value(CommandRunMetricEventKey)?.setAttributes({ command_path: ctx.value(PathKey) }); } // compile walks the Handler tree into a Commander Command tree. diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index 1d2b22a12c..5d644082bb 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -40,15 +40,20 @@ describe("DefaultTelemetryClient", () => { metricSinks: [fileSystemSink], }); - const recorder = client.getAttributesRecorder("cli.command_run", { + const metricEvent = client.startMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); - await client.emit("cli.command_run", 123, recorder); + await metricEvent.end(123); - recorder.record({ exit_reason: "failure" }); - await client.emit("cli.command_run", 456, recorder); + // Start a second event with failure + const metricEvent2 = client.startMetricEvent("cli.command_run", { + exit_reason: "failure", + command_path: "/agentcore", + }); + + await metricEvent2.end(456); await client.shutdown(); expect(fileSystemSink.getName()).toBe("FileSystemSink"); @@ -129,24 +134,20 @@ describe("DefaultTelemetryClient", () => { auditFilePath, }); - await enabledClient.emit( - "cli.command_run", - 123, - enabledClient.getAttributesRecorder("cli.command_run", { - exit_reason: "success", - command_path: "/agentcore", - is_tui: true, - }), - ); - await disabledClient.emit( - "cli.command_run", - 123, - enabledClient.getAttributesRecorder("cli.command_run", { - exit_reason: "failure", - command_path: "/agentcore", - is_tui: false, - }), - ); + const enabledEvent = enabledClient.startMetricEvent("cli.command_run", { + exit_reason: "success", + command_path: "/agentcore", + is_tui: true, + }); + await enabledEvent.end(123); + + const disabledEvent = disabledClient.startMetricEvent("cli.command_run", { + exit_reason: "failure", + command_path: "/agentcore", + is_tui: false, + }); + await disabledEvent.end(123); + await Promise.all([enabledClient.shutdown(), disabledClient.shutdown()]); const auditLines = (await readFile(auditFilePath, "utf8")).trimEnd().split("\n"); @@ -170,7 +171,7 @@ describe("DefaultTelemetryClient", () => { }); }); - test("throws when recorder has incomplete attributes", async () => { + test("throws when metric event has incomplete attributes", async () => { const client = new DefaultTelemetryClient({ logger, sessionId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", @@ -178,9 +179,9 @@ describe("DefaultTelemetryClient", () => { metricSinks: [], }); - const recorder = client.getAttributesRecorder("cli.command_run"); + const metricEvent = client.startMetricEvent("cli.command_run"); - expect(() => client.emit("cli.command_run", 100, recorder)).toThrow(); + await expect(metricEvent.end(100)).rejects.toThrow(); await client.shutdown(); }); @@ -197,11 +198,11 @@ describe("DefaultTelemetryClient", () => { globalConfigAccessor: new TestGlobalConfigAccessor(), metricSinks: [sink], }); - const recorder = client.getAttributesRecorder("cli.command_run", { + const metricEvent = client.startMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); - await client.emit("cli.command_run", 1, recorder); + await metricEvent.end(1); await client.shutdown(); await assertLogsMatch(tempDir, [ @@ -241,13 +242,13 @@ describe("DefaultTelemetryClient", () => { metricSinks: [badSink, goodSink], }); - const recorder = client.getAttributesRecorder("cli.command_run", { + const metricEvent = client.startMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); - // emit should not throw even though the sink's record() throws - await client.emit("cli.command_run", 100, recorder); + // end should not throw even though the sink's send() throws + await metricEvent.end(100); // shutdown should not throw even though the sink's shutdown() rejects await client.shutdown(); diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx index 0f2182f3f9..45c22706fd 100644 --- a/src/telemetry/client.tsx +++ b/src/telemetry/client.tsx @@ -8,7 +8,7 @@ import { type ValueOf, METRICS, type MetricName, - type AttributesRecorder, + type MetricEvent, } from "./types"; import type { GlobalConfigAccessor } from "../globalConfig"; import { FileSystemSink } from "./fileSystemSink"; @@ -31,6 +31,7 @@ export class DefaultTelemetryClient implements TelemetryClient { private readonly auditFilePath: string; private globalConfigAccessor: GlobalConfigAccessor; private readonly metricSinksOverride: MetricSink[] | undefined; + constructor(config: DefaultTelemetryClientConfig) { this.logger = config.logger; this.sessionId = config.sessionId; @@ -41,42 +42,16 @@ export class DefaultTelemetryClient implements TelemetryClient { path.join(os.homedir(), ".agentcore", "telemetry", `${this.sessionId}.jsonl`); } - getAttributesRecorder( - _metricName: TMetricName, - initialAttributes: Partial> = {}, - ): AttributesRecorder> { - return new InMemoryAttributesRecorder(initialAttributes); - } - - async emit( + startMetricEvent( metricName: TMetricName, - metricValue: ValueOf, - attributesRecorder: AttributesRecorder>, - ): Promise { - const metricSinks = await this.getMetricSinks(); - const resourceAttributes = await this.getResourceAttributes(); - - const metricAttributes = METRICS[metricName]["attributeSchema"].parse( - attributesRecorder.getAttributes(), - ); - const validatedMetricValue = METRICS[metricName]["valueSchema"].parse(metricValue); - - // merge in resource attributes with metric attributes before sending to sink. - const attributes = { - ...resourceAttributes, - ...metricAttributes, - }; - - 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.getName()}'`); - // do not allow a single sink failure to fail other sinks. - } + initialAttributes: Partial> = {}, + ): MetricEvent { + return new InMemoryMetricEvent({ + metricName, + initialAttributes, + logger: this.logger, + getSinks: () => this.getMetricSinks(), + getResourceAttributes: () => this.getResourceAttributes(), }); } @@ -134,25 +109,58 @@ function once(fn: () => Promise): () => Promise { return () => (cachedPromise ??= fn()); } -/** An in-memory implementation of {@link AttributesRecorder} that stores the data in memory **/ -class InMemoryAttributesRecorder< - TAttributes extends Record, -> implements AttributesRecorder { - private data: Partial; +type InMemoryMetricEventConfig = { + metricName: TMetricName; + initialAttributes?: Partial>; + logger: Logger; + getSinks: () => Promise; + getResourceAttributes: () => Promise; +}; - constructor(initialAttributes?: Partial) { - this.data = initialAttributes ?? {}; +/** An in-memory implementation of {@link MetricEvent} that accumulates attributes and emits on end() **/ +class InMemoryMetricEvent implements MetricEvent { + private data: Partial>; + private readonly metricName: TMetricName; + private readonly logger: Logger; + private readonly getSinks: () => Promise; + private readonly getResourceAttributes: () => Promise; + + constructor(config: InMemoryMetricEventConfig) { + this.metricName = config.metricName; + this.data = config.initialAttributes ?? {}; + this.logger = config.logger; + this.getSinks = config.getSinks; + this.getResourceAttributes = config.getResourceAttributes; } - record(newData: Partial): Partial { + setAttributes(newData: Partial>): void { this.data = { ...this.data, ...newData, }; - return this.data; } - getAttributes(): Partial { - return this.data; + async end(value: ValueOf): Promise { + const metricAttributes = METRICS[this.metricName]["attributeSchema"].parse(this.data); + const validatedValue = METRICS[this.metricName]["valueSchema"].parse(value); + const resourceAttributes = await this.getResourceAttributes(); + + const attributes = { + ...resourceAttributes, + ...metricAttributes, + }; + + const sinks = await this.getSinks(); + + sinks.forEach((sink) => { + try { + sink.send(this.metricName, validatedValue, 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.getName()}'`); + } + }); } } diff --git a/src/telemetry/fileSystemSink.tsx b/src/telemetry/fileSystemSink.tsx index 28d259757e..26283c8197 100644 --- a/src/telemetry/fileSystemSink.tsx +++ b/src/telemetry/fileSystemSink.tsx @@ -26,7 +26,11 @@ export class FileSystemSink implements MetricSink { this.pendingWrite = Promise.resolve(); } - send(metricName: string, value: number, attributes: Record): void { + send( + metricName: string, + value: number, + attributes: Record, + ): void { this.pendingWrite = this.pendingWrite.then(() => this.appendEntry({ metricName, value, attrs: attributes }), ); @@ -35,7 +39,7 @@ export class FileSystemSink implements MetricSink { private async appendEntry(entry: { metricName: string; value: number; - attrs: Record; + attrs: Record; }): Promise { await mkdir(dirname(this.filePath), { recursive: true }); await appendFile(this.filePath, JSON.stringify(entry) + "\n"); diff --git a/src/telemetry/index.tsx b/src/telemetry/index.tsx index ab5bd3b345..6f28d14244 100644 --- a/src/telemetry/index.tsx +++ b/src/telemetry/index.tsx @@ -1,2 +1,2 @@ export { DefaultTelemetryClient } from "./client"; -export { type AttributesOf, type AttributesRecorder } from "./types"; +export { type AttributesOf, type MetricEvent } from "./types"; diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index d8f8a92f1d..ed8e127749 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -6,37 +6,26 @@ import { commandRunSchema } from "./shapes"; */ export interface TelemetryClient { /** - * Get a recorder for accumulating attributes associated with a specific metric. + * Start a new metric event that accumulates attributes over its lifecycle. */ - getAttributesRecorder( + startMetricEvent( metricName: TMetricName, - initialAttributes: Partial>, - ): AttributesRecorder>; - - /** - * Emits the given data, attaching the attributes stored in the given attribute recorder - */ - emit( - metricName: TMetricName, - metricValue: ValueOf, - attributesRecorder: AttributesRecorder>, - ): Promise; + initialAttributes?: Partial>, + ): MetricEvent; shutdown(): Promise; } /** - * A strongly typed recorder for accumulating metric attributes + * A single in-flight metric event. Accumulate attributes with record(), then + * call end() to validate and emit. Throws if the metric is invalid. */ -export interface AttributesRecorder> { - /** - * Retrieves the underlying attributes - */ - getAttributes(): Partial; - /** - * Add attributes that overwrite existing values if already set. - */ - record(data: Partial): Partial; +export interface MetricEvent { + /** Accumulate attributes incrementally. Later calls overwrite earlier values for the same key. */ + setAttributes(data: Partial>): void; + + /** Validate attributes, emit value + attributes to all sinks. */ + end(value: ValueOf): Promise; } /** diff --git a/src/tui/index.tsx b/src/tui/index.tsx index 31fad8c129..9586554390 100644 --- a/src/tui/index.tsx +++ b/src/tui/index.tsx @@ -7,7 +7,7 @@ import { CommandKey, contextKey, type ContextKey, - TelemetryAttributesRecorderKey, + CommandRunMetricEventKey, } from "../router"; import type { AppIO } from "../io"; import type { Core } from "../handlers/types"; @@ -44,8 +44,7 @@ export async function renderTuiAt( core: Core, io: AppIO, ): Promise { - const telemetryAttributesRecorder = ctx.value(TelemetryAttributesRecorderKey); - telemetryAttributesRecorder?.record({ is_tui: true }); + ctx.value(CommandRunMetricEventKey)?.setAttributes({ is_tui: true }); if (!io.stdin.isTTY || !io.stdout.isTTY) { throw new InvalidEnvironmentError("interactive mode requires a TTY on stdin and stdout"); From f01e5ff7d2a57ee336a1629c59cd057287c78452 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Fri, 31 Jul 2026 12:37:55 +0000 Subject: [PATCH 10/11] refactor(tel): rename end to emit --- src/index.ts | 2 +- src/router/router.test.ts | 2 +- src/telemetry/client.test.tsx | 14 +++++++------- src/telemetry/client.tsx | 2 +- src/telemetry/types.tsx | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/index.ts b/src/index.ts index 9ab15d439d..905b76a4ef 100644 --- a/src/index.ts +++ b/src/index.ts @@ -93,7 +93,7 @@ process.exit( throw error; } finally { try { - await commandRunMetricEvent.end(Date.now() - startTime); + await commandRunMetricEvent.emit(Date.now() - startTime); } catch (e) { const error = AgentCoreCLIError.fromError(e); rootLogger.child({ error: error.json() }).warn("failed to emit telemetry"); diff --git a/src/router/router.test.ts b/src/router/router.test.ts index 88ba197762..e62716d575 100644 --- a/src/router/router.test.ts +++ b/src/router/router.test.ts @@ -721,7 +721,7 @@ test.each([ commandRunMetricEvent.setAttributes({ exit_reason: "success" }); } - await commandRunMetricEvent.end(100); + await commandRunMetricEvent.emit(100); expect(recordedMetrics).toHaveLength(1); expect(recordedMetrics[0]!.metricName).toBe("cli.command_run"); diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index 5d644082bb..e3add6637d 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -45,7 +45,7 @@ describe("DefaultTelemetryClient", () => { command_path: "/agentcore", }); - await metricEvent.end(123); + await metricEvent.emit(123); // Start a second event with failure const metricEvent2 = client.startMetricEvent("cli.command_run", { @@ -53,7 +53,7 @@ describe("DefaultTelemetryClient", () => { command_path: "/agentcore", }); - await metricEvent2.end(456); + await metricEvent2.emit(456); await client.shutdown(); expect(fileSystemSink.getName()).toBe("FileSystemSink"); @@ -139,14 +139,14 @@ describe("DefaultTelemetryClient", () => { command_path: "/agentcore", is_tui: true, }); - await enabledEvent.end(123); + await enabledEvent.emit(123); const disabledEvent = disabledClient.startMetricEvent("cli.command_run", { exit_reason: "failure", command_path: "/agentcore", is_tui: false, }); - await disabledEvent.end(123); + await disabledEvent.emit(123); await Promise.all([enabledClient.shutdown(), disabledClient.shutdown()]); @@ -181,7 +181,7 @@ describe("DefaultTelemetryClient", () => { const metricEvent = client.startMetricEvent("cli.command_run"); - await expect(metricEvent.end(100)).rejects.toThrow(); + await expect(metricEvent.emit(100)).rejects.toThrow(); await client.shutdown(); }); @@ -202,7 +202,7 @@ describe("DefaultTelemetryClient", () => { exit_reason: "success", command_path: "/agentcore", }); - await metricEvent.end(1); + await metricEvent.emit(1); await client.shutdown(); await assertLogsMatch(tempDir, [ @@ -248,7 +248,7 @@ describe("DefaultTelemetryClient", () => { }); // end should not throw even though the sink's send() throws - await metricEvent.end(100); + await metricEvent.emit(100); // shutdown should not throw even though the sink's shutdown() rejects await client.shutdown(); diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx index 45c22706fd..85df9589e3 100644 --- a/src/telemetry/client.tsx +++ b/src/telemetry/client.tsx @@ -140,7 +140,7 @@ class InMemoryMetricEvent implements MetricEvent }; } - async end(value: ValueOf): Promise { + async emit(value: ValueOf): Promise { const metricAttributes = METRICS[this.metricName]["attributeSchema"].parse(this.data); const validatedValue = METRICS[this.metricName]["valueSchema"].parse(value); const resourceAttributes = await this.getResourceAttributes(); diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index ed8e127749..4b1d50579e 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -24,8 +24,8 @@ export interface MetricEvent { /** Accumulate attributes incrementally. Later calls overwrite earlier values for the same key. */ setAttributes(data: Partial>): void; - /** Validate attributes, emit value + attributes to all sinks. */ - end(value: ValueOf): Promise; + /** Validate attributes and emit value + attributes to all sinks. */ + emit(value: ValueOf): Promise; } /** From 0139beb01b0f450a1f0d9298fd50c6e5627de168 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Fri, 31 Jul 2026 13:43:08 +0000 Subject: [PATCH 11/11] refactor(tel): rename start --> create --- src/index.ts | 2 +- src/router/router.test.ts | 2 +- src/telemetry/client.test.tsx | 16 ++++++++-------- src/telemetry/client.tsx | 2 +- src/telemetry/types.tsx | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/index.ts b/src/index.ts index 905b76a4ef..20c7aa212d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -50,7 +50,7 @@ process.exit( globalConfigAccessor, }); - const commandRunMetricEvent = telemetryClient.startMetricEvent("cli.command_run", { + const commandRunMetricEvent = telemetryClient.createMetricEvent("cli.command_run", { exit_reason: "success", }); diff --git a/src/router/router.test.ts b/src/router/router.test.ts index e62716d575..bb8db16045 100644 --- a/src/router/router.test.ts +++ b/src/router/router.test.ts @@ -706,7 +706,7 @@ test.each([ const root = new Router("agentcore"); root.handler(get); - const commandRunMetricEvent = telemetryClient.startMetricEvent("cli.command_run"); + const commandRunMetricEvent = telemetryClient.createMetricEvent("cli.command_run"); const ctx = ValueContext.EmptyContext().withValue( CommandRunMetricEventKey, commandRunMetricEvent, diff --git a/src/telemetry/client.test.tsx b/src/telemetry/client.test.tsx index e3add6637d..f76b3882cb 100644 --- a/src/telemetry/client.test.tsx +++ b/src/telemetry/client.test.tsx @@ -40,15 +40,15 @@ describe("DefaultTelemetryClient", () => { metricSinks: [fileSystemSink], }); - const metricEvent = client.startMetricEvent("cli.command_run", { + const metricEvent = client.createMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); await metricEvent.emit(123); - // Start a second event with failure - const metricEvent2 = client.startMetricEvent("cli.command_run", { + // create a second event with failure + const metricEvent2 = client.createMetricEvent("cli.command_run", { exit_reason: "failure", command_path: "/agentcore", }); @@ -134,14 +134,14 @@ describe("DefaultTelemetryClient", () => { auditFilePath, }); - const enabledEvent = enabledClient.startMetricEvent("cli.command_run", { + const enabledEvent = enabledClient.createMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", is_tui: true, }); await enabledEvent.emit(123); - const disabledEvent = disabledClient.startMetricEvent("cli.command_run", { + const disabledEvent = disabledClient.createMetricEvent("cli.command_run", { exit_reason: "failure", command_path: "/agentcore", is_tui: false, @@ -179,7 +179,7 @@ describe("DefaultTelemetryClient", () => { metricSinks: [], }); - const metricEvent = client.startMetricEvent("cli.command_run"); + const metricEvent = client.createMetricEvent("cli.command_run"); await expect(metricEvent.emit(100)).rejects.toThrow(); await client.shutdown(); @@ -198,7 +198,7 @@ describe("DefaultTelemetryClient", () => { globalConfigAccessor: new TestGlobalConfigAccessor(), metricSinks: [sink], }); - const metricEvent = client.startMetricEvent("cli.command_run", { + const metricEvent = client.createMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); @@ -242,7 +242,7 @@ describe("DefaultTelemetryClient", () => { metricSinks: [badSink, goodSink], }); - const metricEvent = client.startMetricEvent("cli.command_run", { + const metricEvent = client.createMetricEvent("cli.command_run", { exit_reason: "success", command_path: "/agentcore", }); diff --git a/src/telemetry/client.tsx b/src/telemetry/client.tsx index 85df9589e3..94538abec9 100644 --- a/src/telemetry/client.tsx +++ b/src/telemetry/client.tsx @@ -42,7 +42,7 @@ export class DefaultTelemetryClient implements TelemetryClient { path.join(os.homedir(), ".agentcore", "telemetry", `${this.sessionId}.jsonl`); } - startMetricEvent( + createMetricEvent( metricName: TMetricName, initialAttributes: Partial> = {}, ): MetricEvent { diff --git a/src/telemetry/types.tsx b/src/telemetry/types.tsx index 4b1d50579e..c1b38d1840 100644 --- a/src/telemetry/types.tsx +++ b/src/telemetry/types.tsx @@ -8,7 +8,7 @@ export interface TelemetryClient { /** * Start a new metric event that accumulates attributes over its lifecycle. */ - startMetricEvent( + createMetricEvent( metricName: TMetricName, initialAttributes?: Partial>, ): MetricEvent;