Skip to content

Commit 52a24ce

Browse files
HweinstockHweinstock
authored
feat: wire telemetry into all remove.* commands (#1069)
1 parent be65201 commit 52a24ce

31 files changed

Lines changed: 310 additions & 136 deletions

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import {
55
readProjectConfig,
66
runCLI,
77
} from '../src/test-utils/index.js';
8+
import { createTelemetryHelper } from '../src/test-utils/telemetry-helper.js';
89
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
910

11+
const telemetry = createTelemetryHelper();
12+
1013
async function runSuccess(args: string[], cwd: string) {
1114
const result = await runCLI(args, cwd);
1215
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
@@ -38,6 +41,7 @@ describe('integration: add and remove ab-test', () => {
3841

3942
afterAll(async () => {
4043
await project.cleanup();
44+
telemetry.destroy();
4145
});
4246

4347
it('requires --name for JSON mode', async () => {
@@ -154,17 +158,26 @@ describe('integration: add and remove ab-test', () => {
154158
});
155159

156160
it('removes ab-test', async () => {
157-
const json = await runSuccess(['remove', 'ab-test', '--name', 'MyIntegTest', '--json'], project.projectPath);
161+
const result = await runCLI(['remove', 'ab-test', '--name', 'MyIntegTest', '--json'], project.projectPath, {
162+
env: telemetry.env,
163+
});
164+
expect(result.exitCode).toBe(0);
165+
const json = JSON.parse(result.stdout);
158166
expect(json.success).toBe(true);
159167

160-
// Verify removal from agentcore.json
161168
const spec = await readProjectConfig(project.projectPath);
162169
const abTest = spec.abTests?.find((t: { name: string }) => t.name === 'MyIntegTest');
163170
expect(abTest).toBeUndefined();
171+
telemetry.assertMetricEmitted({ command: 'remove.ab-test', exit_reason: 'success' });
164172
});
165173

166174
it('remove returns error for non-existent test', async () => {
167-
const json = await runFailure(['remove', 'ab-test', '--name', 'DoesNotExist', '--json'], project.projectPath);
175+
const result = await runCLI(['remove', 'ab-test', '--name', 'DoesNotExist', '--json'], project.projectPath, {
176+
env: telemetry.env,
177+
});
178+
expect(result.exitCode).toBe(1);
179+
const json = JSON.parse(result.stdout);
168180
expect(json.error).toContain('not found');
181+
telemetry.assertMetricEmitted({ command: 'remove.ab-test', exit_reason: 'failure' });
169182
});
170183
});

‎integ-tests/add-remove-config-bundle.test.ts‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,13 @@ import {
77
runFailure,
88
runSuccess,
99
} from '../src/test-utils/index.js';
10+
import { createTelemetryHelper } from '../src/test-utils/telemetry-helper.js';
1011
import { writeFile } from 'node:fs/promises';
1112
import { join } from 'node:path';
1213
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
1314

15+
const telemetry = createTelemetryHelper();
16+
1417
describe('integration: add and remove config-bundle', () => {
1518
let project: TestProject;
1619

@@ -20,6 +23,7 @@ describe('integration: add and remove config-bundle', () => {
2023

2124
afterAll(async () => {
2225
await project.cleanup();
26+
telemetry.destroy();
2327
});
2428

2529
// ── Add lifecycle ─────────────────────────────────────────────────────
@@ -40,7 +44,7 @@ describe('integration: add and remove config-bundle', () => {
4044
expect(json.bundleName).toBe('InlineBundle');
4145

4246
const config = await readProjectConfig(project.projectPath);
43-
const bundle = config.configBundles!.find(b => b.name === 'InlineBundle');
47+
const bundle = config.configBundles.find(b => b.name === 'InlineBundle');
4448
expect(bundle).toBeDefined();
4549
expect(bundle!.type).toBe('ConfigurationBundle');
4650
expect(bundle!.branchName).toBe('mainline');
@@ -68,7 +72,7 @@ describe('integration: add and remove config-bundle', () => {
6872
expect(json.bundleName).toBe('FileBundle');
6973

7074
const config = await readProjectConfig(project.projectPath);
71-
const bundle = config.configBundles!.find(b => b.name === 'FileBundle');
75+
const bundle = config.configBundles.find(b => b.name === 'FileBundle');
7276
expect(bundle).toBeDefined();
7377
expect(Object.keys(bundle!.components)).toHaveLength(2);
7478
});
@@ -102,7 +106,7 @@ describe('integration: add and remove config-bundle', () => {
102106
expect(json.bundleName).toBe('FullOptsBundle');
103107

104108
const config = await readProjectConfig(project.projectPath);
105-
const bundle = config.configBundles!.find(b => b.name === 'FullOptsBundle');
109+
const bundle = config.configBundles.find(b => b.name === 'FullOptsBundle');
106110
expect(bundle).toBeDefined();
107111
expect(bundle!.description).toBe('A bundle with all optional fields');
108112
expect(bundle!.branchName).toBe('feature-branch');
@@ -127,7 +131,7 @@ describe('integration: add and remove config-bundle', () => {
127131
expect(json.bundleName).toBe('PlaceholderBundle');
128132

129133
const config = await readProjectConfig(project.projectPath);
130-
const bundle = config.configBundles!.find(b => b.name === 'PlaceholderBundle');
134+
const bundle = config.configBundles.find(b => b.name === 'PlaceholderBundle');
131135
expect(bundle).toBeDefined();
132136
const keys = Object.keys(bundle!.components);
133137
expect(keys).toContain('{{runtime:AgentA}}');
@@ -228,37 +232,45 @@ describe('integration: add and remove config-bundle', () => {
228232

229233
describe('remove config-bundle', () => {
230234
it('removes an existing config bundle', async () => {
231-
const json = await runSuccess(
235+
const result = await runCLI(
232236
['remove', 'config-bundle', '--name', 'InlineBundle', '--json'],
233-
project.projectPath
237+
project.projectPath,
238+
{ env: telemetry.env }
234239
);
235240

241+
expect(result.exitCode).toBe(0);
242+
const json = JSON.parse(result.stdout);
236243
expect(json.success).toBe(true);
237244

238245
const config = await readProjectConfig(project.projectPath);
239-
const bundle = config.configBundles!.find(b => b.name === 'InlineBundle');
246+
const bundle = config.configBundles.find(b => b.name === 'InlineBundle');
240247
expect(bundle).toBeUndefined();
248+
telemetry.assertMetricEmitted({ command: 'remove.config-bundle', exit_reason: 'success' });
241249
});
242250

243251
it('returns error for non-existent bundle', async () => {
244-
const json = await runFailure(
252+
const result = await runCLI(
245253
['remove', 'config-bundle', '--name', 'DoesNotExist', '--json'],
246-
project.projectPath
254+
project.projectPath,
255+
{ env: telemetry.env }
247256
);
248257

258+
expect(result.exitCode).toBe(1);
259+
const json = JSON.parse(result.stdout);
249260
expect(json.error).toContain('not found');
261+
telemetry.assertMetricEmitted({ command: 'remove.config-bundle', exit_reason: 'failure' });
250262
});
251263

252264
it('removes all remaining config bundles one by one', async () => {
253265
const configBefore = await readProjectConfig(project.projectPath);
254-
const remaining = configBefore.configBundles!.map(b => b.name);
266+
const remaining = configBefore.configBundles.map(b => b.name);
255267

256268
for (const name of remaining) {
257269
await runSuccess(['remove', 'config-bundle', '--name', name, '--json'], project.projectPath);
258270
}
259271

260272
const configAfter = await readProjectConfig(project.projectPath);
261-
expect(configAfter.configBundles!).toHaveLength(0);
273+
expect(configAfter.configBundles).toHaveLength(0);
262274
});
263275
});
264276

@@ -282,21 +294,21 @@ describe('integration: add and remove config-bundle', () => {
282294
}
283295

284296
const config = await readProjectConfig(project.projectPath);
285-
expect(config.configBundles!).toHaveLength(bundleNames.length);
297+
expect(config.configBundles).toHaveLength(bundleNames.length);
286298

287299
for (const name of bundleNames) {
288-
expect(config.configBundles!.find(b => b.name === name)).toBeDefined();
300+
expect(config.configBundles.find(b => b.name === name)).toBeDefined();
289301
}
290302
});
291303

292304
it('removing one bundle does not affect others', async () => {
293305
await runSuccess(['remove', 'config-bundle', '--name', 'BundleBeta', '--json'], project.projectPath);
294306

295307
const config = await readProjectConfig(project.projectPath);
296-
expect(config.configBundles!).toHaveLength(2);
297-
expect(config.configBundles!.find(b => b.name === 'BundleAlpha')).toBeDefined();
298-
expect(config.configBundles!.find(b => b.name === 'BundleGamma')).toBeDefined();
299-
expect(config.configBundles!.find(b => b.name === 'BundleBeta')).toBeUndefined();
308+
expect(config.configBundles).toHaveLength(2);
309+
expect(config.configBundles.find(b => b.name === 'BundleAlpha')).toBeDefined();
310+
expect(config.configBundles.find(b => b.name === 'BundleGamma')).toBeDefined();
311+
expect(config.configBundles.find(b => b.name === 'BundleBeta')).toBeUndefined();
300312
});
301313

302314
afterAll(async () => {

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import { createTestProject, parseJsonOutput, readProjectConfig, 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 { afterAll, beforeAll, describe, expect, it } from 'vitest';
45

6+
const telemetry = createTelemetryHelper();
7+
58
/** Run a CLI command and assert it succeeds, returning parsed JSON output. */
69
async function runSuccess(args: string[], cwd: string) {
7-
const result = await runCLI(args, cwd);
10+
const result = await runCLI(args, cwd, { env: telemetry.env });
811
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
912
const json: unknown = parseJsonOutput(result.stdout);
1013
expect(json).toHaveProperty('success', true);
@@ -13,7 +16,7 @@ async function runSuccess(args: string[], cwd: string) {
1316

1417
/** Run a CLI command and assert it fails, returning parsed JSON output. */
1518
async function runFailure(args: string[], cwd: string) {
16-
const result = await runCLI(args, cwd);
19+
const result = await runCLI(args, cwd, { env: telemetry.env });
1720
expect(result.exitCode).toBe(1);
1821
const json: unknown = parseJsonOutput(result.stdout);
1922
expect(json).toHaveProperty('success', false);
@@ -35,6 +38,7 @@ describe('integration: add and remove evaluators and online eval configs', () =>
3538

3639
afterAll(async () => {
3740
await project.cleanup();
41+
telemetry.destroy();
3842
});
3943

4044
describe('evaluator and online eval lifecycle', () => {
@@ -123,25 +127,29 @@ describe('integration: add and remove evaluators and online eval configs', () =>
123127

124128
const config = await readProjectConfig(project.projectPath);
125129
expect(config.onlineEvalConfigs.find(c => c.name === configName)).toBeUndefined();
130+
telemetry.assertMetricEmitted({ command: 'remove.online-eval', exit_reason: 'success' });
126131
});
127132

128133
it('removes the evaluator after online eval is gone', async () => {
129134
await runSuccess(['remove', 'evaluator', '--name', evalName, '--json'], project.projectPath);
130135

131136
const config = await readProjectConfig(project.projectPath);
132137
expect(config.evaluators.find(e => e.name === evalName)).toBeUndefined();
138+
telemetry.assertMetricEmitted({ command: 'remove.evaluator', exit_reason: 'success' });
133139
});
134140
});
135141

136142
describe('error cases', () => {
137143
it('fails to remove non-existent evaluator', async () => {
138144
const json = await runFailure(['remove', 'evaluator', '--name', 'NonExistent', '--json'], project.projectPath);
139145
expect(json.error).toContain('not found');
146+
telemetry.assertMetricEmitted({ command: 'remove.evaluator', exit_reason: 'failure' });
140147
});
141148

142149
it('fails to remove non-existent online eval config', async () => {
143150
const json = await runFailure(['remove', 'online-eval', '--name', 'NonExistent', '--json'], project.projectPath);
144151
expect(json.error).toContain('not found');
152+
telemetry.assertMetricEmitted({ command: 'remove.online-eval', exit_reason: 'failure' });
145153
});
146154

147155
it('rejects evaluator with missing --level', async () => {

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

Lines changed: 30 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,25 @@ 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' });
101+
});
102+
103+
it('fails to remove non-existent gateway', async () => {
104+
const result = await runCLI(['remove', 'gateway', '--name', 'NonExistent', '--json'], project.projectPath, {
105+
env: telemetry.env,
106+
});
107+
expect(result.exitCode).toBe(1);
108+
telemetry.assertMetricEmitted({ command: 'remove.gateway', exit_reason: 'failure' });
109+
});
110+
111+
it('fails to remove non-existent gateway target', async () => {
112+
const result = await runCLI(
113+
['remove', 'gateway-target', '--name', 'NonExistent', '--json'],
114+
project.projectPath,
115+
{ env: telemetry.env }
116+
);
117+
expect(result.exitCode).toBe(1);
118+
telemetry.assertMetricEmitted({ command: 'remove.gateway-target', exit_reason: 'failure' });
91119
});
92120
});
93121
});

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { createTestProject, readProjectConfig, 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 { afterAll, beforeAll, describe, expect, it } from 'vitest';
45

6+
const telemetry = createTelemetryHelper();
7+
58
describe('integration: add and remove policy engines and policies', () => {
69
let project: TestProject;
710

@@ -16,6 +19,7 @@ describe('integration: add and remove policy engines and policies', () => {
1619

1720
afterAll(async () => {
1821
await project.cleanup();
22+
telemetry.destroy();
1923
});
2024

2125
describe('policy engine lifecycle', () => {
@@ -111,7 +115,8 @@ describe('integration: add and remove policy engines and policies', () => {
111115
it('removes a policy with --engine flag', async () => {
112116
const result = await runCLI(
113117
['remove', 'policy', '--name', policyName, '--engine', engineName, '--json'],
114-
project.projectPath
118+
project.projectPath,
119+
{ env: telemetry.env }
115120
);
116121

117122
expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0);
@@ -125,6 +130,7 @@ describe('integration: add and remove policy engines and policies', () => {
125130
expect(engine, `Engine "${engineName}" should still exist`).toBeDefined();
126131
const policy = engine!.policies.find(p => p.name === policyName);
127132
expect(policy, `Policy "${policyName}" should be removed`).toBeUndefined();
133+
telemetry.assertMetricEmitted({ command: 'remove.policy', exit_reason: 'success' });
128134
});
129135
});
130136

@@ -272,24 +278,29 @@ describe('integration: add and remove policy engines and policies', () => {
272278
});
273279

274280
it('fails to remove non-existent policy', async () => {
275-
const result = await runCLI(['remove', 'policy', '--name', 'NonExistentPolicy', '--json'], project.projectPath);
281+
const result = await runCLI(['remove', 'policy', '--name', 'NonExistentPolicy', '--json'], project.projectPath, {
282+
env: telemetry.env,
283+
});
276284

277285
expect(result.exitCode).toBe(1);
278286
const json = JSON.parse(result.stdout);
279287
expect(json.success).toBe(false);
280288
expect(json.error).toContain('not found');
289+
telemetry.assertMetricEmitted({ command: 'remove.policy', exit_reason: 'failure' });
281290
});
282291

283292
it('fails to remove non-existent policy engine', async () => {
284293
const result = await runCLI(
285294
['remove', 'policy-engine', '--name', 'NonExistentEngine', '--json'],
286-
project.projectPath
295+
project.projectPath,
296+
{ env: telemetry.env }
287297
);
288298

289299
expect(result.exitCode).toBe(1);
290300
const json = JSON.parse(result.stdout);
291301
expect(json.success).toBe(false);
292302
expect(json.error).toContain('not found');
303+
telemetry.assertMetricEmitted({ command: 'remove.policy-engine', exit_reason: 'failure' });
293304
});
294305

295306
it('requires --engine when adding a policy', async () => {

0 commit comments

Comments
 (0)