Skip to content

Commit f69e252

Browse files
authored
feat: record command attrs on telemetry failure via fallbackAttrs (#1204)
* feat: record command attrs on telemetry failure via fallbackAttrs Add optional fallbackAttrs parameter to client.withCommandRun so command-specific attributes are recorded even when the callback throws. - client.ts: accept fallbackAttrs, use on failure instead of {} - client.ts: run resilientParse on all non-empty attrs (not just success) - cli-command-run.ts: withCommandRunTelemetry passes attrs as fallbackAttrs - cli-command-run.ts: runCliCommand accepts optional knownAttrs param - command.tsx: extract knownAttrs upfront, pass to runCliCommand - client.test.ts: add unit tests for fallbackAttrs behavior - create-edge-cases.test.ts: assert attrs present on failure entry * chore: rebase onto mainline
1 parent f010c12 commit f69e252

6 files changed

Lines changed: 174 additions & 93 deletions

File tree

integ-tests/create-edge-cases.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create edge cases',
3838
telemetry.assertMetricEmitted({
3939
command: 'create',
4040
exit_reason: 'failure',
41+
language: 'python',
42+
has_agent: 'true',
4143
});
4244
});
4345

src/cli/commands/create/command.tsx

Lines changed: 86 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -114,92 +114,99 @@ async function handleCreateCLI(options: CreateOptions): Promise<void> {
114114
process.exit(0);
115115
}
116116

117-
await runCliCommand('create', !!options.json, async () => {
118-
const validation = validateCreateOptions(options, cwd);
119-
if (!validation.valid) {
120-
throw new Error(validation.error);
121-
}
122-
const green = '\x1b[32m';
123-
const reset = '\x1b[0m';
117+
const knownAttrs = {
118+
language: standardize(Language, options.language),
119+
framework: standardize(Framework, options.framework),
120+
model_provider: standardize(ModelProviderEnum, options.modelProvider),
121+
memory: standardize(Memory, options.memory ?? 'none'),
122+
protocol: standardize(Protocol, options.protocol ?? 'http'),
123+
build: standardize(Build, options.build ?? 'codezip'),
124+
agent_type: standardize(AgentType, options.type ?? 'create'),
125+
network_mode: standardize(NetworkModeEnum, options.networkMode ?? 'public'),
126+
has_agent: options.agent !== false,
127+
};
124128

125-
// Progress callback for real-time output
126-
const onProgress: ProgressCallback | undefined = options.json
127-
? undefined
128-
: (step, status) => {
129-
if (status === 'done') {
130-
console.log(`${green}[done]${reset} ${step}`);
131-
} else if (status === 'error') {
132-
console.log(`\x1b[31m[error]${reset} ${step}`);
133-
}
134-
// 'start' is silent - we only show when done
135-
};
129+
await runCliCommand(
130+
'create',
131+
!!options.json,
132+
async () => {
133+
const validation = validateCreateOptions(options, cwd);
134+
if (!validation.valid) {
135+
throw new Error(validation.error);
136+
}
137+
const green = '\x1b[32m';
138+
const reset = '\x1b[0m';
136139

137-
// Commander.js --no-agent sets agent=false, not noAgent=true
138-
const skipAgent = options.agent === false;
140+
// Progress callback for real-time output
141+
const onProgress: ProgressCallback | undefined = options.json
142+
? undefined
143+
: (step, status) => {
144+
if (status === 'done') {
145+
console.log(`${green}[done]${reset} ${step}`);
146+
} else if (status === 'error') {
147+
console.log(`\x1b[31m[error]${reset} ${step}`);
148+
}
149+
// 'start' is silent - we only show when done
150+
};
139151

140-
const result = skipAgent
141-
? await createProject({
142-
name: projectName!,
143-
cwd,
144-
skipGit: options.skipGit,
145-
skipInstall: options.skipInstall,
146-
onProgress,
147-
})
148-
: await createProjectWithAgent({
149-
name: name!,
150-
projectName,
151-
cwd,
152-
type: options.type as 'create' | 'import' | undefined,
153-
buildType: (options.build as BuildType) ?? 'CodeZip',
154-
language: (options.language as TargetLanguage) ?? (options.type === 'import' ? 'Python' : undefined),
155-
framework: options.framework as SDKFramework | undefined,
156-
modelProvider: options.modelProvider as ModelProvider | undefined,
157-
apiKey: options.apiKey,
158-
memory: (options.memory as 'none' | 'shortTerm' | 'longAndShortTerm') ?? 'none',
159-
protocol: options.protocol as ProtocolMode | undefined,
160-
agentId: options.agentId,
161-
agentAliasId: options.agentAliasId,
162-
region: options.region,
163-
networkMode: options.networkMode as NetworkMode | undefined,
164-
subnets: parseCommaSeparatedList(options.subnets),
165-
securityGroups: parseCommaSeparatedList(options.securityGroups),
166-
idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined,
167-
maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined,
168-
sessionStorageMountPath: options.sessionStorageMountPath,
169-
withConfigBundle: options.withConfigBundle,
170-
skipGit: options.skipGit,
171-
skipInstall: options.skipInstall,
172-
skipPythonSetup: options.skipPythonSetup,
173-
onProgress,
174-
});
152+
// Commander.js --no-agent sets agent=false, not noAgent=true
153+
const skipAgent = options.agent === false;
175154

176-
if (!result.success) {
177-
throw result.error;
178-
}
155+
const result = skipAgent
156+
? await createProject({
157+
name: projectName!,
158+
cwd,
159+
skipGit: options.skipGit,
160+
skipInstall: options.skipInstall,
161+
onProgress,
162+
})
163+
: await createProjectWithAgent({
164+
name: name!,
165+
projectName,
166+
cwd,
167+
type: options.type as 'create' | 'import' | undefined,
168+
buildType: (options.build as BuildType) ?? 'CodeZip',
169+
language: (options.language as TargetLanguage) ?? (options.type === 'import' ? 'Python' : undefined),
170+
framework: options.framework as SDKFramework | undefined,
171+
modelProvider: options.modelProvider as ModelProvider | undefined,
172+
apiKey: options.apiKey,
173+
memory: (options.memory as 'none' | 'shortTerm' | 'longAndShortTerm') ?? 'none',
174+
protocol: options.protocol as ProtocolMode | undefined,
175+
agentId: options.agentId,
176+
agentAliasId: options.agentAliasId,
177+
region: options.region,
178+
networkMode: options.networkMode as NetworkMode | undefined,
179+
subnets: parseCommaSeparatedList(options.subnets),
180+
securityGroups: parseCommaSeparatedList(options.securityGroups),
181+
idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined,
182+
maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined,
183+
sessionStorageMountPath: options.sessionStorageMountPath,
184+
withConfigBundle: options.withConfigBundle,
185+
skipGit: options.skipGit,
186+
skipInstall: options.skipInstall,
187+
skipPythonSetup: options.skipPythonSetup,
188+
onProgress,
189+
});
179190

180-
if (options.json) {
181-
console.log(JSON.stringify(serializeResult(result)));
182-
} else {
183-
printCreateSummary(projectName!, result.agentName, options.language, options.framework);
184-
if (options.skipInstall) {
185-
console.log(
186-
"\nDependency installation was skipped. Run 'npm install' in agentcore/cdk/ and 'uv sync' in your agent directory manually."
187-
);
191+
if (!result.success) {
192+
throw result.error;
188193
}
189-
}
190194

191-
return {
192-
language: standardize(Language, options.language),
193-
framework: standardize(Framework, options.framework),
194-
model_provider: standardize(ModelProviderEnum, options.modelProvider),
195-
memory: standardize(Memory, options.memory ?? 'none'),
196-
protocol: standardize(Protocol, options.protocol ?? 'http'),
197-
build: standardize(Build, options.build ?? 'codezip'),
198-
agent_type: standardize(AgentType, options.type ?? 'create'),
199-
network_mode: standardize(NetworkModeEnum, options.networkMode ?? 'public'),
200-
has_agent: options.agent !== false,
201-
};
202-
});
195+
if (options.json) {
196+
console.log(JSON.stringify(result));
197+
} else {
198+
printCreateSummary(projectName!, result.agentName, options.language, options.framework);
199+
if (options.skipInstall) {
200+
console.log(
201+
"\nDependency installation was skipped. Run 'npm install' in agentcore/cdk/ and 'uv sync' in your agent directory manually."
202+
);
203+
}
204+
}
205+
206+
return knownAttrs;
207+
},
208+
knownAttrs
209+
);
203210
}
204211

205212
export const registerCreate = (program: Command) => {

src/cli/telemetry/README.md

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@ async function withCommandRunTelemetry<C extends Command, R extends OperationRes
6262

6363
**Behavior:**
6464

65-
- On success (`{ success: true }`): records success telemetry with `attrs`, returns the result.
66-
- On failure (`{ success: false, error }`): records failure telemetry, returns the result to the caller.
67-
- On throw: records failure telemetry, returns `{ success: false, error }` so callers don't leak unhandled rejections.
65+
- On success: records `attrs` with `exit_reason: 'success'`, returns the result.
66+
- On failure/throw: records `attrs` with `exit_reason: 'failure'`, returns `{ success: false, error }`.
6867
- If telemetry is unavailable: runs `fn()` untracked.
6968

69+
Since `attrs` are passed upfront, they are always recorded — even on failure.
70+
7071
**Example with attributes:**
7172

7273
```ts
@@ -95,6 +96,22 @@ await runCliCommand('add.widget', !!options.json, async () => {
9596
});
9697
```
9798

99+
To record attrs on failure, pass `knownAttrs` as the fourth argument:
100+
101+
```ts
102+
const knownAttrs = { widget_type: standardize(WidgetType, options.type), count: options.items.length };
103+
await runCliCommand(
104+
'add.widget',
105+
!!options.json,
106+
async () => {
107+
const result = await widgetPrimitive.add(options);
108+
if (!result.success) throw new Error(result.error);
109+
return knownAttrs;
110+
},
111+
knownAttrs
112+
);
113+
```
114+
98115
## Key Points
99116

100117
- Telemetry never crashes the CLI — `standardize()` falls back gracefully, `resilientParse` defaults invalid fields to

src/cli/telemetry/__tests__/client.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,5 +173,54 @@ describe('TelemetryClient', () => {
173173
exit_reason: 'cancel',
174174
});
175175
});
176+
177+
it('records fallbackAttrs on failure when provided', async () => {
178+
const sink = new InMemorySink();
179+
const client = new TelemetryClient(sink);
180+
181+
await expect(
182+
client.withCommandRun(
183+
'create',
184+
async () => {
185+
throw new Error('validation failed');
186+
},
187+
{
188+
language: 'python',
189+
framework: 'strands',
190+
model_provider: 'bedrock',
191+
memory: 'none',
192+
protocol: 'http',
193+
build: 'codezip',
194+
agent_type: 'create',
195+
network_mode: 'public',
196+
has_agent: true,
197+
}
198+
)
199+
).rejects.toThrow('validation failed');
200+
201+
expect(sink.metrics).toHaveLength(1);
202+
expect(sink.metrics[0]!.attrs).toMatchObject({
203+
exit_reason: 'failure',
204+
error_name: 'UnknownError',
205+
language: 'python',
206+
framework: 'strands',
207+
model_provider: 'bedrock',
208+
has_agent: 'true',
209+
});
210+
});
211+
212+
it('records empty attrs on failure when fallbackAttrs not provided', async () => {
213+
const sink = new InMemorySink();
214+
const client = new TelemetryClient(sink);
215+
216+
await expect(
217+
client.withCommandRun('deploy', async () => {
218+
throw new Error('boom');
219+
})
220+
).rejects.toThrow('boom');
221+
222+
expect(sink.metrics).toHaveLength(1);
223+
expect(sink.metrics[0]!.attrs.language).toBeUndefined();
224+
});
176225
});
177226
});

src/cli/telemetry/cli-command-run.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,15 @@ export async function withCommandRunTelemetry<C extends Command, R extends Opera
3131

3232
let result: R | undefined;
3333
try {
34-
await client.withCommandRun(command, async () => {
35-
result = await fn();
36-
if (!result.success) throw result.error;
37-
return attrs;
38-
});
34+
await client.withCommandRun(
35+
command,
36+
async () => {
37+
result = await fn();
38+
if (!result.success) throw result.error;
39+
return attrs;
40+
},
41+
attrs
42+
);
3943
} catch (e) {
4044
// withCommandRun re-throws after recording failure telemetry.
4145
// If result was set, fn() returned a failure result — return it directly.
@@ -52,19 +56,21 @@ export async function withCommandRunTelemetry<C extends Command, R extends Opera
5256
* Record telemetry, print errors, and exit the process.
5357
* Use in CLI command handlers where the command is the final action.
5458
* The callback returns attrs on success and throws on failure.
59+
* Pass knownAttrs to record command-specific attributes even on failure.
5560
*/
5661
export async function runCliCommand<C extends Command>(
5762
command: C,
5863
json: boolean,
59-
fn: () => Promise<CommandAttrs<C>>
64+
fn: () => Promise<CommandAttrs<C>>,
65+
knownAttrs?: Partial<CommandAttrs<C>>
6066
): Promise<never> {
6167
try {
6268
const client = await getTelemetryClient();
6369
if (!client) {
6470
await fn();
6571
process.exit(0);
6672
}
67-
await client.withCommandRun(command, fn);
73+
await client.withCommandRun(command, fn, knownAttrs);
6874
process.exit(0);
6975
} catch (error) {
7076
if (json) {

src/cli/telemetry/client.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ export class TelemetryClient {
2626
*/
2727
async withCommandRun<C extends Command>(
2828
command: C,
29-
fn: () => CommandAttrs<C> | typeof CANCELLED | Promise<CommandAttrs<C> | typeof CANCELLED>
29+
fn: () => CommandAttrs<C> | typeof CANCELLED | Promise<CommandAttrs<C> | typeof CANCELLED>,
30+
fallbackAttrs?: Partial<CommandAttrs<C>>
3031
): Promise<void> {
3132
const start = performance.now();
3233
try {
@@ -43,7 +44,7 @@ export class TelemetryClient {
4344
error_name: classifyError(err),
4445
is_user_error: isUserError(err),
4546
};
46-
this.recordCommandRun(command, failureResult, {}, Math.round(performance.now() - start));
47+
this.recordCommandRun(command, failureResult, fallbackAttrs ?? {}, Math.round(performance.now() - start));
4748
throw err;
4849
} finally {
4950
try {
@@ -75,9 +76,8 @@ export class TelemetryClient {
7576

7677
// Validate command attrs resiliently: invalid fields default to 'unknown'
7778
// instead of dropping the entire metric.
78-
// On failure/cancel the callback attrs are empty so validation is skipped.
7979
const validatedAttrs =
80-
result.exit_reason !== 'failure' && result.exit_reason !== 'cancel'
80+
Object.keys(attrs as Record<string, unknown>).length > 0
8181
? resilientParse(COMMAND_SCHEMAS[command], attrs as Record<string, unknown>)
8282
: attrs;
8383

0 commit comments

Comments
 (0)