Skip to content

Commit 33464fc

Browse files
kaizenccgithub-actions
kaizencc
and
github-actions
authored
chore(cli): integ tests assert telemetry successfully sent to endpoint (#775)
This PR includes the following: - more robust trace messaging for telemetry success / failure - use of `Funnel` class to combine `FileSink` and `EndpointSink` - update integ tests to assert on trace messages that the telemetry sent returns 200 OK - integ tests for the production endpoint This allows us to test the production endpoint before enabling telemetry for all users. The integ tests enable telemetry explicitly by providing an environment variable that will be removed once we launch telemetry. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions <[email protected]> Co-authored-by: github-actions <[email protected]>
1 parent 2d46213 commit 33464fc

File tree

13 files changed

+128
-57
lines changed

13 files changed

+128
-57
lines changed

‎packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ export interface CdkCliOptions extends ShellOptions {
218218
options?: string[];
219219
neverRequireApproval?: boolean;
220220
verbose?: boolean;
221+
verboseLevel?: number;
221222
telemetryFile?: string;
222223
}
223224

@@ -579,9 +580,19 @@ export class TestFixture extends ShellHelper {
579580
public async cdk(args: string[], options: CdkCliOptions = {}) {
580581
const verbose = options.verbose ?? true;
581582

583+
if (!verbose && options.verboseLevel) {
584+
throw new Error(`Invalid verbose state: verbose is false and verboseLevel is ${options.verboseLevel}`);
585+
}
586+
587+
const verboseLevel = options.verboseLevel ?? 1;
588+
582589
await this.cli.makeCliAvailable();
583590

584-
return this.shell(['cdk', ...(verbose ? ['-v'] : []), ...args], {
591+
return this.shell([
592+
'cdk',
593+
...(verbose ? [`-${'v'.repeat(verboseLevel)}`] : []),
594+
...args,
595+
], {
585596
...options,
586597
modEnv: {
587598
...this.cdkShellEnv(),

‎packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-cli-telemetry-disable-sends-no-data.integtest.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { TELEMETRY_ENDPOINT } from './constants';
2+
import { integTest, withDefaultFixture } from '../../lib';
3+
4+
jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
5+
6+
integTest(
7+
'CLI Telemetry --disable does not send to endpoint',
8+
withDefaultFixture(async (fixture) => {
9+
const output = await fixture.cdk(['cli-telemetry', '--disable'], { verboseLevel: 3, modEnv: { TELEMETRY_ENDPOINT: TELEMETRY_ENDPOINT } });
10+
11+
// Check the trace that telemetry was not executed successfully
12+
expect(output).not.toContain('Telemetry Sent Successfully');
13+
14+
// Check the trace that endpoint telemetry was never connected
15+
expect(output).toContain('Endpoint Telemetry NOT connected');
16+
}),
17+
);

‎packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-deploy-telemetry.integtest.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as path from 'path';
22
import * as fs from 'fs-extra';
3+
import { TELEMETRY_ENDPOINT } from './constants';
34
import { integTest, withDefaultFixture } from '../../lib';
45

56
jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
@@ -10,9 +11,15 @@ integTest(
1011
const telemetryFile = path.join(fixture.integTestDir, 'telemetry.json');
1112

1213
// Deploy stack while collecting telemetry
13-
await fixture.cdkDeploy('test-1', {
14+
const deployOutput = await fixture.cdkDeploy('test-1', {
1415
telemetryFile,
16+
verboseLevel: 3, // trace mode
17+
modEnv: { TELEMETRY_ENDPOINT: TELEMETRY_ENDPOINT },
1518
});
19+
20+
// Check the trace that telemetry was executed successfully
21+
expect(deployOutput).toContain('Telemetry Sent Successfully');
22+
1623
const json = fs.readJSONSync(telemetryFile);
1724
expect(json).toEqual([
1825
expect.objectContaining({

‎packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-synth-telemetry-with-errors.integtest.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as path from 'path';
22
import * as fs from 'fs-extra';
3+
import { TELEMETRY_ENDPOINT } from './constants';
34
import { integTest, withDefaultFixture } from '../../lib';
45

56
jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
@@ -12,19 +13,23 @@ integTest(
1213
allowErrExit: true,
1314
modEnv: {
1415
INTEG_STACK_SET: 'stage-with-errors',
16+
TELEMETRY_ENDPOINT: TELEMETRY_ENDPOINT,
1517
},
18+
verboseLevel: 3, // trace mode
1619
});
1720

1821
expect(output).toContain('This is an error');
1922

23+
// Check the trace that telemetry was executed successfully despite error in synth
24+
expect(output).toContain('Telemetry Sent Successfully');
25+
2026
const json = fs.readJSONSync(telemetryFile);
2127
expect(json).toEqual([
2228
expect.objectContaining({
2329
event: expect.objectContaining({
2430
command: expect.objectContaining({
2531
path: ['synth'],
26-
parameters: {
27-
verbose: 1,
32+
parameters: expect.objectContaining({
2833
unstable: '<redacted>',
2934
['telemetry-file']: '<redacted>',
3035
lookups: true,
@@ -37,7 +42,7 @@ integTest(
3742
validation: true,
3843
quiet: false,
3944
yes: false,
40-
},
45+
}),
4146
config: {
4247
context: {},
4348
},
@@ -72,8 +77,7 @@ integTest(
7277
event: expect.objectContaining({
7378
command: expect.objectContaining({
7479
path: ['synth'],
75-
parameters: {
76-
verbose: 1,
80+
parameters: expect.objectContaining({
7781
unstable: '<redacted>',
7882
['telemetry-file']: '<redacted>',
7983
lookups: true,
@@ -86,7 +90,7 @@ integTest(
8690
validation: true,
8791
quiet: false,
8892
yes: false,
89-
},
93+
}),
9094
config: {
9195
context: {},
9296
},

‎packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-synth-telemetry.integtest.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as path from 'path';
22
import * as fs from 'fs-extra';
3+
import { TELEMETRY_ENDPOINT } from './constants';
34
import { integTest, withDefaultFixture } from '../../lib';
45

56
jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
@@ -8,15 +9,22 @@ integTest(
89
'cdk synth with telemetry data',
910
withDefaultFixture(async (fixture) => {
1011
const telemetryFile = path.join(fixture.integTestDir, `telemetry-${Date.now()}.json`);
11-
await fixture.cdk(['synth', fixture.fullStackName('test-1'), '--unstable=telemetry', `--telemetry-file=${telemetryFile}`]);
12+
13+
const synthOutput = await fixture.cdk(
14+
['synth', fixture.fullStackName('test-1'), '--unstable=telemetry', `--telemetry-file=${telemetryFile}`],
15+
{ modEnv: { TELEMETRY_ENDPOINT: TELEMETRY_ENDPOINT }, verboseLevel: 3 }, // trace mode
16+
);
17+
18+
// Check the trace that telemetry was executed successfully
19+
expect(synthOutput).toContain('Telemetry Sent Successfully');
20+
1221
const json = fs.readJSONSync(telemetryFile);
1322
expect(json).toEqual([
1423
expect.objectContaining({
1524
event: expect.objectContaining({
1625
command: expect.objectContaining({
1726
path: ['synth', '$STACKS_1'],
18-
parameters: {
19-
verbose: 1,
27+
parameters: expect.objectContaining({
2028
unstable: '<redacted>',
2129
['telemetry-file']: '<redacted>',
2230
lookups: true,
@@ -29,7 +37,7 @@ integTest(
2937
validation: true,
3038
quiet: false,
3139
yes: false,
32-
},
40+
}),
3341
config: {
3442
context: {},
3543
},
@@ -65,8 +73,7 @@ integTest(
6573
event: expect.objectContaining({
6674
command: expect.objectContaining({
6775
path: ['synth', '$STACKS_1'],
68-
parameters: {
69-
verbose: 1,
76+
parameters: expect.objectContaining({
7077
unstable: '<redacted>',
7178
['telemetry-file']: '<redacted>',
7279
lookups: true,
@@ -79,7 +86,7 @@ integTest(
7986
validation: true,
8087
quiet: false,
8188
yes: false,
82-
},
89+
}),
8390
config: {
8491
context: {},
8592
},

‎packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const TELEMETRY_ENDPOINT = 'https://cdk-cli-telemetry.us-east-1.api.aws/metrics';

‎packages/aws-cdk/lib/cli/cdk-toolkit.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,7 @@ export class CdkToolkit {
205205
await this.props.configuration.saveContext();
206206
}
207207

208-
public async cliTelemetryStatus(versionReporting: boolean = true) {
209-
// recreate the version-reporting property in args rather than bring the entire args object over
210-
const args = { ['version-reporting']: versionReporting };
208+
public async cliTelemetryStatus(args: any) {
211209
const canCollect = canCollectTelemetry(args, this.props.configuration.context);
212210
if (canCollect) {
213211
await this.ioHost.asIoHelper().defaults.info('CLI Telemetry is enabled. See https://docs.aws.amazon.com/cdk/v2/guide/cli-telemetry.html for ways to disable.');

‎packages/aws-cdk/lib/cli/cli.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
541541
}
542542

543543
if (args.status) {
544-
return cli.cliTelemetryStatus(args['version-reporting']);
544+
return cli.cliTelemetryStatus(args);
545545
} else {
546546
const enable = args.enable ?? !args.disable;
547547
return cli.cliTelemetry(enable);

‎packages/aws-cdk/lib/cli/io-host/cli-io-host.ts‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,15 @@ import * as promptly from 'promptly';
99
import type { IoHelper, ActivityPrinterProps, IActivityPrinter } from '../../../lib/api-private';
1010
import { asIoHelper, IO, isMessageRelevantForLevel, CurrentActivityPrinter, HistoryActivityPrinter } from '../../../lib/api-private';
1111
import { StackActivityProgress } from '../../commands/deploy';
12+
import { canCollectTelemetry } from '../telemetry/collect-telemetry';
1213
import type { EventResult } from '../telemetry/messages';
1314
import { CLI_PRIVATE_IO, CLI_TELEMETRY_CODES } from '../telemetry/messages';
1415
import type { EventType } from '../telemetry/schema';
1516
import { TelemetrySession } from '../telemetry/session';
17+
import { EndpointTelemetrySink } from '../telemetry/sink/endpoint-sink';
1618
import { FileTelemetrySink } from '../telemetry/sink/file-sink';
19+
import { Funnel } from '../telemetry/sink/funnel';
20+
import type { ITelemetrySink } from '../telemetry/sink/sink-interface';
1721
import { isCI } from '../util/ci';
1822

1923
export type { IIoHost, IoMessage, IoMessageCode, IoMessageLevel, IoRequest };
@@ -180,33 +184,45 @@ export class CliIoHost implements IIoHost {
180184
this.logLevel = props.logLevel ?? 'info';
181185
this.isCI = props.isCI ?? isCI();
182186
this.requireDeployApproval = props.requireDeployApproval ?? RequireApproval.BROADENING;
183-
184187
this.stackProgress = props.stackProgress ?? StackActivityProgress.BAR;
185188
this.autoRespond = props.autoRespond ?? false;
186189
}
187190

188-
public async startTelemetry(args: any, context: Context, _proxyAgent?: Agent) {
189-
let sink;
191+
public async startTelemetry(args: any, context: Context, proxyAgent?: Agent) {
192+
let sinks: ITelemetrySink[] = [];
190193
const telemetryFilePath = args['telemetry-file'];
191194
if (telemetryFilePath) {
192-
sink = new FileTelemetrySink({
193-
ioHost: this,
194-
logFilePath: telemetryFilePath,
195-
});
195+
try {
196+
sinks.push(new FileTelemetrySink({
197+
ioHost: this,
198+
logFilePath: telemetryFilePath,
199+
}));
200+
await this.asIoHelper().defaults.trace('File Telemetry connected');
201+
} catch (e: any) {
202+
await this.asIoHelper().defaults.trace(`File Telemetry instantiation failed: ${e.message}`);
203+
}
196204
}
197-
// TODO: uncomment this at launch
198-
// if (canCollectTelemetry(args, context)) {
199-
// sink = new EndpointTelemetrySink({
200-
// ioHost: this,
201-
// agent: proxyAgent,
202-
// endpoint: '', // TODO: add endpoint
203-
// });
204-
// }
205-
206-
if (sink) {
205+
206+
const telemetryEndpoint = process.env.TELEMETRY_ENDPOINT;
207+
if (canCollectTelemetry(args, context) && telemetryEndpoint) {
208+
try {
209+
sinks.push(new EndpointTelemetrySink({
210+
ioHost: this,
211+
agent: proxyAgent,
212+
endpoint: telemetryEndpoint,
213+
}));
214+
await this.asIoHelper().defaults.trace('Endpoint Telemetry connected');
215+
} catch (e: any) {
216+
await this.asIoHelper().defaults.trace(`Endpoint Telemetry instantiation failed: ${e.message}`);
217+
}
218+
} else {
219+
await this.asIoHelper().defaults.trace('Endpoint Telemetry NOT connected');
220+
}
221+
222+
if (sinks.length > 0) {
207223
this.telemetry = new TelemetrySession({
208224
ioHost: this,
209-
client: sink,
225+
client: new Funnel({ sinks }),
210226
arguments: args,
211227
context: context,
212228
});

‎packages/aws-cdk/lib/cli/telemetry/collect-telemetry.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import type { Context } from '../../api/context';
66
export function canCollectTelemetry(args: any, context: Context): boolean {
77
if ((['true', '1'].includes(process.env.CDK_DISABLE_CLI_TELEMETRY ?? '')) ||
88
['false', false].includes(context.get('cli-telemetry')) ||
9-
(args['version-reporting'] !== undefined && !args['version-reporting'])) /* aliased with telemetry option */ {
9+
(args['version-reporting'] !== undefined && !args['version-reporting']) || /* aliased with telemetry option */
10+
(args._[0] === 'cli-telemetry' && args.disable)) /* special case for `cdk cli-telemetry --disable` */ {
1011
return false;
1112
}
1213

0 commit comments

Comments
 (0)