From 5da7e4bcccc3d3c8c583a498d691efcb3855be7c Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 03:00:00 +0000 Subject: [PATCH 01/15] refactor: decouple TelemetryClient from command_run logic TelemetryClient is now a generic metric emitter (emit, flush, shutdown). withCommandRun + recordCommandRun moved to cli-command-run.ts as standalone functions. All sinks accept metricName as first arg to record(). --- src/cli/commands/dev/command.tsx | 4 +- src/cli/commands/help/command.tsx | 5 +- src/cli/telemetry/__tests__/client.test.ts | 360 +++++++++--------- .../__tests__/composite-sink.test.ts | 4 +- .../__tests__/filesystem-sink.test.ts | 11 +- src/cli/telemetry/cli-command-run.ts | 72 +++- src/cli/telemetry/client.ts | 104 +---- src/cli/telemetry/index.ts | 3 +- src/cli/telemetry/sinks/filesystem-sink.ts | 11 +- src/cli/telemetry/sinks/in-memory-sink.ts | 5 +- src/cli/telemetry/sinks/metric-sink.ts | 6 +- src/cli/telemetry/sinks/otel-metric-sink.ts | 22 +- 12 files changed, 310 insertions(+), 297 deletions(-) diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index b6cb0a7ba..ed348d685 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -25,7 +25,7 @@ import { loadProjectConfig, } from '../../operations/dev'; import { OtelCollector, startOtelCollector } from '../../operations/dev/otel'; -import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; +import { withCommandRun, withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; import { Protocol, standardize } from '../../telemetry/schemas/common-shapes.js'; import { FatalError } from '../../tui/components'; @@ -492,7 +492,7 @@ export const registerDev = (program: Command) => { invoke_count: 0, }; if (client) { - await client.withCommandRun('dev', () => devAttrs); + await withCommandRun(client, 'dev', () => devAttrs); } await runBrowserMode({ workingDir, diff --git a/src/cli/commands/help/command.tsx b/src/cli/commands/help/command.tsx index 338684d7e..d1f17e7b4 100644 --- a/src/cli/commands/help/command.tsx +++ b/src/cli/commands/help/command.tsx @@ -1,3 +1,4 @@ +import { withCommandRun } from '../../telemetry/cli-command-run.js'; import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; import type { Command } from '@commander-js/extra-typings'; @@ -44,7 +45,7 @@ export const registerHelp = (program: Command) => { .description('Display help topics') .action(async () => { const client = await TelemetryClientAccessor.get(); - await client.withCommandRun('help', () => { + await withCommandRun(client, 'help', () => { console.log('Available help topics: modes'); console.log('Run `agentcore help ` for details.'); return {}; @@ -56,7 +57,7 @@ export const registerHelp = (program: Command) => { .description('Explain interactive vs non-interactive modes') .action(async () => { const client = await TelemetryClientAccessor.get(); - await client.withCommandRun('help.modes', () => { + await withCommandRun(client, 'help.modes', () => { console.log(MODES_HELP); return {}; }); diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index cfba2a0c2..d681f78f2 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -1,226 +1,228 @@ /* eslint-disable @typescript-eslint/require-await */ -import { CANCELLED, TelemetryClient } from '../client'; +import { CANCELLED, withCommandRun } from '../cli-command-run'; +import { TelemetryClient } from '../client'; import { InMemorySink } from '../sinks/in-memory-sink'; import { describe, expect, it } from 'vitest'; -describe('TelemetryClient', () => { - describe('withCommandRun', () => { - it('records success with returned attrs', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await client.withCommandRun('update', async () => ({ check_only: true })); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - command_group: 'update', - command: 'update', - exit_reason: 'success', - check_only: 'true', - }); +describe('withCommandRun', () => { + it('records success with returned attrs', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await withCommandRun(client, 'update', async () => ({ check_only: true })); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.metric).toBe('command_run'); + expect(sink.metrics[0]!.attrs).toMatchObject({ + command_group: 'update', + command: 'update', + exit_reason: 'success', + check_only: 'true', }); + }); - it('accepts sync callbacks', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('accepts sync callbacks', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - await client.withCommandRun('telemetry.disable', () => ({})); + await withCommandRun(client, 'telemetry.disable', () => ({})); - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ exit_reason: 'success' }); - }); + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ exit_reason: 'success' }); + }); - it('records failure and re-throws on error', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await expect( - client.withCommandRun('deploy', async () => { - throw new Error('boom'); - }) - ).rejects.toThrow('boom'); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - command_group: 'deploy', - exit_reason: 'failure', - error_name: 'UnknownError', - }); + it('records failure and re-throws on error', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await expect( + withCommandRun(client, 'deploy', async () => { + throw new Error('boom'); + }) + ).rejects.toThrow('boom'); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + command_group: 'deploy', + exit_reason: 'failure', + error_name: 'UnknownError', }); + }); - it('classifies PackagingError subclasses', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('classifies PackagingError subclasses', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - class MissingDependencyError extends Error { - constructor() { - super('missing dep'); - this.name = 'MissingDependencyError'; - } + class MissingDependencyError extends Error { + constructor() { + super('missing dep'); + this.name = 'MissingDependencyError'; } + } - await expect( - client.withCommandRun('deploy', async () => { - throw new MissingDependencyError(); - }) - ).rejects.toThrow(); + await expect( + withCommandRun(client, 'deploy', async () => { + throw new MissingDependencyError(); + }) + ).rejects.toThrow(); - expect(sink.metrics[0]!.attrs).toMatchObject({ - error_name: 'PackagingError', - is_user_error: 'false', - }); + expect(sink.metrics[0]!.attrs).toMatchObject({ + error_name: 'PackagingError', + is_user_error: 'false', }); + }); - it('marks credential errors as user errors', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('marks credential errors as user errors', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - class AwsCredentialsError extends Error { - constructor() { - super('creds expired'); - this.name = 'AwsCredentialsError'; - } + class AwsCredentialsError extends Error { + constructor() { + super('creds expired'); + this.name = 'AwsCredentialsError'; } + } - await expect( - client.withCommandRun('invoke', async () => { - throw new AwsCredentialsError(); - }) - ).rejects.toThrow(); + await expect( + withCommandRun(client, 'invoke', async () => { + throw new AwsCredentialsError(); + }) + ).rejects.toThrow(); - expect(sink.metrics[0]!.attrs).toMatchObject({ - error_name: 'CredentialsError', - is_user_error: 'true', - }); + expect(sink.metrics[0]!.attrs).toMatchObject({ + error_name: 'CredentialsError', + is_user_error: 'true', }); + }); - it('records duration as a non-negative integer', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await client.withCommandRun('telemetry.disable', async () => { - await new Promise(r => globalThis.setTimeout(r, 5)); - return {}; - }); + it('records duration as a non-negative integer', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - expect(sink.metrics[0]!.value).toBeGreaterThanOrEqual(0); - expect(Number.isInteger(sink.metrics[0]!.value)).toBe(true); + await withCommandRun(client, 'telemetry.disable', async () => { + await new Promise(r => globalThis.setTimeout(r, 5)); + return {}; }); - it('converts boolean attrs to strings', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + expect(sink.metrics[0]!.value).toBeGreaterThanOrEqual(0); + expect(Number.isInteger(sink.metrics[0]!.value)).toBe(true); + }); - await client.withCommandRun('update', async () => ({ check_only: true })); + it('converts boolean attrs to strings', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - expect(sink.metrics[0]!.attrs.check_only).toBe('true'); + await withCommandRun(client, 'update', async () => ({ check_only: true })); + + expect(sink.metrics[0]!.attrs.check_only).toBe('true'); + }); + + it('publishes metric with unknown defaults for incomplete success payloads', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await withCommandRun( + client, + 'create', + // @ts-expect-error — intentionally incomplete + async () => ({ language: 'python' }) + ); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + exit_reason: 'success', + language: 'python', + framework: 'unknown', + model_provider: 'unknown', }); + }); - it('publishes metric with unknown defaults for incomplete success payloads', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('defaults invalid attrs to unknown while preserving valid ones', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - // Missing required attrs for 'create' — should still publish with 'unknown' defaults - await client.withCommandRun( - 'create', - // @ts-expect-error — intentionally incomplete - async () => ({ language: 'python' }) - ); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - exit_reason: 'success', - language: 'python', - framework: 'unknown', - model_provider: 'unknown', - }); + await withCommandRun( + client, + 'create', + // @ts-expect-error — intentionally invalid enum value + async () => ({ + language: 'rust', // invalid enum + framework: 'strands', + model_provider: 'bedrock', + memory: 'shortterm', + protocol: 'mcp', + build: 'codezip', + agent_type: 'create', + network_mode: 'public', + has_agent: true, + }) + ); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs.language).toBe('unknown'); + expect(sink.metrics[0]!.attrs.framework).toBe('strands'); + }); + + it('records cancel when callback returns CANCELLED', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await withCommandRun(client, 'deploy', () => CANCELLED); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + command_group: 'deploy', + exit_reason: 'cancel', }); + }); - it('defaults invalid attrs to unknown while preserving valid ones', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('records fallbackAttrs on failure when provided', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - await client.withCommandRun( + await expect( + withCommandRun( + client, 'create', - // @ts-expect-error — intentionally invalid enum value - async () => ({ - language: 'rust', // invalid enum + async () => { + throw new Error('validation failed'); + }, + { + language: 'python', framework: 'strands', model_provider: 'bedrock', - memory: 'shortterm', - protocol: 'mcp', + memory: 'none', + protocol: 'http', build: 'codezip', agent_type: 'create', network_mode: 'public', has_agent: true, - }) - ); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs.language).toBe('unknown'); - expect(sink.metrics[0]!.attrs.framework).toBe('strands'); - }); - - it('records cancel when callback returns CANCELLED', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await client.withCommandRun('deploy', () => CANCELLED); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - command_group: 'deploy', - exit_reason: 'cancel', - }); - }); - - it('records fallbackAttrs on failure when provided', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await expect( - client.withCommandRun( - 'create', - async () => { - throw new Error('validation failed'); - }, - { - language: 'python', - framework: 'strands', - model_provider: 'bedrock', - memory: 'none', - protocol: 'http', - build: 'codezip', - agent_type: 'create', - network_mode: 'public', - has_agent: true, - } - ) - ).rejects.toThrow('validation failed'); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - exit_reason: 'failure', - error_name: 'UnknownError', - language: 'python', - framework: 'strands', - model_provider: 'bedrock', - has_agent: 'true', - }); + } + ) + ).rejects.toThrow('validation failed'); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + exit_reason: 'failure', + error_name: 'UnknownError', + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + has_agent: 'true', }); + }); - it('records empty attrs on failure when fallbackAttrs not provided', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('records empty attrs on failure when fallbackAttrs not provided', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); - await expect( - client.withCommandRun('deploy', async () => { - throw new Error('boom'); - }) - ).rejects.toThrow('boom'); + await expect( + withCommandRun(client, 'deploy', async () => { + throw new Error('boom'); + }) + ).rejects.toThrow('boom'); - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs.language).toBeUndefined(); - }); + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs.language).toBeUndefined(); }); }); diff --git a/src/cli/telemetry/__tests__/composite-sink.test.ts b/src/cli/telemetry/__tests__/composite-sink.test.ts index e07b2d65f..ed7194e1e 100644 --- a/src/cli/telemetry/__tests__/composite-sink.test.ts +++ b/src/cli/telemetry/__tests__/composite-sink.test.ts @@ -8,7 +8,7 @@ describe('CompositeSink', () => { const b = new InMemorySink(); const composite = new CompositeSink([a, b]); - composite.record(100, { command: 'deploy' }); + composite.record('command_run', 100, { command: 'deploy' }); expect(a.metrics).toHaveLength(1); expect(b.metrics).toHaveLength(1); @@ -26,7 +26,7 @@ describe('CompositeSink', () => { const good = new InMemorySink(); const composite = new CompositeSink([bad, good]); - composite.record(100, { command: 'deploy' }); + composite.record('command_run', 100, { command: 'deploy' }); expect(good.metrics).toHaveLength(1); }); diff --git a/src/cli/telemetry/__tests__/filesystem-sink.test.ts b/src/cli/telemetry/__tests__/filesystem-sink.test.ts index 50d8d4620..96a894acc 100644 --- a/src/cli/telemetry/__tests__/filesystem-sink.test.ts +++ b/src/cli/telemetry/__tests__/filesystem-sink.test.ts @@ -28,12 +28,13 @@ describe('FileSystemSink', () => { it('writes each record as a JSONL line on disk', async () => { const sink = createSink(); - sink.record(42, { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }); + sink.record('command_run', 42, { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }); await sink.flush(); const entries = await readJsonl(join(outputDir, 'test-session.json')); expect(entries).toHaveLength(1); expect(entries[0]).toMatchObject({ + metric: 'command_run', value: 42, attrs: { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }, }); @@ -41,8 +42,8 @@ describe('FileSystemSink', () => { it('appends multiple records as separate lines', async () => { const sink = createSink(); - sink.record(10, { command_group: 'add', command: 'add.agent' }); - sink.record(20, { command_group: 'add', command: 'add.memory' }); + sink.record('command_run', 10, { command_group: 'add', command: 'add.agent' }); + sink.record('command_run', 20, { command_group: 'add', command: 'add.memory' }); await sink.flush(); const entries = await readJsonl(join(outputDir, 'test-session.json')); @@ -55,7 +56,7 @@ describe('FileSystemSink', () => { const nested = join(tmp.testDir, 'deep', 'nested', 'telemetry'); const filePath = join(nested, 'test.json'); const sink = new FileSystemSink({ filePath }); - sink.record(1, { command_group: 'status', command: 'status' }); + sink.record('command_run', 1, { command_group: 'status', command: 'status' }); await sink.flush(); const entries = await readJsonl(filePath); @@ -70,7 +71,7 @@ describe('FileSystemSink', () => { it('shutdown logs audit message when records were written', async () => { const logged: string[] = []; const sink = createSink({ log: msg => logged.push(msg) }); - sink.record(99, { command_group: 'invoke', command: 'invoke' }); + sink.record('command_run', 99, { command_group: 'invoke', command: 'invoke' }); await sink.shutdown(); expect(logged).toHaveLength(1); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 2fabd39fe..e1d37c50a 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -1,9 +1,14 @@ import type { Result } from '../../lib/result'; import { getErrorMessage } from '../errors'; import { TelemetryClientAccessor } from './client-accessor.js'; -import type { Command, CommandAttrs } from './schemas/command-run.js'; +import { TelemetryClient } from './client.js'; +import { classifyError, isUserError } from './error-classification.js'; +import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from './schemas/command-run.js'; +import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; +import { performance } from 'perf_hooks'; -export type OperationResult = Result; +/** Return this from the withCommandRun callback to record a cancellation. */ +export const CANCELLED = Symbol('cancelled'); async function getTelemetryClient() { try { @@ -13,6 +18,62 @@ async function getTelemetryClient() { } } +function recordCommandRun( + client: TelemetryClient, + command: C, + result: CommandResult, + attrs: CommandAttrs | Partial>, + durationMs: number +): void { + CommandResultSchema.parse(result); + + const validatedAttrs = + Object.keys(attrs as Record).length > 0 + ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) + : attrs; + + client.emit('command_run', durationMs, { + command_group: deriveCommandGroup(command), + command, + ...result, + ...validatedAttrs, + }); +} + +/** + * Wrap a command action with telemetry recording. + * + * Return attrs on success, or CANCELLED on user cancellation. + * Unhandled throws are classified as failures and re-thrown. + */ +export async function withCommandRun( + client: TelemetryClient, + command: C, + fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED>, + fallbackAttrs?: Partial> +): Promise { + const start = performance.now(); + try { + const result = await fn(); + const durationMs = Math.round(performance.now() - start); + if (result === CANCELLED) { + recordCommandRun(client, command, { exit_reason: 'cancel' }, {}, durationMs); + } else { + recordCommandRun(client, command, { exit_reason: 'success' }, result, durationMs); + } + } catch (err) { + const failureResult: CommandResult & { exit_reason: 'failure' } = { + exit_reason: 'failure', + error_name: classifyError(err), + is_user_error: isUserError(err), + }; + recordCommandRun(client, command, failureResult, fallbackAttrs ?? {}, Math.round(performance.now() - start)); + throw err; + } finally { + await client.flush(); + } +} + /** * Record telemetry for an operation and return its result. * Use in TUI hooks and CLI paths where the caller handles output and control flow. @@ -21,7 +82,7 @@ async function getTelemetryClient() { * is returned to the caller. If the callback throws, telemetry is recorded and * the exception propagates. If telemetry is unavailable, the callback runs untracked. */ -export async function withCommandRunTelemetry( +export async function withCommandRunTelemetry( command: C, attrs: CommandAttrs, fn: () => Promise @@ -31,7 +92,8 @@ export async function withCommandRunTelemetry { result = await fn(); @@ -70,7 +132,7 @@ export async function runCliCommand( await fn(); process.exit(0); } - await client.withCommandRun(command, fn, knownAttrs); + await withCommandRun(client, command, fn, knownAttrs); process.exit(0); } catch (error) { if (json) { diff --git a/src/cli/telemetry/client.ts b/src/cli/telemetry/client.ts index 2341231ea..54164ac16 100644 --- a/src/cli/telemetry/client.ts +++ b/src/cli/telemetry/client.ts @@ -1,104 +1,40 @@ -import { classifyError, isUserError } from './error-classification.js'; -import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from './schemas/command-run.js'; -import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; import type { MetricSink } from './sinks/metric-sink.js'; -import { performance } from 'perf_hooks'; - -/** Return this from the withCommandRun callback to record a cancellation. */ -export const CANCELLED = Symbol('cancelled'); +/** + * Generic metric emitter. + */ export class TelemetryClient { constructor(private readonly sink: MetricSink) {} - /** - * Wrap a command action with telemetry recording. - * - * Return attrs on success, or CANCELLED on user cancellation. - * Unhandled throws are classified as failures and re-thrown. - * - * ```ts - * await client.withCommandRun('deploy', async () => { - * if (userCancelled) return CANCELLED; - * const result = await runDeploy(options); - * return { runtime_count: result.runtimes.length, ... }; - * }); - * ``` - */ - async withCommandRun( - command: C, - fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED>, - fallbackAttrs?: Partial> - ): Promise { - const start = performance.now(); + emit(metricName: string, value: number, attrs: Record): void { try { - const result = await fn(); - const durationMs = Math.round(performance.now() - start); - if (result === CANCELLED) { - this.recordCommandRun(command, { exit_reason: 'cancel' }, {}, durationMs); - } else { - this.recordCommandRun(command, { exit_reason: 'success' }, result, durationMs); - } - } catch (err) { - const failureResult: CommandResult & { exit_reason: 'failure' } = { - exit_reason: 'failure', - error_name: classifyError(err), - is_user_error: isUserError(err), - }; - this.recordCommandRun(command, failureResult, fallbackAttrs ?? {}, Math.round(performance.now() - start)); - throw err; - } finally { - try { - await this.sink.flush(); - } catch { - /* telemetry must not mask command errors */ + const otelAttrs: Record = {}; + for (const [k, v] of Object.entries(attrs)) { + if (typeof v === 'boolean') { + otelAttrs[k] = String(v); + } else if (typeof v === 'string' || typeof v === 'number') { + otelAttrs[k] = v; + } } + this.sink.record(metricName, value, otelAttrs); + } catch { + // Telemetry must never affect CLI behavior } } - async shutdown(): Promise { + async flush(): Promise { try { - await this.sink.shutdown(); + await this.sink.flush(); } catch { - /* telemetry must not affect CLI behavior */ + /* telemetry must not mask command errors */ } } - private recordCommandRun( - command: C, - result: CommandResult, - attrs: CommandAttrs | Partial>, - durationMs: number - ): void { + async shutdown(): Promise { try { - // CommandResult is built internally — hard parse is intentional since - // a metric without a valid exit_reason is meaningless. - CommandResultSchema.parse(result); - - // Validate command attrs resiliently: invalid fields default to 'unknown' - // instead of dropping the entire metric. - const validatedAttrs = - Object.keys(attrs as Record).length > 0 - ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) - : attrs; - - const otelAttrs: Record = { - command_group: deriveCommandGroup(command), - command, - }; - - for (const obj of [result, validatedAttrs]) { - for (const [k, v] of Object.entries(obj)) { - if (typeof v === 'boolean') { - otelAttrs[k] = String(v); - } else if (typeof v === 'string' || typeof v === 'number') { - otelAttrs[k] = v; - } - } - } - - this.sink.record(durationMs, otelAttrs); + await this.sink.shutdown(); } catch { - // Telemetry must never affect CLI behavior + /* telemetry must not affect CLI behavior */ } } } diff --git a/src/cli/telemetry/index.ts b/src/cli/telemetry/index.ts index 778376713..80f92f3db 100644 --- a/src/cli/telemetry/index.ts +++ b/src/cli/telemetry/index.ts @@ -1,7 +1,8 @@ export { resolveTelemetryPreference, resolveResourceAttributes, resolveAuditFilePath } from './config.js'; export type { TelemetryPreference } from './config.js'; export { TelemetryClientAccessor } from './client-accessor.js'; -export { TelemetryClient, CANCELLED } from './client.js'; +export { TelemetryClient } from './client.js'; +export { CANCELLED, withCommandRun } from './cli-command-run.js'; export { type MetricSink, CompositeSink } from './sinks/metric-sink.js'; export { OtelMetricSink, type OtelMetricSinkConfig } from './sinks/otel-metric-sink.js'; export { FileSystemSink, type FileSystemSinkConfig } from './sinks/filesystem-sink.js'; diff --git a/src/cli/telemetry/sinks/filesystem-sink.ts b/src/cli/telemetry/sinks/filesystem-sink.ts index a9868f2b9..6e0492af7 100644 --- a/src/cli/telemetry/sinks/filesystem-sink.ts +++ b/src/cli/telemetry/sinks/filesystem-sink.ts @@ -20,10 +20,10 @@ export class FileSystemSink implements MetricSink { this.log = config.log ?? (msg => console.log(msg)); } - record(value: number, attrs: Record): void { + record(metricName: string, value: number, attrs: Record): void { this.hasRecords = true; this.pendingWrite = this.pendingWrite.then(() => - this.appendEntry({ value, attrs: { ...this.resource, ...attrs } }) + this.appendEntry({ metric: metricName, value, attrs: { ...this.resource, ...attrs } }) ); } @@ -38,10 +38,13 @@ export class FileSystemSink implements MetricSink { } } - // Promise chain that serializes async writes so record() can stay synchronous. private pendingWrite: Promise = Promise.resolve(); - private async appendEntry(entry: { value: number; attrs: Record }): Promise { + private async appendEntry(entry: { + metric: string; + value: number; + attrs: Record; + }): Promise { await mkdir(dirname(this.filePath), { recursive: true }); await appendFile(this.filePath, JSON.stringify(entry) + '\n'); } diff --git a/src/cli/telemetry/sinks/in-memory-sink.ts b/src/cli/telemetry/sinks/in-memory-sink.ts index aab23680c..5511c5c4b 100644 --- a/src/cli/telemetry/sinks/in-memory-sink.ts +++ b/src/cli/telemetry/sinks/in-memory-sink.ts @@ -1,6 +1,7 @@ import type { MetricSink } from './metric-sink.js'; export interface RecordedMetric { + metric: string; value: number; attrs: Record; } @@ -8,8 +9,8 @@ export interface RecordedMetric { export class InMemorySink implements MetricSink { readonly metrics: RecordedMetric[] = []; - record(value: number, attrs: Record): void { - this.metrics.push({ value, attrs }); + record(metricName: string, value: number, attrs: Record): void { + this.metrics.push({ metric: metricName, value, attrs }); } // eslint-disable-next-line @typescript-eslint/no-empty-function diff --git a/src/cli/telemetry/sinks/metric-sink.ts b/src/cli/telemetry/sinks/metric-sink.ts index 6622a34e7..9074039a6 100644 --- a/src/cli/telemetry/sinks/metric-sink.ts +++ b/src/cli/telemetry/sinks/metric-sink.ts @@ -2,7 +2,7 @@ * A destination for metric data. Implementations handle transport (OTel, file, etc.). */ export interface MetricSink { - record(value: number, attrs: Record): void; + record(metricName: string, value: number, attrs: Record): void; flush(timeoutMs?: number): Promise; shutdown(): Promise; } @@ -14,10 +14,10 @@ export interface MetricSink { export class CompositeSink implements MetricSink { constructor(private readonly sinks: MetricSink[]) {} - record(value: number, attrs: Record): void { + record(metricName: string, value: number, attrs: Record): void { for (const sink of this.sinks) { try { - sink.record(value, attrs); + sink.record(metricName, value, attrs); } catch { // Individual sink failure must not affect others } diff --git a/src/cli/telemetry/sinks/otel-metric-sink.ts b/src/cli/telemetry/sinks/otel-metric-sink.ts index 0bc6721f5..dd8cb3860 100644 --- a/src/cli/telemetry/sinks/otel-metric-sink.ts +++ b/src/cli/telemetry/sinks/otel-metric-sink.ts @@ -1,6 +1,6 @@ import type { ResourceAttributes } from '../schemas/common-attributes.js'; import type { MetricSink } from './metric-sink.js'; -import type { Histogram } from '@opentelemetry/api'; +import type { Histogram, Meter } from '@opentelemetry/api'; import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-http'; import { resourceFromAttributes } from '@opentelemetry/resources'; import { AggregationTemporality, MeterProvider, PeriodicExportingMetricReader } from '@opentelemetry/sdk-metrics'; @@ -13,15 +13,18 @@ export interface OtelMetricSinkConfig { export class OtelMetricSink implements MetricSink { private readonly meterProvider: MeterProvider; - private readonly histogram: Histogram; + private readonly meter: Meter; + private readonly histograms = new Map(); constructor(config: OtelMetricSinkConfig) { const resource = resourceFromAttributes(config.resource); + const url = config.endpoint.endsWith('/v1/metrics') ? config.endpoint : `${config.endpoint}/v1/metrics`; const exporter = new OTLPMetricExporter({ - url: `${config.endpoint}/v1/metrics`, + url, headers: { 'X-Installation-Id': config.resource['agentcore-cli.installation_id'] }, temporalityPreference: AggregationTemporality.DELTA, }); + this.meterProvider = new MeterProvider({ resource, readers: [ @@ -32,13 +35,16 @@ export class OtelMetricSink implements MetricSink { }), ], }); - this.histogram = this.meterProvider - .getMeter('agentcore-cli') - .createHistogram('cli.command_run', { description: 'CLI command execution' }); + this.meter = this.meterProvider.getMeter('agentcore-cli'); } - record(value: number, attrs: Record): void { - this.histogram.record(value, attrs); + record(metricName: string, value: number, attrs: Record): void { + let histogram = this.histograms.get(metricName); + if (!histogram) { + histogram = this.meter.createHistogram(metricName, { description: metricName }); + this.histograms.set(metricName, histogram); + } + histogram.record(value, attrs); } async flush(timeoutMs = 5_000): Promise { From ffbe31f690d3c2ce5db7ceded69fa0845b5719e5 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 03:04:40 +0000 Subject: [PATCH 02/15] refactor: make withCommandRun internal, migrate consumers - help: migrated to withCommandRunTelemetry - dev browser mode: migrated to direct client.emit() - withCommandRun and CANCELLED are now module-private - Tests rewritten to go through withCommandRunTelemetry --- src/cli/commands/dev/command.tsx | 25 +-- src/cli/commands/help/command.tsx | 13 +- src/cli/telemetry/__tests__/client.test.ts | 188 +++++++-------------- src/cli/telemetry/cli-command-run.ts | 18 +- src/cli/telemetry/index.ts | 1 - 5 files changed, 86 insertions(+), 159 deletions(-) diff --git a/src/cli/commands/dev/command.tsx b/src/cli/commands/dev/command.tsx index ed348d685..926cf3045 100644 --- a/src/cli/commands/dev/command.tsx +++ b/src/cli/commands/dev/command.tsx @@ -25,7 +25,7 @@ import { loadProjectConfig, } from '../../operations/dev'; import { OtelCollector, startOtelCollector } from '../../operations/dev/otel'; -import { withCommandRun, withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; +import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; import { Protocol, standardize } from '../../telemetry/schemas/common-shapes.js'; import { FatalError } from '../../tui/components'; @@ -479,20 +479,21 @@ export const registerDev = (program: Command) => { // Default: launch web UI in browser // NOTE: Do not copy this pattern. runBrowserMode blocks forever (internal // await new Promise(() => {})) so we cannot use withCommandRunTelemetry here. - // We emit telemetry eagerly before the blocking call. If startup fails, the - // error propagates to the outer catch. Prefer withCommandRunTelemetry for - // commands that return. + // We emit telemetry eagerly before the blocking call. { const client = await TelemetryClientAccessor.get().catch(() => undefined); - const devAttrs = { - action: 'server' as const, - ui_mode: 'browser' as const, - has_stream: false, - protocol: standardize(Protocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), - invoke_count: 0, - }; if (client) { - await withCommandRun(client, 'dev', () => devAttrs); + client.emit('cli.command_run', 0, { + command_group: 'dev', + command: 'dev', + exit_reason: 'success', + action: 'server', + ui_mode: 'browser', + has_stream: false, + protocol: standardize(Protocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()), + invoke_count: 0, + }); + await client.flush(); } await runBrowserMode({ workingDir, diff --git a/src/cli/commands/help/command.tsx b/src/cli/commands/help/command.tsx index d1f17e7b4..b4ab65a70 100644 --- a/src/cli/commands/help/command.tsx +++ b/src/cli/commands/help/command.tsx @@ -1,5 +1,4 @@ -import { withCommandRun } from '../../telemetry/cli-command-run.js'; -import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; +import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; import type { Command } from '@commander-js/extra-typings'; const MODES_HELP = ` @@ -44,11 +43,10 @@ export const registerHelp = (program: Command) => { .command('help') .description('Display help topics') .action(async () => { - const client = await TelemetryClientAccessor.get(); - await withCommandRun(client, 'help', () => { + await withCommandRunTelemetry('help', {}, () => { console.log('Available help topics: modes'); console.log('Run `agentcore help ` for details.'); - return {}; + return { success: true as const }; }); }); @@ -56,10 +54,9 @@ export const registerHelp = (program: Command) => { .command('modes') .description('Explain interactive vs non-interactive modes') .action(async () => { - const client = await TelemetryClientAccessor.get(); - await withCommandRun(client, 'help.modes', () => { + await withCommandRunTelemetry('help.modes', {}, () => { console.log(MODES_HELP); - return {}; + return { success: true as const }; }); }); }; diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index d681f78f2..4722878a6 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -1,18 +1,27 @@ /* eslint-disable @typescript-eslint/require-await */ -import { CANCELLED, withCommandRun } from '../cli-command-run'; +import { withCommandRunTelemetry } from '../cli-command-run'; import { TelemetryClient } from '../client'; +import { TelemetryClientAccessor } from '../client-accessor'; import { InMemorySink } from '../sinks/in-memory-sink'; -import { describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -describe('withCommandRun', () => { - it('records success with returned attrs', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); +let sink: InMemorySink; + +beforeEach(() => { + sink = new InMemorySink(); + vi.spyOn(TelemetryClientAccessor, 'get').mockResolvedValue(new TelemetryClient(sink)); +}); - await withCommandRun(client, 'update', async () => ({ check_only: true })); +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('withCommandRunTelemetry', () => { + it('records success with returned attrs', async () => { + await withCommandRunTelemetry('update', { check_only: true }, async () => ({ success: true })); expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.metric).toBe('command_run'); + expect(sink.metrics[0]!.metric).toBe('cli.command_run'); expect(sink.metrics[0]!.attrs).toMatchObject({ command_group: 'update', command: 'update', @@ -21,26 +30,13 @@ describe('withCommandRun', () => { }); }); - it('accepts sync callbacks', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await withCommandRun(client, 'telemetry.disable', () => ({})); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ exit_reason: 'success' }); - }); - - it('records failure and re-throws on error', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await expect( - withCommandRun(client, 'deploy', async () => { - throw new Error('boom'); - }) - ).rejects.toThrow('boom'); + it('records failure when callback returns failure result', async () => { + const result = await withCommandRunTelemetry('deploy', {} as never, async () => ({ + success: false as const, + error: new Error('boom'), + })); + expect(result.success).toBe(false); expect(sink.metrics).toHaveLength(1); expect(sink.metrics[0]!.attrs).toMatchObject({ command_group: 'deploy', @@ -50,9 +46,6 @@ describe('withCommandRun', () => { }); it('classifies PackagingError subclasses', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - class MissingDependencyError extends Error { constructor() { super('missing dep'); @@ -60,11 +53,10 @@ describe('withCommandRun', () => { } } - await expect( - withCommandRun(client, 'deploy', async () => { - throw new MissingDependencyError(); - }) - ).rejects.toThrow(); + await withCommandRunTelemetry('deploy', {} as never, async () => ({ + success: false as const, + error: new MissingDependencyError(), + })); expect(sink.metrics[0]!.attrs).toMatchObject({ error_name: 'PackagingError', @@ -73,9 +65,6 @@ describe('withCommandRun', () => { }); it('marks credential errors as user errors', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - class AwsCredentialsError extends Error { constructor() { super('creds expired'); @@ -83,11 +72,10 @@ describe('withCommandRun', () => { } } - await expect( - withCommandRun(client, 'invoke', async () => { - throw new AwsCredentialsError(); - }) - ).rejects.toThrow(); + await withCommandRunTelemetry('invoke', {} as never, async () => ({ + success: false as const, + error: new AwsCredentialsError(), + })); expect(sink.metrics[0]!.attrs).toMatchObject({ error_name: 'CredentialsError', @@ -96,12 +84,9 @@ describe('withCommandRun', () => { }); it('records duration as a non-negative integer', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await withCommandRun(client, 'telemetry.disable', async () => { + await withCommandRunTelemetry('telemetry.disable', {}, async () => { await new Promise(r => globalThis.setTimeout(r, 5)); - return {}; + return { success: true as const }; }); expect(sink.metrics[0]!.value).toBeGreaterThanOrEqual(0); @@ -109,44 +94,16 @@ describe('withCommandRun', () => { }); it('converts boolean attrs to strings', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await withCommandRun(client, 'update', async () => ({ check_only: true })); + await withCommandRunTelemetry('update', { check_only: true }, async () => ({ success: true })); expect(sink.metrics[0]!.attrs.check_only).toBe('true'); }); - it('publishes metric with unknown defaults for incomplete success payloads', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await withCommandRun( - client, - 'create', - // @ts-expect-error — intentionally incomplete - async () => ({ language: 'python' }) - ); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - exit_reason: 'success', - language: 'python', - framework: 'unknown', - model_provider: 'unknown', - }); - }); - it('defaults invalid attrs to unknown while preserving valid ones', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await withCommandRun( - client, + await withCommandRunTelemetry( 'create', - // @ts-expect-error — intentionally invalid enum value - async () => ({ - language: 'rust', // invalid enum + { + language: 'rust' as never, framework: 'strands', model_provider: 'bedrock', memory: 'shortterm', @@ -155,7 +112,8 @@ describe('withCommandRun', () => { agent_type: 'create', network_mode: 'public', has_agent: true, - }) + }, + async () => ({ success: true }) ); expect(sink.metrics).toHaveLength(1); @@ -163,43 +121,22 @@ describe('withCommandRun', () => { expect(sink.metrics[0]!.attrs.framework).toBe('strands'); }); - it('records cancel when callback returns CANCELLED', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await withCommandRun(client, 'deploy', () => CANCELLED); - - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs).toMatchObject({ - command_group: 'deploy', - exit_reason: 'cancel', - }); - }); - - it('records fallbackAttrs on failure when provided', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); - - await expect( - withCommandRun( - client, - 'create', - async () => { - throw new Error('validation failed'); - }, - { - language: 'python', - framework: 'strands', - model_provider: 'bedrock', - memory: 'none', - protocol: 'http', - build: 'codezip', - agent_type: 'create', - network_mode: 'public', - has_agent: true, - } - ) - ).rejects.toThrow('validation failed'); + it('records fallbackAttrs on failure', async () => { + await withCommandRunTelemetry( + 'create', + { + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + memory: 'none', + protocol: 'http', + build: 'codezip', + agent_type: 'create', + network_mode: 'public', + has_agent: true, + }, + async () => ({ success: false as const, error: new Error('validation failed') }) + ); expect(sink.metrics).toHaveLength(1); expect(sink.metrics[0]!.attrs).toMatchObject({ @@ -212,17 +149,12 @@ describe('withCommandRun', () => { }); }); - it('records empty attrs on failure when fallbackAttrs not provided', async () => { - const sink = new InMemorySink(); - const client = new TelemetryClient(sink); + it('runs untracked when telemetry client is unavailable', async () => { + vi.spyOn(TelemetryClientAccessor, 'get').mockRejectedValue(new Error('no client')); - await expect( - withCommandRun(client, 'deploy', async () => { - throw new Error('boom'); - }) - ).rejects.toThrow('boom'); + const result = await withCommandRunTelemetry('deploy', {} as never, async () => ({ success: true })); - expect(sink.metrics).toHaveLength(1); - expect(sink.metrics[0]!.attrs.language).toBeUndefined(); + expect(result).toEqual({ success: true }); + expect(sink.metrics).toHaveLength(0); }); }); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index e1d37c50a..4f47e66f0 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -7,8 +7,8 @@ import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; import { performance } from 'perf_hooks'; -/** Return this from the withCommandRun callback to record a cancellation. */ -export const CANCELLED = Symbol('cancelled'); +/** Return this from the trackCommandRun callback to record a cancellation. */ +const CANCELLED = Symbol('cancelled'); async function getTelemetryClient() { try { @@ -32,7 +32,7 @@ function recordCommandRun( ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) : attrs; - client.emit('command_run', durationMs, { + client.emit('cli.command_run', durationMs, { command_group: deriveCommandGroup(command), command, ...result, @@ -41,12 +41,10 @@ function recordCommandRun( } /** - * Wrap a command action with telemetry recording. - * * Return attrs on success, or CANCELLED on user cancellation. * Unhandled throws are classified as failures and re-thrown. */ -export async function withCommandRun( +async function trackCommandRun( client: TelemetryClient, command: C, fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED>, @@ -85,14 +83,14 @@ export async function withCommandRun( export async function withCommandRunTelemetry( command: C, attrs: CommandAttrs, - fn: () => Promise + fn: () => R | Promise ): Promise { const client = await getTelemetryClient(); if (!client) return fn(); let result: R | undefined; try { - await withCommandRun( + await trackCommandRun( client, command, async () => { @@ -103,7 +101,7 @@ export async function withCommandRunTelemetry( await fn(); process.exit(0); } - await withCommandRun(client, command, fn, knownAttrs); + await trackCommandRun(client, command, fn, knownAttrs); process.exit(0); } catch (error) { if (json) { diff --git a/src/cli/telemetry/index.ts b/src/cli/telemetry/index.ts index 80f92f3db..96c059198 100644 --- a/src/cli/telemetry/index.ts +++ b/src/cli/telemetry/index.ts @@ -2,7 +2,6 @@ export { resolveTelemetryPreference, resolveResourceAttributes, resolveAuditFile export type { TelemetryPreference } from './config.js'; export { TelemetryClientAccessor } from './client-accessor.js'; export { TelemetryClient } from './client.js'; -export { CANCELLED, withCommandRun } from './cli-command-run.js'; export { type MetricSink, CompositeSink } from './sinks/metric-sink.js'; export { OtelMetricSink, type OtelMetricSinkConfig } from './sinks/otel-metric-sink.js'; export { FileSystemSink, type FileSystemSinkConfig } from './sinks/filesystem-sink.js'; From 992ad6d60b2798d9890acd716990cf6bf1c028c9 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 03:12:32 +0000 Subject: [PATCH 03/15] feat: add typed metric registry, MetricName enforced at compile time --- src/cli/telemetry/client.ts | 3 +- src/cli/telemetry/schemas/command-run.ts | 26 ++--- src/cli/telemetry/schemas/common-shapes.ts | 117 +++++++++++++++++++++ src/cli/telemetry/schemas/index.ts | 10 +- src/cli/telemetry/schemas/registry.ts | 24 +++++ 5 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 src/cli/telemetry/schemas/registry.ts diff --git a/src/cli/telemetry/client.ts b/src/cli/telemetry/client.ts index 54164ac16..8be2da8e9 100644 --- a/src/cli/telemetry/client.ts +++ b/src/cli/telemetry/client.ts @@ -1,3 +1,4 @@ +import type { MetricAttrs, MetricName } from './schemas/registry.js'; import type { MetricSink } from './sinks/metric-sink.js'; /** @@ -6,7 +7,7 @@ import type { MetricSink } from './sinks/metric-sink.js'; export class TelemetryClient { constructor(private readonly sink: MetricSink) {} - emit(metricName: string, value: number, attrs: Record): void { + emit(metricName: M, value: number, attrs: MetricAttrs & Record): void { try { const otelAttrs: Record = {}; for (const [k, v] of Object.entries(attrs)) { diff --git a/src/cli/telemetry/schemas/command-run.ts b/src/cli/telemetry/schemas/command-run.ts index fae8f8394..55639e397 100644 --- a/src/cli/telemetry/schemas/command-run.ts +++ b/src/cli/telemetry/schemas/command-run.ts @@ -5,6 +5,8 @@ import { AuthType, AuthorizerType, Build, + CommandGroup as CommandGroupSchema, + Command as CommandSchema, Count, CredentialType, EvaluatorType, @@ -30,6 +32,9 @@ import { } from './common-shapes.js'; import { z } from 'zod'; +export type Command = z.infer; +export type CommandGroup = z.infer; + // --------------------------------------------------------------------------- // Per-command attribute schemas // All schemas use safeSchema() which rejects z.string() at compile time. @@ -213,33 +218,18 @@ export const COMMAND_SCHEMAS = { 'telemetry.disable': NoAttrs, 'telemetry.enable': NoAttrs, 'telemetry.status': NoAttrs, -} as const satisfies Record>; +} as const satisfies Record>; // --------------------------------------------------------------------------- // Derived types // --------------------------------------------------------------------------- -export type Command = keyof typeof COMMAND_SCHEMAS; export type CommandAttrs = z.infer<(typeof COMMAND_SCHEMAS)[C]>; -/** Extract the command group prefix from a dotted command key (e.g. 'add' from 'add.agent'). */ -type CommandGroup = { - [C in Command]: C extends `${infer G}.${string}` ? G : C; -}[Command]; - -/** - * Type-safe lookup of a subcommand under a command group. - * Produces a compile-time error if `${G}.${S}` is not a registered command. - * - * @example - * SubCommand<'remove', 'agent'> // → 'remove.agent' - * SubCommand<'add', 'memory'> // → 'add.memory' - * SubCommand<'remove', 'bogus'> // → never (compile error at call site) - */ export type SubCommand = Extract; /** Derive command_group from command key (e.g. 'add.agent' → 'add') */ -export function deriveCommandGroup(command: Command): string { +export function deriveCommandGroup(command: Command): CommandGroup { const dot = command.indexOf('.'); - return dot === -1 ? command : command.slice(0, dot); + return (dot === -1 ? command : command.slice(0, dot)) as CommandGroup; } diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 429716096..a4413537f 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -121,3 +121,120 @@ export const FailureResult = z.object({ }); export const CommandResultSchema = z.discriminatedUnion('exit_reason', [SuccessResult, CancelResult, FailureResult]); export type CommandResult = z.infer; + +// --------------------------------------------------------------------------- +// ATTRIBUTES namespace — single source of truth for all attribute definitions. +// New code should use ATTRIBUTES.X; existing named exports remain for compat. +// --------------------------------------------------------------------------- + +export const Command = z.enum([ + 'create', + 'add.agent', + 'add.memory', + 'add.credential', + 'add.evaluator', + 'add.online-eval', + 'add.gateway', + 'add.gateway-target', + 'add.policy-engine', + 'add.policy', + 'add.runtime-endpoint', + 'deploy', + 'dev', + 'invoke', + 'status', + 'logs', + 'logs.evals', + 'run.eval', + 'fetch.access', + 'update', + 'pause.online-eval', + 'resume.online-eval', + 'traces.list', + 'traces.get', + 'evals.history', + 'import', + 'import.runtime', + 'import.memory', + 'package', + 'validate', + 'help.modes', + 'help', + 'remove.all', + 'remove.agent', + 'remove.memory', + 'remove.credential', + 'remove.evaluator', + 'remove.online-eval', + 'remove.gateway', + 'remove.gateway-target', + 'remove.policy-engine', + 'remove.policy', + 'remove.runtime-endpoint', + 'remove.config-bundle', + 'remove.ab-test', + 'telemetry.disable', + 'telemetry.enable', + 'telemetry.status', +]); + +export const CommandGroup = z.enum([ + 'create', + 'add', + 'deploy', + 'dev', + 'invoke', + 'status', + 'logs', + 'run', + 'fetch', + 'update', + 'pause', + 'resume', + 'traces', + 'evals', + 'import', + 'package', + 'validate', + 'help', + 'remove', + 'telemetry', +]); + +export const ATTRIBUTES = { + // Primitives + Count, + + // Enums + Action, + AgentType, + AttachMode, + AuthType, + AuthorizerType, + Build, + Command, + CommandGroup, + CredentialType, + ErrorCategory, + EvaluatorType, + ExitReason, + FilterState, + FilterType, + Framework, + GatewayTargetHost, + GatewayTargetType, + Language, + Level, + Memory, + Mode, + ModelProvider, + NetworkMode, + OutboundAuth, + PolicyEngineMode, + Protocol, + RefType, + ResourceType, + SourceType, + UiMode, + ValidationMode, +} as const; diff --git a/src/cli/telemetry/schemas/index.ts b/src/cli/telemetry/schemas/index.ts index 7110f61d9..de0c38bf9 100644 --- a/src/cli/telemetry/schemas/index.ts +++ b/src/cli/telemetry/schemas/index.ts @@ -1,4 +1,5 @@ export { + ATTRIBUTES, CommandResultSchema, Count, ErrorCategory, @@ -10,4 +11,11 @@ export { type CommandResult, } from './common-shapes.js'; export { ResourceAttributesSchema, type ResourceAttributes } from './common-attributes.js'; -export { COMMAND_SCHEMAS, deriveCommandGroup, type Command, type CommandAttrs } from './command-run.js'; +export { + COMMAND_SCHEMAS, + deriveCommandGroup, + type Command, + type CommandGroup, + type CommandAttrs, +} from './command-run.js'; +export { METRICS, type MetricName, type MetricAttrs, type Instrument } from './registry.js'; diff --git a/src/cli/telemetry/schemas/registry.ts b/src/cli/telemetry/schemas/registry.ts new file mode 100644 index 000000000..1500e90cf --- /dev/null +++ b/src/cli/telemetry/schemas/registry.ts @@ -0,0 +1,24 @@ +import { ATTRIBUTES, safeSchema } from './common-shapes.js'; +import { z } from 'zod'; + +/** + * Metric registry — single source of truth for all metrics the CLI can emit. + * Adding a new metric = adding one entry here. + * + * The schema defines the base attributes for compile-time type safety on emit(). + * For cli.command_run, per-command attributes are validated separately via COMMAND_SCHEMAS. + */ +export const METRICS = { + 'cli.command_run': { + instrument: 'histogram', + schema: safeSchema({ + command: ATTRIBUTES.Command, + command_group: ATTRIBUTES.CommandGroup, + exit_reason: ATTRIBUTES.ExitReason, + }), + }, +} as const; + +export type MetricName = keyof typeof METRICS; +export type Instrument = (typeof METRICS)[MetricName]['instrument']; +export type MetricAttrs = z.infer<(typeof METRICS)[M]['schema']>; From 7a1ded1f69d0b106d340acece594689641b11fc0 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 03:42:44 +0000 Subject: [PATCH 04/15] docs: update telemetry README with new metric and new command guides --- src/cli/telemetry/README.md | 153 ++++++++++++++++++------------------ 1 file changed, 77 insertions(+), 76 deletions(-) diff --git a/src/cli/telemetry/README.md b/src/cli/telemetry/README.md index 49f98bbcb..76f497208 100644 --- a/src/cli/telemetry/README.md +++ b/src/cli/telemetry/README.md @@ -1,121 +1,122 @@ -# Adding New Telemetry Metrics +# Telemetry -## Overview +## Architecture -Every CLI command emits a `command_run` metric with a command key, exit reason, and command-specific attributes. This -guide shows how to add telemetry to a new command. +``` +ATTRIBUTES (common-shapes.ts) METRICS registry (registry.ts) + │ │ + ▼ ▼ + COMMAND_SCHEMAS (command-run.ts) TelemetryClient.emit() + │ │ + ▼ ▼ + withCommandRunTelemetry / runCliCommand MetricSink (otel, filesystem, in-memory) +``` -## Step 1: Register the command in `schemas/command-run.ts` +- `TelemetryClient` — generic emitter. Knows nothing about specific metrics. +- `cli-command-run.ts` — command_run-specific logic (timing, error classification). +- `schemas/registry.ts` — all metric definitions. `MetricName` type derived from here. +- `schemas/common-shapes.ts` — all attribute definitions in `ATTRIBUTES` namespace. -Add an entry to `COMMAND_SCHEMAS`: +## Adding a New Metric -```ts -// No attributes: -'remove.widget': NoAttrs, +### 1. Define attributes in `schemas/common-shapes.ts` -// With attributes: -'add.widget': safeSchema({ - widget_type: WidgetType, // z.enum(), z.boolean(), z.number(), or z.literal() only - count: Count, -}), -``` +Skip if reusing existing attributes. -`safeSchema` enforces allowed field types at compile time. No `z.string()` fields. +```ts +export const ToolName = z.enum(['read_file', 'write_file', 'search']); +``` -## Step 2: Add enums to `schemas/common-shapes.ts` +Add to the `ATTRIBUTES` object: ```ts -export const WidgetType = z.enum(['basic', 'advanced']); +export const ATTRIBUTES = { + // ...existing + ToolName, +} as const; ``` -Use `standardize()` to normalize input before recording: +### 2. Register the metric in `schemas/registry.ts` ```ts -import { WidgetType, standardize } from '../telemetry/schemas/common-shapes.js'; +export const METRICS = { + 'cli.command_run': { ... }, + 'cli.mcp_tool_call': { + instrument: 'histogram', + schema: safeSchema({ + tool_name: ATTRIBUTES.ToolName, + success: z.boolean(), + }), + }, +} as const; +``` -const type = standardize(WidgetType, userInput); +### 3. Emit it + +```ts +client.emit('cli.mcp_tool_call', durationMs, { tool_name: 'read_file', success: true }); ``` -## Step 3: Instrument the command handler +Wrong metric name or missing/invalid attrs = compile error. -Use **`withCommandRunTelemetry`** — the primary helper for recording telemetry: +--- -```ts -import { withCommandRunTelemetry } from '../telemetry/cli-command-run.js'; +## Adding a New Command (to `cli.command_run`) + +### 1. Add the command name to `Command` enum in `schemas/common-shapes.ts` -const result = await withCommandRunTelemetry('remove.gateway', {}, () => this.remove(name)); +```ts +export const Command = z.enum([ + // ...existing + 'add.widget', +]); ``` -**Signature:** +If it introduces a new group, add to `CommandGroup` too. + +### 2. Define the command's attribute schema in `schemas/command-run.ts` ```ts -async function withCommandRunTelemetry( - command: C, - attrs: CommandAttrs, - fn: () => Promise -): Promise; +const AddWidgetAttrs = safeSchema({ + widget_type: WidgetType, + count: Count, +}); ``` -- `command` — the registered command key (e.g. `'add.widget'`) -- `attrs` — attribute object matching the schema registered in Step 1 -- `fn` — async callback returning `Result` (from `src/lib/result.ts`) +Add to `COMMAND_SCHEMAS`: -**Behavior:** +```ts +'add.widget': AddWidgetAttrs, +``` -- On success: records `attrs` with `exit_reason: 'success'`, returns the result. -- On failure/throw: records `attrs` with `exit_reason: 'failure'`, returns `{ success: false, error }`. -- If telemetry is unavailable: runs `fn()` untracked. +Compile error if the key doesn't match the `Command` enum. -Since `attrs` are passed upfront, they are always recorded — even on failure. +### 3. Instrument the handler -**Example with attributes:** +Use `withCommandRunTelemetry`: ```ts const result = await withCommandRunTelemetry( 'add.widget', - { widget_type: standardize(WidgetType, config.type), count: config.items.length }, + { widget_type: standardize(WidgetType, input), count: items.length }, () => widgetPrimitive.add(config) ); - -if (!result.success) { - console.error(result.error); - process.exit(1); -} ``` -### `runCliCommand` (alternative for top-level CLI handlers) - -For CLI handlers that own `process.exit`, use `runCliCommand` instead. The callback throws on failure and returns attrs -on success: +Or `runCliCommand` for top-level CLI handlers that own `process.exit`: ```ts -await runCliCommand('add.widget', !!options.json, async () => { - const result = await widgetPrimitive.add(options); - if (!result.success) throw new Error(result.error); - return { widget_type: standardize(WidgetType, options.type), count: options.items.length }; +await runCliCommand('add.widget', !!opts.json, async () => { + await widgetPrimitive.add(opts); + return { widget_type: standardize(WidgetType, opts.type), count: opts.items.length }; }); ``` -To record attrs on failure, pass `knownAttrs` as the fourth argument: - -```ts -const knownAttrs = { widget_type: standardize(WidgetType, options.type), count: options.items.length }; -await runCliCommand( - 'add.widget', - !!options.json, - async () => { - const result = await widgetPrimitive.add(options); - if (!result.success) throw new Error(result.error); - return knownAttrs; - }, - knownAttrs -); -``` +--- -## Key Points +## Key Rules -- Telemetry never crashes the CLI — `standardize()` falls back gracefully, `resilientParse` defaults invalid fields to - `'unknown'`. -- Prefer `withCommandRunTelemetry` for new code — it returns the `Result` for the caller to handle output and control - flow. -- Use `runCliCommand` only when the handler owns `process.exit` and prints its own output. +- `safeSchema` only allows `z.enum()`, `z.boolean()`, `z.number()`, `z.literal()`. No `z.string()`. +- `standardize(schema, value)` lowercases and validates enum values. Invalid values fall through gracefully. +- `resilientParse` validates each field independently — one bad field defaults to `'unknown'`, never drops the metric. +- Telemetry never crashes the CLI. From 50e781d38854fa82c2dfa0d87faefcaa7b740534 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 03:50:56 +0000 Subject: [PATCH 05/15] fix: restore try/catch in recordCommandRun to prevent telemetry crashes --- src/cli/telemetry/cli-command-run.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 4f47e66f0..aee3dc867 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -25,19 +25,23 @@ function recordCommandRun( attrs: CommandAttrs | Partial>, durationMs: number ): void { - CommandResultSchema.parse(result); + try { + CommandResultSchema.parse(result); - const validatedAttrs = - Object.keys(attrs as Record).length > 0 - ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) - : attrs; + const validatedAttrs = + Object.keys(attrs as Record).length > 0 + ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) + : attrs; - client.emit('cli.command_run', durationMs, { - command_group: deriveCommandGroup(command), - command, - ...result, - ...validatedAttrs, - }); + client.emit('cli.command_run', durationMs, { + command_group: deriveCommandGroup(command), + command, + ...result, + ...validatedAttrs, + }); + } catch { + // Telemetry must never affect CLI behavior + } } /** From 328e823f6e5067f07c2d17b3c8f842914056a661 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 12:35:58 +0000 Subject: [PATCH 06/15] =?UTF-8?q?fix:=20tighten=20emit()=20type=20safety,?= =?UTF-8?q?=20use=20MetricName=20in=20sinks,=20rename=20mode=E2=86=92deplo?= =?UTF-8?q?y=5Fmode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove `& Record` from emit() — typos in attrs are now compile errors - MetricSink.record() uses MetricName instead of string - Remove unused instrument field from registry (YAGNI) - Rename deploy `mode` attr to `deploy_mode` to avoid collision with session Mode - Registry schema includes all per-command attrs as optional for full type coverage --- .../commands/deploy/__tests__/utils.test.ts | 6 +- src/cli/commands/deploy/utils.ts | 8 +- src/cli/telemetry/README.md | 15 +++- .../__tests__/composite-sink.test.ts | 4 +- .../__tests__/filesystem-sink.test.ts | 12 +-- src/cli/telemetry/cli-command-run.ts | 13 +-- src/cli/telemetry/client.ts | 2 +- .../schemas/__tests__/command-run.test.ts | 6 +- src/cli/telemetry/schemas/command-run.ts | 35 ++------ .../telemetry/schemas/common-attributes.ts | 5 -- src/cli/telemetry/schemas/common-shapes.ts | 12 ++- src/cli/telemetry/schemas/index.ts | 2 +- src/cli/telemetry/schemas/registry.ts | 82 +++++++++++++++++-- src/cli/telemetry/sinks/filesystem-sink.ts | 3 +- src/cli/telemetry/sinks/in-memory-sink.ts | 5 +- src/cli/telemetry/sinks/metric-sink.ts | 6 +- src/cli/telemetry/sinks/otel-metric-sink.ts | 3 +- 17 files changed, 132 insertions(+), 87 deletions(-) diff --git a/src/cli/commands/deploy/__tests__/utils.test.ts b/src/cli/commands/deploy/__tests__/utils.test.ts index 8bf07311a..2ba22c15a 100644 --- a/src/cli/commands/deploy/__tests__/utils.test.ts +++ b/src/cli/commands/deploy/__tests__/utils.test.ts @@ -24,7 +24,7 @@ describe('computeDeployAttrs', () => { gateway_target_count: 3, policy_engine_count: 2, policy_count: 3, - mode: 'diff', + deploy_mode: 'diff', }); }); @@ -39,7 +39,7 @@ describe('computeDeployAttrs', () => { gateway_target_count: 0, policy_engine_count: 0, policy_count: 0, - mode: 'deploy', + deploy_mode: 'deploy', }); }); @@ -49,6 +49,6 @@ describe('computeDeployAttrs', () => { expect(attrs.runtime_count).toBe(1); expect(attrs.memory_count).toBe(0); - expect(attrs.mode).toBe('dry-run'); + expect(attrs.deploy_mode).toBe('dry-run'); }); }); diff --git a/src/cli/commands/deploy/utils.ts b/src/cli/commands/deploy/utils.ts index d0db1ec08..773dbf98c 100644 --- a/src/cli/commands/deploy/utils.ts +++ b/src/cli/commands/deploy/utils.ts @@ -1,6 +1,8 @@ import type { AgentCoreProjectSpec } from '../../../schema'; +import { DeployMode as DeployModeSchema } from '../../telemetry/schemas/common-shapes.js'; +import { z } from 'zod'; -export type DeployMode = 'deploy' | 'dry-run' | 'diff'; +export type DeployMode = z.infer; export const DEFAULT_DEPLOY_ATTRS = { runtime_count: 0, @@ -12,7 +14,7 @@ export const DEFAULT_DEPLOY_ATTRS = { gateway_target_count: 0, policy_engine_count: 0, policy_count: 0, - mode: 'deploy' as DeployMode, + deploy_mode: 'deploy' as DeployMode, }; export function computeDeployAttrs(projectSpec: Partial, mode: DeployMode) { @@ -28,6 +30,6 @@ export function computeDeployAttrs(projectSpec: Partial, m gateway_target_count: gateways.reduce((sum, g) => sum + (g.targets ?? []).length, 0), policy_engine_count: policyEngines.length, policy_count: policyEngines.reduce((sum, pe) => sum + (pe.policies ?? []).length, 0), - mode, + deploy_mode: mode, }; } diff --git a/src/cli/telemetry/README.md b/src/cli/telemetry/README.md index 76f497208..cd6dcae6c 100644 --- a/src/cli/telemetry/README.md +++ b/src/cli/telemetry/README.md @@ -42,7 +42,6 @@ export const ATTRIBUTES = { export const METRICS = { 'cli.command_run': { ... }, 'cli.mcp_tool_call': { - instrument: 'histogram', schema: safeSchema({ tool_name: ATTRIBUTES.ToolName, success: z.boolean(), @@ -91,7 +90,19 @@ Add to `COMMAND_SCHEMAS`: Compile error if the key doesn't match the `Command` enum. -### 3. Instrument the handler +### 3. Add the new attributes as optional fields in `schemas/registry.ts` + +```ts +const CommandRunSchema = safeSchema({ ... }).extend({ + // ...existing + widget_type: ATTRIBUTES.WidgetType.optional(), + count: z.number().optional(), +}); +``` + +This ensures `emit()` accepts the new fields with compile-time checking. + +### 4. Instrument the handler Use `withCommandRunTelemetry`: diff --git a/src/cli/telemetry/__tests__/composite-sink.test.ts b/src/cli/telemetry/__tests__/composite-sink.test.ts index ed7194e1e..a3a7afb6e 100644 --- a/src/cli/telemetry/__tests__/composite-sink.test.ts +++ b/src/cli/telemetry/__tests__/composite-sink.test.ts @@ -8,7 +8,7 @@ describe('CompositeSink', () => { const b = new InMemorySink(); const composite = new CompositeSink([a, b]); - composite.record('command_run', 100, { command: 'deploy' }); + composite.record('cli.command_run', 100, { command: 'deploy' }); expect(a.metrics).toHaveLength(1); expect(b.metrics).toHaveLength(1); @@ -26,7 +26,7 @@ describe('CompositeSink', () => { const good = new InMemorySink(); const composite = new CompositeSink([bad, good]); - composite.record('command_run', 100, { command: 'deploy' }); + composite.record('cli.command_run', 100, { command: 'deploy' }); expect(good.metrics).toHaveLength(1); }); diff --git a/src/cli/telemetry/__tests__/filesystem-sink.test.ts b/src/cli/telemetry/__tests__/filesystem-sink.test.ts index 96a894acc..87865063c 100644 --- a/src/cli/telemetry/__tests__/filesystem-sink.test.ts +++ b/src/cli/telemetry/__tests__/filesystem-sink.test.ts @@ -28,13 +28,13 @@ describe('FileSystemSink', () => { it('writes each record as a JSONL line on disk', async () => { const sink = createSink(); - sink.record('command_run', 42, { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }); + sink.record('cli.command_run', 42, { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }); await sink.flush(); const entries = await readJsonl(join(outputDir, 'test-session.json')); expect(entries).toHaveLength(1); expect(entries[0]).toMatchObject({ - metric: 'command_run', + metric: 'cli.command_run', value: 42, attrs: { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }, }); @@ -42,8 +42,8 @@ describe('FileSystemSink', () => { it('appends multiple records as separate lines', async () => { const sink = createSink(); - sink.record('command_run', 10, { command_group: 'add', command: 'add.agent' }); - sink.record('command_run', 20, { command_group: 'add', command: 'add.memory' }); + sink.record('cli.command_run', 10, { command_group: 'add', command: 'add.agent' }); + sink.record('cli.command_run', 20, { command_group: 'add', command: 'add.memory' }); await sink.flush(); const entries = await readJsonl(join(outputDir, 'test-session.json')); @@ -56,7 +56,7 @@ describe('FileSystemSink', () => { const nested = join(tmp.testDir, 'deep', 'nested', 'telemetry'); const filePath = join(nested, 'test.json'); const sink = new FileSystemSink({ filePath }); - sink.record('command_run', 1, { command_group: 'status', command: 'status' }); + sink.record('cli.command_run', 1, { command_group: 'status', command: 'status' }); await sink.flush(); const entries = await readJsonl(filePath); @@ -71,7 +71,7 @@ describe('FileSystemSink', () => { it('shutdown logs audit message when records were written', async () => { const logged: string[] = []; const sink = createSink({ log: msg => logged.push(msg) }); - sink.record('command_run', 99, { command_group: 'invoke', command: 'invoke' }); + sink.record('cli.command_run', 99, { command_group: 'invoke', command: 'invoke' }); await sink.shutdown(); expect(logged).toHaveLength(1); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index aee3dc867..4a4c99f13 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -7,9 +7,6 @@ import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; import { performance } from 'perf_hooks'; -/** Return this from the trackCommandRun callback to record a cancellation. */ -const CANCELLED = Symbol('cancelled'); - async function getTelemetryClient() { try { return await TelemetryClientAccessor.get(); @@ -45,24 +42,20 @@ function recordCommandRun( } /** - * Return attrs on success, or CANCELLED on user cancellation. + * Return attrs on success * Unhandled throws are classified as failures and re-thrown. */ async function trackCommandRun( client: TelemetryClient, command: C, - fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED>, + fn: () => CommandAttrs | Promise>, fallbackAttrs?: Partial> ): Promise { const start = performance.now(); try { const result = await fn(); const durationMs = Math.round(performance.now() - start); - if (result === CANCELLED) { - recordCommandRun(client, command, { exit_reason: 'cancel' }, {}, durationMs); - } else { - recordCommandRun(client, command, { exit_reason: 'success' }, result, durationMs); - } + recordCommandRun(client, command, { exit_reason: 'success' }, result, durationMs); } catch (err) { const failureResult: CommandResult & { exit_reason: 'failure' } = { exit_reason: 'failure', diff --git a/src/cli/telemetry/client.ts b/src/cli/telemetry/client.ts index 8be2da8e9..10157a465 100644 --- a/src/cli/telemetry/client.ts +++ b/src/cli/telemetry/client.ts @@ -7,7 +7,7 @@ import type { MetricSink } from './sinks/metric-sink.js'; export class TelemetryClient { constructor(private readonly sink: MetricSink) {} - emit(metricName: M, value: number, attrs: MetricAttrs & Record): void { + emit(metricName: M, value: number, attrs: MetricAttrs): void { try { const otelAttrs: Record = {}; for (const [k, v] of Object.entries(attrs)) { diff --git a/src/cli/telemetry/schemas/__tests__/command-run.test.ts b/src/cli/telemetry/schemas/__tests__/command-run.test.ts index eae73cabc..6090decb0 100644 --- a/src/cli/telemetry/schemas/__tests__/command-run.test.ts +++ b/src/cli/telemetry/schemas/__tests__/command-run.test.ts @@ -47,7 +47,7 @@ describe('COMMAND_SCHEMAS', () => { gateway_target_count: 3, policy_engine_count: 0, policy_count: 0, - mode: 'diff', + deploy_mode: 'diff', }; expect(COMMAND_SCHEMAS.deploy.parse(attrs)).toEqual(attrs); }); @@ -64,7 +64,7 @@ describe('COMMAND_SCHEMAS', () => { gateway_target_count: 0, policy_engine_count: 0, policy_count: 0, - mode: 'deploy', + deploy_mode: 'deploy', }) ).toThrow(); }); @@ -81,7 +81,7 @@ describe('COMMAND_SCHEMAS', () => { gateway_target_count: 0, policy_engine_count: 0, policy_count: 0, - mode: 'deploy', + deploy_mode: 'deploy', }) ).toThrow(); }); diff --git a/src/cli/telemetry/schemas/command-run.ts b/src/cli/telemetry/schemas/command-run.ts index 55639e397..7ea530139 100644 --- a/src/cli/telemetry/schemas/command-run.ts +++ b/src/cli/telemetry/schemas/command-run.ts @@ -9,6 +9,7 @@ import { Command as CommandSchema, Count, CredentialType, + DeployMode, EvaluatorType, FilterState, FilterType, @@ -35,11 +36,6 @@ import { z } from 'zod'; export type Command = z.infer; export type CommandGroup = z.infer; -// --------------------------------------------------------------------------- -// Per-command attribute schemas -// All schemas use safeSchema() which rejects z.string() at compile time. -// --------------------------------------------------------------------------- - const CreateAttrs = safeSchema({ language: Language, framework: Framework, @@ -106,7 +102,7 @@ const DeployAttrs = safeSchema({ gateway_target_count: Count, policy_engine_count: Count, policy_count: Count, - mode: z.enum(['deploy', 'dry-run', 'diff']), + deploy_mode: DeployMode, }); const DevAttrs = safeSchema({ @@ -146,15 +142,12 @@ const PauseResumeOnlineEvalAttrs = safeSchema({ ref_type: RefType }); const NoAttrs = safeSchema({}); -// --------------------------------------------------------------------------- -// Command schema registry — single source of truth -// --------------------------------------------------------------------------- - +/* + Mapping of commands to required attributes. + This is chosen over discriminated unions to avoid complexity in the root-level definition. +*/ export const COMMAND_SCHEMAS = { - // create create: CreateAttrs, - - // add 'add.agent': AddAgentAttrs, 'add.memory': AddMemoryAttrs, 'add.credential': AddCredentialAttrs, @@ -165,33 +158,17 @@ export const COMMAND_SCHEMAS = { 'add.policy-engine': AddPolicyEngineAttrs, 'add.policy': AddPolicyAttrs, 'add.runtime-endpoint': NoAttrs, - - // deploy deploy: DeployAttrs, - - // dev / invoke dev: DevAttrs, invoke: InvokeAttrs, - - // status / logs status: StatusAttrs, logs: LogsAttrs, 'logs.evals': LogsEvalsAttrs, - - // run 'run.eval': RunEvalAttrs, - - // fetch 'fetch.access': FetchAccessAttrs, - - // update update: UpdateAttrs, - - // pause / resume 'pause.online-eval': PauseResumeOnlineEvalAttrs, 'resume.online-eval': PauseResumeOnlineEvalAttrs, - - // no command-specific attributes 'traces.list': NoAttrs, 'traces.get': NoAttrs, 'evals.history': NoAttrs, diff --git a/src/cli/telemetry/schemas/common-attributes.ts b/src/cli/telemetry/schemas/common-attributes.ts index 3085db7cc..4be71a5a5 100644 --- a/src/cli/telemetry/schemas/common-attributes.ts +++ b/src/cli/telemetry/schemas/common-attributes.ts @@ -9,11 +9,6 @@ const MAX_ATTR_LENGTH = 64; /** * Resource attributes attached to every metric datapoint. * Set once per session, not per-event. - * - * Constraints are intentionally strict to prevent PII leakage: - * - IDs must be UUID format (no user-chosen strings) - * - Version strings are pattern-constrained - * - All free-text fields are length-bounded */ export const ResourceAttributesSchema = z.object({ 'service.name': z.literal('agentcore-cli'), diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index a4413537f..47a1be3e5 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -122,10 +122,7 @@ export const FailureResult = z.object({ export const CommandResultSchema = z.discriminatedUnion('exit_reason', [SuccessResult, CancelResult, FailureResult]); export type CommandResult = z.infer; -// --------------------------------------------------------------------------- -// ATTRIBUTES namespace — single source of truth for all attribute definitions. -// New code should use ATTRIBUTES.X; existing named exports remain for compat. -// --------------------------------------------------------------------------- +export const DeployMode = z.enum(['deploy', 'dry-run', 'diff']); export const Command = z.enum([ 'create', @@ -201,11 +198,11 @@ export const CommandGroup = z.enum([ 'telemetry', ]); +/* + All attributes the CLI may attach to a metric. +*/ export const ATTRIBUTES = { - // Primitives Count, - - // Enums Action, AgentType, AttachMode, @@ -215,6 +212,7 @@ export const ATTRIBUTES = { Command, CommandGroup, CredentialType, + DeployMode, ErrorCategory, EvaluatorType, ExitReason, diff --git a/src/cli/telemetry/schemas/index.ts b/src/cli/telemetry/schemas/index.ts index de0c38bf9..2d263b122 100644 --- a/src/cli/telemetry/schemas/index.ts +++ b/src/cli/telemetry/schemas/index.ts @@ -18,4 +18,4 @@ export { type CommandGroup, type CommandAttrs, } from './command-run.js'; -export { METRICS, type MetricName, type MetricAttrs, type Instrument } from './registry.js'; +export { METRICS, type MetricName, type MetricAttrs } from './registry.js'; diff --git a/src/cli/telemetry/schemas/registry.ts b/src/cli/telemetry/schemas/registry.ts index 1500e90cf..eb1e4e104 100644 --- a/src/cli/telemetry/schemas/registry.ts +++ b/src/cli/telemetry/schemas/registry.ts @@ -5,20 +5,84 @@ import { z } from 'zod'; * Metric registry — single source of truth for all metrics the CLI can emit. * Adding a new metric = adding one entry here. * - * The schema defines the base attributes for compile-time type safety on emit(). - * For cli.command_run, per-command attributes are validated separately via COMMAND_SCHEMAS. + * The schema defines attributes for compile-time type safety on emit(). + * For cli.command_run, required fields are always present; optional fields + * are the union of all per-command attributes. */ + +const CommandRunSchema = safeSchema({ + command: ATTRIBUTES.Command, + command_group: ATTRIBUTES.CommandGroup, + exit_reason: ATTRIBUTES.ExitReason, +}).extend({ + // Error fields (present on failure) + error_name: ATTRIBUTES.ErrorCategory.optional(), + is_user_error: z.boolean().optional(), + + // Per-command attributes (optional — depends on which command ran) + action: ATTRIBUTES.Action.optional(), + agent_type: ATTRIBUTES.AgentType.optional(), + attach_gateway_count: z.number().optional(), + attach_mode: ATTRIBUTES.AttachMode.optional(), + auth_type: ATTRIBUTES.AuthType.optional(), + authorizer_type: ATTRIBUTES.AuthorizerType.optional(), + build: ATTRIBUTES.Build.optional(), + check_only: z.boolean().optional(), + credential_count: z.number().optional(), + credential_type: ATTRIBUTES.CredentialType.optional(), + enable_on_create: z.boolean().optional(), + evaluator_count: z.number().optional(), + evaluator_type: ATTRIBUTES.EvaluatorType.optional(), + filter_state: ATTRIBUTES.FilterState.optional(), + filter_type: ATTRIBUTES.FilterType.optional(), + framework: ATTRIBUTES.Framework.optional(), + gateway_count: z.number().optional(), + gateway_target_count: z.number().optional(), + has_agent: z.boolean().optional(), + has_assertions: z.boolean().optional(), + has_expected_response: z.boolean().optional(), + has_expected_trajectory: z.boolean().optional(), + has_follow: z.boolean().optional(), + has_level_filter: z.boolean().optional(), + has_policy_engine: z.boolean().optional(), + has_query: z.boolean().optional(), + has_session_id: z.boolean().optional(), + has_stream: z.boolean().optional(), + host: ATTRIBUTES.GatewayTargetHost.optional(), + invoke_count: z.number().optional(), + language: ATTRIBUTES.Language.optional(), + level: ATTRIBUTES.Level.optional(), + memory: ATTRIBUTES.Memory.optional(), + memory_count: z.number().optional(), + deploy_mode: ATTRIBUTES.DeployMode.optional(), + model_provider: ATTRIBUTES.ModelProvider.optional(), + network_mode: ATTRIBUTES.NetworkMode.optional(), + online_eval_count: z.number().optional(), + outbound_auth: ATTRIBUTES.OutboundAuth.optional(), + policy_count: z.number().optional(), + policy_engine_count: z.number().optional(), + policy_engine_mode: ATTRIBUTES.PolicyEngineMode.optional(), + protocol: ATTRIBUTES.Protocol.optional(), + ref_type: ATTRIBUTES.RefType.optional(), + resource_type: ATTRIBUTES.ResourceType.optional(), + runtime_count: z.number().optional(), + semantic_search: z.boolean().optional(), + source_type: ATTRIBUTES.SourceType.optional(), + strategy_count: z.number().optional(), + strategy_episodic: z.boolean().optional(), + strategy_semantic: z.boolean().optional(), + strategy_summarization: z.boolean().optional(), + strategy_user_preference: z.boolean().optional(), + target_type: ATTRIBUTES.GatewayTargetType.optional(), + ui_mode: ATTRIBUTES.UiMode.optional(), + validation_mode: ATTRIBUTES.ValidationMode.optional(), +}); + export const METRICS = { 'cli.command_run': { - instrument: 'histogram', - schema: safeSchema({ - command: ATTRIBUTES.Command, - command_group: ATTRIBUTES.CommandGroup, - exit_reason: ATTRIBUTES.ExitReason, - }), + schema: CommandRunSchema, }, } as const; export type MetricName = keyof typeof METRICS; -export type Instrument = (typeof METRICS)[MetricName]['instrument']; export type MetricAttrs = z.infer<(typeof METRICS)[M]['schema']>; diff --git a/src/cli/telemetry/sinks/filesystem-sink.ts b/src/cli/telemetry/sinks/filesystem-sink.ts index 6e0492af7..43638b91a 100644 --- a/src/cli/telemetry/sinks/filesystem-sink.ts +++ b/src/cli/telemetry/sinks/filesystem-sink.ts @@ -1,3 +1,4 @@ +import type { MetricName } from '../schemas/registry.js'; import type { MetricSink } from './metric-sink.js'; import { appendFile, mkdir } from 'fs/promises'; import { dirname } from 'path'; @@ -20,7 +21,7 @@ export class FileSystemSink implements MetricSink { this.log = config.log ?? (msg => console.log(msg)); } - record(metricName: string, value: number, attrs: Record): void { + record(metricName: MetricName, value: number, attrs: Record): void { this.hasRecords = true; this.pendingWrite = this.pendingWrite.then(() => this.appendEntry({ metric: metricName, value, attrs: { ...this.resource, ...attrs } }) diff --git a/src/cli/telemetry/sinks/in-memory-sink.ts b/src/cli/telemetry/sinks/in-memory-sink.ts index 5511c5c4b..7fa5c3398 100644 --- a/src/cli/telemetry/sinks/in-memory-sink.ts +++ b/src/cli/telemetry/sinks/in-memory-sink.ts @@ -1,7 +1,8 @@ +import type { MetricName } from '../schemas/registry.js'; import type { MetricSink } from './metric-sink.js'; export interface RecordedMetric { - metric: string; + metric: MetricName; value: number; attrs: Record; } @@ -9,7 +10,7 @@ export interface RecordedMetric { export class InMemorySink implements MetricSink { readonly metrics: RecordedMetric[] = []; - record(metricName: string, value: number, attrs: Record): void { + record(metricName: MetricName, value: number, attrs: Record): void { this.metrics.push({ metric: metricName, value, attrs }); } diff --git a/src/cli/telemetry/sinks/metric-sink.ts b/src/cli/telemetry/sinks/metric-sink.ts index 9074039a6..38ac48f43 100644 --- a/src/cli/telemetry/sinks/metric-sink.ts +++ b/src/cli/telemetry/sinks/metric-sink.ts @@ -1,8 +1,10 @@ +import type { MetricName } from '../schemas/registry.js'; + /** * A destination for metric data. Implementations handle transport (OTel, file, etc.). */ export interface MetricSink { - record(metricName: string, value: number, attrs: Record): void; + record(metricName: MetricName, value: number, attrs: Record): void; flush(timeoutMs?: number): Promise; shutdown(): Promise; } @@ -14,7 +16,7 @@ export interface MetricSink { export class CompositeSink implements MetricSink { constructor(private readonly sinks: MetricSink[]) {} - record(metricName: string, value: number, attrs: Record): void { + record(metricName: MetricName, value: number, attrs: Record): void { for (const sink of this.sinks) { try { sink.record(metricName, value, attrs); diff --git a/src/cli/telemetry/sinks/otel-metric-sink.ts b/src/cli/telemetry/sinks/otel-metric-sink.ts index dd8cb3860..2fe44edbb 100644 --- a/src/cli/telemetry/sinks/otel-metric-sink.ts +++ b/src/cli/telemetry/sinks/otel-metric-sink.ts @@ -1,4 +1,5 @@ import type { ResourceAttributes } from '../schemas/common-attributes.js'; +import type { MetricName } from '../schemas/registry.js'; import type { MetricSink } from './metric-sink.js'; import type { Histogram, Meter } from '@opentelemetry/api'; import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-http'; @@ -38,7 +39,7 @@ export class OtelMetricSink implements MetricSink { this.meter = this.meterProvider.getMeter('agentcore-cli'); } - record(metricName: string, value: number, attrs: Record): void { + record(metricName: MetricName, value: number, attrs: Record): void { let histogram = this.histograms.get(metricName); if (!histogram) { histogram = this.meter.createHistogram(metricName, { description: metricName }); From 6468ed749c31809355fe74668166e6d22ea537d4 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 12:54:45 +0000 Subject: [PATCH 07/15] simplify: derive registry types from COMMAND_SCHEMAS, remove manual CommandGroup enum - Optional fields in registry derived automatically from COMMAND_SCHEMAS (eliminates 55 lines) - CommandGroup reverted to type-level derivation from Command (zero maintenance) - Removed CommandGroup z.enum from common-shapes.ts --- src/cli/telemetry/schemas/command-run.ts | 3 +- src/cli/telemetry/schemas/common-shapes.ts | 120 +++++++++++---------- src/cli/telemetry/schemas/registry.ts | 92 +++------------- 3 files changed, 81 insertions(+), 134 deletions(-) diff --git a/src/cli/telemetry/schemas/command-run.ts b/src/cli/telemetry/schemas/command-run.ts index 7ea530139..41e860ee2 100644 --- a/src/cli/telemetry/schemas/command-run.ts +++ b/src/cli/telemetry/schemas/command-run.ts @@ -5,7 +5,6 @@ import { AuthType, AuthorizerType, Build, - CommandGroup as CommandGroupSchema, Command as CommandSchema, Count, CredentialType, @@ -34,7 +33,7 @@ import { import { z } from 'zod'; export type Command = z.infer; -export type CommandGroup = z.infer; +export type CommandGroup = { [C in Command]: C extends `${infer G}.${string}` ? G : C }[Command]; const CreateAttrs = safeSchema({ language: Language, diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 47a1be3e5..5338cfc7c 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -175,64 +175,70 @@ export const Command = z.enum([ 'telemetry.status', ]); -export const CommandGroup = z.enum([ - 'create', - 'add', - 'deploy', - 'dev', - 'invoke', - 'status', - 'logs', - 'run', - 'fetch', - 'update', - 'pause', - 'resume', - 'traces', - 'evals', - 'import', - 'package', - 'validate', - 'help', - 'remove', - 'telemetry', -]); - /* - All attributes the CLI may attach to a metric. + All attributes the CLI may attach to a metric. + Keys are the field names as they appear in emitted metrics. */ export const ATTRIBUTES = { - Count, - Action, - AgentType, - AttachMode, - AuthType, - AuthorizerType, - Build, - Command, - CommandGroup, - CredentialType, - DeployMode, - ErrorCategory, - EvaluatorType, - ExitReason, - FilterState, - FilterType, - Framework, - GatewayTargetHost, - GatewayTargetType, - Language, - Level, - Memory, - Mode, - ModelProvider, - NetworkMode, - OutboundAuth, - PolicyEngineMode, - Protocol, - RefType, - ResourceType, - SourceType, - UiMode, - ValidationMode, + action: Action, + agent_type: AgentType, + attach_gateway_count: Count, + attach_mode: AttachMode, + auth_type: AuthType, + authorizer_type: AuthorizerType, + build: Build, + check_only: z.boolean(), + command: Command, + credential_count: Count, + credential_type: CredentialType, + deploy_mode: DeployMode, + enable_on_create: z.boolean(), + error_name: ErrorCategory, + evaluator_count: Count, + evaluator_type: EvaluatorType, + exit_reason: ExitReason, + filter_state: FilterState, + filter_type: FilterType, + framework: Framework, + gateway_count: Count, + gateway_target_count: Count, + has_agent: z.boolean(), + has_assertions: z.boolean(), + has_expected_response: z.boolean(), + has_expected_trajectory: z.boolean(), + has_follow: z.boolean(), + has_level_filter: z.boolean(), + has_policy_engine: z.boolean(), + has_query: z.boolean(), + has_session_id: z.boolean(), + has_stream: z.boolean(), + host: GatewayTargetHost, + invoke_count: Count, + is_user_error: z.boolean(), + language: Language, + level: Level, + memory: Memory, + memory_count: Count, + mode: Mode, + model_provider: ModelProvider, + network_mode: NetworkMode, + online_eval_count: Count, + outbound_auth: OutboundAuth, + policy_count: Count, + policy_engine_count: Count, + policy_engine_mode: PolicyEngineMode, + protocol: Protocol, + ref_type: RefType, + resource_type: ResourceType, + runtime_count: Count, + semantic_search: z.boolean(), + source_type: SourceType, + strategy_count: Count, + strategy_episodic: z.boolean(), + strategy_semantic: z.boolean(), + strategy_summarization: z.boolean(), + strategy_user_preference: z.boolean(), + target_type: GatewayTargetType, + ui_mode: UiMode, + validation_mode: ValidationMode, } as const; diff --git a/src/cli/telemetry/schemas/registry.ts b/src/cli/telemetry/schemas/registry.ts index eb1e4e104..61b9a7b34 100644 --- a/src/cli/telemetry/schemas/registry.ts +++ b/src/cli/telemetry/schemas/registry.ts @@ -1,88 +1,30 @@ -import { ATTRIBUTES, safeSchema } from './common-shapes.js'; +import { COMMAND_SCHEMAS, type Command, type CommandGroup } from './command-run.js'; +import { ATTRIBUTES } from './common-shapes.js'; import { z } from 'zod'; /** * Metric registry — single source of truth for all metrics the CLI can emit. - * Adding a new metric = adding one entry here. * - * The schema defines attributes for compile-time type safety on emit(). - * For cli.command_run, required fields are always present; optional fields - * are the union of all per-command attributes. + * Per-command optional fields are derived from COMMAND_SCHEMAS automatically. + * Adding a new command's attrs to COMMAND_SCHEMAS is sufficient — no manual + * update here needed. */ -const CommandRunSchema = safeSchema({ - command: ATTRIBUTES.Command, - command_group: ATTRIBUTES.CommandGroup, - exit_reason: ATTRIBUTES.ExitReason, -}).extend({ - // Error fields (present on failure) - error_name: ATTRIBUTES.ErrorCategory.optional(), - is_user_error: z.boolean().optional(), +// Merge all per-command schemas into a single partial type +type AllCommandSchemas = (typeof COMMAND_SCHEMAS)[keyof typeof COMMAND_SCHEMAS]; +type MergedCommandAttrs = Partial>; - // Per-command attributes (optional — depends on which command ran) - action: ATTRIBUTES.Action.optional(), - agent_type: ATTRIBUTES.AgentType.optional(), - attach_gateway_count: z.number().optional(), - attach_mode: ATTRIBUTES.AttachMode.optional(), - auth_type: ATTRIBUTES.AuthType.optional(), - authorizer_type: ATTRIBUTES.AuthorizerType.optional(), - build: ATTRIBUTES.Build.optional(), - check_only: z.boolean().optional(), - credential_count: z.number().optional(), - credential_type: ATTRIBUTES.CredentialType.optional(), - enable_on_create: z.boolean().optional(), - evaluator_count: z.number().optional(), - evaluator_type: ATTRIBUTES.EvaluatorType.optional(), - filter_state: ATTRIBUTES.FilterState.optional(), - filter_type: ATTRIBUTES.FilterType.optional(), - framework: ATTRIBUTES.Framework.optional(), - gateway_count: z.number().optional(), - gateway_target_count: z.number().optional(), - has_agent: z.boolean().optional(), - has_assertions: z.boolean().optional(), - has_expected_response: z.boolean().optional(), - has_expected_trajectory: z.boolean().optional(), - has_follow: z.boolean().optional(), - has_level_filter: z.boolean().optional(), - has_policy_engine: z.boolean().optional(), - has_query: z.boolean().optional(), - has_session_id: z.boolean().optional(), - has_stream: z.boolean().optional(), - host: ATTRIBUTES.GatewayTargetHost.optional(), - invoke_count: z.number().optional(), - language: ATTRIBUTES.Language.optional(), - level: ATTRIBUTES.Level.optional(), - memory: ATTRIBUTES.Memory.optional(), - memory_count: z.number().optional(), - deploy_mode: ATTRIBUTES.DeployMode.optional(), - model_provider: ATTRIBUTES.ModelProvider.optional(), - network_mode: ATTRIBUTES.NetworkMode.optional(), - online_eval_count: z.number().optional(), - outbound_auth: ATTRIBUTES.OutboundAuth.optional(), - policy_count: z.number().optional(), - policy_engine_count: z.number().optional(), - policy_engine_mode: ATTRIBUTES.PolicyEngineMode.optional(), - protocol: ATTRIBUTES.Protocol.optional(), - ref_type: ATTRIBUTES.RefType.optional(), - resource_type: ATTRIBUTES.ResourceType.optional(), - runtime_count: z.number().optional(), - semantic_search: z.boolean().optional(), - source_type: ATTRIBUTES.SourceType.optional(), - strategy_count: z.number().optional(), - strategy_episodic: z.boolean().optional(), - strategy_semantic: z.boolean().optional(), - strategy_summarization: z.boolean().optional(), - strategy_user_preference: z.boolean().optional(), - target_type: ATTRIBUTES.GatewayTargetType.optional(), - ui_mode: ATTRIBUTES.UiMode.optional(), - validation_mode: ATTRIBUTES.ValidationMode.optional(), -}); +type CommandRunAttrs = { + command: Command; + command_group: CommandGroup; + exit_reason: z.infer; + error_name?: z.infer; + is_user_error?: boolean; +} & MergedCommandAttrs; export const METRICS = { - 'cli.command_run': { - schema: CommandRunSchema, - }, + 'cli.command_run': {}, } as const; export type MetricName = keyof typeof METRICS; -export type MetricAttrs = z.infer<(typeof METRICS)[M]['schema']>; +export type MetricAttrs = M extends 'cli.command_run' ? CommandRunAttrs : never; From 201185c943325b9d2b34b8e1cf9d3fef43e78bd7 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 13:12:05 +0000 Subject: [PATCH 08/15] simplify: remove dead exports, Command z.enum, and fix deploy/utils dependency - Derive Command from keyof COMMAND_SCHEMAS (zero maintenance, as before) - Remove Command z.enum from common-shapes.ts (~50 lines) - Remove mode from ATTRIBUTES (resource attr, not metric attr) - Remove unused barrel re-exports (CompositeSink, classifyError, isUserError, CommandGroup) - Revert deploy/utils.ts to simple literal union (no zod dependency) --- src/cli/commands/deploy/utils.ts | 5 +- src/cli/telemetry/index.ts | 3 +- src/cli/telemetry/schemas/command-run.ts | 12 ++--- src/cli/telemetry/schemas/common-shapes.ts | 58 ++-------------------- src/cli/telemetry/schemas/index.ts | 8 +-- 5 files changed, 11 insertions(+), 75 deletions(-) diff --git a/src/cli/commands/deploy/utils.ts b/src/cli/commands/deploy/utils.ts index 773dbf98c..c866ded9e 100644 --- a/src/cli/commands/deploy/utils.ts +++ b/src/cli/commands/deploy/utils.ts @@ -1,8 +1,5 @@ import type { AgentCoreProjectSpec } from '../../../schema'; -import { DeployMode as DeployModeSchema } from '../../telemetry/schemas/common-shapes.js'; -import { z } from 'zod'; - -export type DeployMode = z.infer; +import type { DeployMode } from '../../telemetry/schemas/common-shapes'; export const DEFAULT_DEPLOY_ATTRS = { runtime_count: 0, diff --git a/src/cli/telemetry/index.ts b/src/cli/telemetry/index.ts index 96c059198..16395562f 100644 --- a/src/cli/telemetry/index.ts +++ b/src/cli/telemetry/index.ts @@ -2,7 +2,6 @@ export { resolveTelemetryPreference, resolveResourceAttributes, resolveAuditFile export type { TelemetryPreference } from './config.js'; export { TelemetryClientAccessor } from './client-accessor.js'; export { TelemetryClient } from './client.js'; -export { type MetricSink, CompositeSink } from './sinks/metric-sink.js'; +export { type MetricSink } from './sinks/metric-sink.js'; export { OtelMetricSink, type OtelMetricSinkConfig } from './sinks/otel-metric-sink.js'; export { FileSystemSink, type FileSystemSinkConfig } from './sinks/filesystem-sink.js'; -export { classifyError, isUserError } from './error-classification.js'; diff --git a/src/cli/telemetry/schemas/command-run.ts b/src/cli/telemetry/schemas/command-run.ts index 41e860ee2..d5fd1cd48 100644 --- a/src/cli/telemetry/schemas/command-run.ts +++ b/src/cli/telemetry/schemas/command-run.ts @@ -5,10 +5,9 @@ import { AuthType, AuthorizerType, Build, - Command as CommandSchema, Count, CredentialType, - DeployMode, + DeployModeSchema, EvaluatorType, FilterState, FilterType, @@ -32,9 +31,6 @@ import { } from './common-shapes.js'; import { z } from 'zod'; -export type Command = z.infer; -export type CommandGroup = { [C in Command]: C extends `${infer G}.${string}` ? G : C }[Command]; - const CreateAttrs = safeSchema({ language: Language, framework: Framework, @@ -101,7 +97,7 @@ const DeployAttrs = safeSchema({ gateway_target_count: Count, policy_engine_count: Count, policy_count: Count, - deploy_mode: DeployMode, + deploy_mode: DeployModeSchema, }); const DevAttrs = safeSchema({ @@ -194,12 +190,14 @@ export const COMMAND_SCHEMAS = { 'telemetry.disable': NoAttrs, 'telemetry.enable': NoAttrs, 'telemetry.status': NoAttrs, -} as const satisfies Record>; +} as const satisfies Record>; // --------------------------------------------------------------------------- // Derived types // --------------------------------------------------------------------------- +export type Command = keyof typeof COMMAND_SCHEMAS; +export type CommandGroup = { [C in Command]: C extends `${infer G}.${string}` ? G : C }[Command]; export type CommandAttrs = z.infer<(typeof COMMAND_SCHEMAS)[C]>; export type SubCommand = Extract; diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 5338cfc7c..733a4829c 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -122,58 +122,8 @@ export const FailureResult = z.object({ export const CommandResultSchema = z.discriminatedUnion('exit_reason', [SuccessResult, CancelResult, FailureResult]); export type CommandResult = z.infer; -export const DeployMode = z.enum(['deploy', 'dry-run', 'diff']); - -export const Command = z.enum([ - 'create', - 'add.agent', - 'add.memory', - 'add.credential', - 'add.evaluator', - 'add.online-eval', - 'add.gateway', - 'add.gateway-target', - 'add.policy-engine', - 'add.policy', - 'add.runtime-endpoint', - 'deploy', - 'dev', - 'invoke', - 'status', - 'logs', - 'logs.evals', - 'run.eval', - 'fetch.access', - 'update', - 'pause.online-eval', - 'resume.online-eval', - 'traces.list', - 'traces.get', - 'evals.history', - 'import', - 'import.runtime', - 'import.memory', - 'package', - 'validate', - 'help.modes', - 'help', - 'remove.all', - 'remove.agent', - 'remove.memory', - 'remove.credential', - 'remove.evaluator', - 'remove.online-eval', - 'remove.gateway', - 'remove.gateway-target', - 'remove.policy-engine', - 'remove.policy', - 'remove.runtime-endpoint', - 'remove.config-bundle', - 'remove.ab-test', - 'telemetry.disable', - 'telemetry.enable', - 'telemetry.status', -]); +export const DeployModeSchema = z.enum(['deploy', 'dry-run', 'diff']); +export type DeployMode = z.infer; /* All attributes the CLI may attach to a metric. @@ -188,10 +138,9 @@ export const ATTRIBUTES = { authorizer_type: AuthorizerType, build: Build, check_only: z.boolean(), - command: Command, credential_count: Count, credential_type: CredentialType, - deploy_mode: DeployMode, + deploy_mode: DeployModeSchema, enable_on_create: z.boolean(), error_name: ErrorCategory, evaluator_count: Count, @@ -219,7 +168,6 @@ export const ATTRIBUTES = { level: Level, memory: Memory, memory_count: Count, - mode: Mode, model_provider: ModelProvider, network_mode: NetworkMode, online_eval_count: Count, diff --git a/src/cli/telemetry/schemas/index.ts b/src/cli/telemetry/schemas/index.ts index 2d263b122..21793c93b 100644 --- a/src/cli/telemetry/schemas/index.ts +++ b/src/cli/telemetry/schemas/index.ts @@ -11,11 +11,5 @@ export { type CommandResult, } from './common-shapes.js'; export { ResourceAttributesSchema, type ResourceAttributes } from './common-attributes.js'; -export { - COMMAND_SCHEMAS, - deriveCommandGroup, - type Command, - type CommandGroup, - type CommandAttrs, -} from './command-run.js'; +export { COMMAND_SCHEMAS, deriveCommandGroup, type Command, type CommandAttrs } from './command-run.js'; export { METRICS, type MetricName, type MetricAttrs } from './registry.js'; From ab570dd190cc2ac382e921b613f190987ebbcb37 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 13:19:06 +0000 Subject: [PATCH 09/15] =?UTF-8?q?fix:=20remove=20dead=20CancelResult=20sch?= =?UTF-8?q?ema=20=E2=80=94=20no=20code=20path=20produces=20cancel=20events?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/cli/telemetry/schemas/common-shapes.ts | 5 ++--- src/cli/telemetry/schemas/index.ts | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 733a4829c..8bfdb666e 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -55,7 +55,7 @@ export const AuthorizerType = z.enum(['aws_iam', 'custom_jwt', 'none']); export const Build = z.enum(['codezip', 'container']); export const CredentialType = z.enum(['api-key', 'oauth']); export const EvaluatorType = z.enum(['llm-as-a-judge', 'code-based']); -export const ExitReason = z.enum(['success', 'failure', 'cancel']); +export const ExitReason = z.enum(['success', 'failure']); export const FilterState = z.enum(['deployed', 'local-only', 'pending-removal', 'none']); export const FilterType = z.enum([ 'agent', @@ -113,13 +113,12 @@ export const ErrorCategory = z.enum([ // Common result shapes — reusable across metrics export const SuccessResult = z.object({ exit_reason: z.literal('success') }); -export const CancelResult = z.object({ exit_reason: z.literal('cancel') }); export const FailureResult = z.object({ exit_reason: z.literal('failure'), error_name: ErrorCategory, is_user_error: z.boolean(), }); -export const CommandResultSchema = z.discriminatedUnion('exit_reason', [SuccessResult, CancelResult, FailureResult]); +export const CommandResultSchema = z.discriminatedUnion('exit_reason', [SuccessResult, FailureResult]); export type CommandResult = z.infer; export const DeployModeSchema = z.enum(['deploy', 'dry-run', 'diff']); diff --git a/src/cli/telemetry/schemas/index.ts b/src/cli/telemetry/schemas/index.ts index 21793c93b..14ae632a9 100644 --- a/src/cli/telemetry/schemas/index.ts +++ b/src/cli/telemetry/schemas/index.ts @@ -7,7 +7,6 @@ export { FailureResult, Mode, SuccessResult, - CancelResult, type CommandResult, } from './common-shapes.js'; export { ResourceAttributesSchema, type ResourceAttributes } from './common-attributes.js'; From 1021dc3f930a3be9c280bb3d2c3e74b7d08cd68a Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 13:27:28 +0000 Subject: [PATCH 10/15] =?UTF-8?q?fix:=20rename=20stale=20mode=E2=86=92depl?= =?UTF-8?q?oy=5Fmode=20in=20useDeployFlow=20diff=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/cli/tui/screens/deploy/useDeployFlow.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/tui/screens/deploy/useDeployFlow.ts b/src/cli/tui/screens/deploy/useDeployFlow.ts index e65aa77f5..cdeff0915 100644 --- a/src/cli/tui/screens/deploy/useDeployFlow.ts +++ b/src/cli/tui/screens/deploy/useDeployFlow.ts @@ -747,7 +747,7 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState const attrs = context ? computeDeployAttrs(context.projectSpec, 'diff') - : { ...DEFAULT_DEPLOY_ATTRS, mode: 'diff' as const }; + : { ...DEFAULT_DEPLOY_ATTRS, deploy_mode: 'diff' as const }; const run = async (): Promise<{ success: true } | { success: false; error: Error }> => { setDiffStep(prev => ({ ...prev, status: 'running' })); From 36f3c36bb22ee216ecc51dae95c6fe583acdb03f Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 13:30:15 +0000 Subject: [PATCH 11/15] test: add coverage for callback-throws path in withCommandRunTelemetry --- src/cli/telemetry/__tests__/client.test.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index 4722878a6..54242ccaa 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -157,4 +157,26 @@ describe('withCommandRunTelemetry', () => { expect(result).toEqual({ success: true }); expect(sink.metrics).toHaveLength(0); }); + + it('records failure and returns error result when callback throws', async () => { + type R = { success: true } | { success: false; error: Error }; + const result = await withCommandRunTelemetry<'telemetry.disable', R>( + 'telemetry.disable', + {}, + async (): Promise => { + throw new Error('network timeout'); + } + ); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.message).toBe('network timeout'); + } + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + command: 'telemetry.disable', + exit_reason: 'failure', + error_name: 'UnknownError', + }); + }); }); From bed69e46823c130a7dd3d3ddac1bc3d905bb82a6 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Thu, 14 May 2026 13:45:32 +0000 Subject: [PATCH 12/15] docs: rewrite telemetry README to match implementation --- src/cli/telemetry/README.md | 70 +++++++++---------------------------- 1 file changed, 17 insertions(+), 53 deletions(-) diff --git a/src/cli/telemetry/README.md b/src/cli/telemetry/README.md index cd6dcae6c..cab2322e1 100644 --- a/src/cli/telemetry/README.md +++ b/src/cli/telemetry/README.md @@ -1,22 +1,5 @@ # Telemetry -## Architecture - -``` -ATTRIBUTES (common-shapes.ts) METRICS registry (registry.ts) - │ │ - ▼ ▼ - COMMAND_SCHEMAS (command-run.ts) TelemetryClient.emit() - │ │ - ▼ ▼ - withCommandRunTelemetry / runCliCommand MetricSink (otel, filesystem, in-memory) -``` - -- `TelemetryClient` — generic emitter. Knows nothing about specific metrics. -- `cli-command-run.ts` — command_run-specific logic (timing, error classification). -- `schemas/registry.ts` — all metric definitions. `MetricName` type derived from here. -- `schemas/common-shapes.ts` — all attribute definitions in `ATTRIBUTES` namespace. - ## Adding a New Metric ### 1. Define attributes in `schemas/common-shapes.ts` @@ -27,27 +10,30 @@ Skip if reusing existing attributes. export const ToolName = z.enum(['read_file', 'write_file', 'search']); ``` -Add to the `ATTRIBUTES` object: +Add to the `ATTRIBUTES` object using the field name as the key: ```ts export const ATTRIBUTES = { // ...existing - ToolName, + tool_name: ToolName, } as const; ``` ### 2. Register the metric in `schemas/registry.ts` +Add an entry to `METRICS` and a corresponding `MetricAttrs` branch: + ```ts export const METRICS = { - 'cli.command_run': { ... }, - 'cli.mcp_tool_call': { - schema: safeSchema({ - tool_name: ATTRIBUTES.ToolName, - success: z.boolean(), - }), - }, + 'cli.command_run': {}, + 'cli.mcp_tool_call': {}, } as const; + +export type MetricAttrs = M extends 'cli.command_run' + ? CommandRunAttrs + : M extends 'cli.mcp_tool_call' + ? { tool_name: z.infer; success: boolean } + : never; ``` ### 3. Emit it @@ -56,24 +42,13 @@ export const METRICS = { client.emit('cli.mcp_tool_call', durationMs, { tool_name: 'read_file', success: true }); ``` -Wrong metric name or missing/invalid attrs = compile error. +Wrong metric name or missing attrs = compile error. --- ## Adding a New Command (to `cli.command_run`) -### 1. Add the command name to `Command` enum in `schemas/common-shapes.ts` - -```ts -export const Command = z.enum([ - // ...existing - 'add.widget', -]); -``` - -If it introduces a new group, add to `CommandGroup` too. - -### 2. Define the command's attribute schema in `schemas/command-run.ts` +### 1. Define the command's attribute schema in `schemas/command-run.ts` ```ts const AddWidgetAttrs = safeSchema({ @@ -88,21 +63,10 @@ Add to `COMMAND_SCHEMAS`: 'add.widget': AddWidgetAttrs, ``` -Compile error if the key doesn't match the `Command` enum. - -### 3. Add the new attributes as optional fields in `schemas/registry.ts` - -```ts -const CommandRunSchema = safeSchema({ ... }).extend({ - // ...existing - widget_type: ATTRIBUTES.WidgetType.optional(), - count: z.number().optional(), -}); -``` - -This ensures `emit()` accepts the new fields with compile-time checking. +The `Command` type and optional fields in `MetricAttrs<'cli.command_run'>` are derived automatically from +`COMMAND_SCHEMAS`. -### 4. Instrument the handler +### 2. Instrument the handler Use `withCommandRunTelemetry`: From 4f4c48fabe0a5fc7834a57012345e873cdbbb6c6 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Fri, 15 May 2026 13:16:50 +0000 Subject: [PATCH 13/15] fix: wrap no-client case in try-catch for consistency with other case --- src/cli/telemetry/cli-command-run.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 4a4c99f13..780a7f6a1 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -75,7 +75,8 @@ async function trackCommandRun( * * If the callback returns a failure result, telemetry is recorded and the result * is returned to the caller. If the callback throws, telemetry is recorded and - * the exception propagates. If telemetry is unavailable, the callback runs untracked. + * the exception is converted to a result type such that callers do not need to handle result + try/catch. + * If telemetry is unavailable, the callback runs untracked. */ export async function withCommandRunTelemetry( command: C, @@ -83,10 +84,10 @@ export async function withCommandRunTelemetry R | Promise ): Promise { const client = await getTelemetryClient(); - if (!client) return fn(); let result: R | undefined; try { + if (!client) return fn(); await trackCommandRun( client, command, From 7702fceac8578cdfc8a34b1081162302e0300649 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Fri, 15 May 2026 13:19:54 +0000 Subject: [PATCH 14/15] docs: update outdated docstring --- src/cli/telemetry/schemas/common-shapes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 8bfdb666e..7e6bf727b 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -30,7 +30,7 @@ export function resilientParse( * Lowercase a CLI value and parse it through a Zod enum, returning the narrowed type. * The `as` cast on the failure branch is intentional: invalid values pass through to * recordCommandRun, where COMMAND_SCHEMAS[command].parse(attrs) validates the full - * attr object in a try/catch — silently dropping the metric if any field is invalid. + * attr object with resilient parsing. * This ensures telemetry never crashes the CLI while keeping the happy-path type-safe. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any From 4a2b6ea1139962f08228b523ea4e4f9f43b21b53 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Sat, 16 May 2026 00:14:19 +0000 Subject: [PATCH 15/15] feat(telemetry): add MetricRegistry type with descriptions --- src/cli/telemetry/README.md | 8 ++++---- src/cli/telemetry/schemas/registry.ts | 11 +++++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/cli/telemetry/README.md b/src/cli/telemetry/README.md index cab2322e1..6750c3108 100644 --- a/src/cli/telemetry/README.md +++ b/src/cli/telemetry/README.md @@ -21,13 +21,13 @@ export const ATTRIBUTES = { ### 2. Register the metric in `schemas/registry.ts` -Add an entry to `METRICS` and a corresponding `MetricAttrs` branch: +Add an entry to `METRICS` with a description, and a corresponding `MetricAttrs` branch: ```ts export const METRICS = { - 'cli.command_run': {}, - 'cli.mcp_tool_call': {}, -} as const; + 'cli.command_run': { description: 'CLI/TUI Command Execution' }, + 'cli.mcp_tool_call': { description: 'MCP tool invocation' }, +} as const satisfies MetricRegistry; export type MetricAttrs = M extends 'cli.command_run' ? CommandRunAttrs diff --git a/src/cli/telemetry/schemas/registry.ts b/src/cli/telemetry/schemas/registry.ts index 30e2871ed..5f547b0e8 100644 --- a/src/cli/telemetry/schemas/registry.ts +++ b/src/cli/telemetry/schemas/registry.ts @@ -22,9 +22,16 @@ type CommandRunAttrs = { error_source?: z.infer; } & MergedCommandAttrs; +interface MetricRegistryItem { + description?: string; +} +type MetricRegistry = Record; + export const METRICS = { - 'cli.command_run': {}, -} as const; + 'cli.command_run': { + description: 'CLI/TUI Command Execution', + }, +} as const satisfies MetricRegistry; export type MetricName = keyof typeof METRICS; export type MetricAttrs = M extends 'cli.command_run' ? CommandRunAttrs : never;