feat(cli): send telemetry from a detached subprocess to unblock CLI exit

.projenrc.ts

@@ -1455,6 +1455,10 @@ new BundleCli(cli, {
   test: 'bin/cdk --version',
   entryPoints: [
     'lib/index.js',
+    // The detached telemetry sender. A separate entry point so that it stands on its own in the
+    // published package (where `dependencies` are stripped), which is what lets it use the real
+    // `proxy-agent` instead of hand-rolling proxy support out of Node built-ins.
+    'lib/cli/telemetry/sender-bundle.js',
   ],
   minifyWhitespace: true,
 });

packages/@aws-cdk-testing/cli-integ/lib/telemetry-endpoint.ts

@@ -0,0 +1,189 @@
+import { promises as fs } from 'fs';
+import * as net from 'net';
+import * as os from 'os';
+import * as path from 'path';
+import * as mockttp from 'mockttp';
+import { sleep } from './aws';
+
+/**
+ * A local stand-in for the telemetry endpoint.
+ *
+ * Built on the same mockttp machinery as `startProxyServer`, which matters for one specific reason:
+ * mockttp mints a leaf certificate for the requested host signed by the CA we hand it, so the CLI
+ * can be pointed at `caBundlePath` with `--ca-bundle-path` and the delivery will actually complete a
+ * TLS handshake. A bare self-signed certificate would fail hostname verification instead.
+ *
+ * Dispose it in a `finally` block.
+ */
+export interface TelemetryEndpoint {
+  /**
+   * URL to point `TELEMETRY_ENDPOINT` at.
+   */
+  readonly url: string;
+
+  /**
+   * Path to the CA certificate that signs this endpoint's certificate.
+   *
+   * Pass through `NODE_EXTRA_CA_CERTS` so the CLI, and the detached sender it spawns, will trust it.
+   *
+   * Deliberately NOT `--ca-bundle-path` or `AWS_CA_BUNDLE`: those REPLACE the trust store for the
+   * whole CLI, so the SDK's own calls to public AWS endpoints stop verifying. `STS.GetCallerIdentity`
+   * then fails to find an issuer, the default account never resolves, and the app exits non-zero
+   * before any telemetry assertion is reached. `NODE_EXTRA_CA_CERTS` adds to the store instead.
+   */
+  readonly caBundlePath: string;
+
+  /**
+   * Every telemetry batch this endpoint has received so far.
+   */
+  batches(): Promise<TelemetryBatch[]>;
+
+  /**
+   * Wait for at least one batch to arrive.
+   *
+   * Delivery happens in a detached child that outlives the CLI, so tests have to poll rather than
+   * assert immediately after the command returns.
+   *
+   * @returns the first batch, or undefined if none arrived in time
+   */
+  waitForBatch(timeoutMs?: number): Promise<TelemetryBatch | undefined>;
+
+  dispose(): Promise<void>;
+}
+
+/**
+ * A batch of events as the endpoint received it.
+ */
+export interface TelemetryBatch {
+  readonly events: Array<Record<string, any>>;
+}
+
+/**
+ * Options for `startTelemetryEndpoint`.
+ */
+export interface TelemetryEndpointOptions {
+  /**
+   * Status code to answer with.
+   *
+   * @default 200
+   */
+  readonly statusCode?: number;
+
+  /**
+   * Where to put the generated certificate directory.
+   *
+   * @default the OS temp directory
+   */
+  readonly certDirRoot?: string;
+}
+
+export async function startTelemetryEndpoint(options: TelemetryEndpointOptions = {}): Promise<TelemetryEndpoint> {
+  const certDir = await fs.mkdtemp(path.join(options.certDirRoot ?? os.tmpdir(), 'cdk-telemetry-'));
+  const certPath = path.join(certDir, 'cert.pem');
+  const keyPath = path.join(certDir, 'key.pem');
+
+  const { key, cert } = await mockttp.generateCACertificate();
+  await fs.writeFile(keyPath, key);
+  await fs.writeFile(certPath, cert);
+
+  const server = mockttp.getLocal({ https: { keyPath, certPath } });
+  const endpoint = await server.forPost('/metrics').thenReply(
+    options.statusCode ?? 200,
+    JSON.stringify({ ok: true }),
+    { 'content-type': 'application/json' },
+  );
+
+  // No port argument: mockttp picks a free one itself. Naming a port -- even a random one out of a
+  // range -- collides once suites run in parallel, and there is no retry to recover from it.
+  await server.start();
+
+  const batches = async (): Promise<TelemetryBatch[]> => {
+    const requests = await endpoint.getSeenRequests();
+    return requests.map((req) => JSON.parse(req.body.buffer.toString('utf-8')) as TelemetryBatch);
+  };
+
+  return {
+    // `localhost` rather than 127.0.0.1: the certificate mockttp mints covers the hostname, and this
+    // is the name the sender will verify against.
+    url: `https://localhost:${server.port}/metrics`,
+    caBundlePath: certPath,
+    batches,
+    waitForBatch: (timeoutMs = 30_000) => waitFor(async () => (await batches())[0], timeoutMs),
+    async dispose() {
+      await server.stop();
+      await fs.rm(certDir, { recursive: true, force: true });
+    },
+  };
+}
+
+/**
+ * A TCP listener that accepts connections and then never answers.
+ *
+ * Stands in for an endpoint that hangs, which is how we tell "the CLI did not wait for delivery"
+ * apart from "delivery happened to be fast".
+ *
+ * Dispose it in a `finally` block.
+ */
+export interface BlackHoleEndpoint {
+  /**
+   * URL to point `TELEMETRY_ENDPOINT` at.
+   */
+  readonly url: string;
+
+  /**
+   * How many connections have been accepted.
+   *
+   * A non-zero count is what proves delivery was actually attempted rather than skipped.
+   */
+  connectionCount(): number;
+
+  /**
+   * Wait for at least one connection to arrive.
+   */
+  waitForConnection(timeoutMs?: number): Promise<boolean>;
+
+  dispose(): Promise<void>;
+}
+
+export async function startBlackHoleEndpoint(): Promise<BlackHoleEndpoint> {
+  const sockets: net.Socket[] = [];
+  let connections = 0;
+
+  const server = net.createServer((socket) => {
+    connections += 1;
+    sockets.push(socket);
+  });
+  await new Promise<void>((ok) => server.listen(0, '127.0.0.1', ok));
+  const port = (server.address() as net.AddressInfo).port;
+
+  return {
+    url: `https://127.0.0.1:${port}/metrics`,
+    connectionCount: () => connections,
+    waitForConnection: (timeoutMs = 30_000) => waitFor(async () => connections > 0 || undefined, timeoutMs).then((x) => x === true),
+    async dispose() {
+      for (const socket of sockets) {
+        socket.destroy();
+      }
+      await new Promise<void>((ok) => server.close(() => ok()));
+    },
+  };
+}
+
+/**
+ * Poll `fn` until it returns something truthy, or give up after `timeoutMs`.
+ *
+ * Deliberately not `eventually` from `./eventually`: that one retries until a call stops THROWING and
+ * rethrows on give-up, whereas both callers here want "returned nothing within the deadline" to be a
+ * plain undefined they can assert on.
+ */
+export async function waitFor<A>(fn: () => Promise<A | undefined>, timeoutMs: number): Promise<A | undefined> {
+  const deadline = Date.now() + timeoutMs;
+  while (Date.now() < deadline) {
+    const result = await fn();
+    if (result) {
+      return result;
+    }
+    await sleep(500);
+  }
+  return undefined;
+}

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

@@ -5,10 +5,10 @@ integTest(
   withDefaultFixture(async (fixture) => {
     const output = await fixture.cdk(['cli-telemetry', '--disable'], { verboseLevel: 3 });
 
-    // Check the trace that telemetry was not executed successfully
-    expect(output).not.toContain('Telemetry Sent Successfully');
+    // Check the trace that telemetry was never handed to a sender
+    expect(output).not.toContain('Telemetry dispatched');
 
     // Check the trace that endpoint telemetry was never connected
-    expect(output).toContain('Endpoint Telemetry NOT connected');
+    expect(output).toContain('Telemetry disabled');
   }),
 );

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

@@ -13,8 +13,8 @@ integTest(
       verboseLevel: 3, // trace mode
     });
 
-    // Check the trace that telemetry was executed successfully
-    expect(deployOutput).toContain('Telemetry Sent Successfully');
+    // Check the trace that telemetry was handed to the detached sender that delivers it
+    expect(deployOutput).toContain('Telemetry dispatched');
 
     const json = fs.readJSONSync(telemetryFile);
     expect(json).toEqual([

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

@@ -23,8 +23,8 @@ integTest(
       modEnv: { DYNAMIC_LAMBDA_PROPERTY_VALUE: 'updated' },
     });
 
-    // Check the trace that telemetry was executed successfully
-    expect(deployOutput).toContain('Telemetry Sent Successfully');
+    // Check the trace that telemetry was handed to the detached sender that delivers it
+    expect(deployOutput).toContain('Telemetry dispatched');
 
     const json = fs.readJSONSync(telemetryFile);
     expect(json).toEqual(

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

@@ -17,8 +17,8 @@ integTest(
       }, // trace mode
     );
 
-    // Check the trace that telemetry was executed successfully
-    expect(synthOutput).toContain('Telemetry Sent Successfully');
+    // Check the trace that telemetry was handed to the detached sender that delivers it
+    expect(synthOutput).toContain('Telemetry dispatched');
 
     const json = fs.readJSONSync(telemetryFile);
     expect(json).toEqual([

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

@@ -17,8 +17,8 @@ integTest(
 
     expect(output).toContain('This is an error');
 
-    // Check the trace that telemetry was executed successfully despite error in synth
-    expect(output).toContain('Telemetry Sent Successfully');
+    // Check the trace that telemetry was handed to the detached sender despite the error in synth
+    expect(output).toContain('Telemetry dispatched');
 
     const json = fs.readJSONSync(telemetryFile);
     expect(json).toEqual([

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

@@ -13,8 +13,8 @@ integTest(
       { verboseLevel: 3 }, // trace mode
     );
 
-    // Check the trace that telemetry was executed successfully
-    expect(synthOutput).toContain('Telemetry Sent Successfully');
+    // Check the trace that telemetry was handed to the detached sender that delivers it
+    expect(synthOutput).toContain('Telemetry dispatched');
 
     const json = fs.readJSONSync(telemetryFile);
     expect(json).toEqual([

packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-telemetry-disable-command-posts-nothing.integtest.ts

@@ -0,0 +1,51 @@
+import { TELEMETRY_QUIET_PERIOD_MS } from './constants';
+import { integTest, sleep, withDefaultFixture } from '../../lib';
+import { startTelemetryEndpoint } from '../../lib/telemetry-endpoint';
+
+/**
+ * Opting out via the persisted setting has to actually stop the data leaving the machine.
+ *
+ * `cli-telemetry --disable` writes to the CDK context rather than reading an environment variable, so
+ * it reaches the same decision by a different route than
+ * `cdk-telemetry-disabled-posts-nothing`. Proven the same way: a real local endpoint, and nothing
+ * POSTed to it by the CLI or by the detached child that outlives it.
+ *
+ * The endpoint's CA is still supplied, via `NODE_EXTRA_CA_CERTS`, even though nothing should reach it:
+ * without a trusted CA "nothing arrived" would also be true of an ENABLED run whose TLS handshake
+ * simply failed, and the test would pass for the wrong reason. Not `--ca-bundle-path`, which REPLACES
+ * the trust store and breaks the SDK's own calls to public AWS endpoints.
+ */
+integTest(
+  'cli-telemetry --disable posts nothing to the endpoint',
+  withDefaultFixture(async (fixture) => {
+    const endpoint = await startTelemetryEndpoint({ certDirRoot: fixture.integTestDir });
+    try {
+      await fixture.cdk(['cli-telemetry', '--disable'], {
+        modEnv: {
+          CDK_HOME: fixture.integTestDir,
+          TELEMETRY_ENDPOINT: endpoint.url,
+        },
+      });
+
+      const output = await fixture.cdkSynth({
+        options: [
+          fixture.fullStackName('test-1'),
+        ],
+        modEnv: {
+          CDK_HOME: fixture.integTestDir,
+          TELEMETRY_ENDPOINT: endpoint.url,
+          NODE_EXTRA_CA_CERTS: endpoint.caBundlePath,
+        },
+        verboseLevel: 3, // trace
+      });
+
+      expect(output).toContain('Telemetry disabled');
+
+      await sleep(TELEMETRY_QUIET_PERIOD_MS);
+
+      expect(await endpoint.batches()).toEqual([]);
+    } finally {
+      await endpoint.dispose();
+    }
+  }),
+);

packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-telemetry-disabled-posts-nothing.integtest.ts

@@ -0,0 +1,45 @@
+import { TELEMETRY_QUIET_PERIOD_MS } from './constants';
+import { integTest, sleep, withDefaultFixture } from '../../lib';
+import { startTelemetryEndpoint } from '../../lib/telemetry-endpoint';
+
+/**
+ * Opting out via the environment has to actually stop the data leaving the machine.
+ *
+ * The other disable tests assert on the CLI's own trace output, which only proves the sink was never
+ * constructed. This points `TELEMETRY_ENDPOINT` at a real local server and proves nothing is POSTed
+ * to it -- including by the detached child, which outlives the CLI and would therefore not show up in
+ * its output at all.
+ *
+ * The endpoint's CA is still supplied, via `NODE_EXTRA_CA_CERTS`, even though nothing should reach it:
+ * without a trusted CA "nothing arrived" would also be true of an ENABLED run whose TLS handshake
+ * simply failed, and the test would pass for the wrong reason. Not `--ca-bundle-path`, which REPLACES
+ * the trust store and breaks the SDK's own calls to public AWS endpoints.
+ */
+integTest(
+  'CDK_DISABLE_CLI_TELEMETRY posts nothing to the endpoint',
+  withDefaultFixture(async (fixture) => {
+    const endpoint = await startTelemetryEndpoint({ certDirRoot: fixture.integTestDir });
+    try {
+      const output = await fixture.cdkSynth({
+        options: [
+          fixture.fullStackName('test-1'),
+        ],
+        modEnv: {
+          CDK_HOME: fixture.integTestDir,
+          TELEMETRY_ENDPOINT: endpoint.url,
+          NODE_EXTRA_CA_CERTS: endpoint.caBundlePath,
+          CDK_DISABLE_CLI_TELEMETRY: 'true',
+        },
+        verboseLevel: 3, // trace
+      });
+
+      expect(output).toContain('Telemetry disabled');
+
+      await sleep(TELEMETRY_QUIET_PERIOD_MS);
+
+      expect(await endpoint.batches()).toEqual([]);
+    } finally {
+      await endpoint.dispose();
+    }
+  }),
+);