Skip to content

Commit 951cf98

Browse files
HweinstockHweinstock
committed
feat: wire telemetry into all remove.* commands
1 parent 90924d6 commit 951cf98

27 files changed

Lines changed: 198 additions & 112 deletions

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,4 @@ ProtocolTesting/
7373
browser-tests/.browser-test-env
7474
browser-tests/test-results/
7575
browser-tests/playwright-report/
76+
src/cli/primitives/.BasePrimitive.ts.swp

‎integ-tests/add-remove-gateway.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { createTestProject, runCLI } from '../src/test-utils/index.js';
22
import type { TestProject } from '../src/test-utils/index.js';
3+
import { createTelemetryHelper } from '../src/test-utils/telemetry-helper.js';
34
import { mkdir, readFile, writeFile } from 'node:fs/promises';
45
import { join } from 'node:path';
56
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
67

8+
const telemetry = createTelemetryHelper();
9+
710
async function readProjectConfig(projectPath: string) {
811
return JSON.parse(await readFile(join(projectPath, 'agentcore/agentcore.json'), 'utf-8'));
912
}
@@ -19,6 +22,7 @@ describe('integration: add and remove gateway with external MCP server', () => {
1922

2023
afterAll(async () => {
2124
await project.cleanup();
25+
telemetry.destroy();
2226
});
2327

2428
describe('gateway lifecycle', () => {
@@ -64,7 +68,9 @@ describe('integration: add and remove gateway with external MCP server', () => {
6468
});
6569

6670
it('removes the gateway target', async () => {
67-
const result = await runCLI(['remove', 'gateway-target', '--name', targetName, '--json'], project.projectPath);
71+
const result = await runCLI(['remove', 'gateway-target', '--name', targetName, '--json'], project.projectPath, {
72+
env: telemetry.env,
73+
});
6874

6975
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
7076
const json = JSON.parse(result.stdout);
@@ -75,10 +81,13 @@ describe('integration: add and remove gateway with external MCP server', () => {
7581
const targets = gateway?.targets ?? [];
7682
const found = targets.find((t: { name: string }) => t.name === targetName);
7783
expect(found, `Target "${targetName}" should be removed`).toBeFalsy();
84+
telemetry.assertMetricEmitted({ command: 'remove.gateway-target', exit_reason: 'success' });
7885
});
7986

8087
it('removes the gateway', async () => {
81-
const result = await runCLI(['remove', 'gateway', '--name', gatewayName, '--json'], project.projectPath);
88+
const result = await runCLI(['remove', 'gateway', '--name', gatewayName, '--json'], project.projectPath, {
89+
env: telemetry.env,
90+
});
8291

8392
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
8493
const json = JSON.parse(result.stdout);
@@ -88,6 +97,7 @@ describe('integration: add and remove gateway with external MCP server', () => {
8897
const gateways = mcpSpec.agentCoreGateways ?? [];
8998
const found = gateways.find((g: { name: string }) => g.name === gatewayName);
9099
expect(found, `Gateway "${gatewayName}" should be removed`).toBeFalsy();
100+
telemetry.assertMetricEmitted({ command: 'remove.gateway', exit_reason: 'success' });
91101
});
92102
});
93103
});

‎integ-tests/add-remove-resources.test.ts‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ describe('integration: add and remove resources', () => {
4141
const found = memories!.some((m: Record<string, unknown>) => m.name === memoryName);
4242
expect(found, `Memory "${memoryName}" should be in config`).toBe(true);
4343

44-
// Verify telemetry
4544
telemetry.assertMetricEmitted({ command: 'add.memory', exit_reason: 'success' });
4645
});
4746

@@ -71,7 +70,6 @@ describe('integration: add and remove resources', () => {
7170
expect(episodic!.reflectionNamespaces, 'Should have reflectionNamespaces').toBeDefined();
7271
expect(episodic!.reflectionNamespaces!.length).toBeGreaterThan(0);
7372

74-
// Verify telemetry
7573
telemetry.assertMetricEmitted({
7674
command: 'add.memory',
7775
exit_reason: 'success',
@@ -84,7 +82,9 @@ describe('integration: add and remove resources', () => {
8482
});
8583

8684
it('removes the memory resource', async () => {
87-
const result = await runCLI(['remove', 'memory', '--name', memoryName, '--json'], project.projectPath);
85+
const result = await runCLI(['remove', 'memory', '--name', memoryName, '--json'], project.projectPath, {
86+
env: telemetry.env,
87+
});
8888

8989
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
9090
const json = JSON.parse(result.stdout);
@@ -95,6 +95,8 @@ describe('integration: add and remove resources', () => {
9595
const memories = (config.memories as Record<string, unknown>[] | undefined) ?? [];
9696
const found = memories.some((m: Record<string, unknown>) => m.name === memoryName);
9797
expect(found, `Memory "${memoryName}" should be removed from config`).toBe(false);
98+
99+
telemetry.assertMetricEmitted({ command: 'remove.memory', exit_reason: 'success' });
98100
});
99101
});
100102

@@ -119,7 +121,6 @@ describe('integration: add and remove resources', () => {
119121
const found = credentials!.some((c: Record<string, unknown>) => c.name === credentialName);
120122
expect(found, `Credential "${credentialName}" should be in config`).toBe(true);
121123

122-
// Verify telemetry
123124
telemetry.assertMetricEmitted({
124125
command: 'add.credential',
125126
exit_reason: 'success',
@@ -128,7 +129,9 @@ describe('integration: add and remove resources', () => {
128129
});
129130

130131
it('removes the credential resource', async () => {
131-
const result = await runCLI(['remove', 'credential', '--name', credentialName, '--json'], project.projectPath);
132+
const result = await runCLI(['remove', 'credential', '--name', credentialName, '--json'], project.projectPath, {
133+
env: telemetry.env,
134+
});
132135

133136
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
134137
const json = JSON.parse(result.stdout);
@@ -139,6 +142,8 @@ describe('integration: add and remove resources', () => {
139142
const credentials = (config.credentials as Record<string, unknown>[] | undefined) ?? [];
140143
const found = credentials.some((c: Record<string, unknown>) => c.name === credentialName);
141144
expect(found, `Credential "${credentialName}" should be removed from config`).toBe(false);
145+
146+
telemetry.assertMetricEmitted({ command: 'remove.credential', exit_reason: 'success' });
142147
});
143148
});
144149

@@ -162,9 +167,37 @@ describe('integration: add and remove resources', () => {
162167
});
163168

164169
it('removes the policy engine resource', async () => {
165-
const result = await runCLI(['remove', 'policy-engine', '--name', engineName, '--json'], project.projectPath);
170+
const result = await runCLI(['remove', 'policy-engine', '--name', engineName, '--json'], project.projectPath, {
171+
env: telemetry.env,
172+
});
166173

167174
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
175+
176+
telemetry.assertMetricEmitted({ command: 'remove.policy-engine', exit_reason: 'success' });
177+
});
178+
});
179+
180+
describe('remove failure telemetry', () => {
181+
it('emits failure telemetry for non-existent resource', async () => {
182+
const result = await runCLI(['remove', 'memory', '--name', 'DoesNotExist', '--json'], project.projectPath, {
183+
env: telemetry.env,
184+
});
185+
186+
expect(result.exitCode).toBe(1);
187+
telemetry.assertMetricEmitted({ command: 'remove.memory', exit_reason: 'failure' });
188+
});
189+
});
190+
191+
describe('remove all', () => {
192+
it('resets all schemas and emits telemetry', async () => {
193+
const result = await runCLI(['remove', 'all', '--yes', '--json'], project.projectPath, {
194+
env: telemetry.env,
195+
});
196+
197+
expect(result.exitCode).toBe(0);
198+
const json = JSON.parse(result.stdout);
199+
expect(json.success).toBe(true);
200+
telemetry.assertMetricEmitted({ command: 'remove.all', exit_reason: 'success' });
168201
});
169202
});
170203
});

‎src/cli/commands/remove/command.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ConfigIO } from '../../../lib';
22
import { getErrorMessage } from '../../errors';
3+
import { runCliCommand } from '../../telemetry/cli-command-run.js';
34
import { COMMAND_DESCRIPTIONS } from '../../tui/copy';
45
import { requireProject, requireTTY } from '../../tui/guards';
56
import { RemoveAllScreen, RemoveFlow } from '../../tui/screens/remove';
@@ -54,9 +55,12 @@ async function handleRemoveAll(_options: RemoveAllOptions): Promise<RemoveResult
5455

5556
async function handleRemoveAllCLI(options: RemoveAllOptions): Promise<void> {
5657
validateRemoveAllOptions(options);
57-
const result = await handleRemoveAll(options);
58-
console.log(JSON.stringify(result));
59-
process.exit(result.success ? 0 : 1);
58+
await runCliCommand('remove.all', !!options.json, async () => {
59+
const result = await handleRemoveAll(options);
60+
if (!result.success) throw new Error(result.error);
61+
console.log(JSON.stringify(result));
62+
return {};
63+
});
6064
}
6165

6266
export const registerRemove = (program: Command): Command => {

‎src/cli/primitives/AgentPrimitive.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import {
3535
import { executeImportAgent } from '../operations/agent/import';
3636
import { setupPythonProject } from '../operations/python';
3737
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
38-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
38+
import { runCliCommand } from '../telemetry/cli-command-run.js';
3939
import {
4040
AgentType,
4141
AuthorizerType,
@@ -283,7 +283,7 @@ export class AgentPrimitive extends BasePrimitive<AddAgentOptions, RemovableReso
283283

284284
// Any flag triggers non-interactive CLI mode
285285
if (cliOptions.name || cliOptions.framework || cliOptions.json) {
286-
await cliCommandRun('add.agent', !!cliOptions.json, async () => {
286+
await runCliCommand('add.agent', !!cliOptions.json, async () => {
287287
const validation = validateAddAgentOptions(cliOptions);
288288
if (!validation.valid) {
289289
throw new Error(validation.error);

‎src/cli/primitives/BasePrimitive.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { ConfigIO, findConfigRoot } from '../../lib';
22
import type { AgentCoreProjectSpec } from '../../schema';
33
import type { ResourceType } from '../commands/remove/types';
44
import { getErrorMessage } from '../errors';
5+
import { trackCommand } from '../telemetry/cli-command-run.js';
6+
import type { SubCommand } from '../telemetry/schemas/command-run.js';
57
import { requireTTY } from '../tui/guards/tty';
68
import { SOURCE_CODE_NOTE } from './constants';
79
import type { AddResult, AddScreenComponent, RemovableResource, RemovalPreview, RemovalResult } from './types';
@@ -120,7 +122,11 @@ export abstract class BasePrimitive<
120122
process.exit(1);
121123
}
122124

123-
const result = await this.remove(cliOptions.name);
125+
const result = await trackCommand<SubCommand<'remove', typeof this.kind>, RemovalResult>(
126+
`remove.${this.kind}`,
127+
{},
128+
() => this.remove(cliOptions.name!)
129+
);
124130
console.log(
125131
JSON.stringify({
126132
success: result.success,

‎src/cli/primitives/CredentialPrimitive.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { CredentialSchema } from '../../schema';
44
import { validateAddCredentialOptions } from '../commands/add/validate';
55
import { getErrorMessage } from '../errors';
66
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
7-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
7+
import { runCliCommand } from '../telemetry/cli-command-run.js';
88
import { CredentialType, standardize } from '../telemetry/schemas/common-shapes.js';
99
import { requireTTY } from '../tui/guards/tty';
1010
import { BasePrimitive } from './BasePrimitive';
@@ -291,7 +291,7 @@ export class CredentialPrimitive extends BasePrimitive<AddCredentialOptions, Rem
291291
cliOptions.scopes
292292
) {
293293
// CLI mode
294-
await cliCommandRun('add.credential', !!cliOptions.json, async () => {
294+
await runCliCommand('add.credential', !!cliOptions.json, async () => {
295295
const validation = validateAddCredentialOptions({
296296
name: cliOptions.name,
297297
type: cliOptions.type as 'api-key' | 'oauth' | undefined,

‎src/cli/primitives/EvaluatorPrimitive.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { EvaluationLevel, Evaluator, EvaluatorConfig } from '../../schema';
33
import { EvaluationLevelSchema, EvaluatorSchema } from '../../schema';
44
import { getErrorMessage } from '../errors';
55
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
6-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
6+
import { runCliCommand } from '../telemetry/cli-command-run.js';
77
import { EvaluatorType, Level, standardize } from '../telemetry/schemas/common-shapes.js';
88
import { renderCodeBasedEvaluatorTemplate } from '../templates/EvaluatorRenderer';
99
import { requireTTY } from '../tui/guards/tty';
@@ -204,7 +204,7 @@ export class EvaluatorPrimitive extends BasePrimitive<AddEvaluatorOptions, Remov
204204
}
205205

206206
if (cliOptions.name || cliOptions.json) {
207-
await cliCommandRun('add.evaluator', !!cliOptions.json, async () => {
207+
await runCliCommand('add.evaluator', !!cliOptions.json, async () => {
208208
const fail = (error: string): never => {
209209
throw new Error(error);
210210
};

‎src/cli/primitives/GatewayPrimitive.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type { AddGatewayOptions as CLIAddGatewayOptions } from '../commands/add/
1212
import { validateAddGatewayOptions } from '../commands/add/validate';
1313
import { getErrorMessage } from '../errors';
1414
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
15-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
15+
import { runCliCommand, trackCommand } from '../telemetry/cli-command-run.js';
1616
import { AuthorizerType, PolicyEngineMode, standardize } from '../telemetry/schemas/common-shapes.js';
1717
import { requireTTY } from '../tui/guards/tty';
1818
import type { AddGatewayConfig } from '../tui/screens/mcp/types';
@@ -188,7 +188,7 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
188188
console.error('No agentcore project found. Run `agentcore create` first.');
189189
process.exit(1);
190190
}
191-
await cliCommandRun('add.gateway', !!cliOptions.json, async () => {
191+
await runCliCommand('add.gateway', !!cliOptions.json, async () => {
192192
const validation = validateAddGatewayOptions(cliOptions);
193193
if (!validation.valid) {
194194
throw new Error(validation.error);
@@ -262,7 +262,7 @@ export class GatewayPrimitive extends BasePrimitive<AddGatewayOptions, Removable
262262
process.exit(1);
263263
}
264264

265-
const result = await this.remove(cliOptions.name);
265+
const result = await trackCommand('remove.gateway', {}, () => this.remove(cliOptions.name!));
266266
console.log(
267267
JSON.stringify({
268268
success: result.success,

‎src/cli/primitives/GatewayTargetPrimitive.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { validateAddGatewayTargetOptions } from '../commands/add/validate';
1414
import { getErrorMessage } from '../errors';
1515
import type { RemovableGatewayTarget } from '../operations/remove/remove-gateway-target';
1616
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
17-
import { cliCommandRun } from '../telemetry/cli-command-run.js';
17+
import { runCliCommand, trackCommand } from '../telemetry/cli-command-run.js';
1818
import {
1919
GATEWAY_TARGET_TYPE_MAP,
2020
GatewayTargetHost,
@@ -309,7 +309,7 @@ export class GatewayTargetPrimitive extends BasePrimitive<AddGatewayTargetOption
309309
process.exit(1);
310310
}
311311

312-
await cliCommandRun('add.gateway-target', !!cliOptions.json, async () => {
312+
await runCliCommand('add.gateway-target', !!cliOptions.json, async () => {
313313
const validation = await validateAddGatewayTargetOptions(cliOptions);
314314
if (!validation.valid) {
315315
throw new Error(validation.error);
@@ -510,7 +510,7 @@ export class GatewayTargetPrimitive extends BasePrimitive<AddGatewayTargetOption
510510
process.exit(1);
511511
}
512512

513-
const result = await this.remove(cliOptions.name);
513+
const result = await trackCommand('remove.gateway-target', {}, () => this.remove(cliOptions.name!));
514514
console.log(
515515
JSON.stringify({
516516
success: result.success,

0 commit comments

Comments
 (0)