diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index e254524bf..96adebafc 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -116,18 +116,49 @@ describe('TelemetryClient', () => { expect(sink.metrics[0]!.attrs.check_only).toBe('true'); }); - it('silently drops invalid success payloads', async () => { + it('publishes metric with unknown defaults for incomplete success payloads', async () => { const sink = new InMemorySink(); const client = new TelemetryClient(sink); - // Missing required attrs for 'create' — should silently drop + // 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(0); + 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 client.withCommandRun( + '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 () => { diff --git a/src/cli/telemetry/client.ts b/src/cli/telemetry/client.ts index 3228f45b1..91dffd94f 100644 --- a/src/cli/telemetry/client.ts +++ b/src/cli/telemetry/client.ts @@ -1,6 +1,6 @@ import { classifyError, isUserError } from './error-classification.js'; import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from './schemas/command-run.js'; -import { type CommandResult, CommandResultSchema } from './schemas/common-shapes.js'; +import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js'; import type { MetricSink } from './sinks/metric-sink.js'; import { performance } from 'perf_hooks'; @@ -69,17 +69,24 @@ export class TelemetryClient { durationMs: number ): void { try { + // CommandResult is built internally — hard parse is intentional since + // a metric without a valid exit_reason is meaningless. CommandResultSchema.parse(result); - if (result.exit_reason !== 'failure' && result.exit_reason !== 'cancel') { - COMMAND_SCHEMAS[command].parse(attrs); - } + + // Validate command attrs resiliently: invalid fields default to 'unknown' + // instead of dropping the entire metric. + // On failure/cancel the callback attrs are empty so validation is skipped. + const validatedAttrs = + result.exit_reason !== 'failure' && result.exit_reason !== 'cancel' + ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) + : attrs; const otelAttrs: Record = { command_group: deriveCommandGroup(command), command, }; - for (const obj of [result, attrs]) { + for (const obj of [result, validatedAttrs]) { for (const [k, v] of Object.entries(obj)) { if (typeof v === 'boolean') { otelAttrs[k] = String(v); diff --git a/src/cli/telemetry/schemas/__tests__/command-run.test.ts b/src/cli/telemetry/schemas/__tests__/command-run.test.ts index 11d293c71..110df1284 100644 --- a/src/cli/telemetry/schemas/__tests__/command-run.test.ts +++ b/src/cli/telemetry/schemas/__tests__/command-run.test.ts @@ -1,6 +1,6 @@ import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from '../command-run'; import { ResourceAttributesSchema } from '../common-attributes'; -import { CommandResultSchema } from '../common-shapes'; +import { CommandResultSchema, resilientParse } from '../common-shapes'; import { describe, expect, expectTypeOf, it } from 'vitest'; import { z } from 'zod'; @@ -170,3 +170,55 @@ describe('type safety', () => { } }); }); + +describe('resilientParse', () => { + it('passes valid attrs through unchanged', () => { + const attrs = { + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + memory: 'shortterm', + protocol: 'mcp', + build: 'codezip', + agent_type: 'create', + network_mode: 'public', + has_agent: true, + }; + expect(resilientParse(COMMAND_SCHEMAS.create, attrs)).toEqual(attrs); + }); + + it('defaults a single invalid enum field to unknown', () => { + const attrs = { + language: 'rust', // invalid + framework: 'strands', + model_provider: 'bedrock', + memory: 'shortterm', + protocol: 'mcp', + build: 'codezip', + agent_type: 'create', + network_mode: 'public', + has_agent: true, + }; + const result = resilientParse(COMMAND_SCHEMAS.create, attrs); + expect(result.language).toBe('unknown'); + expect(result.framework).toBe('strands'); + }); + + it('defaults missing required fields to unknown', () => { + const result = resilientParse(COMMAND_SCHEMAS.create, { language: 'python' }); + expect(result.language).toBe('python'); + expect(result.framework).toBe('unknown'); + expect(result.model_provider).toBe('unknown'); + }); + + it('defaults all fields to unknown when all are invalid', () => { + const result = resilientParse(COMMAND_SCHEMAS.create, {}); + for (const value of Object.values(result)) { + expect(value).toBe('unknown'); + } + }); + + it('returns empty object for no-attrs schemas', () => { + expect(resilientParse(COMMAND_SCHEMAS['telemetry.disable'], {})).toEqual({}); + }); +}); diff --git a/src/cli/telemetry/schemas/common-shapes.ts b/src/cli/telemetry/schemas/common-shapes.ts index 5c5e56493..732bb3d61 100644 --- a/src/cli/telemetry/schemas/common-shapes.ts +++ b/src/cli/telemetry/schemas/common-shapes.ts @@ -8,6 +8,24 @@ export function safeSchema>(shape: T) { return z.object(shape); } +/** + * Validate each field in a schema individually, defaulting to 'unknown' on failure. + * This ensures a single invalid attribute never blocks the entire metric from being published. + * Keys in attrs not present in the schema are omitted from the result. + */ +export function resilientParse( + schema: z.ZodObject, + attrs: Record +): Record { + const result: Record = {}; + for (const key of Object.keys(schema.shape)) { + const field = schema.shape[key] as z.ZodType; + const parsed = field.safeParse(attrs[key]); + result[key] = parsed.success ? parsed.data : 'unknown'; + } + return result; +} + // Primitive types export const Count = z.number().int().nonnegative();