Skip to content

Commit 9db18ad

Browse files
committed
feat: instrument help.modes with telemetry, add audit integ test
1 parent 85582a1 commit 9db18ad

8 files changed

Lines changed: 149 additions & 16 deletions

File tree

integ-tests/help.test.ts

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
1+
import { spawnAndCollect } from '../src/test-utils/cli-runner.js';
12
import { runCLI } from '../src/test-utils/index.js';
2-
import { describe, expect, it } from 'vitest';
3+
import { readdirSync } from 'node:fs';
4+
import { mkdir, readFile, rm } from 'node:fs/promises';
5+
import { tmpdir } from 'node:os';
6+
import { join } from 'node:path';
7+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
38

49
const COMMANDS = [
510
'create',
@@ -38,3 +43,59 @@ describe('CLI help', () => {
3843
}
3944
});
4045
});
46+
47+
describe('help modes telemetry', () => {
48+
let testConfigDir: string;
49+
const cliPath = join(__dirname, '..', 'dist', 'cli', 'index.mjs');
50+
51+
beforeAll(async () => {
52+
testConfigDir = join(tmpdir(), `agentcore-help-telemetry-${Date.now()}`);
53+
await mkdir(testConfigDir, { recursive: true });
54+
});
55+
afterAll(() => rm(testConfigDir, { recursive: true, force: true }));
56+
57+
function run(args: string[], extraEnv: Record<string, string> = {}) {
58+
return spawnAndCollect('node', [cliPath, ...args], tmpdir(), {
59+
AGENTCORE_SKIP_INSTALL: '1',
60+
AGENTCORE_CONFIG_DIR: testConfigDir,
61+
...extraEnv,
62+
});
63+
}
64+
65+
it('writes JSONL audit file when audit is enabled via env var', async () => {
66+
const result = await run(['help', 'modes'], { AGENTCORE_TELEMETRY_AUDIT: '1' });
67+
expect(result.exitCode).toBe(0);
68+
69+
const telemetryDir = join(testConfigDir, 'telemetry');
70+
const files = readdirSync(telemetryDir).filter(f => f.startsWith('help-'));
71+
expect(files).toHaveLength(1);
72+
73+
const content = await readFile(join(telemetryDir, files[0]!), 'utf-8');
74+
const entry = JSON.parse(content.trim());
75+
expect(entry.attrs).toMatchObject({
76+
'service.name': 'agentcore-cli',
77+
'agentcore-cli.mode': 'cli',
78+
command_group: 'help',
79+
command: 'help.modes',
80+
exit_reason: 'success',
81+
});
82+
expect(entry.attrs['agentcore-cli.session_id']).toBeDefined();
83+
expect(entry.attrs['os.type']).toBeDefined();
84+
expect(entry.value).toBeGreaterThanOrEqual(0);
85+
});
86+
87+
it('does not write audit file when audit is not enabled', async () => {
88+
const telemetryDir = join(testConfigDir, 'telemetry');
89+
await rm(telemetryDir, { recursive: true, force: true });
90+
91+
const result = await run(['help', 'modes']);
92+
expect(result.exitCode).toBe(0);
93+
94+
try {
95+
const files = readdirSync(telemetryDir);
96+
expect(files).toHaveLength(0);
97+
} catch {
98+
// telemetry dir doesn't exist — correct
99+
}
100+
});
101+
});

src/cli/cli.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { registerValidate } from './commands/validate';
2121
import { PACKAGE_VERSION } from './constants';
2222
import { getOrCreateInstallationId } from './global-config';
2323
import { ALL_PRIMITIVES } from './primitives';
24+
import { TelemetryClientAccessor } from './telemetry';
2425
import { App } from './tui/App';
2526
import { LayoutProvider } from './tui/context';
2627
import { COMMAND_DESCRIPTIONS } from './tui/copy';
@@ -222,7 +223,12 @@ export const main = async (argv: string[]) => {
222223
printTelemetryNotice();
223224
}
224225

225-
await program.parseAsync(argv);
226+
TelemetryClientAccessor.init(args[0] ?? 'unknown');
227+
try {
228+
await program.parseAsync(argv);
229+
} finally {
230+
await TelemetryClientAccessor.shutdown();
231+
}
226232

227233
// Telemetry notice already printed above; only run update check here.
228234
await printPostCommandNotices(false, updateCheck);

src/cli/commands/help/command.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js';
12
import type { Command } from '@commander-js/extra-typings';
23

34
const MODES_HELP = `
@@ -41,15 +42,23 @@ export const registerHelp = (program: Command) => {
4142
const helpCmd = program
4243
.command('help')
4344
.description('Display help topics')
44-
.action(() => {
45-
console.log('Available help topics: modes');
46-
console.log('Run `agentcore help <topic>` for details.');
45+
.action(async () => {
46+
const client = await TelemetryClientAccessor.get();
47+
await client.withCommandRun('help', () => {
48+
console.log('Available help topics: modes');
49+
console.log('Run `agentcore help <topic>` for details.');
50+
return {};
51+
});
4752
});
4853

4954
helpCmd
5055
.command('modes')
5156
.description('Explain interactive vs non-interactive modes')
52-
.action(() => {
53-
console.log(MODES_HELP);
57+
.action(async () => {
58+
const client = await TelemetryClientAccessor.get();
59+
await client.withCommandRun('help.modes', () => {
60+
console.log(MODES_HELP);
61+
return {};
62+
});
5463
});
5564
};

src/cli/telemetry/__tests__/filesystem-sink.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createTempConfig } from '../../__tests__/helpers/temp-config';
22
import { resolveAuditFilePath } from '../config';
3-
import { FilesystemSink } from '../sinks/filesystem-sink';
3+
import { FileSystemSink } from '../sinks/filesystem-sink';
44
import { readFile } from 'fs/promises';
55
import { join } from 'node:path';
66
import { afterAll, beforeEach, describe, expect, it } from 'vitest';
@@ -10,7 +10,7 @@ const outputDir = join(tmp.configDir, 'telemetry');
1010

1111
function createSink(opts: { dir?: string; log?: (msg: string) => void } = {}) {
1212
const filePath = join(opts.dir ?? outputDir, 'test-session.json');
13-
return new FilesystemSink({ filePath, log: opts.log });
13+
return new FileSystemSink({ filePath, log: opts.log });
1414
}
1515

1616
function readJsonl(path: string): Promise<unknown[]> {
@@ -22,7 +22,7 @@ function readJsonl(path: string): Promise<unknown[]> {
2222
);
2323
}
2424

25-
describe('FilesystemSink', () => {
25+
describe('FileSystemSink', () => {
2626
beforeEach(() => tmp.setup());
2727
afterAll(() => tmp.cleanup());
2828

@@ -54,7 +54,7 @@ describe('FilesystemSink', () => {
5454
it('creates output directory if it does not exist', async () => {
5555
const nested = join(tmp.testDir, 'deep', 'nested', 'telemetry');
5656
const filePath = join(nested, 'test.json');
57-
const sink = new FilesystemSink({ filePath });
57+
const sink = new FileSystemSink({ filePath });
5858
sink.record(1, { command_group: 'status', command: 'status' });
5959
await sink.flush();
6060

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { GLOBAL_CONFIG_DIR, readGlobalConfig } from '../global-config.js';
2+
import { TelemetryClient } from './client.js';
3+
import { resolveAuditFilePath, resolveResourceAttributes } from './config.js';
4+
import { FileSystemSink } from './sinks/filesystem-sink.js';
5+
import { CompositeSink } from './sinks/metric-sink.js';
6+
import { join } from 'path';
7+
8+
/**
9+
* Manages a singleton TelemetryClient. Call init() at startup to configure,
10+
* get() from command handlers to obtain the client, and shutdown() on exit.
11+
* get() lazily initializes if init() was never called.
12+
*/
13+
export class TelemetryClientAccessor {
14+
private static clientPromise: Promise<TelemetryClient> | undefined;
15+
16+
static init(entrypoint: string, mode: 'cli' | 'tui' = 'cli'): void {
17+
this.clientPromise = createClient(entrypoint, mode);
18+
}
19+
20+
static get(): Promise<TelemetryClient> {
21+
this.clientPromise ??= createClient('unknown');
22+
return this.clientPromise;
23+
}
24+
25+
static async shutdown(): Promise<void> {
26+
if (this.clientPromise) {
27+
const client = await this.clientPromise;
28+
await client.shutdown();
29+
}
30+
}
31+
}
32+
33+
async function createClient(entrypoint: string, mode: 'cli' | 'tui' = 'cli'): Promise<TelemetryClient> {
34+
const [resource, config] = await Promise.all([resolveResourceAttributes(mode), readGlobalConfig()]);
35+
36+
const sinks = [];
37+
const audit = process.env.AGENTCORE_TELEMETRY_AUDIT === '1' || config.telemetry?.audit === true;
38+
39+
if (audit) {
40+
const filePath = resolveAuditFilePath(
41+
join(GLOBAL_CONFIG_DIR, 'telemetry'),
42+
entrypoint,
43+
resource['agentcore-cli.session_id']
44+
);
45+
sinks.push(new FileSystemSink({ filePath, resource }));
46+
}
47+
48+
return new TelemetryClient(new CompositeSink(sinks));
49+
}

src/cli/telemetry/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
export { resolveTelemetryPreference, resolveResourceAttributes, resolveAuditFilePath } from './config.js';
22
export type { TelemetryPreference } from './config.js';
3+
export { TelemetryClientAccessor } from './client-accessor.js';
34
export { TelemetryClient, CANCELLED } from './client.js';
45
export { type MetricSink, CompositeSink } from './sinks/metric-sink.js';
56
export { OtelMetricSink, type OtelMetricSinkConfig } from './sinks/otel-metric-sink.js';
6-
export { FilesystemSink, type FilesystemSinkConfig } from './sinks/filesystem-sink.js';
7+
export { FileSystemSink, type FileSystemSinkConfig } from './sinks/filesystem-sink.js';
78
export { classifyError, isUserError } from './error-classification.js';

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ export const COMMAND_SCHEMAS = {
193193
package: NoAttrs,
194194
validate: NoAttrs,
195195
'help.modes': NoAttrs,
196+
help: NoAttrs,
196197
'remove.agent': NoAttrs,
197198
'remove.memory': NoAttrs,
198199
'remove.credential': NoAttrs,

src/cli/telemetry/sinks/filesystem-sink.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,29 @@ import type { MetricSink } from './metric-sink.js';
22
import { appendFile, mkdir } from 'fs/promises';
33
import { dirname } from 'path';
44

5-
export interface FilesystemSinkConfig {
5+
export interface FileSystemSinkConfig {
66
filePath: string;
7+
resource?: Record<string, string | number>;
78
log?: (message: string) => void;
89
}
910

10-
export class FilesystemSink implements MetricSink {
11+
export class FileSystemSink implements MetricSink {
1112
private readonly filePath: string;
13+
private readonly resource: Record<string, string | number>;
1214
private readonly log: (message: string) => void;
1315
private hasRecords = false;
1416

15-
constructor(config: FilesystemSinkConfig) {
17+
constructor(config: FileSystemSinkConfig) {
1618
this.filePath = config.filePath;
19+
this.resource = config.resource ?? {};
1720
this.log = config.log ?? (msg => console.log(msg));
1821
}
1922

2023
record(value: number, attrs: Record<string, string | number>): void {
2124
this.hasRecords = true;
22-
this.pendingWrite = this.pendingWrite.then(() => this.appendEntry({ value, attrs }));
25+
this.pendingWrite = this.pendingWrite.then(() =>
26+
this.appendEntry({ value, attrs: { ...this.resource, ...attrs } })
27+
);
2328
}
2429

2530
async flush(): Promise<void> {
@@ -33,6 +38,7 @@ export class FilesystemSink implements MetricSink {
3338
}
3439
}
3540

41+
// Promise chain that serializes async writes so record() can stay synchronous.
3642
private pendingWrite: Promise<void> = Promise.resolve();
3743

3844
private async appendEntry(entry: { value: number; attrs: Record<string, string | number> }): Promise<void> {

0 commit comments

Comments
 (0)