From 97dbb5c55f854d0b45c52ae0f5b6dbcbc90a9974 Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 01:15:51 +0700 Subject: [PATCH 01/17] feat(cli): replace HashiCorp checkpoint telemetry with Sentry usage metrics --- packages/@cdktn/cli-core/package.json | 1 - .../cli-core/src/test/checkpoint.test.ts | 56 --- packages/@cdktn/commons/src/checkpoint.ts | 201 ---------- packages/@cdktn/commons/src/config.ts | 2 + packages/@cdktn/commons/src/errors.ts | 44 ++- packages/@cdktn/commons/src/identity.ts | 69 ++++ packages/@cdktn/commons/src/index.ts | 3 +- packages/@cdktn/commons/src/telemetry.test.ts | 349 ++++++++++++++++++ packages/@cdktn/commons/src/telemetry.ts | 160 ++++++++ pnpm-lock.yaml | 59 --- 10 files changed, 606 insertions(+), 338 deletions(-) delete mode 100644 packages/@cdktn/cli-core/src/test/checkpoint.test.ts delete mode 100644 packages/@cdktn/commons/src/checkpoint.ts create mode 100644 packages/@cdktn/commons/src/identity.ts create mode 100644 packages/@cdktn/commons/src/telemetry.test.ts create mode 100644 packages/@cdktn/commons/src/telemetry.ts diff --git a/packages/@cdktn/cli-core/package.json b/packages/@cdktn/cli-core/package.json index b402e166b..210b4a072 100644 --- a/packages/@cdktn/cli-core/package.json +++ b/packages/@cdktn/cli-core/package.json @@ -69,7 +69,6 @@ "@types/fs-extra": "11.0.4", "@types/node": "22.20.1", "@types/semver": "7.7.1", - "nock": "^14.0.16", "prettier": "2.8.8", "tsc-files": "1.1.4", "typescript": "~6.0.0" diff --git a/packages/@cdktn/cli-core/src/test/checkpoint.test.ts b/packages/@cdktn/cli-core/src/test/checkpoint.test.ts deleted file mode 100644 index 50eef89b1..000000000 --- a/packages/@cdktn/cli-core/src/test/checkpoint.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) HashiCorp, Inc -// SPDX-License-Identifier: MPL-2.0 -import { ReportRequest, ReportParams } from "@cdktn/commons"; -import nock from "nock"; - -describe("ReportRequest", () => { - const reportParams: ReportParams = { - command: "foo", - product: "cdktn", - version: "0.1", - dateTime: new Date(), - payload: {}, - language: "typescript", - }; - - it("handles request errors", async () => { - nock("https://checkpoint-api.hashicorp.com") - .post(new RegExp("/v1/.*")) - .replyWithError("some request error happened"); - - await ReportRequest(reportParams); - }); - - describe("CHECKPOINT_DISABLE", () => { - let checkPointDisable: any; - - beforeEach(() => { - checkPointDisable = process.env.CHECKPOINT_DISABLE; - }); - - afterEach(() => { - process.env.CHECKPOINT_DISABLE = checkPointDisable; - }); - - it("does not perform request when disabled via ENV", async () => { - process.env.CHECKPOINT_DISABLE = "truthy"; - - const scope = nock("https://checkpoint-api.hashicorp.com") - .post(new RegExp("/v1/.*")) - .reply(); - - await ReportRequest(reportParams); - expect(scope.isDone()).toBeFalsy(); - }); - - it("does perform request by default", async () => { - delete process.env.CHECKPOINT_DISABLE; - const scope = nock("https://checkpoint-api.hashicorp.com") - .post(new RegExp("/v1/*")) - .reply(201, ""); - - await ReportRequest(reportParams); - expect(scope.isDone).toBeTruthy(); - }); - }); -}); diff --git a/packages/@cdktn/commons/src/checkpoint.ts b/packages/@cdktn/commons/src/checkpoint.ts deleted file mode 100644 index 277f6d608..000000000 --- a/packages/@cdktn/commons/src/checkpoint.ts +++ /dev/null @@ -1,201 +0,0 @@ -// Copyright (c) HashiCorp, Inc -// SPDX-License-Identifier: MPL-2.0 -import * as https from "https"; -import { format } from "url"; -import { randomUUID } from "node:crypto"; -import * as os from "os"; -import ciInfo from "ci-info"; -import { logger, processLoggerError } from "./logging"; -import * as path from "path"; -import * as fs from "fs-extra"; -import { DISPLAY_VERSION } from "./version"; - -const BASE_URL = `https://checkpoint-api.hashicorp.com/v1/`; - -const VALID_STATUS_CODES = [200, 201]; - -const MAX_REQUEST_BODY_SIZE = 8192; - -function homeDir() { - return process.env.CDKTF_HOME - ? path.resolve(process.env.CDKTF_HOME) - : path.join( - (os.userInfo().homedir ?? os.homedir()).trim() || "/", - ".cdktf", - ); -} - -export interface ReportParams { - dateTime?: Date; - arch?: string; - os?: string; - payload: Record; - product: string; - runID?: string; - version?: string; - command?: string; - language?: string; - userId?: string; - ci?: string; - projectId?: string; -} - -async function post(url: string, data: string) { - return new Promise((ok, ko) => { - const req = https.request( - format(url), - { - headers: { - Accept: "application/json", - "Content-Length": data.length, - "User-Agent": "OpenConstructs/cdktn-cli", - }, - method: "POST", - }, - (res) => { - if (res.statusCode) { - const statusCode = res.statusCode; - if (!VALID_STATUS_CODES.includes(statusCode)) { - return ko(new Error(res.statusMessage)); - } - } - const data = new Array(); - res.on("data", (chunk) => data.push(chunk)); - res.on("error", (err) => ko(err)); - res.on("end", () => { - return ok(); - }); - }, - ); - - req.setTimeout(1000, () => ko(new Error("request timeout"))); - req.write(data); - req.end(); - req.on("error", (err) => ko(err)); - }); -} - -export async function sendTelemetry( - command: string, - payload: Record, -) { - const reportParams: ReportParams = { - command, - product: "cdktn", - version: `${DISPLAY_VERSION}`, - dateTime: new Date(), - language: payload.language, - payload, - }; - - try { - await ReportRequest(reportParams); - } catch (err) { - logger.error(`Could not send telemetry data: ${err}`); - } -} - -function getId( - filePath: string, - key: string, - forceCreation = false, - explanatoryComment?: string, -): string { - const _uuid = randomUUID(); // create a new UUID in case we don't find one - - let jsonFile; - try { - jsonFile = JSON.parse(fs.readFileSync(filePath, "utf-8")); // we found the file - } catch { - // we found no file, create one if we're forcing a creation - if (forceCreation) { - const _idFile = {} as Record; // compose JSON id file in case we don't find one - if (explanatoryComment) { - _idFile["//"] = explanatoryComment.replace(/\n/g, " "); - } - _idFile[key] = _uuid; - fs.ensureDirSync(path.dirname(filePath)); - fs.writeFileSync(filePath, JSON.stringify(_idFile, null, 2)); - } - return _uuid; - } - - if (jsonFile[key]) { - return jsonFile[key]; // we found an id - } else { - // we found no id, we add it to the file for future use - fs.writeFileSync( - filePath, - JSON.stringify({ ...jsonFile, [key]: _uuid }, null, 2), - ); - return _uuid; - } -} - -export function getProjectId(projectPath = process.cwd()): string { - return getId(path.resolve(projectPath, "cdktf.json"), "projectId"); -} - -export function getUserId(): string { - return getId( - path.resolve(homeDir(), "config.json"), - "userId", - true, - `This signature is a randomly generated UUID used to anonymously differentiate users in telemetry data order to inform product direction. -This signature is random, it is not based on any personally identifiable information. -To create a new signature, you can simply delete this file at any time. -See https://cdktn.io/docs/telemetry for more -information on how to disable it.`, - ); -} - -export async function ReportRequest(reportParams: ReportParams): Promise { - // we won't report when checkpoint is disabled. - // Check at runtime (not import time) to allow tests to modify the env var - if (process.env.CHECKPOINT_DISABLE) { - return; - } - - if (!reportParams.runID) { - reportParams.runID = randomUUID(); - } - - if (!reportParams.dateTime) { - reportParams.dateTime = new Date(); - } - - if (!reportParams.arch) { - reportParams.arch = os.arch(); - } - - if (!reportParams.os) { - reportParams.os = os.platform(); - } - - const ci: string | false = ciInfo.isCI ? ciInfo.name || "unknown" : false; - if (!reportParams.userId && !ci) { - reportParams.userId = getUserId(); - } - - if (ci) { - reportParams.ci = ci; - } - - reportParams.projectId = reportParams.projectId || getProjectId(); - - const postData = JSON.stringify(reportParams); - - if (postData.length > MAX_REQUEST_BODY_SIZE) { - logger.warn( - `Skipped sending telemetry as the request body size was ${postData.length} bytes. The limit is ${MAX_REQUEST_BODY_SIZE} bytes`, - ); - return; - } - - try { - await post(`${BASE_URL}telemetry/${reportParams.product}`, postData); - } catch (e: any) { - // Log errors writing to checkpoint - processLoggerError(e.message); - } -} diff --git a/packages/@cdktn/commons/src/config.ts b/packages/@cdktn/commons/src/config.ts index 2de98a30e..3fd22413f 100644 --- a/packages/@cdktn/commons/src/config.ts +++ b/packages/@cdktn/commons/src/config.ts @@ -301,6 +301,8 @@ interface ConfigBase { * `targetVersions` before running it. */ readonly validateInstalledBinary?: boolean; + readonly sendCrashReports?: boolean; + readonly sendUsageTelemetry?: boolean; } /** diff --git a/packages/@cdktn/commons/src/errors.ts b/packages/@cdktn/commons/src/errors.ts index b5e33434b..c32ef36d6 100644 --- a/packages/@cdktn/commons/src/errors.ts +++ b/packages/@cdktn/commons/src/errors.ts @@ -1,40 +1,40 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 -import { ReportParams, ReportRequest } from "./checkpoint"; -import { DISPLAY_VERSION } from "./version"; import * as Sentry from "@sentry/node"; - -// Errors that will emit telemetry events -async function report(command: string, payload: Record) { - const reportParams: ReportParams = { - command, - product: "cdktn", - version: `${DISPLAY_VERSION}`, - dateTime: new Date(), - payload, - }; - - await ReportRequest(reportParams); -} +// telemetry.ts must never import this module: it would close a cycle through +// terraform.ts and load errors.ts before the factories exist. +import { CommandErrorType, sendErrorTelemetry } from "./telemetry"; type ErrorType = "Internal" | "External" | "Usage"; export function IsErrorType(error: any, type: ErrorType): boolean { return error && error.__type === type; } -function reportPrefixedError(type: ErrorType, command: string) { +/** Metric class of a thrown value: its `Errors` type, "unexpected" otherwise. */ +export function commandErrorType(error: unknown): CommandErrorType { + for (const type of ["Usage", "External", "Internal"] as const) { + if (IsErrorType(error, type)) { + return type; + } + } + return "unexpected"; +} + +function reportPrefixedError(type: ErrorType) { return ( message: string, originalError: Error = new Error(), context?: Record, ) => { - report(command, { ...context, message, type }); const err: any = new Error(`${type} Error: ${message}`); Object.entries(context || {}).forEach(([key, value]) => { err[key] = value; }); err.__type = type; err.stack = originalError.stack; + // errorScope is read here, not when the factory is created, so the + // command set by setScope is the one counted + sendErrorTelemetry(type, errorScope); return err; }; } @@ -44,15 +44,19 @@ function reportPrefixedError(type: ErrorType, command: string) { let errorScope = "unknown"; export const Errors = { // Error within our control - Internal: reportPrefixedError("Internal", errorScope), + Internal: reportPrefixedError("Internal"), // Error in the usage - Usage: reportPrefixedError("Usage", errorScope), + Usage: reportPrefixedError("Usage"), // Error outside of our control (e.g. terraform failed) - External: reportPrefixedError("External", errorScope), + External: reportPrefixedError("External"), // Set the scope for all errors setScope(scope: string) { errorScope = scope; Sentry.getCurrentScope().setTransactionName(scope); }, + + getScope(): string { + return errorScope; + }, }; diff --git a/packages/@cdktn/commons/src/identity.ts b/packages/@cdktn/commons/src/identity.ts new file mode 100644 index 000000000..be1bf2bab --- /dev/null +++ b/packages/@cdktn/commons/src/identity.ts @@ -0,0 +1,69 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import { randomUUID } from "node:crypto"; +import * as os from "os"; +import * as path from "path"; +import * as fs from "fs-extra"; + +function homeDir() { + return process.env.CDKTF_HOME + ? path.resolve(process.env.CDKTF_HOME) + : path.join( + (os.userInfo().homedir ?? os.homedir()).trim() || "/", + ".cdktf", + ); +} + +function getId( + filePath: string, + key: string, + forceCreation = false, + explanatoryComment?: string, +): string { + const _uuid = randomUUID(); // create a new UUID in case we don't find one + + let jsonFile; + try { + jsonFile = JSON.parse(fs.readFileSync(filePath, "utf-8")); // we found the file + } catch { + // we found no file, create one if we're forcing a creation + if (forceCreation) { + const _idFile = {} as Record; // compose JSON id file in case we don't find one + if (explanatoryComment) { + _idFile["//"] = explanatoryComment.replace(/\n/g, " "); + } + _idFile[key] = _uuid; + fs.ensureDirSync(path.dirname(filePath)); + fs.writeFileSync(filePath, JSON.stringify(_idFile, null, 2)); + } + return _uuid; + } + + if (jsonFile[key]) { + return jsonFile[key]; // we found an id + } else { + // we found no id, we add it to the file for future use + fs.writeFileSync( + filePath, + JSON.stringify({ ...jsonFile, [key]: _uuid }, null, 2), + ); + return _uuid; + } +} + +export function getProjectId(projectPath = process.cwd()): string { + return getId(path.resolve(projectPath, "cdktf.json"), "projectId"); +} + +export function getUserId(): string { + return getId( + path.resolve(homeDir(), "config.json"), + "userId", + true, + `This signature is a randomly generated UUID used to anonymously differentiate users in telemetry data order to inform product direction. +This signature is random, it is not based on any personally identifiable information. +To create a new signature, you can simply delete this file at any time. +See https://cdktn.io/docs/telemetry for more +information on how to disable it.`, + ); +} diff --git a/packages/@cdktn/commons/src/index.ts b/packages/@cdktn/commons/src/index.ts index 2737d7ac8..58d775583 100644 --- a/packages/@cdktn/commons/src/index.ts +++ b/packages/@cdktn/commons/src/index.ts @@ -1,16 +1,17 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 -export * from "./checkpoint"; export * from "./construct-maker-target"; export * from "./config"; export * from "./debug"; export * from "./environment"; export * from "./errors"; export * from "./gradle"; +export * from "./identity"; export * from "./logging"; export * from "./module-schema"; export * from "./provider-schema"; +export * from "./telemetry"; export * from "./terraform-module"; export * from "./terraform"; export * from "./util"; diff --git a/packages/@cdktn/commons/src/telemetry.test.ts b/packages/@cdktn/commons/src/telemetry.test.ts new file mode 100644 index 000000000..eccbfbab7 --- /dev/null +++ b/packages/@cdktn/commons/src/telemetry.test.ts @@ -0,0 +1,349 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as Sentry from "@sentry/node"; +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; +import ciInfo from "ci-info"; +import { + sendTelemetry, + flushTelemetry, + getUsageTelemetryConsent, + setUsageTelemetryEnabled, +} from "./telemetry"; +import { Errors } from "./errors"; + +// A real client with a capturing transport proves the metric envelope +// reaches the transport and survives a bounded flush; a mocked @sentry/node +// would pass even if metrics were dropped before exit. + +type MetricItem = { + name: string; + type: string; + value: number; + attributes: Record; +}; + +function parseMetricItems(envelopeBodies: string[]): MetricItem[] { + const items: MetricItem[] = []; + for (const body of envelopeBodies) { + const lines = body.split("\n").filter(Boolean); + for (let i = 0; i < lines.length - 1; i++) { + let header; + try { + header = JSON.parse(lines[i]); + } catch { + continue; + } + if (header.type === "trace_metric") { + const payload = JSON.parse(lines[i + 1]); + items.push(...payload.items); + } + } + } + return items; +} + +function attributeValues(item: MetricItem) { + return Object.fromEntries( + Object.entries(item.attributes).map(([k, v]) => [k, v.value]), + ); +} + +describe("telemetry", () => { + let workdir: string; + let envelopeBodies: string[]; + const originalCwd = process.cwd(); + const originalCheckpointDisable = process.env.CHECKPOINT_DISABLE; + const originalSentryEnvironment = process.env.SENTRY_ENVIRONMENT; + + function initSentryWithCapturingTransport() { + Sentry.init({ + dsn: "https://public@example.invalid/1", + release: "cdktn-cli-test", + environment: "production", + tracesSampleRate: 0, + serverName: "cdktn-cli", + // each test inits its own client; process-level integrations would + // pile up listeners across tests and are irrelevant to metrics + defaultIntegrations: false, + transport: (options) => + Sentry.createTransport(options, async (request) => { + envelopeBodies.push(request.body as string); + return { statusCode: 200 }; + }), + }); + } + + beforeEach(() => { + workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-telemetry-")); + process.chdir(workdir); + envelopeBodies = []; + delete process.env.CHECKPOINT_DISABLE; + delete process.env.SENTRY_ENVIRONMENT; + }); + + afterEach(async () => { + setUsageTelemetryEnabled(undefined); + await Sentry.close(1000); + process.chdir(originalCwd); + fs.removeSync(workdir); + if (originalCheckpointDisable === undefined) { + delete process.env.CHECKPOINT_DISABLE; + } else { + process.env.CHECKPOINT_DISABLE = originalCheckpointDisable; + } + if (originalSentryEnvironment === undefined) { + delete process.env.SENTRY_ENVIRONMENT; + } else { + process.env.SENTRY_ENVIRONMENT = originalSentryEnvironment; + } + }); + + describe("sendTelemetry delivery (real client + capturing transport)", () => { + it("stamps environment attributes on every command metric", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + sendUsageTelemetry: true, + }); + process.env.SENTRY_ENVIRONMENT = "LEAK-ENV-SENTRY"; + initSentryWithCapturingTransport(); + + await sendTelemetry("synth", { totalTime: 1234, language: "typescript" }); + await sendTelemetry("synth", { error: true }); + expect(await Sentry.flush(2000)).toBe(true); + + const items = parseMetricItems(envelopeBodies); + // a run is counted once: as invoked or, for an error payload, as error + expect(items.map((i) => i.name)).toEqual([ + "cli.command.invoked", + "cli.synth.duration", + "cli.command.error", + ]); + const [invoked, duration, error] = items; + + for (const metric of items) { + const values = attributeValues(metric); + expect(values).toMatchObject({ + command: "synth", + ci: ciInfo.isCI ? ciInfo.name || "unknown" : false, + // the SDK stamps the release set in Sentry.init on every metric, + // so the CLI version needs no attribute of its own + "sentry.release": "cdktn-cli-test", + "sentry.environment": "production", + }); + for (const forbidden of [ + "stackName", + "hostname", + "message", + "projectId", + ]) { + expect(values).not.toHaveProperty(forbidden); + } + } + expect(attributeValues(invoked).language).toBe("typescript"); + expect(duration.type).toBe("distribution"); + expect(duration.value).toBe(1234); + expect(attributeValues(error)).toMatchObject({ + error_type: "unexpected", + }); + expect(envelopeBodies.join("\n")).not.toContain("LEAK-ENV-SENTRY"); + }); + + it.each([ + ["Usage", "Usage"], + ["External", "External"], + ["Internal", "Internal"], + ["unexpected", "unexpected"], + ["Something Else", "unexpected"], + [42, "unexpected"], + ])( + "stamps error_type %p as %p on cli.command.error", + async (errorType, expected) => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendUsageTelemetry: true, + }); + initSentryWithCapturingTransport(); + + await sendTelemetry("deploy", { error: true, errorType }); + expect(await Sentry.flush(2000)).toBe(true); + + const error = parseMetricItems(envelopeBodies).find( + (i) => i.name === "cli.command.error", + )!; + expect(error.attributes.error_type.value).toBe(expected); + expect(error.attributes.command.value).toBe("deploy"); + }, + ); + + it("never sends the hostname, username or working directory in any envelope", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendUsageTelemetry: true, + }); + initSentryWithCapturingTransport(); + + await sendTelemetry("synth", { totalTime: 1, language: "typescript" }); + expect(await Sentry.flush(2000)).toBe(true); + + let username: string | undefined; + try { + username = os.userInfo().username; + } catch { + username = process.env.USER; + } + expect(envelopeBodies.length).toBeGreaterThan(0); + for (const body of envelopeBodies) { + expect(body).not.toContain(os.hostname()); + if (username) { + expect(body).not.toContain(username); + } + expect(body).not.toContain(process.cwd()); + expect(body).not.toContain(workdir); + } + }); + }); + + describe("attribute validation", () => { + beforeEach(() => { + initSentryWithCapturingTransport(); + }); + + it("omits a language that is not one of the supported ones", async () => { + await sendTelemetry("convert", { language: "rust; DROP TABLE" }); + expect(await Sentry.flush(2000)).toBe(true); + + const invoked = parseMetricItems(envelopeBodies).find( + (i) => i.name === "cli.command.invoked", + )!; + expect(invoked.attributes).not.toHaveProperty("language"); + expect(envelopeBodies.join("\n")).not.toContain("DROP TABLE"); + }); + }); + + // no DSN means no client: every emitter must stay a silent no-op rather + // than throw into the command it was called from + describe("without an initialized Sentry client", () => { + beforeEach(() => { + Sentry.getGlobalScope().setClient(undefined); + Sentry.getIsolationScope().setClient(undefined); + Sentry.getCurrentScope().setClient(undefined); + setUsageTelemetryEnabled(true); + }); + + it("sends and flushes without throwing", async () => { + expect(Sentry.getClient()).toBeUndefined(); + + await expect( + sendTelemetry("deploy", { language: "typescript" }), + ).resolves.toBeUndefined(); + expect(() => Errors.Internal("boom")).not.toThrow(); + await expect(flushTelemetry(100)).resolves.toBeUndefined(); + expect(envelopeBodies).toHaveLength(0); + }); + }); + + describe("sendTelemetry gating", () => { + // CHECKPOINT_DISABLE > the decision captured at command start > + // sendUsageTelemetry in cdktf.json (absent file = flag unset) > on + it.each([ + { env: undefined, captured: undefined, flag: undefined, emits: true }, + { env: undefined, captured: undefined, flag: true, emits: true }, + { env: undefined, captured: undefined, flag: false, emits: false }, + // convert chdirs into a throwaway project that opts out + { env: undefined, captured: true, flag: false, emits: true }, + { env: undefined, captured: false, flag: true, emits: false }, + { env: "1", captured: undefined, flag: true, emits: false }, + { env: "1", captured: true, flag: true, emits: false }, + ])( + "CHECKPOINT_DISABLE=$env, captured=$captured, flag=$flag -> emits $emits", + async ({ env, captured, flag, emits }) => { + if (flag !== undefined) { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendUsageTelemetry: flag, + }); + } + initSentryWithCapturingTransport(); + setUsageTelemetryEnabled(captured); + if (env !== undefined) { + process.env.CHECKPOINT_DISABLE = env; + } + + await sendTelemetry("convert", {}); + await Sentry.flush(2000); + + const items = parseMetricItems(envelopeBodies); + expect(items.some((i) => i.name === "cli.command.invoked")).toBe(emits); + if (!emits) { + expect(items).toHaveLength(0); + } + }, + ); + }); + + describe("cli.error from the Errors factories", () => { + afterEach(() => { + Errors.setScope("unknown"); + }); + + // the scope is read when the error is built, not when the factory is + // created: a factory-time binding reports every error as "unknown" + it("counts constructed errors by type with the command set at call time", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendUsageTelemetry: true, + }); + initSentryWithCapturingTransport(); + + Errors.setScope("deploy"); + Errors.Usage("no stacks selected", undefined, { stackName: "secret" }); + Errors.External("terraform exited with code 1"); + expect(await Sentry.flush(2000)).toBe(true); + + const errors = parseMetricItems(envelopeBodies).filter( + (i) => i.name === "cli.error", + ); + expect(errors).toHaveLength(2); + expect(errors.map((e) => e.attributes.type.value)).toEqual([ + "Usage", + "External", + ]); + for (const error of errors) { + expect(error.attributes.command.value).toBe("deploy"); + expect(error.attributes).not.toHaveProperty("message"); + expect(error.attributes).not.toHaveProperty("stackName"); + } + expect(JSON.stringify(errors)).not.toContain("secret"); + expect(JSON.stringify(errors)).not.toContain("no stacks selected"); + }); + + it("still returns the typed error when Sentry is not initialized", () => { + const err = Errors.Usage("plain"); + expect(err.message).toBe("Usage Error: plain"); + expect(err.__type).toBe("Usage"); + }); + + it("exposes the scope set for the running command", () => { + expect(Errors.getScope()).toBe("unknown"); + Errors.setScope("provider add"); + expect(Errors.getScope()).toBe("provider add"); + }); + }); + + describe("getUsageTelemetryConsent", () => { + it.each([ + [{ sendUsageTelemetry: true }, true], + [{ sendUsageTelemetry: false }, false], + // init templates render the flag as a string; a boolean-only check + // would opt every freshly init'ed project out + [{ sendUsageTelemetry: "true" }, true], + [{ sendUsageTelemetry: "false" }, false], + [{}, undefined], + ])("reads %j as %p", (config, expected) => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), config); + expect(getUsageTelemetryConsent(workdir)).toBe(expected); + }); + + it("returns undefined when no cdktf.json exists", () => { + expect(getUsageTelemetryConsent(workdir)).toBeUndefined(); + }); + }); +}); diff --git a/packages/@cdktn/commons/src/telemetry.ts b/packages/@cdktn/commons/src/telemetry.ts new file mode 100644 index 000000000..ffae32966 --- /dev/null +++ b/packages/@cdktn/commons/src/telemetry.ts @@ -0,0 +1,160 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as Sentry from "@sentry/node"; +import * as path from "path"; +import * as fs from "fs-extra"; +import ciInfo from "ci-info"; +import { logger } from "./logging"; +import { LANGUAGES } from "./config"; + +type AttributeValue = string | number | boolean; +type Attributes = Record; + +/** Error class of a failed command run: a typed `Errors` value or "unexpected". */ +export const COMMAND_ERROR_TYPES = [ + "Usage", + "External", + "Internal", + "unexpected", +] as const; +export type CommandErrorType = (typeof COMMAND_ERROR_TYPES)[number]; + +/** + * Raw `cdktf.json` read that never throws: telemetry must work outside a + * project and must not depend on cli-core's typed config (commons cannot + * import cli-core). + */ +function readRawCdktfJson(projectPath: string): Record { + try { + return JSON.parse( + fs.readFileSync(path.resolve(projectPath, "cdktf.json"), "utf8"), + ); + } catch { + return {}; + } +} + +/** + * Reads the raw `sendUsageTelemetry` flag from `cdktf.json`. Returns + * `undefined` when the flag is unset or no readable `cdktf.json` exists; + * `isUsageTelemetryEnabled` derives the effective state. + */ +export function getUsageTelemetryConsent( + projectPath = process.cwd(), +): boolean | undefined { + const cdktfJson = readRawCdktfJson(projectPath); + if (!("sendUsageTelemetry" in cdktfJson)) { + return undefined; + } + // init templates render booleans as the strings "true"/"false"; a + // boolean-only check would opt every freshly init'ed project out + return typeof cdktfJson.sendUsageTelemetry === "boolean" + ? cdktfJson.sendUsageTelemetry + : cdktfJson.sendUsageTelemetry === "true"; +} + +// Captured by initializErrorReporting (cli-core) while still in the user's +// cwd: `convert` chdirs into a throwaway project before emitting, so +// re-reading cdktf.json at emission time would consult the wrong project. +let usageTelemetryEnabledState: boolean | undefined; + +export function setUsageTelemetryEnabled(enabled: boolean | undefined): void { + usageTelemetryEnabledState = enabled; +} + +/** + * Whether a command already captured its consent decision. `init` initializes + * reporting for the project it just created and must not overwrite the + * decision of a command (`convert`) that drives it inside a throwaway project. + */ +export function hasCapturedUsageTelemetryDecision(): boolean { + return usageTelemetryEnabledState !== undefined; +} + +/** + * Effective usage-telemetry gate: `CHECKPOINT_DISABLE` > the decision captured + * at command start > `sendUsageTelemetry` in `cdktf.json` > enabled by + * default. Independent of crash reporting (`sendCrashReports`). + */ +export function isUsageTelemetryEnabled(projectPath = process.cwd()): boolean { + if (process.env.CHECKPOINT_DISABLE) { + return false; + } + if (usageTelemetryEnabledState !== undefined) { + return usageTelemetryEnabledState; + } + return getUsageTelemetryConsent(projectPath) !== false; +} + +/** + * Counts an error built by the `Errors` factories as `cli.error` by type and + * command, never message or context (both can carry paths and user input). + * `cli.command.error` counts failed runs; a handled Usage error counts here. + */ +export function sendErrorTelemetry(type: string, command: string): void { + try { + if (!isUsageTelemetryEnabled()) { + return; + } + Sentry.metrics.count("cli.error", 1, { attributes: { type, command } }); + } catch (err) { + logger.debug(`Could not send error telemetry: ${err}`); + } +} + +/** + * Bounded flush of buffered telemetry. Call before any explicit + * `process.exit()` on a path that may have emitted metrics: Sentry buffers + * asynchronously and a hard exit drops the buffer. + */ +export async function flushTelemetry(timeoutMs = 4000): Promise { + try { + await Sentry.flush(timeoutMs); + } catch (err) { + logger.debug(`Could not flush telemetry: ${err}`); + } +} + +/** + * Emits a command's usage telemetry as Sentry metrics; payload fields reach + * the attributes only through the allow-lists above. A silent no-op when + * usage telemetry is disabled or Sentry is not initialized. + * `payload.error` counts the run as `cli.command.error` by `errorType` instead. + */ +export async function sendTelemetry( + command: string, + payload: Record, +): Promise { + try { + if (!isUsageTelemetryEnabled()) { + return; + } + + const ci: string | false = ciInfo.isCI ? ciInfo.name || "unknown" : false; + const attributes: Attributes = { + command, + ci: ci === false ? false : ci, + }; + if (LANGUAGES.includes(payload.language)) { + attributes.language = payload.language; + } + + if (payload.error) { + attributes.error_type = COMMAND_ERROR_TYPES.includes(payload.errorType) + ? payload.errorType + : "unexpected"; + Sentry.metrics.count("cli.command.error", 1, { attributes }); + return; + } + + Sentry.metrics.count("cli.command.invoked", 1, { attributes }); + if (typeof payload.totalTime === "number") { + Sentry.metrics.distribution("cli.synth.duration", payload.totalTime, { + unit: "millisecond", + attributes, + }); + } + } catch (err) { + logger.debug(`Could not send telemetry data: ${err}`); + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1f4b1d054..129217c35 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1069,9 +1069,6 @@ importers: '@types/semver': specifier: 7.7.1 version: 7.7.1 - nock: - specifier: ^14.0.16 - version: 14.0.16 prettier: specifier: 2.8.8 version: 2.8.8 @@ -2926,10 +2923,6 @@ packages: resolution: {integrity: sha512-1rGS+iJtejQ3ybSJfV0if9PzZuSQmORbVF3MkqE0csatF//Rj8raCqptZLMX6rkdfZ8C2Jsw5hU4PrmuCzBk6A==} engines: {node: '>= 14.17.0'} - '@mswjs/interceptors@0.41.9': - resolution: {integrity: sha512-VVPPgHyQ6ShqnrmDWuxjmUIsO9gWyOZFmuOfLd9LfBGQJwZfy0gvv9pbHSJuoFNIYC7ZDX9aoFwowjcdSC4E8w==} - engines: {node: '>=18'} - '@napi-rs/wasm-runtime@0.2.12': resolution: {integrity: sha512-ZVWUcfwY4E/yPitQJl481FjFo3K22D6qF0DuFH6Y/nbnE11GY5uguDxZMGXPQ8WQ0128MXQD7TnfHyK4oWoIJQ==} @@ -3053,15 +3046,6 @@ packages: '@nx/workspace@22.7.9': resolution: {integrity: sha512-IY7ldjs7mtv3/mW5TW8yt4xm2IqX2eWRy0KQ6obKlOM4nZ9lARV2RGcBHsgOZxTTNDSWyz7aWWgxXGnMBs3Y9A==} - '@open-draft/deferred-promise@2.2.0': - resolution: {integrity: sha512-CecwLWx3rhxVQF6V4bAgPS5t+So2sTbPgAzafKkVizyi7tlwpcFpdFqq+wqF2OwNBmqFuu6tOyouTuxgpMfzmA==} - - '@open-draft/logger@0.3.0': - resolution: {integrity: sha512-X2g45fzhxH238HKO4xbSr7+wBS8Fvw6ixhTDuvLd5mqh6bJJCFAPwU9mPDxbcrRtfxv4u5IHCEH77BmxvXmmxQ==} - - '@open-draft/until@2.1.0': - resolution: {integrity: sha512-U69T3ItWHvLwGg5eJ0n3I62nWuE6ilHlmz7zM0npLBRvPRd7e6NYmg54vvRtP5mZG7kZqZCFVdsTWo7BPtBujg==} - '@opentelemetry/api-logs@0.220.0': resolution: {integrity: sha512-CmVa4ImJ+ynfrPMNaAXHET6Bhb44SwzmfyVJFq9ni2jgXJR/l7C6gfVFddNmHP+ZOkP9cf4f9DBe68qVLTHc9w==} engines: {node: '>=8.0.0'} @@ -5827,9 +5811,6 @@ packages: resolution: {integrity: sha512-1Qed0/Hr2m+YqxnM09CjA2d/i6YZNfF6R2oRAOj36eUdS6qIV/huPJNSEpKbupewFs+ZsJlxsjjPbc0/afW6Lw==} engines: {node: '>= 0.4'} - is-node-process@1.2.0: - resolution: {integrity: sha512-Vg4o6/fqPxIjtxgUH5QLJhwZ7gW5diGCVlXpuUfELC62CuxM1iHcRe51f2W1FDy04Ai4KJkagKjx3XaqyfRKXw==} - is-number-object@1.1.1: resolution: {integrity: sha512-lZhclumE1G6VYD8VHe35wFaIif+CTy5SJIi5+3y4psDgWu4wPDoBhF8NxUOinEc7pHgiTsT6MaBb92rKhhD+Xw==} engines: {node: '>= 0.4'} @@ -6695,10 +6676,6 @@ packages: resolution: {integrity: sha512-o2zOYiCpzRqSzPj0Zt/dQ/DqZeYoaQ7TUonc/xUPjCGl9WeHpNbxgVvOquXYAaJzI0M9BXV3HTzG0p8IUAbBTQ==} engines: {node: '>= 10.13'} - nock@14.0.16: - resolution: {integrity: sha512-8r4KEc6nT1D/fdLD/R1BO1CPaVEL8o40u/guFRJlXabN7vr3RmMqyjsY5Krt0nMwhsOAwXQ/mtN5vy5Jh3aErg==} - engines: {node: '>=18.20.0 <20 || >=20.12.1'} - node-addon-api@7.1.1: resolution: {integrity: sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==} @@ -6819,9 +6796,6 @@ packages: resolution: {integrity: sha512-zAKMgGXUim0Jyd6CXK9lraBnD3H5yPGBPPOkC23a2BG6hsm4Zu6OQSjQuEtV0BHDf4aKHcUFvJiGRrFuW3MG8g==} engines: {node: '>=10'} - outvariant@1.4.3: - resolution: {integrity: sha512-+Sl2UErvtsoajRDKCE5/dBz4DIvHXQQnAxtQTF04OJxY0+DyZXSo5P5Bb7XYWOh81syohlYL24hbDwxedPUJCA==} - oxc-parser@0.133.0: resolution: {integrity: sha512-661RSx+ZcjBmjBYid+Fpp/2F5EbtildpeoZh5HdgnGs+jZ03nqQEQW8yGkt4BGyOC3OMPDQQRl8M5kqD2/g6jw==} engines: {node: ^20.19.0 || >=22.12.0} @@ -7501,9 +7475,6 @@ packages: streamx@2.25.0: resolution: {integrity: sha512-0nQuG6jf1w+wddNEEXCF4nTg3LtufWINB5eFEN+5TNZW7KWJp6x87+JFL43vaAUPyCfH1wID+mNVyW6OHtFamg==} - strict-event-emitter@0.5.1: - resolution: {integrity: sha512-vMgjE/GGEPEFnhFub6pa4FmJBRBVOLpIII2hvCZ8Kzb7K0hlHo7mQv6xYrBvCL2LtAIBwFUK8wvuJgTVSQ5MFQ==} - string-argv@0.3.2: resolution: {integrity: sha512-aqD2Q0144Z+/RqG52NeHEkZauTAUWJO8c6yTftGJKO3Tja5tUgIfmIl6kExvhtxSDP7fXB6DvzkfMpCd/F3G+Q==} engines: {node: '>=0.6.19'} @@ -9900,15 +9871,6 @@ snapshots: '@jsii/spec@1.140.0': {} - '@mswjs/interceptors@0.41.9': - dependencies: - '@open-draft/deferred-promise': 2.2.0 - '@open-draft/logger': 0.3.0 - '@open-draft/until': 2.1.0 - is-node-process: 1.2.0 - outvariant: 1.4.3 - strict-event-emitter: 0.5.1 - '@napi-rs/wasm-runtime@0.2.12': dependencies: '@emnapi/core': 1.11.2 @@ -10119,15 +10081,6 @@ snapshots: - '@swc-node/register' - '@swc/core' - '@open-draft/deferred-promise@2.2.0': {} - - '@open-draft/logger@0.3.0': - dependencies: - is-node-process: 1.2.0 - outvariant: 1.4.3 - - '@open-draft/until@2.1.0': {} - '@opentelemetry/api-logs@0.220.0': dependencies: '@opentelemetry/api': 1.9.1 @@ -13050,8 +13003,6 @@ snapshots: is-map@2.0.3: {} - is-node-process@1.2.0: {} - is-number-object@1.1.1: dependencies: call-bound: 1.0.4 @@ -14440,12 +14391,6 @@ snapshots: transitivePeerDependencies: - supports-color - nock@14.0.16: - dependencies: - '@mswjs/interceptors': 0.41.9 - json-stringify-safe: 5.0.1 - propagate: 2.0.1 - node-addon-api@7.1.1: {} node-fetch@2.6.7(encoding@0.1.13): @@ -14696,8 +14641,6 @@ snapshots: strip-ansi: 6.0.1 wcwidth: 1.0.1 - outvariant@1.4.3: {} - oxc-parser@0.133.0: dependencies: '@oxc-project/types': 0.133.0 @@ -15489,8 +15432,6 @@ snapshots: - react-native-b4a optional: true - strict-event-emitter@0.5.1: {} - string-argv@0.3.2: {} string-length@4.0.2: From a937dd47413b9fc48bd91a54c84be5aa532b9001 Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 01:15:52 +0700 Subject: [PATCH 02/17] feat(cli): ask for usage-telemetry consent and persist it in cdktf.json --- .../cli-core/src/lib/error-reporting.ts | 106 ++++-- packages/@cdktn/cli-core/src/lib/init.ts | 3 + .../cli-core/src/test/error-reporting.test.ts | 347 +++++++++++++++++- .../src/test/lib/cdktf-project.test.ts | 1 + .../test/lib/terraform-parallelism.test.ts | 1 + .../cli-core/templates/csharp/cdktf.json | 1 + .../@cdktn/cli-core/templates/go/cdktf.json | 1 + .../@cdktn/cli-core/templates/java/cdktf.json | 1 + .../cli-core/templates/python-pip/cdktf.json | 1 + .../cli-core/templates/python/cdktf.json | 1 + .../cli-core/templates/typescript/cdktf.json | 1 + packages/cdktn-cli/src/bin/cmds/handlers.ts | 46 ++- .../helper/__tests__/init-telemetry.test.ts | 94 +++++ .../src/bin/cmds/helper/error-reporting.ts | 8 + .../cdktn-cli/src/bin/cmds/helper/init.ts | 33 +- packages/cdktn-cli/src/bin/cmds/init.ts | 4 + 16 files changed, 610 insertions(+), 39 deletions(-) create mode 100644 packages/cdktn-cli/src/bin/cmds/helper/__tests__/init-telemetry.test.ts diff --git a/packages/@cdktn/cli-core/src/lib/error-reporting.ts b/packages/@cdktn/cli-core/src/lib/error-reporting.ts index 094141a06..bd0df792c 100644 --- a/packages/@cdktn/cli-core/src/lib/error-reporting.ts +++ b/packages/@cdktn/cli-core/src/lib/error-reporting.ts @@ -4,6 +4,8 @@ import * as Sentry from "@sentry/node"; import { getProjectId, getUserId, + getUsageTelemetryConsent, + setUsageTelemetryEnabled, collectDebugInformation, DISPLAY_VERSION, } from "@cdktn/commons"; @@ -21,6 +23,13 @@ export function shouldReportCrash( fs.readFileSync(path.resolve(projectPath, "cdktf.json"), "utf8"), ); + // tri-state: an absent flag means "unset" and triggers the + // interactive consent prompt; outside a project (no readable + // cdktf.json) crash reporting stays off + if (!("sendCrashReports" in cdktfJson)) { + return undefined; + } + return typeof cdktfJson.sendCrashReports === "boolean" ? cdktfJson.sendCrashReports : cdktfJson.sendCrashReports === "true"; @@ -32,20 +41,35 @@ export function shouldReportCrash( } } -export function persistReportCrashReportDecision( +function persistConsentDecision( + key: "sendCrashReports" | "sendUsageTelemetry", decision: boolean, projectPath = process.cwd(), ) { const cdktfJson = JSON.parse( fs.readFileSync(path.resolve(projectPath, "cdktf.json"), "utf8"), ); - cdktfJson.sendCrashReports = decision; + cdktfJson[key] = decision; fs.writeFileSync( path.resolve(projectPath, "cdktf.json"), JSON.stringify(cdktfJson, null, 2), ); } +export function persistReportCrashReportDecision( + decision: boolean, + projectPath = process.cwd(), +) { + persistConsentDecision("sendCrashReports", decision, projectPath); +} + +export function persistSendUsageTelemetryDecision( + decision: boolean, + projectPath = process.cwd(), +) { + persistConsentDecision("sendUsageTelemetry", decision, projectPath); +} + function isPromise(p: any): p is Promise { return ( typeof p === "object" && @@ -54,33 +78,63 @@ function isPromise(p: any): p is Promise { ); } +/** + * `projectPath` is the project whose consent flags apply: the cwd for every + * command, the freshly created project for `init`. + */ export async function initializErrorReporting( - runConsentPrompt?: () => Promise, + runCrashConsentPrompt?: () => Promise, + runUsageTelemetryConsentPrompt?: () => Promise, + projectPath = process.cwd(), ) { - let shouldReport = shouldReportCrash(); - const ci: string | false = ciInfo.isCI ? ciInfo.name || "unknown" : false; - - // We have no info yet, so we need to ask the user - if (shouldReport === undefined && runConsentPrompt) { - // But only if it's a user - if (ci) { - return; - } + let shouldReport = shouldReportCrash(projectPath); + let usageConsent = getUsageTelemetryConsent(projectPath); + + // Prompting requires a real user at a terminal (TTY and not CI) and a + // cdktf.json to persist the decision into; otherwise fall through to + // the per-flag non-interactive defaults below. + const canPrompt = + Boolean(process.stdout.isTTY) && + !ciInfo.isCI && + !process.env.CI && + fs.existsSync(path.resolve(projectPath, "cdktf.json")); - shouldReport = await runConsentPrompt(); - persistReportCrashReportDecision(shouldReport); + if (canPrompt) { + if (shouldReport === undefined && runCrashConsentPrompt) { + shouldReport = await runCrashConsentPrompt(); + persistReportCrashReportDecision(shouldReport, projectPath); + } + if ( + usageConsent === undefined && + runUsageTelemetryConsentPrompt && + !process.env.CHECKPOINT_DISABLE + ) { + usageConsent = await runUsageTelemetryConsentPrompt(); + persistSendUsageTelemetryDecision(usageConsent, projectPath); + } } - if (!shouldReport) { - logger.debug("Error reporting disabled"); + // Non-interactive defaults: crash reporting is opt-in (off), usage + // telemetry is on unless CHECKPOINT_DISABLE is set. + const crashReportingEnabled = shouldReport === true; + const usageTelemetryEnabled = + !process.env.CHECKPOINT_DISABLE && usageConsent !== false; + + // Capture the decision while we are still in the user's working + // directory: some commands (convert) chdir into a temporary project + // before sendTelemetry runs and must not consult that project's flags. + setUsageTelemetryEnabled(usageTelemetryEnabled); + + if (!crashReportingEnabled && !usageTelemetryEnabled) { + logger.debug("Error reporting and usage telemetry disabled"); return; } if (!process.env.SENTRY_DSN) { - logger.info("Error reporting disabled: SENTRY_DSN not set"); + logger.info("Reporting disabled: SENTRY_DSN not set"); return; } - logger.debug("Initializing error reporting"); + logger.debug("Initializing reporting"); Sentry.init({ dsn: process.env.SENTRY_DSN, @@ -97,6 +151,12 @@ export async function initializErrorReporting( // SDK default flip cannot silently stop delivery. enableMetrics: true, async beforeSend(event, hint) { + // Error/crash events require their own consent: when Sentry is + // initialized only for usage metrics, drop every error event + // (metrics do not pass through beforeSend). + if (!crashReportingEnabled) { + return null; + } if (!hint) { return event; } @@ -136,10 +196,12 @@ export async function initializErrorReporting( }); scope.setTag("projectId", getProjectId()); - logger.debug("Collecting environment information for error reporting"); - collectDebugInformation().then((debugOutput) => { - Sentry.setContext("environment", debugOutput); - }); + if (crashReportingEnabled) { + logger.debug("Collecting environment information for error reporting"); + collectDebugInformation().then((debugOutput) => { + Sentry.setContext("environment", debugOutput); + }); + } } export function captureException({ diff --git a/packages/@cdktn/cli-core/src/lib/init.ts b/packages/@cdktn/cli-core/src/lib/init.ts index 3e1b4825f..c8318d57d 100644 --- a/packages/@cdktn/cli-core/src/lib/init.ts +++ b/packages/@cdktn/cli-core/src/lib/init.ts @@ -43,6 +43,7 @@ export type InitArgs = { projectInfo: Project; templatePath: string; sendCrashReports: boolean; + sendUsageTelemetry: boolean; silent?: boolean; }; @@ -64,6 +65,7 @@ export async function init({ projectInfo, templatePath, sendCrashReports, + sendUsageTelemetry, providers, providersForceLocal, silent, @@ -84,6 +86,7 @@ export async function init({ futureFlags, projectId, sendCrashReports, + sendUsageTelemetry, silent, }); const cdktfConfig = CdktfConfig.read(destination); diff --git a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts index 8e0f4b3f3..54f15719c 100644 --- a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts +++ b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts @@ -21,17 +21,28 @@ jest.mock("@sentry/node", () => ({ jest.mock("ci-info", () => ({ isCI: false, name: null })); +// the capture setters call through so the gating cases below observe the +// real captured state, while init can still be asserted to perform them jest.mock("@cdktn/commons", () => { const actual = jest.requireActual("@cdktn/commons"); return { ...actual, collectDebugInformation: jest.fn().mockResolvedValue({}), + setUsageTelemetryEnabled: jest.fn(actual.setUsageTelemetryEnabled), }; }); import * as Sentry from "@sentry/node"; import ciInfo from "ci-info"; -import { initializErrorReporting } from "../lib/error-reporting"; +import { + isUsageTelemetryEnabled, + setUsageTelemetryEnabled, +} from "@cdktn/commons"; +import { + initializErrorReporting, + shouldReportCrash, + persistSendUsageTelemetryDecision, +} from "../lib/error-reporting"; // the ci-info mock above replaces the module with a plain mutable object, // so tests can flip isCI; the published types declare it readonly @@ -45,6 +56,7 @@ describe("Sentry init hardening", () => { const originalEnv = { CI: process.env.CI, SENTRY_DSN: process.env.SENTRY_DSN, + CHECKPOINT_DISABLE: process.env.CHECKPOINT_DISABLE, }; const originalIsTTY = process.stdout.isTTY; @@ -64,11 +76,13 @@ describe("Sentry init hardening", () => { workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-consent-")); process.chdir(workdir); delete process.env.CI; + delete process.env.CHECKPOINT_DISABLE; process.env.SENTRY_DSN = "https://public@example.invalid/1"; ciInfoMock.isCI = false; }); afterEach(() => { + setUsageTelemetryEnabled(undefined); process.chdir(originalCwd); fs.removeSync(workdir); Object.defineProperty(process.stdout, "isTTY", { @@ -84,13 +98,102 @@ describe("Sentry init hardening", () => { } }); + it("upgrade path: crash set, usage unset, interactive -> prompts ONCE for usage only and persists", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(true); + const crashPrompt = jest.fn().mockResolvedValue(true); + const usagePrompt = jest.fn().mockResolvedValue(true); + + await initializErrorReporting(crashPrompt, usagePrompt); + + expect(crashPrompt).not.toHaveBeenCalled(); + expect(usagePrompt).toHaveBeenCalledTimes(1); + expect(fs.readJsonSync(path.join(workdir, "cdktf.json"))).toMatchObject({ + sendCrashReports: true, + sendUsageTelemetry: true, + }); + expect(Sentry.init).toHaveBeenCalledTimes(1); + }); + + it("both unset, interactive -> prompts for each flag and persists both", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), {}); + setInteractive(true); + const crashPrompt = jest.fn().mockResolvedValue(false); + const usagePrompt = jest.fn().mockResolvedValue(false); + + await initializErrorReporting(crashPrompt, usagePrompt); + + expect(crashPrompt).toHaveBeenCalledTimes(1); + expect(usagePrompt).toHaveBeenCalledTimes(1); + expect(fs.readJsonSync(path.join(workdir, "cdktf.json"))).toMatchObject({ + sendCrashReports: false, + sendUsageTelemetry: false, + }); + // both declined -> no Sentry at all + expect(Sentry.init).not.toHaveBeenCalled(); + }); + + // Every condition that makes prompting impossible falls through to the + // non-interactive defaults: no prompt, nothing persisted, usage default-on. + it.each([ + ["no TTY", () => setInteractive(false), true], + [ + "TTY but ciInfo.isCI", + () => { + setInteractive(true); + ciInfoMock.isCI = true; + }, + true, + ], + [ + "TTY but CI env var", + () => { + setInteractive(true); + process.env.CI = "true"; + }, + true, + ], + [ + "TTY but no cdktf.json (no-project command)", + () => setInteractive(true), + false, + ], + ])( + "usage unset, %s -> no prompt, nothing persisted, default-on init", + async (_case, arrange, hasProject) => { + if (hasProject) { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + }); + } + arrange(); + const crashPrompt = jest.fn(); + const usagePrompt = jest.fn(); + + await initializErrorReporting(crashPrompt, usagePrompt); + + expect(crashPrompt).not.toHaveBeenCalled(); + expect(usagePrompt).not.toHaveBeenCalled(); + if (hasProject) { + expect( + fs.readJsonSync(path.join(workdir, "cdktf.json")).sendUsageTelemetry, + ).toBeUndefined(); + } else { + expect(fs.existsSync(path.join(workdir, "cdktf.json"))).toBe(false); + } + expect(Sentry.init).toHaveBeenCalledTimes(1); + }, + ); + it("starts a fresh trace so nothing seeded from SENTRY_TRACE/SENTRY_BAGGAGE propagates", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: true, sendUsageTelemetry: true, }); - await initializErrorReporting(jest.fn()); + await initializErrorReporting(jest.fn(), jest.fn()); expect(mockScope.setPropagationContext).toHaveBeenCalledWith( expect.objectContaining({ @@ -99,6 +202,144 @@ describe("Sentry init hardening", () => { ); }); + it("captures the usage decision while still in the user's cwd", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + }); + setInteractive(false); + + await initializErrorReporting(); + + expect(setUsageTelemetryEnabled).toHaveBeenCalledWith(true); + }); + + it("reads and persists against an explicit project path, not the cwd", async () => { + // init creates the project in a destination directory and initializes + // reporting against it; the cwd may hold an unrelated (or no) cdktf.json + const destination = path.join(workdir, "new-project"); + fs.mkdirpSync(destination); + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + sendUsageTelemetry: false, + }); + fs.writeJsonSync(path.join(destination, "cdktf.json"), { + sendCrashReports: false, + sendUsageTelemetry: true, + }); + setInteractive(true); + const crashPrompt = jest.fn(); + const usagePrompt = jest.fn(); + + await initializErrorReporting(crashPrompt, usagePrompt, destination); + + expect(crashPrompt).not.toHaveBeenCalled(); + expect(usagePrompt).not.toHaveBeenCalled(); + expect(setUsageTelemetryEnabled).toHaveBeenCalledWith(true); + expect(isUsageTelemetryEnabled()).toBe(true); + // usage-only consent: the client exists but drops error events + await expect(initOptions().beforeSend({}, undefined)).resolves.toBeNull(); + }); + + it("prompts into the explicit project path when its flags are unset", async () => { + const destination = path.join(workdir, "new-project"); + fs.mkdirpSync(destination); + fs.writeJsonSync(path.join(destination, "cdktf.json"), {}); + setInteractive(true); + const crashPrompt = jest.fn().mockResolvedValue(true); + const usagePrompt = jest.fn().mockResolvedValue(false); + + await initializErrorReporting(crashPrompt, usagePrompt, destination); + + expect(fs.readJsonSync(path.join(destination, "cdktf.json"))).toEqual({ + sendCrashReports: true, + sendUsageTelemetry: false, + }); + expect(fs.existsSync(path.join(workdir, "cdktf.json"))).toBe(false); + }); + + it("CHECKPOINT_DISABLE -> no usage prompt, no init when crash is off", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + }); + setInteractive(true); + process.env.CHECKPOINT_DISABLE = "1"; + const usagePrompt = jest.fn(); + + await initializErrorReporting(jest.fn(), usagePrompt); + + expect(usagePrompt).not.toHaveBeenCalled(); + expect(Sentry.init).not.toHaveBeenCalled(); + }); + + it("CHECKPOINT_DISABLE does NOT affect crash reporting", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(false); + process.env.CHECKPOINT_DISABLE = "1"; + + await initializErrorReporting(); + + expect(Sentry.init).toHaveBeenCalledTimes(1); + }); + + it("explicit sendUsageTelemetry: false + crash off -> Sentry never initialized", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + sendUsageTelemetry: false, + }); + setInteractive(false); + + await initializErrorReporting(); + + expect(Sentry.init).not.toHaveBeenCalled(); + }); + + it("no SENTRY_DSN -> no init even with consent", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + sendUsageTelemetry: true, + }); + setInteractive(false); + delete process.env.SENTRY_DSN; + + await initializErrorReporting(); + + expect(Sentry.init).not.toHaveBeenCalled(); + }); + + // The resolved decision is captured for the whole run: commands that chdir + // into another project (convert) must not re-read that project's flag. + it.each([ + [ + "CHECKPOINT_DISABLE", + { CHECKPOINT_DISABLE: "1" }, + { sendUsageTelemetry: true }, + false, + ], + ["flag unset, no TTY", {}, { sendUsageTelemetry: false }, true], + ])( + "captures the usage decision at init (%s) for every later project directory", + async (_case, env, otherProject, expected) => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + }); + setInteractive(false); + Object.assign(process.env, env); + const other = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-other-")); + fs.writeJsonSync(path.join(other, "cdktf.json"), otherProject); + + try { + await initializErrorReporting(); + delete process.env.CHECKPOINT_DISABLE; + + expect(isUsageTelemetryEnabled(other)).toBe(expected); + } finally { + fs.removeSync(other); + } + }, + ); + it("init options pin release, tracesSampleRate 0, a fixed environment, a fixed serverName and enableMetrics", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: true, @@ -118,4 +359,106 @@ describe("Sentry init hardening", () => { enableMetrics: true, }); }); + + describe("beforeSend", () => { + const boom = { message: "boom" }; + + it("passes error events through when crash reporting is consented, even under CHECKPOINT_DISABLE", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(false); + process.env.CHECKPOINT_DISABLE = "1"; + + await initializErrorReporting(); + + expect( + await initOptions().beforeSend(boom, { + originalException: new Error("boom"), + }), + ).toBe(boom); + }); + + it("drops error events on a usage-only init (crash declined)", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + sendUsageTelemetry: true, + }); + setInteractive(false); + + await initializErrorReporting(); + + expect(Sentry.init).toHaveBeenCalledTimes(1); + expect( + await initOptions().beforeSend(boom, { + originalException: new Error("boom"), + }), + ).toBeNull(); + }); + + it("still drops Usage Errors when crash reporting is enabled", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(false); + + await initializErrorReporting(); + + expect( + await initOptions().beforeSend( + { message: "x" }, + { originalException: new Error("Usage Error: bad input") }, + ), + ).toBeNull(); + }); + }); +}); + +describe("shouldReportCrash tri-state", () => { + let workdir: string; + + beforeEach(() => { + workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-crash-")); + }); + + afterEach(() => { + fs.removeSync(workdir); + }); + + it.each([ + [{ sendCrashReports: true }, true], + [{ sendCrashReports: false }, false], + [{ sendCrashReports: "true" }, true], + [{ sendCrashReports: "false" }, false], + // undefined, not false, for an absent flag: that is what triggers the + // crash-consent prompt + [{}, undefined], + ])("reads %j as %p", (config, expected) => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), config); + expect(shouldReportCrash(workdir)).toBe(expected); + }); + + it("returns false outside a project (missing cdktf.json)", () => { + expect(shouldReportCrash(workdir)).toBe(false); + }); +}); + +describe("persistSendUsageTelemetryDecision", () => { + it("writes the decision without clobbering other keys", () => { + const workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-persist-")); + try { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + sendCrashReports: true, + }); + persistSendUsageTelemetryDecision(false, workdir); + expect(fs.readJsonSync(path.join(workdir, "cdktf.json"))).toEqual({ + language: "typescript", + sendCrashReports: true, + sendUsageTelemetry: false, + }); + } finally { + fs.removeSync(workdir); + } + }); }); diff --git a/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts b/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts index c5e23de6d..f04d12893 100644 --- a/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts +++ b/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts @@ -52,6 +52,7 @@ describeIfDistExists(__dirname)("CdktfProject", () => { Name: "cdktf-api-test", }, sendCrashReports: false, + sendUsageTelemetry: false, dist: path.join(__dirname, "../../../../../../dist"), }); diff --git a/packages/@cdktn/cli-core/src/test/lib/terraform-parallelism.test.ts b/packages/@cdktn/cli-core/src/test/lib/terraform-parallelism.test.ts index 7b75b4366..cf507b5cb 100644 --- a/packages/@cdktn/cli-core/src/test/lib/terraform-parallelism.test.ts +++ b/packages/@cdktn/cli-core/src/test/lib/terraform-parallelism.test.ts @@ -110,6 +110,7 @@ describeIfDistExists(__dirname)("terraform parallelism", () => { Name: projectName, }, sendCrashReports: false, + sendUsageTelemetry: false, dist: path.join(__dirname, "../../../../../../dist"), }); diff --git a/packages/@cdktn/cli-core/templates/csharp/cdktf.json b/packages/@cdktn/cli-core/templates/csharp/cdktf.json index c99823a2b..6f16bdbf2 100644 --- a/packages/@cdktn/cli-core/templates/csharp/cdktf.json +++ b/packages/@cdktn/cli-core/templates/csharp/cdktf.json @@ -7,6 +7,7 @@ "terraform": ">=1.5.7", "opentofu": ">=1.6.0" }, + "sendUsageTelemetry": "{{sendUsageTelemetry}}", "terraformProviders": [], "terraformModules": [], "context": { diff --git a/packages/@cdktn/cli-core/templates/go/cdktf.json b/packages/@cdktn/cli-core/templates/go/cdktf.json index ac3437e60..99cdc414a 100644 --- a/packages/@cdktn/cli-core/templates/go/cdktf.json +++ b/packages/@cdktn/cli-core/templates/go/cdktf.json @@ -8,6 +8,7 @@ "terraform": ">=1.5.7", "opentofu": ">=1.6.0" }, + "sendUsageTelemetry": "{{sendUsageTelemetry}}", "terraformProviders": [], "terraformModules": [], "context": { diff --git a/packages/@cdktn/cli-core/templates/java/cdktf.json b/packages/@cdktn/cli-core/templates/java/cdktf.json index 13460b4b5..d42b58146 100644 --- a/packages/@cdktn/cli-core/templates/java/cdktf.json +++ b/packages/@cdktn/cli-core/templates/java/cdktf.json @@ -3,6 +3,7 @@ "app": "./gradlew run", "projectId": "{{projectId}}", "sendCrashReports": "{{sendCrashReports}}", + "sendUsageTelemetry": "{{sendUsageTelemetry}}", "codeMakerOutput": "imports", "targetVersions": { "terraform": ">=1.5.7", diff --git a/packages/@cdktn/cli-core/templates/python-pip/cdktf.json b/packages/@cdktn/cli-core/templates/python-pip/cdktf.json index 1984ec45a..b2a977ff4 100644 --- a/packages/@cdktn/cli-core/templates/python-pip/cdktf.json +++ b/packages/@cdktn/cli-core/templates/python-pip/cdktf.json @@ -7,6 +7,7 @@ "terraform": ">=1.5.7", "opentofu": ">=1.6.0" }, + "sendUsageTelemetry": "{{sendUsageTelemetry}}", "terraformProviders": [], "terraformModules": [], "codeMakerOutput": "imports", diff --git a/packages/@cdktn/cli-core/templates/python/cdktf.json b/packages/@cdktn/cli-core/templates/python/cdktf.json index 28e589d8c..f02016fca 100644 --- a/packages/@cdktn/cli-core/templates/python/cdktf.json +++ b/packages/@cdktn/cli-core/templates/python/cdktf.json @@ -7,6 +7,7 @@ "terraform": ">=1.5.7", "opentofu": ">=1.6.0" }, + "sendUsageTelemetry": "{{sendUsageTelemetry}}", "terraformProviders": [], "terraformModules": [], "codeMakerOutput": "imports", diff --git a/packages/@cdktn/cli-core/templates/typescript/cdktf.json b/packages/@cdktn/cli-core/templates/typescript/cdktf.json index 0d75dc8d4..33885c144 100644 --- a/packages/@cdktn/cli-core/templates/typescript/cdktf.json +++ b/packages/@cdktn/cli-core/templates/typescript/cdktf.json @@ -7,6 +7,7 @@ "terraform": ">=1.5.7", "opentofu": ">=1.6.0" }, + "sendUsageTelemetry": "{{sendUsageTelemetry}}", "terraformProviders": [], "terraformModules": [], "context": { diff --git a/packages/cdktn-cli/src/bin/cmds/handlers.ts b/packages/cdktn-cli/src/bin/cmds/handlers.ts index 2bd396eb4..f4795ef7b 100644 --- a/packages/cdktn-cli/src/bin/cmds/handlers.ts +++ b/packages/cdktn-cli/src/bin/cmds/handlers.ts @@ -57,7 +57,10 @@ import { verifySimilarLibraryVersion, } from "./helper/check-environment"; import { sanitizeVarFiles } from "./helper/var-files"; -import { askForCrashReportingConsent } from "./helper/error-reporting"; +import { + askForCrashReportingConsent, + askForUsageTelemetryConsent, +} from "./helper/error-reporting"; import { startPerformanceMonitoring } from "./helper/performance"; import path from "path"; import os from "os"; @@ -146,6 +149,7 @@ export async function convert({ projectDescription: "Temporary project for conversion", local: true, enableCrashReporting: false, + enableUsageTelemetry: false, fromTerraformProject: "no", dist: pkg.version === "0.0.0" ? dist : undefined, cdktfVersion: pkg.version, @@ -177,7 +181,10 @@ export async function convert({ } export async function deploy(argv: any) { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); await checkEnvironment(); @@ -229,7 +236,10 @@ export async function deploy(argv: any) { } export async function destroy(argv: any) { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); await checkEnvironment(); @@ -267,7 +277,10 @@ export async function destroy(argv: any) { } export async function diff(argv: any) { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); await checkEnvironment(); @@ -315,7 +328,10 @@ export async function get(argv: { try { throwIfNotProjectDirectory(); await displayVersionMessage(); - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); await checkEnvironment(); await verifySimilarLibraryVersion(); const config = readConfigSync(); // read config again to be up-to-date (if called via 'add' command) @@ -391,7 +407,10 @@ export async function init(argv: any) { } export async function list(argv: any) { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); await checkEnvironment(); @@ -451,7 +470,10 @@ export async function synth(argv: any) { : () => {}; try { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); await checkEnvironment(); @@ -482,7 +504,10 @@ export async function synth(argv: any) { } export async function watch(argv: any) { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); const command = argv.app; @@ -511,7 +536,10 @@ export async function watch(argv: any) { } export async function output(argv: any) { - await initializErrorReporting(askForCrashReportingConsent); + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); throwIfNotProjectDirectory(); await displayVersionMessage(); await checkEnvironment(); diff --git a/packages/cdktn-cli/src/bin/cmds/helper/__tests__/init-telemetry.test.ts b/packages/cdktn-cli/src/bin/cmds/helper/__tests__/init-telemetry.test.ts new file mode 100644 index 000000000..49a09b581 --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/helper/__tests__/init-telemetry.test.ts @@ -0,0 +1,94 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; + +jest.mock("@cdktn/cli-core", () => { + const actual = jest.requireActual("@cdktn/cli-core"); + return { + ...actual, + init: jest.fn().mockResolvedValue(false), + initializErrorReporting: jest.fn().mockResolvedValue(undefined), + CdktfConfig: { + read: jest.fn(() => ({ + language: "typescript", + codeMakerOutput: ".gen", + })), + }, + }; +}); + +jest.mock("@cdktn/commons", () => { + const actual = jest.requireActual("@cdktn/commons"); + return { ...actual, sendTelemetry: jest.fn().mockResolvedValue(undefined) }; +}); + +jest.mock("../terraform-check", () => ({ + getTerraformVersion: jest.fn().mockResolvedValue(undefined), + terraformCheck: jest.fn().mockResolvedValue(undefined), +})); + +import { initializErrorReporting } from "@cdktn/cli-core"; +import { sendTelemetry, setUsageTelemetryEnabled } from "@cdktn/commons"; +import { runInit } from "../init"; + +const callOrder = (mock: unknown) => + (mock as jest.Mock).mock.invocationCallOrder[0]; + +describe("runInit telemetry wiring", () => { + let destination: string; + + const init = () => + runInit({ + destination, + local: true, + silent: true, + nonInteractive: true, + template: "typescript", + projectName: "test", + projectDescription: "test", + fromTerraformProject: "no", + enableCrashReporting: false, + enableUsageTelemetry: true, + providers: ["aws"], + }); + + beforeEach(() => { + jest.clearAllMocks(); + destination = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-init-")); + setUsageTelemetryEnabled(undefined); + }); + + afterEach(() => { + setUsageTelemetryEnabled(undefined); + fs.removeSync(destination); + }); + + it("initializes reporting against the destination before sending the init metric", async () => { + await init(); + + expect(initializErrorReporting).toHaveBeenCalledWith( + undefined, + undefined, + path.resolve(destination), + ); + expect(callOrder(initializErrorReporting)).toBeLessThan( + callOrder(sendTelemetry), + ); + expect(sendTelemetry).toHaveBeenCalledWith("init", { + template: "typescript", + addedProviders: ["aws"], + language: "typescript", + }); + }); + + it("leaves an already captured decision alone (convert drives init in a throwaway project)", async () => { + setUsageTelemetryEnabled(false); + + await init(); + + expect(initializErrorReporting).not.toHaveBeenCalled(); + expect(sendTelemetry).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts b/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts index 9fe802470..0c2db1197 100644 --- a/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts +++ b/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts @@ -12,3 +12,11 @@ export async function askForCrashReportingConsent() { default: true, }); } + +export async function askForUsageTelemetryConsent() { + return await confirm({ + message: + "Do you want to send anonymous usage telemetry (command, language, timing) to the CDKTN team? Refer to https://cdktn.io/docs/telemetry for more information", + default: true, + }); +} diff --git a/packages/cdktn-cli/src/bin/cmds/helper/init.ts b/packages/cdktn-cli/src/bin/cmds/helper/init.ts index 89398a5ce..5d442a3c0 100644 --- a/packages/cdktn-cli/src/bin/cmds/helper/init.ts +++ b/packages/cdktn-cli/src/bin/cmds/helper/init.ts @@ -16,6 +16,7 @@ import { Project, CdktfConfig, getAllPrebuiltProviders, + initializErrorReporting, } from "@cdktn/cli-core"; import { convertProject, @@ -34,6 +35,7 @@ import { logFileName, logger, Errors, + hasCapturedUsageTelemetryDecision, sendTelemetry, ConstructsMakerProviderTarget, } from "@cdktn/commons"; @@ -42,7 +44,10 @@ import ciDetect from "@npmcli/ci-detect"; import { isInteractiveTerminal } from "./check-environment"; import { getTerraformVersion } from "./terraform-check"; import * as semver from "semver"; -import { askForCrashReportingConsent } from "./error-reporting"; +import { + askForCrashReportingConsent, + askForUsageTelemetryConsent, +} from "./error-reporting"; const chalkColour = new chalk.Instance(); @@ -78,6 +83,7 @@ type Options = { destination: string; fromTerraformProject?: string; enableCrashReporting?: boolean; + enableUsageTelemetry?: boolean; tfeHostname?: string; silent?: boolean; nonInteractive?: boolean; @@ -141,7 +147,6 @@ This means that your Terraform state file will be stored locally on disk in a fi argv.projectDescription, ); const projectId = randomUUID(); - telemetryData.projectId = projectId; let fromTerraformProject = argv.fromTerraformProject || undefined; if (!fromTerraformProject) { @@ -173,9 +178,15 @@ This means that your Terraform state file will be stored locally on disk in a fi } const ci: string | false = ciDetect(); + // Prompts only run for a real user at a terminal; non-interactive + // defaults are crash reporting off, usage telemetry on. + const interactive = !ci && !argv.nonInteractive && isInteractiveTerminal(); const sendCrashReports = argv.enableCrashReporting ?? - (ci ? false : await askForCrashReportingConsent()); + (interactive ? await askForCrashReportingConsent() : false); + const sendUsageTelemetry = + argv.enableUsageTelemetry ?? + (interactive ? await askForUsageTelemetryConsent() : true); const providers = argv.providers?.length || argv.nonInteractive ? argv.providers @@ -226,13 +237,14 @@ This means that your Terraform state file will be stored locally on disk in a fi projectInfo, templatePath: templateInfo.Path, sendCrashReports: sendCrashReports, + sendUsageTelemetry: sendUsageTelemetry, providers, providersForceLocal: argv.providersForceLocal, silent: argv.silent, }); if (convertResult && importPath) { - const { code, cdktfJson, stats } = convertResult; + const { code, cdktfJson } = convertResult; const mainTs = fs.readFileSync( path.resolve(destination, "main.ts"), @@ -270,8 +282,6 @@ This means that your Terraform state file will be stored locally on disk in a fi } execSync("npm run get", { cwd: destination }); } - - telemetryData.conversionStats = stats; } if (templateInfo.cleanupTemporaryFiles) { @@ -284,6 +294,17 @@ This means that your Terraform state file will be stored locally on disk in a fi telemetryData.addedProviders = providers; } + // The consent answers are now persisted in the new project's cdktf.json; + // reporting is initialized against it so the init metric honours them. + // convert drives init inside a throwaway project and already captured its own. + if (!hasCapturedUsageTelemetryDecision()) { + await initializErrorReporting( + undefined, + undefined, + path.resolve(destination), + ); + } + await sendTelemetry("init", { ...telemetryData, language: cdktfConfig.language, diff --git a/packages/cdktn-cli/src/bin/cmds/init.ts b/packages/cdktn-cli/src/bin/cmds/init.ts index 1a4c744b7..de07dc42d 100644 --- a/packages/cdktn-cli/src/bin/cmds/init.ts +++ b/packages/cdktn-cli/src/bin/cmds/init.ts @@ -49,6 +49,10 @@ class Command extends BaseCommand { type: "boolean", desc: "Enable crash reporting for the CLI, refer to https://cdktn.io/docs/telemetry#crash-reporting for more details", }) + .option("enable-usage-telemetry", { + type: "boolean", + desc: "Enable anonymous usage telemetry for the CLI, refer to https://cdktn.io/docs/telemetry for more details", + }) .option("providers", { describe: "Providers to add to your project", type: "array", From 02e9b82ffa25711ab9379d409d6a1665e6895863 Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 01:15:54 +0700 Subject: [PATCH 03/17] feat(cli): count failed command runs and flush through flushTelemetry --- .../@cdktn/cli-core/src/lib/synth-stack.ts | 32 +++- .../cli-core/src/test/lib/synth-stack.test.ts | 65 +++++++++ .../error-handling.integration.test.ts | 51 ++++++- .../src/bin/__tests__/error-handling.test.ts | 138 +++++++++++++++++- .../bin/cmds/__tests__/error-scope.test.ts | 62 ++++++++ .../src/bin/cmds/ui/__tests__/get.test.ts | 107 ++++++++++++++ packages/cdktn-cli/src/bin/cmds/ui/get.ts | 8 +- packages/cdktn-cli/src/bin/error-handling.ts | 34 ++++- 8 files changed, 478 insertions(+), 19 deletions(-) create mode 100644 packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts create mode 100644 packages/cdktn-cli/src/bin/cmds/__tests__/error-scope.test.ts create mode 100644 packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts diff --git a/packages/@cdktn/cli-core/src/lib/synth-stack.ts b/packages/@cdktn/cli-core/src/lib/synth-stack.ts index 5f811bd8c..a67338c98 100644 --- a/packages/@cdktn/cli-core/src/lib/synth-stack.ts +++ b/packages/@cdktn/cli-core/src/lib/synth-stack.ts @@ -11,7 +11,14 @@ import { TerraformStackMetadata, } from "cdktn"; import { performance } from "perf_hooks"; -import { logger, readConfigSync, sendTelemetry, shell } from "@cdktn/commons"; +import { + commandErrorType, + flushTelemetry, + logger, + readConfigSync, + sendTelemetry, + shell, +} from "@cdktn/commons"; import { CdktfConfig } from "./cdktf-config"; import { format } from "@cdktn/hcl-tools"; @@ -164,12 +171,15 @@ Command output on stdout: ` : "" }`; - await this.synthErrorTelemetry(synthOrigin); if (graceful) { e.errorOutput = errorOutput; throw e; } console.error(`ERROR: ${errorOutput}`); + // hard exit skips the entrypoint's failure reporter and flush, so + // count and flush the failed run here (bounded) + await this.synthErrorTelemetry(e, synthOrigin); + await flushTelemetry(); process.exit(1); } @@ -190,6 +200,8 @@ Command output on stdout: throw new Error(errorMessage); } logger.error(errorMessage); + await this.synthErrorTelemetry(e, synthOrigin); + await flushTelemetry(); process.exit(1); } @@ -290,8 +302,20 @@ Command output on stdout: }); } - public static async synthErrorTelemetry(synthOrigin?: SynthOrigin) { - await sendTelemetry("synth", { error: true, synthOrigin }); + /** + * One `cli.command.error` per failed run: counted here only on the paths + * above that exit the process themselves; anything thrown is counted by + * the CLI entrypoint's failure reporter instead. + */ + public static async synthErrorTelemetry( + error: unknown, + synthOrigin?: SynthOrigin, + ) { + await sendTelemetry("synth", { + error: true, + errorType: commandErrorType(error), + synthOrigin, + }); } } diff --git a/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts b/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts new file mode 100644 index 000000000..87e0069d1 --- /dev/null +++ b/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts @@ -0,0 +1,65 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; +import { SynthStack } from "../../lib/synth-stack"; + +jest.mock("@cdktn/commons", () => ({ + ...jest.requireActual("@cdktn/commons"), + sendTelemetry: jest.fn().mockResolvedValue(undefined), + flushTelemetry: jest.fn().mockResolvedValue(undefined), +})); + +describe("SynthStack.synth failure paths count the run before exiting", () => { + const commons = jest.requireMock("@cdktn/commons") as { + sendTelemetry: jest.Mock; + flushTelemetry: jest.Mock; + }; + let outdir: string; + let exitSpy: jest.SpyInstance; + let errorSpy: jest.SpyInstance; + + beforeEach(() => { + outdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-synth-stack-")); + commons.sendTelemetry.mockClear(); + commons.flushTelemetry.mockClear(); + // a real exit would end jest; the sentinel stops synth where exit would + exitSpy = jest.spyOn(process, "exit").mockImplementation((( + code: number, + ) => { + throw new Error(`exit ${code}`); + }) as never); + errorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + exitSpy.mockRestore(); + errorSpy.mockRestore(); + fs.removeSync(outdir); + }); + + it.each([ + ["the app exits non-zero", 'node -e "process.exit(1)"'], + ["the app never writes a manifest", 'node -e ""'], + ])( + "emits one cli.command.error, flushes, then exits 1 when %s", + async (_case, app) => { + await expect( + SynthStack.synth(new AbortController().signal, app, outdir), + ).rejects.toThrow("exit 1"); + + expect(commons.sendTelemetry).toHaveBeenCalledTimes(1); + expect(commons.sendTelemetry).toHaveBeenCalledWith("synth", { + error: true, + errorType: "unexpected", + synthOrigin: undefined, + }); + expect(commons.flushTelemetry).toHaveBeenCalledTimes(1); + expect(commons.sendTelemetry.mock.invocationCallOrder[0]).toBeLessThan( + commons.flushTelemetry.mock.invocationCallOrder[0], + ); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + ); +}); diff --git a/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts b/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts index 51e840849..b2ed0271e 100644 --- a/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts +++ b/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts @@ -18,6 +18,7 @@ function fixtureSource(errorHandlingPath: string): string { return ` import yargs from "yargs"; import * as Sentry from "@sentry/node"; + import { Errors, setUsageTelemetryEnabled } from "@cdktn/commons"; import { runCli } from ${JSON.stringify(errorHandlingPath)}; if (process.argv.includes("--with-listener")) { @@ -40,6 +41,9 @@ function fixtureSource(errorHandlingPath: string): string { serverName: "cdktn-cli", }); } + if (process.env.TEST_USAGE_TELEMETRY === "1") { + setUsageTelemetryEnabled(true); + } // cdktn.ts' completion function, minus the manifest: answers for "diff" // only after a turn of the event loop @@ -77,6 +81,7 @@ function fixtureSource(errorHandlingPath: string): string { "throws an async Error that should reach Sentry", () => {}, async () => { + Errors.setScope("capturedboom"); throw new Error("empirical-sentry-message"); }, ) @@ -178,6 +183,32 @@ function expectNoRuntimeNoise(output: string) { expect(output).not.toContain("Node.js v"); } +type MetricItem = { + name: string; + attributes: Record; +}; + +// A third copy of the envelope parser (test helper, sink, here); unifying +// them is tracked as a follow-up. +function parseMetricItems(bodies: string[]): MetricItem[] { + const items: MetricItem[] = []; + for (const body of bodies) { + const lines = body.split("\n").filter(Boolean); + for (let i = 0; i < lines.length - 1; i++) { + let header; + try { + header = JSON.parse(lines[i]); + } catch { + continue; + } + if (header?.type === "trace_metric") { + items.push(...JSON.parse(lines[i + 1]).items); + } + } + } + return items; +} + describe("runCli child-process smoke test", () => { let bundlePath: string; @@ -195,7 +226,7 @@ describe("runCli child-process smoke test", () => { // The fixture sits under os.tmpdir() with no node_modules ancestry, so its // bare imports are aliased to the workspace copies error-handling.ts itself - // resolves: one Sentry client. + // resolves: one Sentry client, one telemetry state. await esbuild.build({ entryPoints: [fixturePath], bundle: true, @@ -205,6 +236,7 @@ describe("runCli child-process smoke test", () => { alias: { yargs: require.resolve("yargs"), "@sentry/node": require.resolve("@sentry/node"), + "@cdktn/commons": require.resolve("@cdktn/commons"), }, }); }, 60000); @@ -240,14 +272,20 @@ describe("runCli child-process smoke test", () => { expect(output).not.toContain("Node.js v"); }); - it("delivers the crash to Sentry before the process exits", async () => { + it("delivers the failed-command metric alongside the crash in the same run", async () => { const sink = await startSentrySink(); try { const dsn = `http://public@127.0.0.1:${sink.port}/1`; + // the jest preset sets CHECKPOINT_DISABLE, which would gate the metric + const { CHECKPOINT_DISABLE: _disabled, ...env } = process.env; const result = await execa( process.execPath, [bundlePath, "capturedboom"], - { env: { ...process.env, TEST_SENTRY_DSN: dsn }, reject: false }, + { + env: { ...env, TEST_SENTRY_DSN: dsn, TEST_USAGE_TELEMETRY: "1" }, + extendEnv: false, + reject: false, + }, ); // the process exits only after reportFailure awaited the flush, so @@ -257,10 +295,15 @@ describe("runCli child-process smoke test", () => { .requests() .filter((r) => r.url.includes("/envelope/")) .map((r) => r.body); - expect(bodies.length).toBeGreaterThan(0); expect(bodies.some((b) => b.includes("empirical-sentry-message"))).toBe( true, ); + const metrics = parseMetricItems(bodies); + const error = metrics.find((m) => m.name === "cli.command.error")!; + expect(error).toBeDefined(); + expect(error.attributes.error_type.value).toBe("unexpected"); + expect(error.attributes.command.value).toBe("capturedboom"); + expect(JSON.stringify(metrics)).not.toContain("empirical-sentry-message"); } finally { await sink.close(); } diff --git a/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts b/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts index da011274c..62f94c61f 100644 --- a/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts +++ b/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts @@ -1,8 +1,10 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 import yargs, { Argv } from "yargs"; -import { Errors } from "@cdktn/commons"; +import * as Sentry from "@sentry/node"; +import { Errors, setUsageTelemetryEnabled } from "@cdktn/commons"; import { + defaultDeps, describeError, reportFailure, runCli, @@ -35,6 +37,7 @@ function makeDeps(): FailureReporterDeps { new Promise((resolve) => setImmediate(() => resolve({ node: "24" }))), ), captureException: jest.fn(), + sendCommandErrorTelemetry: jest.fn().mockResolvedValue(undefined), flushTelemetry: jest.fn().mockResolvedValue(undefined), }; } @@ -314,6 +317,139 @@ describe("reportFailure", () => { }); }); +describe("reportFailure failed-command metric", () => { + afterEach(() => { + Errors.setScope("unknown"); + }); + + it.each([ + ["Usage", () => Errors.Usage("bad-usage-message")], + ["External", () => Errors.External("bad-external-message")], + ["Internal", () => Errors.Internal("bad-internal-message")], + ["unexpected", () => new Error("boom-message")], + ["unexpected", () => "raw-string-message"], + ])( + "counts the failure once as %s under the command scope", + async (errorType, makeError) => { + const deps = makeDeps(); + Errors.setScope("deploy"); + await reportFailure({ message: null, error: makeError() }, deps); + + expect(deps.sendCommandErrorTelemetry).toHaveBeenCalledTimes(1); + expect(deps.sendCommandErrorTelemetry).toHaveBeenCalledWith( + "deploy", + errorType, + ); + }, + ); + + it("counts a yargs validation failure (message, no error) as Usage", async () => { + const deps = makeDeps(); + await reportFailure({ message: "Invalid values: nope" }, deps); + + expect(deps.sendCommandErrorTelemetry).toHaveBeenCalledTimes(1); + expect(deps.sendCommandErrorTelemetry).toHaveBeenCalledWith( + "unknown", + "Usage", + ); + }); + + it("counts the failure before the flush, after the crash capture", async () => { + const deps = makeDeps(); + const order: string[] = []; + (deps.captureException as jest.Mock).mockImplementation(() => + order.push("capture"), + ); + (deps.sendCommandErrorTelemetry as jest.Mock).mockImplementation( + async () => { + order.push("metric"); + }, + ); + (deps.flushTelemetry as jest.Mock).mockImplementation(async () => { + order.push("flush"); + }); + await reportFailure({ message: null, error: new Error("boom") }, deps); + + expect(order).toEqual(["capture", "metric", "flush"]); + expect(deps.flushTelemetry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + it("still flushes when the metric emission itself rejects", async () => { + const deps = makeDeps(); + (deps.sendCommandErrorTelemetry as jest.Mock).mockRejectedValue( + new Error("metrics exploded"), + ); + const code = await reportFailure( + { message: null, error: Errors.Usage("x") }, + deps, + ); + + expect(code).toBe(1); + expect(deps.flushTelemetry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + describe("with the default (commons) emitter", () => { + const originalCheckpointDisable = process.env.CHECKPOINT_DISABLE; + let count: jest.SpyInstance; + + beforeEach(() => { + // the jest preset disables usage telemetry process-wide + delete process.env.CHECKPOINT_DISABLE; + count = jest.spyOn(Sentry.metrics, "count").mockImplementation(() => {}); + }); + + afterEach(() => { + count.mockRestore(); + setUsageTelemetryEnabled(undefined); + if (originalCheckpointDisable === undefined) { + delete process.env.CHECKPOINT_DISABLE; + } else { + process.env.CHECKPOINT_DISABLE = originalCheckpointDisable; + } + }); + + // Only the metric seams are real here: log and capture stay mocked so + // the test neither prints nor spawns debug collection. + function realEmitterDeps(): FailureReporterDeps { + return { + ...makeDeps(), + sendCommandErrorTelemetry: defaultDeps.sendCommandErrorTelemetry, + flushTelemetry: defaultDeps.flushTelemetry, + }; + } + + it("emits cli.command.error with error_type when usage telemetry is on", async () => { + setUsageTelemetryEnabled(true); + Errors.setScope("deploy"); + const error = Errors.External("terraform exited 1"); + count.mockClear(); // the factory above counted a cli.error + + await reportFailure({ message: null, error }, realEmitterDeps()); + + expect(count).toHaveBeenCalledTimes(1); + expect(count).toHaveBeenCalledWith( + "cli.command.error", + 1, + expect.objectContaining({ + attributes: expect.objectContaining({ + command: "deploy", + error_type: "External", + }), + }), + ); + }); + + it("emits nothing when usage telemetry is off", async () => { + setUsageTelemetryEnabled(false); + const error = new Error("boom"); + + await reportFailure({ message: null, error }, realEmitterDeps()); + + expect(count).not.toHaveBeenCalled(); + }); + }); +}); + describe("runCli", () => { it("reports an async Error thrown by a command handler exactly once, with no orphaned rejection", async () => { const deps = makeDeps(); diff --git a/packages/cdktn-cli/src/bin/cmds/__tests__/error-scope.test.ts b/packages/cdktn-cli/src/bin/cmds/__tests__/error-scope.test.ts new file mode 100644 index 000000000..eba5e94be --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/__tests__/error-scope.test.ts @@ -0,0 +1,62 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import { Errors } from "@cdktn/commons"; + +// The real command modules run against a stubbed handler bundle: nothing is +// executed but the scope assignment the metrics and cli.error counts read. +const handlers = new Proxy( + {}, + { get: () => jest.fn().mockResolvedValue(undefined) }, +); +jest.mock("../helper/utilities", () => ({ + ...jest.requireActual("../helper/utilities"), + requireHandlers: () => handlers, +})); + +import convertCmd from "../convert"; +import debugCmd from "../debug"; +import deployCmd from "../deploy"; +import destroyCmd from "../destroy"; +import diffCmd from "../diff"; +import getCmd from "../get"; +import initCmd from "../init"; +import listCmd from "../list"; +import loginCmd from "../login"; +import outputCmd from "../output"; +import providerAddCmd from "../provider-add"; +import providerListCmd from "../provider-list"; +import providerUpgradeCmd from "../provider-upgrade"; +import synthCmd from "../synth"; +import watchCmd from "../watch"; + +type ScopedCommand = { handler: (args: any) => Promise | void }; + +describe("command scope wiring", () => { + afterEach(() => { + Errors.setScope("unknown"); + }); + + // without this every metric and error count of the command would report + // command: "unknown" + it.each<[string, ScopedCommand]>([ + ["convert", convertCmd], + ["debug", debugCmd], + ["deploy", deployCmd], + ["destroy", destroyCmd], + ["diff", diffCmd], + ["get", getCmd], + ["init", initCmd], + ["list", listCmd], + ["login", loginCmd], + ["output", outputCmd], + ["provider add", providerAddCmd], + ["provider list", providerListCmd], + ["provider upgrade", providerUpgradeCmd], + ["synth", synthCmd], + ["watch", watchCmd], + ])("the %s handler sets the error scope", async (scope, cmd) => { + await cmd.handler({}); + + expect(Errors.getScope()).toBe(scope); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts b/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts new file mode 100644 index 000000000..334e9220c --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts @@ -0,0 +1,107 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import yargs from "yargs"; +import { Errors, Language } from "@cdktn/commons"; + +const mockGet = jest.fn(); +jest.mock("@cdktn/cli-core", () => ({ + ...jest.requireActual("@cdktn/cli-core"), + get: (...args: unknown[]) => mockGet(...args), +})); + +const mockSendTelemetry = jest.fn().mockResolvedValue(undefined); +jest.mock("@cdktn/commons", () => ({ + ...jest.requireActual("@cdktn/commons"), + sendTelemetry: (...args: unknown[]) => mockSendTelemetry(...args), +})); + +jest.mock("../../helper/tty-stream", () => ({ + StreamRenderer: jest.fn().mockImplementation(() => ({ + start: jest.fn(), + stop: jest.fn(), + setBar: jest.fn(), + })), +})); + +import { runGet } from "../get"; +import { defaultDeps, runCli } from "../../../error-handling"; + +const config = { + codeMakerOutput: ".gen", + language: Language.TYPESCRIPT, + constraints: [], + parallelism: 1, + silent: true, +}; + +describe("runGet telemetry", () => { + let errorSpy: jest.SpyInstance; + + beforeEach(() => { + mockGet.mockReset(); + mockSendTelemetry.mockClear(); + errorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + errorSpy.mockRestore(); + Errors.setScope("unknown"); + }); + + it("sends one get metric on success", async () => { + mockGet.mockResolvedValue(undefined); + + await runGet(config); + + expect(mockSendTelemetry).toHaveBeenCalledTimes(1); + expect(mockSendTelemetry).toHaveBeenCalledWith("get", { + language: "typescript", + }); + }); + + it("rethrows a generation failure without counting it: the entrypoint counts the run", async () => { + const failure = Errors.External("schema fetch failed"); + mockGet.mockRejectedValue(failure); + + await expect(runGet(config)).rejects.toBe(failure); + + expect(mockSendTelemetry).not.toHaveBeenCalled(); + }); + + it("a failing get run yields exactly one cli.command.error and no cli.command.invoked", async () => { + mockGet.mockRejectedValue(Errors.External("schema fetch failed")); + const cli = yargs(["get"]) + .exitProcess(false) + .command( + "get", + "generates bindings", + () => {}, + async () => { + Errors.setScope("get"); + await runGet(config); + }, + ); + const exitSpy = jest + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + + try { + await runCli(cli, { + log: jest.fn(), + logError: jest.fn(), + collectDebugInformation: jest.fn().mockResolvedValue({}), + captureException: jest.fn(), + flushTelemetry: jest.fn().mockResolvedValue(undefined), + sendCommandErrorTelemetry: defaultDeps.sendCommandErrorTelemetry, + }); + } finally { + exitSpy.mockRestore(); + } + + expect(mockSendTelemetry).toHaveBeenCalledTimes(1); + expect(mockSendTelemetry).toHaveBeenCalledWith("get", { + error: true, + errorType: "External", + }); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/ui/get.ts b/packages/cdktn-cli/src/bin/cmds/ui/get.ts index 22f7eef4c..4c3b8b55f 100644 --- a/packages/cdktn-cli/src/bin/cmds/ui/get.ts +++ b/packages/cdktn-cli/src/bin/cmds/ui/get.ts @@ -67,13 +67,11 @@ export async function runGet({ stream?.setBar(`${status}...`, { spinner: true }); }, providerSchemaCachePath, - reportTelemetry: async (payload) => - sendTelemetry("get", { - language: payload.targetLanguage, - ...payload.trackingPayload, - }), }); + await sendTelemetry("get", { language }); } catch (e: any) { + // not counted here: the entrypoint's failure reporter counts the run + // once, under the command's scope stream?.stop(); if (!IsErrorType(e, "Usage")) { console.error(e); diff --git a/packages/cdktn-cli/src/bin/error-handling.ts b/packages/cdktn-cli/src/bin/error-handling.ts index a3211363d..cbbcfc7fb 100644 --- a/packages/cdktn-cli/src/bin/error-handling.ts +++ b/packages/cdktn-cli/src/bin/error-handling.ts @@ -2,7 +2,15 @@ // SPDX-License-Identifier: MPL-2.0 import * as yargs from "yargs"; import * as Sentry from "@sentry/node"; -import { IsErrorType, collectDebugInformation } from "@cdktn/commons"; +import { + CommandErrorType, + Errors, + IsErrorType, + collectDebugInformation, + commandErrorType, + flushTelemetry, + sendTelemetry, +} from "@cdktn/commons"; export type CliFailure = { message?: string | null; error?: unknown }; @@ -11,7 +19,12 @@ export interface FailureReporterDeps { logError(msg: string): void; // default: console.error collectDebugInformation(): Promise>; captureException(error: unknown): void; // default: Sentry.captureException - flushTelemetry(timeoutMs: number): Promise; // default: Sentry.flush + // default: commons sendTelemetry, which applies the usage-telemetry gate + sendCommandErrorTelemetry( + command: string, + errorType: CommandErrorType, + ): Promise; + flushTelemetry(timeoutMs: number): Promise; // default: commons flushTelemetry } export const SENTRY_FLUSH_TIMEOUT_MS = 4000; @@ -51,9 +64,9 @@ export const defaultDeps: FailureReporterDeps = { captureException: (error) => { Sentry.captureException(error); }, - flushTelemetry: async (timeoutMs) => { - await Sentry.flush(timeoutMs); - }, + sendCommandErrorTelemetry: (command, errorType) => + sendTelemetry(command, { error: true, errorType }), + flushTelemetry, }; export async function reportFailure( @@ -95,6 +108,17 @@ export async function reportFailure( deps.logError(`Error while reporting failure: ${describeError(e).message}`); } + // The one place a failure that reaches the entrypoint is counted; synth + // failures that exit inside cli-core count themselves and never get here. + // A yargs validation failure carries a message but no error. + try { + await deps.sendCommandErrorTelemetry( + Errors.getScope(), + error === undefined || error === null ? "Usage" : commandErrorType(error), + ); + } catch { + /* never mask the original error */ + } try { await deps.flushTelemetry(SENTRY_FLUSH_TIMEOUT_MS); } catch { From 2fb48c0ee60997eaf52e07eaa2196c09c326403d Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 17:35:26 +0700 Subject: [PATCH 04/17] fix(cli): collect crash and usage consent in convert before the temporary project --- .../cmds/__tests__/handlers-convert.test.ts | 178 ++++++++++++++++++ packages/cdktn-cli/src/bin/cmds/handlers.ts | 7 +- 2 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts diff --git a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts new file mode 100644 index 000000000..5d4edaaf1 --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts @@ -0,0 +1,178 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; + +// Only the seams around the conversion are stubbed: consent runs through the +// real initializErrorReporting against the cdktf.json of the test project. +const mockCrashPrompt = jest.fn(); +const mockUsagePrompt = jest.fn(); +jest.mock("../helper/error-reporting", () => ({ + askForCrashReportingConsent: () => mockCrashPrompt(), + askForUsageTelemetryConsent: () => mockUsagePrompt(), +})); + +const mockRunInit = jest.fn(); +jest.mock("../helper/init", () => ({ + ...jest.requireActual("../helper/init"), + runInit: (...args: unknown[]) => mockRunInit(...args), +})); + +const mockConvert = jest.fn(); +jest.mock("@cdktn/hcl2cdk", () => ({ + ...jest.requireActual("@cdktn/hcl2cdk"), + convert: (...args: unknown[]) => mockConvert(...args), +})); + +jest.mock("@cdktn/provider-schema", () => ({ + readSchema: jest.fn().mockResolvedValue({ providerSchema: {} }), +})); + +jest.mock("../helper/utilities", () => ({ + ...jest.requireActual("../helper/utilities"), + readStreamAsString: jest.fn().mockResolvedValue("resource {}"), +})); + +jest.mock("../helper/terraform-check", () => ({ + terraformCheck: jest.fn().mockResolvedValue(undefined), + getTerraformVersion: jest.fn().mockResolvedValue(null), +})); + +jest.mock("../helper/version-check", () => ({ + displayVersionMessage: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock("../helper/check-environment", () => ({ + ...jest.requireActual("../helper/check-environment"), + checkEnvironment: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock("ci-info", () => ({ isCI: false, name: null })); + +import { setUsageTelemetryEnabled } from "@cdktn/commons"; +import { convert } from "../handlers"; + +describe("convert consent", () => { + let workdir: string; + const originalCwd = process.cwd(); + const originalIsTTY = process.stdout.isTTY; + const originalEnv = { + CI: process.env.CI, + SENTRY_DSN: process.env.SENTRY_DSN, + CHECKPOINT_DISABLE: process.env.CHECKPOINT_DISABLE, + }; + let logSpy: jest.SpyInstance; + + const setInteractive = (interactive: boolean) => { + Object.defineProperty(process.stdout, "isTTY", { + value: interactive, + configurable: true, + }); + }; + + const cdktfJson = () => fs.readJsonSync(path.join(workdir, "cdktf.json")); + + beforeEach(() => { + jest.clearAllMocks(); + workdir = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-convert-")), + ); + process.chdir(workdir); + delete process.env.CI; + delete process.env.SENTRY_DSN; + delete process.env.CHECKPOINT_DISABLE; + setUsageTelemetryEnabled(undefined); + mockCrashPrompt.mockResolvedValue(false); + mockUsagePrompt.mockResolvedValue(true); + mockRunInit.mockResolvedValue({ + needsGet: false, + codeMakerOutput: ".gen", + language: "typescript", + }); + mockConvert.mockResolvedValue({ all: "", stats: {} }); + logSpy = jest.spyOn(console, "log").mockImplementation(() => {}); + }); + + afterEach(() => { + logSpy.mockRestore(); + setUsageTelemetryEnabled(undefined); + setInteractive(originalIsTTY); + process.chdir(originalCwd); + fs.removeSync(workdir); + for (const [key, value] of Object.entries(originalEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + it("prompts for both flags in the user's project before the temporary project work", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + app: "npx ts-node main.ts", + }); + setInteractive(true); + const cwdAtPrompt: string[] = []; + mockUsagePrompt.mockImplementation(async () => { + cwdAtPrompt.push(process.cwd()); + return true; + }); + + // a non-typescript target drives init inside a throwaway project + await convert({ language: "python", provider: [] }); + + expect(mockCrashPrompt).toHaveBeenCalledTimes(1); + expect(mockUsagePrompt).toHaveBeenCalledTimes(1); + expect(cwdAtPrompt).toEqual([workdir]); + expect(cdktfJson()).toMatchObject({ + sendCrashReports: false, + sendUsageTelemetry: true, + }); + expect(mockUsagePrompt.mock.invocationCallOrder[0]).toBeLessThan( + mockRunInit.mock.invocationCallOrder[0], + ); + // the decision captured in the user's cwd survives the throwaway + // project, which opts out + expect(mockRunInit.mock.calls[0][0]).toMatchObject({ + enableUsageTelemetry: false, + }); + expect(process.cwd()).toBe(workdir); + }); + + it("does not prompt again when the flags are already set", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + app: "npx ts-node main.ts", + sendCrashReports: false, + sendUsageTelemetry: false, + }); + setInteractive(true); + + await convert({ language: "typescript", provider: [] }); + + expect(mockCrashPrompt).not.toHaveBeenCalled(); + expect(mockUsagePrompt).not.toHaveBeenCalled(); + expect(cdktfJson()).toMatchObject({ + sendCrashReports: false, + sendUsageTelemetry: false, + }); + }); + + it("stays prompt-free without a terminal and persists nothing", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + app: "npx ts-node main.ts", + }); + setInteractive(false); + + await convert({ language: "typescript", provider: [] }); + + expect(mockCrashPrompt).not.toHaveBeenCalled(); + expect(mockUsagePrompt).not.toHaveBeenCalled(); + expect(cdktfJson()).not.toHaveProperty("sendUsageTelemetry"); + expect(cdktfJson()).not.toHaveProperty("sendCrashReports"); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/handlers.ts b/packages/cdktn-cli/src/bin/cmds/handlers.ts index f4795ef7b..d13abbb78 100644 --- a/packages/cdktn-cli/src/bin/cmds/handlers.ts +++ b/packages/cdktn-cli/src/bin/cmds/handlers.ts @@ -93,7 +93,12 @@ export async function convert({ stack, experimentalProviderSchemaCachePath, }: any) { - await initializErrorReporting(); + // Consent is read and persisted against the user's project before the + // conversion chdirs into a throwaway one. + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); await displayVersionMessage(); const pkg = readPackageJson(); From 3ccd05044a5f9bdc278d715fd7af3762511320ba Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 17:36:46 +0700 Subject: [PATCH 05/17] fix(cli): throw usage errors instead of exiting from the synth, watch and terraform checks --- .../bin/cmds/__tests__/handlers-watch.test.ts | 132 ++++++++++++++++++ packages/cdktn-cli/src/bin/cmds/handlers.ts | 10 +- .../helper/__tests__/terraform-check.test.ts | 94 +++++++++++++ .../src/bin/cmds/helper/terraform-check.ts | 32 ++--- 4 files changed, 244 insertions(+), 24 deletions(-) create mode 100644 packages/cdktn-cli/src/bin/cmds/__tests__/handlers-watch.test.ts create mode 100644 packages/cdktn-cli/src/bin/cmds/helper/__tests__/terraform-check.test.ts diff --git a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-watch.test.ts b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-watch.test.ts new file mode 100644 index 000000000..998d92fc1 --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-watch.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; +import yargs from "yargs"; +import * as Sentry from "@sentry/node"; +import { Errors, setUsageTelemetryEnabled } from "@cdktn/commons"; + +jest.mock("../helper/version-check", () => ({ + displayVersionMessage: jest.fn().mockResolvedValue(undefined), +})); + +const mockRunWatch = jest.fn(); +jest.mock("../ui/watch", () => ({ + runWatch: (...args: unknown[]) => mockRunWatch(...args), +})); + +jest.mock("ci-info", () => ({ isCI: false, name: null })); + +import { watch } from "../handlers"; +import { + defaultDeps, + FailureReporterDeps, + runCli, + SENTRY_FLUSH_TIMEOUT_MS, +} from "../../error-handling"; + +// The guard runs after reporting is initialized, so its failure must reach +// the entrypoint's reporter like any other thrown error: one message, one +// cli.command.error, one flush, one exit. +describe("watch --auto-approve guard", () => { + let workdir: string; + const originalCwd = process.cwd(); + const originalEnv = { + SENTRY_DSN: process.env.SENTRY_DSN, + CHECKPOINT_DISABLE: process.env.CHECKPOINT_DISABLE, + }; + let count: jest.SpyInstance; + + beforeEach(() => { + workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-watch-")); + process.chdir(workdir); + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + app: "npx ts-node main.ts", + sendCrashReports: false, + }); + delete process.env.SENTRY_DSN; + // the jest preset disables usage telemetry process-wide + delete process.env.CHECKPOINT_DISABLE; + setUsageTelemetryEnabled(undefined); + count = jest.spyOn(Sentry.metrics, "count").mockImplementation(() => {}); + }); + + afterEach(() => { + count.mockRestore(); + setUsageTelemetryEnabled(undefined); + Errors.setScope("unknown"); + process.chdir(originalCwd); + fs.removeSync(workdir); + for (const [key, value] of Object.entries(originalEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + it("yields one usage message, one cli.command.error and one flush before a single exit", async () => { + const cli = yargs(["watch"]) + .exitProcess(false) + .command( + "watch", + "watches", + () => {}, + async (argv) => { + Errors.setScope("watch"); + await watch({ ...argv, autoApprove: false }); + }, + ); + const deps: FailureReporterDeps = { + log: jest.fn(), + logError: jest.fn(), + collectDebugInformation: jest.fn().mockResolvedValue({}), + captureException: jest.fn(), + // the real emitter: the metric below is the commons one, gated by the + // decision initializErrorReporting captured + sendCommandErrorTelemetry: defaultDeps.sendCommandErrorTelemetry, + flushTelemetry: jest.fn().mockResolvedValue(undefined), + }; + const exitSpy = jest + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + + let exitCalls: unknown[][]; + try { + await runCli(cli, deps); + } finally { + // read before mockRestore(), which also resets the recorded calls + exitCalls = exitSpy.mock.calls; + exitSpy.mockRestore(); + } + + expect(mockRunWatch).not.toHaveBeenCalled(); + expect(exitCalls).toEqual([[1]]); + expect(deps.logError).toHaveBeenCalledTimes(1); + expect(deps.logError).toHaveBeenCalledWith( + expect.stringContaining("--auto-approve flag must be set"), + ); + expect(deps.captureException).not.toHaveBeenCalled(); + // the Usage factory counts a cli.error; the run itself is counted once + const commandErrors = count.mock.calls.filter( + ([name]) => name === "cli.command.error", + ); + expect(commandErrors).toHaveLength(1); + expect(commandErrors[0][2]).toEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ + command: "watch", + error_type: "Usage", + }), + }), + ); + expect(deps.flushTelemetry).toHaveBeenCalledTimes(1); + expect(deps.flushTelemetry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + expect(count.mock.invocationCallOrder.at(-1)).toBeLessThan( + (deps.flushTelemetry as jest.Mock).mock.invocationCallOrder[0], + ); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/handlers.ts b/packages/cdktn-cli/src/bin/cmds/handlers.ts index d13abbb78..c281e41b0 100644 --- a/packages/cdktn-cli/src/bin/cmds/handlers.ts +++ b/packages/cdktn-cli/src/bin/cmds/handlers.ts @@ -491,10 +491,9 @@ export async function synth(argv: any) { checkCodeMakerOutput && !(await fs.pathExists(config.codeMakerOutput)) ) { - console.error( - `ERROR: synthesis failed, run "cdktn get" to generate providers in ${config.codeMakerOutput}`, + throw Errors.Usage( + `synthesis failed, run "cdktn get" to generate providers in ${config.codeMakerOutput}`, ); - process.exit(1); } await terraformCheck(); @@ -523,10 +522,9 @@ export async function watch(argv: any) { const parallelism = argv.parallelism; if (!autoApprove) { - console.error( - chalkColour`{redBright ERROR: The watch command always automatically deploys and approves changes. To make this behaviour explicit the --auto-approve flag must be set}`, + throw Errors.Usage( + "The watch command always automatically deploys and approves changes. To make this behaviour explicit the --auto-approve flag must be set", ); - process.exit(1); } await terraformCheck(); diff --git a/packages/cdktn-cli/src/bin/cmds/helper/__tests__/terraform-check.test.ts b/packages/cdktn-cli/src/bin/cmds/helper/__tests__/terraform-check.test.ts new file mode 100644 index 000000000..b04b87c44 --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/helper/__tests__/terraform-check.test.ts @@ -0,0 +1,94 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; +import { Errors, IsErrorType } from "@cdktn/commons"; + +const mockVersion = jest.fn(); +jest.mock("@cdktn/cli-core", () => ({ + ...jest.requireActual("@cdktn/cli-core"), + TerraformCli: jest.fn().mockImplementation(() => ({ + version: () => mockVersion(), + })), +})); + +import { terraformCheck } from "../terraform-check"; +import { + FailureReporterDeps, + reportFailure, + SENTRY_FLUSH_TIMEOUT_MS, +} from "../../../error-handling"; + +describe("terraformCheck", () => { + let workdir: string; + const originalCwd = process.cwd(); + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-tf-check-")); + process.chdir(workdir); + mockVersion.mockResolvedValue("Terraform v1.5.0"); + warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + process.chdir(originalCwd); + fs.removeSync(workdir); + Errors.setScope("unknown"); + }); + + it("resolves on a supported version and warns on an old one", async () => { + await expect(terraformCheck()).resolves.toBeUndefined(); + expect(warnSpy).not.toHaveBeenCalled(); + + mockVersion.mockResolvedValue("Terraform v1.1.0"); + await expect(terraformCheck()).resolves.toBeUndefined(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("unsupported Terraform version [1.1.0]"), + ); + }); + + it("throws a Usage error for a legacy terraform.tfstate instead of exiting", async () => { + fs.writeFileSync(path.join(workdir, "terraform.tfstate"), "{}"); + const exitSpy = jest.spyOn(process, "exit"); + + try { + await expect(terraformCheck()).rejects.toMatchObject({ __type: "Usage" }); + expect(exitSpy).not.toHaveBeenCalled(); + } finally { + exitSpy.mockRestore(); + } + }); + + it("is counted once as a failed run and flushed by the entrypoint reporter", async () => { + fs.writeFileSync(path.join(workdir, "terraform.tfstate"), "{}"); + Errors.setScope("deploy"); + const error = await terraformCheck().catch((e) => e); + expect(IsErrorType(error, "Usage")).toBe(true); + const deps: FailureReporterDeps = { + log: jest.fn(), + logError: jest.fn(), + collectDebugInformation: jest.fn().mockResolvedValue({}), + captureException: jest.fn(), + sendCommandErrorTelemetry: jest.fn().mockResolvedValue(undefined), + flushTelemetry: jest.fn().mockResolvedValue(undefined), + }; + + const code = await reportFailure({ message: null, error }, deps); + + expect(code).toBe(1); + expect(deps.logError).toHaveBeenCalledTimes(1); + expect(deps.logError).toHaveBeenCalledWith( + expect.stringContaining("Found 'terraform.tfstate'"), + ); + expect(deps.sendCommandErrorTelemetry).toHaveBeenCalledTimes(1); + expect(deps.sendCommandErrorTelemetry).toHaveBeenCalledWith( + "deploy", + "Usage", + ); + expect(deps.flushTelemetry).toHaveBeenCalledTimes(1); + expect(deps.flushTelemetry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts b/packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts index 247f6c2d4..cb3dec184 100644 --- a/packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts +++ b/packages/cdktn-cli/src/bin/cmds/helper/terraform-check.ts @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 import { TerraformCli, SynthesizedStack } from "@cdktn/cli-core"; -import { logger } from "@cdktn/commons"; +import { Errors, logger } from "@cdktn/commons"; import * as semver from "semver"; import { existsSync } from "fs-extra"; import * as path from "path"; @@ -45,27 +45,23 @@ export const getTerraformVersion = async (): Promise => { } }; +// Throws rather than exits so the failure is reported (and counted) once by +// the CLI entrypoint; an undeterminable version only warns. export const terraformCheck = async (): Promise => { - try { - if (existsSync(path.join(process.cwd(), "terraform.tfstate"))) { - throw new Error(` + if (existsSync(path.join(process.cwd(), "terraform.tfstate"))) { + throw Errors.Usage(` CDK Terrain now supports multiple stacks! Found 'terraform.tfstate' Terraform state file. Please rename it to match the stack name. Learn more https://cdktn.io/docs/concepts/stacks#multiple-stacks `); - } - const cleanTerraformVersion = await getTerraformVersion(); + } + const cleanTerraformVersion = await getTerraformVersion(); - if (cleanTerraformVersion !== null) { - if ( - cleanTerraformVersion && - semver.lt(cleanTerraformVersion, MIN_SUPPORTED_VERSION) - ) { - const warningMessage = `Warning: unsupported Terraform version [${cleanTerraformVersion}] - please upgrade to >=${MIN_SUPPORTED_VERSION}`; - console.warn(warningMessage); - } - } - } catch (e: any) { - console.error(e.message); - process.exit(1); + if ( + cleanTerraformVersion && + semver.lt(cleanTerraformVersion, MIN_SUPPORTED_VERSION) + ) { + console.warn( + `Warning: unsupported Terraform version [${cleanTerraformVersion}] - please upgrade to >=${MIN_SUPPORTED_VERSION}`, + ); } }; From c52191eb25db8e08d6246476c7744f4e57408f64 Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 17:37:15 +0700 Subject: [PATCH 06/17] fix(cli): normalise string-valued consent flags when parsing cdktf.json --- packages/@cdktn/commons/src/config.test.ts | 39 ++++++++++++++++++++++ packages/@cdktn/commons/src/config.ts | 32 ++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/packages/@cdktn/commons/src/config.test.ts b/packages/@cdktn/commons/src/config.test.ts index df9ff6531..e0eeceb74 100644 --- a/packages/@cdktn/commons/src/config.test.ts +++ b/packages/@cdktn/commons/src/config.test.ts @@ -1,6 +1,7 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 import { parseConfig } from "./config"; +import { logger } from "./logging"; import * as fs from "fs-extra"; import * as os from "os"; import * as path from "path"; @@ -726,6 +727,44 @@ describe("parseConfig", () => { }); }); + describe("consent flags", () => { + it.each([ + [true, true], + [false, false], + // the init templates render the flags as strings + ["true", true], + ["false", false], + ["yes", undefined], + [1, undefined], + [null, undefined], + ])("normalises %p to %p", (value, expected) => { + const config = parseConfig( + JSON.stringify({ sendCrashReports: value, sendUsageTelemetry: value }), + ); + expect(config.sendCrashReports).toBe(expected); + expect(config.sendUsageTelemetry).toBe(expected); + expect("sendUsageTelemetry" in config).toBe(expected !== undefined); + }); + + it("leaves an absent flag absent", () => { + const config = parseConfig(JSON.stringify({ sendCrashReports: true })); + expect(config.sendCrashReports).toBe(true); + expect("sendUsageTelemetry" in config).toBe(false); + }); + + it("logs a rejected value at debug level", () => { + const debug = jest.spyOn(logger, "debug").mockImplementation(() => {}); + try { + parseConfig(JSON.stringify({ sendUsageTelemetry: "yes" })); + expect(debug).toHaveBeenCalledWith( + expect.stringContaining("Ignoring sendUsageTelemetry"), + ); + } finally { + debug.mockRestore(); + } + }); + }); + describe("targetVersions", () => { const ENV_KEY = "CDKTF_CONTEXT_JSON"; let envBefore: string | undefined; diff --git a/packages/@cdktn/commons/src/config.ts b/packages/@cdktn/commons/src/config.ts index 3fd22413f..bd34d90ce 100644 --- a/packages/@cdktn/commons/src/config.ts +++ b/packages/@cdktn/commons/src/config.ts @@ -301,10 +301,30 @@ interface ConfigBase { * `targetVersions` before running it. */ readonly validateInstalledBinary?: boolean; + // On disk either flag may be the string "true"/"false" (the init templates + // render it that way); parseConfig normalises both to a boolean. readonly sendCrashReports?: boolean; readonly sendUsageTelemetry?: boolean; } +const CONSENT_FLAGS = ["sendCrashReports", "sendUsageTelemetry"] as const; + +function normalizeConsentFlag( + key: (typeof CONSENT_FLAGS)[number], + value: unknown, +): boolean | undefined { + if (typeof value === "boolean") { + return value; + } + if (value === "true" || value === "false") { + return value === "true"; + } + logger.debug( + `Ignoring ${key} in ${CONFIG_FILE}: expected a boolean, got ${JSON.stringify(value)}`, + ); + return undefined; +} + /** * Validates the shape of a `targetVersions` declaration; returns a list of * human-readable problems (empty when valid). @@ -408,6 +428,18 @@ export const parseConfig = (configJSON?: string) => { ); } + const flags = config as unknown as Record; + for (const key of CONSENT_FLAGS) { + if (key in flags) { + const normalized = normalizeConsentFlag(key, flags[key]); + if (normalized === undefined) { + delete flags[key]; + } else { + flags[key] = normalized; + } + } + } + const targetVersionProblems = validateTargetVersions(config.targetVersions); if (targetVersionProblems.length > 0) { throw new Error( From 52e0a989d048ac2bcdd68816b236ebf0bdd3e3ab Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 17:40:11 +0700 Subject: [PATCH 07/17] feat(cli): count every command run as invoked at start and as completed or error once --- .../cli-core/src/lib/error-reporting.ts | 6 ++ .../@cdktn/cli-core/src/lib/synth-stack.ts | 9 +- packages/@cdktn/cli-core/src/lib/watch.ts | 3 +- .../cli-core/src/test/error-reporting.test.ts | 85 +++++++++++++++++ .../cli-core/src/test/lib/synth-stack.test.ts | 8 +- packages/@cdktn/commons/src/telemetry.test.ts | 91 ++++++++++++++++++- packages/@cdktn/commons/src/telemetry.ts | 74 ++++++++++++--- .../src/bin/cmds/ui/__tests__/get.test.ts | 2 +- 8 files changed, 254 insertions(+), 24 deletions(-) diff --git a/packages/@cdktn/cli-core/src/lib/error-reporting.ts b/packages/@cdktn/cli-core/src/lib/error-reporting.ts index bd0df792c..71141bb73 100644 --- a/packages/@cdktn/cli-core/src/lib/error-reporting.ts +++ b/packages/@cdktn/cli-core/src/lib/error-reporting.ts @@ -2,10 +2,12 @@ // SPDX-License-Identifier: MPL-2.0 import * as Sentry from "@sentry/node"; import { + Errors, getProjectId, getUserId, getUsageTelemetryConsent, setUsageTelemetryEnabled, + startCommandTelemetry, collectDebugInformation, DISPLAY_VERSION, } from "@cdktn/commons"; @@ -202,6 +204,10 @@ export async function initializErrorReporting( Sentry.setContext("environment", debugOutput); }); } + + // The run is counted as started here, under the command scope every + // command sets before it initializes reporting. + await startCommandTelemetry(Errors.getScope(), projectPath); } export function captureException({ diff --git a/packages/@cdktn/cli-core/src/lib/synth-stack.ts b/packages/@cdktn/cli-core/src/lib/synth-stack.ts index a67338c98..e2ca06804 100644 --- a/packages/@cdktn/cli-core/src/lib/synth-stack.ts +++ b/packages/@cdktn/cli-core/src/lib/synth-stack.ts @@ -12,6 +12,7 @@ import { } from "cdktn"; import { performance } from "perf_hooks"; import { + Errors, commandErrorType, flushTelemetry, logger, @@ -303,15 +304,15 @@ Command output on stdout: } /** - * One `cli.command.error` per failed run: counted here only on the paths - * above that exit the process themselves; anything thrown is counted by - * the CLI entrypoint's failure reporter instead. + * One `cli.command.error` per failed run, under the running command (a + * deploy's synth fails the deploy). Only the self-exiting paths above count + * here; anything thrown is counted by the entrypoint's failure reporter. */ public static async synthErrorTelemetry( error: unknown, synthOrigin?: SynthOrigin, ) { - await sendTelemetry("synth", { + await sendTelemetry(Errors.getScope(), { error: true, errorType: commandErrorType(error), synthOrigin, diff --git a/packages/@cdktn/cli-core/src/lib/watch.ts b/packages/@cdktn/cli-core/src/lib/watch.ts index ad055155c..8ab2c864b 100644 --- a/packages/@cdktn/cli-core/src/lib/watch.ts +++ b/packages/@cdktn/cli-core/src/lib/watch.ts @@ -8,7 +8,7 @@ import { } from "./cdktf-project"; import * as fs from "fs"; import * as chokidar from "chokidar"; -import { logger, Errors, sendTelemetry } from "@cdktn/commons"; +import { logger, Errors } from "@cdktn/commons"; import { CdktfStack } from "./cdktf-stack"; // In this very first iteration we will find out which files to watch by asking the user to provide the files @@ -185,6 +185,5 @@ export async function watch( // initially run once onFileChange(); - await sendTelemetry("watch", { event: "start" }); await stopped; } diff --git a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts index 54f15719c..1188753e7 100644 --- a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts +++ b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts @@ -17,6 +17,7 @@ jest.mock("@sentry/node", () => ({ addBreadcrumb: jest.fn(), // the commons logger records every debug line flush: jest.fn().mockResolvedValue(true), close: jest.fn().mockResolvedValue(true), + metrics: { count: jest.fn(), distribution: jest.fn() }, })); jest.mock("ci-info", () => ({ isCI: false, name: null })); @@ -35,7 +36,9 @@ jest.mock("@cdktn/commons", () => { import * as Sentry from "@sentry/node"; import ciInfo from "ci-info"; import { + Errors, isUsageTelemetryEnabled, + resetCommandTelemetry, setUsageTelemetryEnabled, } from "@cdktn/commons"; import { @@ -83,6 +86,8 @@ describe("Sentry init hardening", () => { afterEach(() => { setUsageTelemetryEnabled(undefined); + resetCommandTelemetry(); + Errors.setScope("unknown"); process.chdir(originalCwd); fs.removeSync(workdir); Object.defineProperty(process.stdout, "isTTY", { @@ -360,6 +365,86 @@ describe("Sentry init hardening", () => { }); }); + describe("start-of-command metric", () => { + const invokedCalls = () => + (Sentry.metrics.count as jest.Mock).mock.calls.filter( + ([name]) => name === "cli.command.invoked", + ); + + it("counts the run once as cli.command.invoked under the command scope, after init", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "python", + sendCrashReports: false, + }); + setInteractive(false); + Errors.setScope("deploy"); + + await initializErrorReporting(); + // init runs get, which initializes reporting again + await initializErrorReporting(); + + expect(invokedCalls()).toHaveLength(1); + expect(invokedCalls()[0][2]).toEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ + command: "deploy", + language: "python", + }), + }), + ); + expect( + (Sentry.init as jest.Mock).mock.invocationCallOrder[0], + ).toBeLessThan( + (Sentry.metrics.count as jest.Mock).mock.invocationCallOrder[0], + ); + }); + + it("reads the language from the explicit project path", async () => { + const destination = path.join(workdir, "new-project"); + fs.mkdirpSync(destination); + fs.writeJsonSync(path.join(destination, "cdktf.json"), { + language: "go", + sendCrashReports: false, + }); + setInteractive(false); + Errors.setScope("init"); + + await initializErrorReporting(undefined, undefined, destination); + + expect(invokedCalls()[0][2]).toEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ + command: "init", + language: "go", + }), + }), + ); + }); + + it.each([ + ["usage telemetry declined", { sendUsageTelemetry: false }, {}], + ["CHECKPOINT_DISABLE", {}, { CHECKPOINT_DISABLE: "1" }], + ["no SENTRY_DSN", {}, { SENTRY_DSN: undefined }], + ])("emits nothing when %s", async (_case, flags, env) => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + ...flags, + }); + setInteractive(false); + for (const [key, value] of Object.entries(env)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + + await initializErrorReporting(); + + expect(Sentry.metrics.count).not.toHaveBeenCalled(); + }); + }); + describe("beforeSend", () => { const boom = { message: "boom" }; diff --git a/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts b/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts index 87e0069d1..c732cace0 100644 --- a/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts +++ b/packages/@cdktn/cli-core/src/test/lib/synth-stack.test.ts @@ -3,6 +3,7 @@ import * as fs from "fs-extra"; import * as os from "os"; import * as path from "path"; +import { Errors } from "@cdktn/commons"; import { SynthStack } from "../../lib/synth-stack"; jest.mock("@cdktn/commons", () => ({ @@ -37,20 +38,23 @@ describe("SynthStack.synth failure paths count the run before exiting", () => { exitSpy.mockRestore(); errorSpy.mockRestore(); fs.removeSync(outdir); + Errors.setScope("unknown"); }); it.each([ ["the app exits non-zero", 'node -e "process.exit(1)"'], ["the app never writes a manifest", 'node -e ""'], ])( - "emits one cli.command.error, flushes, then exits 1 when %s", + "emits one cli.command.error under the running command, flushes, then exits 1 when %s", async (_case, app) => { + // a deploy's synth: the deploy is the run that failed + Errors.setScope("deploy"); await expect( SynthStack.synth(new AbortController().signal, app, outdir), ).rejects.toThrow("exit 1"); expect(commons.sendTelemetry).toHaveBeenCalledTimes(1); - expect(commons.sendTelemetry).toHaveBeenCalledWith("synth", { + expect(commons.sendTelemetry).toHaveBeenCalledWith("deploy", { error: true, errorType: "unexpected", synthOrigin: undefined, diff --git a/packages/@cdktn/commons/src/telemetry.test.ts b/packages/@cdktn/commons/src/telemetry.test.ts index eccbfbab7..d24f8e025 100644 --- a/packages/@cdktn/commons/src/telemetry.test.ts +++ b/packages/@cdktn/commons/src/telemetry.test.ts @@ -7,6 +7,8 @@ import * as path from "path"; import ciInfo from "ci-info"; import { sendTelemetry, + startCommandTelemetry, + resetCommandTelemetry, flushTelemetry, getUsageTelemetryConsent, setUsageTelemetryEnabled, @@ -85,6 +87,7 @@ describe("telemetry", () => { afterEach(async () => { setUsageTelemetryEnabled(undefined); + resetCommandTelemetry(); await Sentry.close(1000); process.chdir(originalCwd); fs.removeSync(workdir); @@ -109,18 +112,19 @@ describe("telemetry", () => { process.env.SENTRY_ENVIRONMENT = "LEAK-ENV-SENTRY"; initSentryWithCapturingTransport(); + await startCommandTelemetry("synth"); await sendTelemetry("synth", { totalTime: 1234, language: "typescript" }); await sendTelemetry("synth", { error: true }); expect(await Sentry.flush(2000)).toBe(true); const items = parseMetricItems(envelopeBodies); - // a run is counted once: as invoked or, for an error payload, as error expect(items.map((i) => i.name)).toEqual([ "cli.command.invoked", + "cli.command.completed", "cli.synth.duration", "cli.command.error", ]); - const [invoked, duration, error] = items; + const [invoked, completed, duration, error] = items; for (const metric of items) { const values = attributeValues(metric); @@ -141,7 +145,10 @@ describe("telemetry", () => { expect(values).not.toHaveProperty(forbidden); } } + // the language comes from cdktf.json at start and from the payload + // at the end expect(attributeValues(invoked).language).toBe("typescript"); + expect(attributeValues(completed).language).toBe("typescript"); expect(duration.type).toBe("distribution"); expect(duration.value).toBe(1234); expect(attributeValues(error)).toMatchObject({ @@ -212,6 +219,21 @@ describe("telemetry", () => { await sendTelemetry("convert", { language: "rust; DROP TABLE" }); expect(await Sentry.flush(2000)).toBe(true); + const completed = parseMetricItems(envelopeBodies).find( + (i) => i.name === "cli.command.completed", + )!; + expect(completed.attributes).not.toHaveProperty("language"); + expect(envelopeBodies.join("\n")).not.toContain("DROP TABLE"); + }); + + it("omits an unsupported language read from cdktf.json at start", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "rust; DROP TABLE", + }); + + await startCommandTelemetry("synth"); + expect(await Sentry.flush(2000)).toBe(true); + const invoked = parseMetricItems(envelopeBodies).find( (i) => i.name === "cli.command.invoked", )!; @@ -220,6 +242,67 @@ describe("telemetry", () => { }); }); + describe("run lifecycle", () => { + beforeEach(() => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "python", + sendUsageTelemetry: true, + }); + initSentryWithCapturingTransport(); + }); + + it("counts the run once as invoked at start, even when reporting is initialized again", async () => { + await startCommandTelemetry("init"); + // init runs get, which initializes reporting a second time + await startCommandTelemetry("get"); + expect(await Sentry.flush(2000)).toBe(true); + + const items = parseMetricItems(envelopeBodies); + expect(items.map((i) => i.name)).toEqual(["cli.command.invoked"]); + expect(attributeValues(items[0])).toMatchObject({ + command: "init", + language: "python", + }); + }); + + it("counts only the run's own command as completed; a nested operation adds its own metrics", async () => { + await startCommandTelemetry("deploy"); + // the synth a deploy drives, then the deploy itself + await sendTelemetry("synth", { totalTime: 42, language: "python" }); + await sendTelemetry("deploy", { language: "python" }); + expect(await Sentry.flush(2000)).toBe(true); + + const items = parseMetricItems(envelopeBodies); + expect(items.map((i) => [i.name, attributeValues(i).command])).toEqual([ + ["cli.command.invoked", "deploy"], + ["cli.synth.duration", "synth"], + ["cli.command.completed", "deploy"], + ]); + }); + + it("counts a failed run once as error, never as completed", async () => { + await startCommandTelemetry("synth"); + await sendTelemetry("synth", { error: true, errorType: "External" }); + expect(await Sentry.flush(2000)).toBe(true); + + expect(parseMetricItems(envelopeBodies).map((i) => i.name)).toEqual([ + "cli.command.invoked", + "cli.command.error", + ]); + }); + + it("emits nothing at start when usage telemetry is off, and stays a no-op afterwards", async () => { + setUsageTelemetryEnabled(false); + + await startCommandTelemetry("synth"); + setUsageTelemetryEnabled(true); + await startCommandTelemetry("synth"); + await Sentry.flush(2000); + + expect(parseMetricItems(envelopeBodies)).toHaveLength(0); + }); + }); + // no DSN means no client: every emitter must stay a silent no-op rather // than throw into the command it was called from describe("without an initialized Sentry client", () => { @@ -268,11 +351,15 @@ describe("telemetry", () => { process.env.CHECKPOINT_DISABLE = env; } + await startCommandTelemetry("convert"); await sendTelemetry("convert", {}); await Sentry.flush(2000); const items = parseMetricItems(envelopeBodies); expect(items.some((i) => i.name === "cli.command.invoked")).toBe(emits); + expect(items.some((i) => i.name === "cli.command.completed")).toBe( + emits, + ); if (!emits) { expect(items).toHaveLength(0); } diff --git a/packages/@cdktn/commons/src/telemetry.ts b/packages/@cdktn/commons/src/telemetry.ts index ffae32966..c1748cf1c 100644 --- a/packages/@cdktn/commons/src/telemetry.ts +++ b/packages/@cdktn/commons/src/telemetry.ts @@ -58,6 +58,11 @@ export function getUsageTelemetryConsent( // re-reading cdktf.json at emission time would consult the wrong project. let usageTelemetryEnabledState: boolean | undefined; +// The command this run was started as; set once by startCommandTelemetry so +// a second reporting init (init runs get, provider add runs get) and the +// operations a command drives (a deploy's synth) never count as runs. +let startedCommand: string | undefined; + export function setUsageTelemetryEnabled(enabled: boolean | undefined): void { usageTelemetryEnabledState = enabled; } @@ -115,11 +120,59 @@ export async function flushTelemetry(timeoutMs = 4000): Promise { } } +// A run counts once as cli.command.invoked at start, then once as either +// cli.command.completed (with the scalars known only at the end) or +// cli.command.error; a nested operation (a deploy's synth) adds only its own. +function commandAttributes(command: string, language: unknown): Attributes { + const ci: string | false = ciInfo.isCI ? ciInfo.name || "unknown" : false; + const attributes: Attributes = { + command, + ci: ci === false ? false : ci, + }; + if (LANGUAGES.includes(language as any)) { + attributes.language = language as string; + } + return attributes; +} + +/** + * Counts the run as `cli.command.invoked` (an attempt, whatever its outcome). + * Called once consent is captured and Sentry initialized; later calls in the + * same process are no-ops. + */ +export async function startCommandTelemetry( + command: string, + projectPath = process.cwd(), +): Promise { + if (startedCommand !== undefined) { + return; + } + startedCommand = command; + try { + if (!isUsageTelemetryEnabled()) { + return; + } + const attributes = commandAttributes( + command, + readRawCdktfJson(projectPath).language, + ); + Sentry.metrics.count("cli.command.invoked", 1, { attributes }); + } catch (err) { + logger.debug(`Could not send telemetry data: ${err}`); + } +} + +/** Forgets the started run; tests run many commands in one process. */ +export function resetCommandTelemetry(): void { + startedCommand = undefined; +} + /** - * Emits a command's usage telemetry as Sentry metrics; payload fields reach - * the attributes only through the allow-lists above. A silent no-op when - * usage telemetry is disabled or Sentry is not initialized. - * `payload.error` counts the run as `cli.command.error` by `errorType` instead. + * Emits the usage telemetry of a finished command or operation as Sentry + * metrics; payload fields reach the attributes only through the allow-lists + * above. A silent no-op when usage telemetry is disabled or Sentry is not + * initialized. `payload.error` counts the run as `cli.command.error` by + * `errorType` instead. */ export async function sendTelemetry( command: string, @@ -130,14 +183,7 @@ export async function sendTelemetry( return; } - const ci: string | false = ciInfo.isCI ? ciInfo.name || "unknown" : false; - const attributes: Attributes = { - command, - ci: ci === false ? false : ci, - }; - if (LANGUAGES.includes(payload.language)) { - attributes.language = payload.language; - } + const attributes = commandAttributes(command, payload.language); if (payload.error) { attributes.error_type = COMMAND_ERROR_TYPES.includes(payload.errorType) @@ -147,7 +193,9 @@ export async function sendTelemetry( return; } - Sentry.metrics.count("cli.command.invoked", 1, { attributes }); + if (startedCommand === undefined || startedCommand === command) { + Sentry.metrics.count("cli.command.completed", 1, { attributes }); + } if (typeof payload.totalTime === "number") { Sentry.metrics.distribution("cli.synth.duration", payload.totalTime, { unit: "millisecond", diff --git a/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts b/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts index 334e9220c..e04becd36 100644 --- a/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts +++ b/packages/cdktn-cli/src/bin/cmds/ui/__tests__/get.test.ts @@ -68,7 +68,7 @@ describe("runGet telemetry", () => { expect(mockSendTelemetry).not.toHaveBeenCalled(); }); - it("a failing get run yields exactly one cli.command.error and no cli.command.invoked", async () => { + it("a failing get run yields exactly one cli.command.error and no cli.command.completed", async () => { mockGet.mockRejectedValue(Errors.External("schema fetch failed")); const cli = yargs(["get"]) .exitProcess(false) From 9e6fafd7e0fc18b9b6bd3c56bf7a29cda9c7d98a Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 17:41:00 +0700 Subject: [PATCH 08/17] test(cli): split consent gating from the Sentry init hardening cases --- .../cli-core/src/test/error-reporting.test.ts | 341 +++++++++--------- 1 file changed, 171 insertions(+), 170 deletions(-) diff --git a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts index 1188753e7..851515e58 100644 --- a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts +++ b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts @@ -53,8 +53,22 @@ const ciInfoMock = ciInfo as unknown as { isCI: boolean }; const initOptions = () => (Sentry.init as jest.Mock).mock.calls.at(-1)![0]; -describe("Sentry init hardening", () => { - let workdir: string; +const setInteractive = (interactive: boolean) => { + Object.defineProperty(process.stdout, "isTTY", { + value: interactive, + configurable: true, + }); + if (interactive) { + delete process.env.CI; + ciInfoMock.isCI = false; + } +}; + +let workdir: string; + +// Shared by every describe below: a temp project as cwd, an interactive- +// capable terminal and a DSN, restored after each case. +function useReportingFixture() { const originalCwd = process.cwd(); const originalEnv = { CI: process.env.CI, @@ -63,17 +77,6 @@ describe("Sentry init hardening", () => { }; const originalIsTTY = process.stdout.isTTY; - const setInteractive = (interactive: boolean) => { - Object.defineProperty(process.stdout, "isTTY", { - value: interactive, - configurable: true, - }); - if (interactive) { - delete process.env.CI; - ciInfoMock.isCI = false; - } - }; - beforeEach(() => { jest.clearAllMocks(); workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-consent-")); @@ -102,6 +105,100 @@ describe("Sentry init hardening", () => { } } }); +} + +describe("Sentry init hardening", () => { + useReportingFixture(); + + it("starts a fresh trace so nothing seeded from SENTRY_TRACE/SENTRY_BAGGAGE propagates", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + sendUsageTelemetry: true, + }); + + await initializErrorReporting(jest.fn(), jest.fn()); + + expect(mockScope.setPropagationContext).toHaveBeenCalledWith( + expect.objectContaining({ + traceId: expect.stringMatching(/^[0-9a-f]{32}$/), + }), + ); + }); + it("init options pin release, tracesSampleRate 0, a fixed environment, a fixed serverName and enableMetrics", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(false); + + await initializErrorReporting(); + + // fixed values are the production-side lock that neither the hostname + // nor SENTRY_ENVIRONMENT reaches Sentry (the commons delivery tests set + // their own init options) + expect(initOptions()).toMatchObject({ + release: expect.stringMatching(/^cdktn-cli-/), + tracesSampleRate: 0, + environment: "production", + serverName: "cdktn-cli", + enableMetrics: true, + }); + }); + describe("beforeSend", () => { + const boom = { message: "boom" }; + + it("passes error events through when crash reporting is consented, even under CHECKPOINT_DISABLE", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(false); + process.env.CHECKPOINT_DISABLE = "1"; + + await initializErrorReporting(); + + expect( + await initOptions().beforeSend(boom, { + originalException: new Error("boom"), + }), + ).toBe(boom); + }); + + it("drops error events on a usage-only init (crash declined)", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: false, + sendUsageTelemetry: true, + }); + setInteractive(false); + + await initializErrorReporting(); + + expect(Sentry.init).toHaveBeenCalledTimes(1); + expect( + await initOptions().beforeSend(boom, { + originalException: new Error("boom"), + }), + ).toBeNull(); + }); + + it("still drops Usage Errors when crash reporting is enabled", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + }); + setInteractive(false); + + await initializErrorReporting(); + + expect( + await initOptions().beforeSend( + { message: "x" }, + { originalException: new Error("Usage Error: bad input") }, + ), + ).toBeNull(); + }); + }); +}); + +describe("consent gating (initializErrorReporting)", () => { + useReportingFixture(); it("upgrade path: crash set, usage unset, interactive -> prompts ONCE for usage only and persists", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { @@ -121,7 +218,6 @@ describe("Sentry init hardening", () => { }); expect(Sentry.init).toHaveBeenCalledTimes(1); }); - it("both unset, interactive -> prompts for each flag and persists both", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), {}); setInteractive(true); @@ -191,22 +287,6 @@ describe("Sentry init hardening", () => { expect(Sentry.init).toHaveBeenCalledTimes(1); }, ); - - it("starts a fresh trace so nothing seeded from SENTRY_TRACE/SENTRY_BAGGAGE propagates", async () => { - fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - sendCrashReports: true, - sendUsageTelemetry: true, - }); - - await initializErrorReporting(jest.fn(), jest.fn()); - - expect(mockScope.setPropagationContext).toHaveBeenCalledWith( - expect.objectContaining({ - traceId: expect.stringMatching(/^[0-9a-f]{32}$/), - }), - ); - }); - it("captures the usage decision while still in the user's cwd", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: false, @@ -217,7 +297,6 @@ describe("Sentry init hardening", () => { expect(setUsageTelemetryEnabled).toHaveBeenCalledWith(true); }); - it("reads and persists against an explicit project path, not the cwd", async () => { // init creates the project in a destination directory and initializes // reporting against it; the cwd may hold an unrelated (or no) cdktf.json @@ -244,7 +323,6 @@ describe("Sentry init hardening", () => { // usage-only consent: the client exists but drops error events await expect(initOptions().beforeSend({}, undefined)).resolves.toBeNull(); }); - it("prompts into the explicit project path when its flags are unset", async () => { const destination = path.join(workdir, "new-project"); fs.mkdirpSync(destination); @@ -261,7 +339,6 @@ describe("Sentry init hardening", () => { }); expect(fs.existsSync(path.join(workdir, "cdktf.json"))).toBe(false); }); - it("CHECKPOINT_DISABLE -> no usage prompt, no init when crash is off", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: false, @@ -275,7 +352,6 @@ describe("Sentry init hardening", () => { expect(usagePrompt).not.toHaveBeenCalled(); expect(Sentry.init).not.toHaveBeenCalled(); }); - it("CHECKPOINT_DISABLE does NOT affect crash reporting", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: true, @@ -287,7 +363,6 @@ describe("Sentry init hardening", () => { expect(Sentry.init).toHaveBeenCalledTimes(1); }); - it("explicit sendUsageTelemetry: false + crash off -> Sentry never initialized", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: false, @@ -299,7 +374,6 @@ describe("Sentry init hardening", () => { expect(Sentry.init).not.toHaveBeenCalled(); }); - it("no SENTRY_DSN -> no init even with consent", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { sendCrashReports: true, @@ -344,158 +418,85 @@ describe("Sentry init hardening", () => { } }, ); +}); - it("init options pin release, tracesSampleRate 0, a fixed environment, a fixed serverName and enableMetrics", async () => { +describe("start-of-command metric (initializErrorReporting)", () => { + useReportingFixture(); + + const invokedCalls = () => + (Sentry.metrics.count as jest.Mock).mock.calls.filter( + ([name]) => name === "cli.command.invoked", + ); + + it("counts the run once as cli.command.invoked under the command scope, after init", async () => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - sendCrashReports: true, + language: "python", + sendCrashReports: false, }); setInteractive(false); + Errors.setScope("deploy"); + await initializErrorReporting(); + // init runs get, which initializes reporting again await initializErrorReporting(); - // fixed values are the production-side lock that neither the hostname - // nor SENTRY_ENVIRONMENT reaches Sentry (the commons delivery tests set - // their own init options) - expect(initOptions()).toMatchObject({ - release: expect.stringMatching(/^cdktn-cli-/), - tracesSampleRate: 0, - environment: "production", - serverName: "cdktn-cli", - enableMetrics: true, - }); + expect(invokedCalls()).toHaveLength(1); + expect(invokedCalls()[0][2]).toEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ + command: "deploy", + language: "python", + }), + }), + ); + expect((Sentry.init as jest.Mock).mock.invocationCallOrder[0]).toBeLessThan( + (Sentry.metrics.count as jest.Mock).mock.invocationCallOrder[0], + ); }); - describe("start-of-command metric", () => { - const invokedCalls = () => - (Sentry.metrics.count as jest.Mock).mock.calls.filter( - ([name]) => name === "cli.command.invoked", - ); - - it("counts the run once as cli.command.invoked under the command scope, after init", async () => { - fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - language: "python", - sendCrashReports: false, - }); - setInteractive(false); - Errors.setScope("deploy"); - - await initializErrorReporting(); - // init runs get, which initializes reporting again - await initializErrorReporting(); - - expect(invokedCalls()).toHaveLength(1); - expect(invokedCalls()[0][2]).toEqual( - expect.objectContaining({ - attributes: expect.objectContaining({ - command: "deploy", - language: "python", - }), - }), - ); - expect( - (Sentry.init as jest.Mock).mock.invocationCallOrder[0], - ).toBeLessThan( - (Sentry.metrics.count as jest.Mock).mock.invocationCallOrder[0], - ); + it("reads the language from the explicit project path", async () => { + const destination = path.join(workdir, "new-project"); + fs.mkdirpSync(destination); + fs.writeJsonSync(path.join(destination, "cdktf.json"), { + language: "go", + sendCrashReports: false, }); + setInteractive(false); + Errors.setScope("init"); - it("reads the language from the explicit project path", async () => { - const destination = path.join(workdir, "new-project"); - fs.mkdirpSync(destination); - fs.writeJsonSync(path.join(destination, "cdktf.json"), { - language: "go", - sendCrashReports: false, - }); - setInteractive(false); - Errors.setScope("init"); - - await initializErrorReporting(undefined, undefined, destination); + await initializErrorReporting(undefined, undefined, destination); - expect(invokedCalls()[0][2]).toEqual( - expect.objectContaining({ - attributes: expect.objectContaining({ - command: "init", - language: "go", - }), + expect(invokedCalls()[0][2]).toEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ + command: "init", + language: "go", }), - ); - }); - - it.each([ - ["usage telemetry declined", { sendUsageTelemetry: false }, {}], - ["CHECKPOINT_DISABLE", {}, { CHECKPOINT_DISABLE: "1" }], - ["no SENTRY_DSN", {}, { SENTRY_DSN: undefined }], - ])("emits nothing when %s", async (_case, flags, env) => { - fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - sendCrashReports: true, - ...flags, - }); - setInteractive(false); - for (const [key, value] of Object.entries(env)) { - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } - - await initializErrorReporting(); - - expect(Sentry.metrics.count).not.toHaveBeenCalled(); - }); + }), + ); }); - describe("beforeSend", () => { - const boom = { message: "boom" }; - - it("passes error events through when crash reporting is consented, even under CHECKPOINT_DISABLE", async () => { - fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - sendCrashReports: true, - }); - setInteractive(false); - process.env.CHECKPOINT_DISABLE = "1"; - - await initializErrorReporting(); - - expect( - await initOptions().beforeSend(boom, { - originalException: new Error("boom"), - }), - ).toBe(boom); - }); - - it("drops error events on a usage-only init (crash declined)", async () => { - fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - sendCrashReports: false, - sendUsageTelemetry: true, - }); - setInteractive(false); - - await initializErrorReporting(); - - expect(Sentry.init).toHaveBeenCalledTimes(1); - expect( - await initOptions().beforeSend(boom, { - originalException: new Error("boom"), - }), - ).toBeNull(); + it.each([ + ["usage telemetry declined", { sendUsageTelemetry: false }, {}], + ["CHECKPOINT_DISABLE", {}, { CHECKPOINT_DISABLE: "1" }], + ["no SENTRY_DSN", {}, { SENTRY_DSN: undefined }], + ])("emits nothing when %s", async (_case, flags, env) => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: true, + ...flags, }); + setInteractive(false); + for (const [key, value] of Object.entries(env)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } - it("still drops Usage Errors when crash reporting is enabled", async () => { - fs.writeJsonSync(path.join(workdir, "cdktf.json"), { - sendCrashReports: true, - }); - setInteractive(false); - - await initializErrorReporting(); + await initializErrorReporting(); - expect( - await initOptions().beforeSend( - { message: "x" }, - { originalException: new Error("Usage Error: bad input") }, - ), - ).toBeNull(); - }); + expect(Sentry.metrics.count).not.toHaveBeenCalled(); }); }); From a2c8a2ed0521aa14d296ead281fb92c50413c4df Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 18:02:25 +0700 Subject: [PATCH 09/17] fix(commons): share the command scope across the bundle's module copies --- packages/@cdktn/commons/src/errors.test.ts | 25 ++++++++++++++++++++++ packages/@cdktn/commons/src/errors.ts | 18 ++++++++++------ 2 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 packages/@cdktn/commons/src/errors.test.ts diff --git a/packages/@cdktn/commons/src/errors.test.ts b/packages/@cdktn/commons/src/errors.test.ts new file mode 100644 index 000000000..a58dce534 --- /dev/null +++ b/packages/@cdktn/commons/src/errors.test.ts @@ -0,0 +1,25 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import { Errors } from "./errors"; + +describe("Errors scope", () => { + afterEach(() => { + Errors.setScope("unknown"); + }); + + // the bundle has one copy of this module per entry point: the command + // modules in bin/cdktn.js set the scope, bin/cmds/handlers.js reads it + it("is shared with a second copy of the module", () => { + let second: typeof Errors | undefined; + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + second = require("./errors").Errors; + }); + + Errors.setScope("deploy"); + expect(second!.getScope()).toBe("deploy"); + + second!.setScope("synth"); + expect(Errors.getScope()).toBe("synth"); + }); +}); diff --git a/packages/@cdktn/commons/src/errors.ts b/packages/@cdktn/commons/src/errors.ts index c32ef36d6..35bd4e03d 100644 --- a/packages/@cdktn/commons/src/errors.ts +++ b/packages/@cdktn/commons/src/errors.ts @@ -32,16 +32,22 @@ function reportPrefixedError(type: ErrorType) { }); err.__type = type; err.stack = originalError.stack; - // errorScope is read here, not when the factory is created, so the + // the scope is read here, not when the factory is created, so the // command set by setScope is the one counted - sendErrorTelemetry(type, errorScope); + sendErrorTelemetry(type, scopeStore().scope); return err; }; } // The CLI only deals with one command at a time, so we can just use the same -// scope for all errors and set it once during initialization. -let errorScope = "unknown"; +// scope for all errors and set it once during initialization. The bundle +// carries one copy of this module per entry point (bin/cdktn.js sets the +// scope, bin/cmds/handlers.js counts under it), so it lives on globalThis. +const SCOPE_KEY = Symbol.for("cdktn.errorScope"); +function scopeStore(): { scope: string } { + const globals = globalThis as { [SCOPE_KEY]?: { scope: string } }; + return (globals[SCOPE_KEY] ??= { scope: "unknown" }); +} export const Errors = { // Error within our control Internal: reportPrefixedError("Internal"), @@ -52,11 +58,11 @@ export const Errors = { // Set the scope for all errors setScope(scope: string) { - errorScope = scope; + scopeStore().scope = scope; Sentry.getCurrentScope().setTransactionName(scope); }, getScope(): string { - return errorScope; + return scopeStore().scope; }, }; From 8249adc571d31ab18abc38b1bc1bb51102d73ff8 Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 18:18:04 +0700 Subject: [PATCH 10/17] fix(commons): share the consent decision, started command and language across the bundle's module copies --- packages/@cdktn/commons/src/errors.ts | 20 +++---- packages/@cdktn/commons/src/process-state.ts | 10 ++++ packages/@cdktn/commons/src/telemetry.test.ts | 60 ++++++++++++++++++- packages/@cdktn/commons/src/telemetry.ts | 60 ++++++++++++------- 4 files changed, 116 insertions(+), 34 deletions(-) create mode 100644 packages/@cdktn/commons/src/process-state.ts diff --git a/packages/@cdktn/commons/src/errors.ts b/packages/@cdktn/commons/src/errors.ts index 35bd4e03d..2dfabcee3 100644 --- a/packages/@cdktn/commons/src/errors.ts +++ b/packages/@cdktn/commons/src/errors.ts @@ -4,6 +4,7 @@ import * as Sentry from "@sentry/node"; // telemetry.ts must never import this module: it would close a cycle through // terraform.ts and load errors.ts before the factories exist. import { CommandErrorType, sendErrorTelemetry } from "./telemetry"; +import { processState } from "./process-state"; type ErrorType = "Internal" | "External" | "Usage"; export function IsErrorType(error: any, type: ErrorType): boolean { @@ -34,20 +35,17 @@ function reportPrefixedError(type: ErrorType) { err.stack = originalError.stack; // the scope is read here, not when the factory is created, so the // command set by setScope is the one counted - sendErrorTelemetry(type, scopeStore().scope); + sendErrorTelemetry(type, scopeState.scope); return err; }; } // The CLI only deals with one command at a time, so we can just use the same -// scope for all errors and set it once during initialization. The bundle -// carries one copy of this module per entry point (bin/cdktn.js sets the -// scope, bin/cmds/handlers.js counts under it), so it lives on globalThis. -const SCOPE_KEY = Symbol.for("cdktn.errorScope"); -function scopeStore(): { scope: string } { - const globals = globalThis as { [SCOPE_KEY]?: { scope: string } }; - return (globals[SCOPE_KEY] ??= { scope: "unknown" }); -} +// scope for all errors and set it once during initialization (bin/cdktn.js +// sets it, the bundle copy in bin/cmds/handlers.js counts under it). +const scopeState = processState("cdktn.errorScope", () => ({ + scope: "unknown", +})); export const Errors = { // Error within our control Internal: reportPrefixedError("Internal"), @@ -58,11 +56,11 @@ export const Errors = { // Set the scope for all errors setScope(scope: string) { - scopeStore().scope = scope; + scopeState.scope = scope; Sentry.getCurrentScope().setTransactionName(scope); }, getScope(): string { - return scopeStore().scope; + return scopeState.scope; }, }; diff --git a/packages/@cdktn/commons/src/process-state.ts b/packages/@cdktn/commons/src/process-state.ts new file mode 100644 index 000000000..78e33944e --- /dev/null +++ b/packages/@cdktn/commons/src/process-state.ts @@ -0,0 +1,10 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 + +// The CLI bundle carries one copy of commons per esbuild entry point +// (bin/cdktn.js and bin/cmds/handlers.js); state a command sets in one copy +// and reads in the other has to live on globalThis, keyed by a shared symbol. +export function processState(key: string, init: () => T): T { + const globals = globalThis as { [key: symbol]: T | undefined }; + return (globals[Symbol.for(key)] ??= init()); +} diff --git a/packages/@cdktn/commons/src/telemetry.test.ts b/packages/@cdktn/commons/src/telemetry.test.ts index d24f8e025..b1974dd54 100644 --- a/packages/@cdktn/commons/src/telemetry.test.ts +++ b/packages/@cdktn/commons/src/telemetry.test.ts @@ -11,6 +11,8 @@ import { resetCommandTelemetry, flushTelemetry, getUsageTelemetryConsent, + hasCapturedUsageTelemetryDecision, + isUsageTelemetryEnabled, setUsageTelemetryEnabled, } from "./telemetry"; import { Errors } from "./errors"; @@ -146,13 +148,14 @@ describe("telemetry", () => { } } // the language comes from cdktf.json at start and from the payload - // at the end + // at the end; the error metric has no payload and reuses the start read expect(attributeValues(invoked).language).toBe("typescript"); expect(attributeValues(completed).language).toBe("typescript"); expect(duration.type).toBe("distribution"); expect(duration.value).toBe(1234); expect(attributeValues(error)).toMatchObject({ error_type: "unexpected", + language: "typescript", }); expect(envelopeBodies.join("\n")).not.toContain("LEAK-ENV-SENTRY"); }); @@ -303,6 +306,61 @@ describe("telemetry", () => { }); }); + // the bundle has one copy of this module per entry point: bin/cdktn.js + // captures the decision and starts the run, bin/cmds/handlers.js emits + describe("shared across module copies", () => { + type Telemetry = typeof import("./telemetry"); + let second: Telemetry; + + beforeEach(() => { + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + second = require("./telemetry"); + }); + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "go", + sendUsageTelemetry: true, + }); + initSentryWithCapturingTransport(); + }); + + it("observes the captured decision of the other copy", () => { + expect(second.hasCapturedUsageTelemetryDecision()).toBe(false); + + setUsageTelemetryEnabled(false); + expect(second.hasCapturedUsageTelemetryDecision()).toBe(true); + expect(second.isUsageTelemetryEnabled()).toBe(false); + + second.setUsageTelemetryEnabled(true); + expect(hasCapturedUsageTelemetryDecision()).toBe(true); + expect(isUsageTelemetryEnabled()).toBe(true); + }); + + it("observes the started command and its language from the other copy", async () => { + await startCommandTelemetry("deploy"); + // the handlers' copy: a nested start is a no-op, only the run's own + // command completes, and the error carries the language read at start + await second.startCommandTelemetry("synth"); + await second.sendTelemetry("synth", { language: "go" }); + await second.sendTelemetry("deploy", { error: true, errorType: "Usage" }); + expect(await Sentry.flush(2000)).toBe(true); + + const items = parseMetricItems(envelopeBodies); + expect(items.map((i) => [i.name, attributeValues(i).command])).toEqual([ + ["cli.command.invoked", "deploy"], + ["cli.command.error", "deploy"], + ]); + expect(attributeValues(items[1]).language).toBe("go"); + + second.resetCommandTelemetry(); + await startCommandTelemetry("get"); + expect(await Sentry.flush(2000)).toBe(true); + expect( + parseMetricItems(envelopeBodies).map((i) => attributeValues(i).command), + ).toEqual(["deploy", "deploy", "get"]); + }); + }); + // no DSN means no client: every emitter must stay a silent no-op rather // than throw into the command it was called from describe("without an initialized Sentry client", () => { diff --git a/packages/@cdktn/commons/src/telemetry.ts b/packages/@cdktn/commons/src/telemetry.ts index c1748cf1c..13becc162 100644 --- a/packages/@cdktn/commons/src/telemetry.ts +++ b/packages/@cdktn/commons/src/telemetry.ts @@ -6,6 +6,7 @@ import * as fs from "fs-extra"; import ciInfo from "ci-info"; import { logger } from "./logging"; import { LANGUAGES } from "./config"; +import { processState } from "./process-state"; type AttributeValue = string | number | boolean; type Attributes = Record; @@ -53,18 +54,28 @@ export function getUsageTelemetryConsent( : cdktfJson.sendUsageTelemetry === "true"; } -// Captured by initializErrorReporting (cli-core) while still in the user's -// cwd: `convert` chdirs into a throwaway project before emitting, so -// re-reading cdktf.json at emission time would consult the wrong project. -let usageTelemetryEnabledState: boolean | undefined; - -// The command this run was started as; set once by startCommandTelemetry so -// a second reporting init (init runs get, provider add runs get) and the -// operations a command drives (a deploy's synth) never count as runs. -let startedCommand: string | undefined; +type CommandTelemetryState = { + // Captured by initializErrorReporting (cli-core) while still in the user's + // cwd: `convert` chdirs into a throwaway project before emitting, so + // re-reading cdktf.json at emission time would consult the wrong project. + usageTelemetryEnabled?: boolean; + // The command this run was started as; set once by startCommandTelemetry so + // a second reporting init (init runs get, provider add runs get) and the + // operations a command drives (a deploy's synth) never count as runs. + startedCommand?: string; + // The raw `language` of the project the run started in, so every metric of + // the run (the error metric has no payload) carries the one `invoked` did. + language?: unknown; +}; + +// Set by the entrypoint's bundle copy, read by the handlers' copy. +const state = processState( + "cdktn.commandTelemetry", + () => ({}), +); export function setUsageTelemetryEnabled(enabled: boolean | undefined): void { - usageTelemetryEnabledState = enabled; + state.usageTelemetryEnabled = enabled; } /** @@ -73,7 +84,7 @@ export function setUsageTelemetryEnabled(enabled: boolean | undefined): void { * decision of a command (`convert`) that drives it inside a throwaway project. */ export function hasCapturedUsageTelemetryDecision(): boolean { - return usageTelemetryEnabledState !== undefined; + return state.usageTelemetryEnabled !== undefined; } /** @@ -85,8 +96,8 @@ export function isUsageTelemetryEnabled(projectPath = process.cwd()): boolean { if (process.env.CHECKPOINT_DISABLE) { return false; } - if (usageTelemetryEnabledState !== undefined) { - return usageTelemetryEnabledState; + if (state.usageTelemetryEnabled !== undefined) { + return state.usageTelemetryEnabled; } return getUsageTelemetryConsent(projectPath) !== false; } @@ -144,18 +155,16 @@ export async function startCommandTelemetry( command: string, projectPath = process.cwd(), ): Promise { - if (startedCommand !== undefined) { + if (state.startedCommand !== undefined) { return; } - startedCommand = command; + state.startedCommand = command; + state.language = readRawCdktfJson(projectPath).language; try { if (!isUsageTelemetryEnabled()) { return; } - const attributes = commandAttributes( - command, - readRawCdktfJson(projectPath).language, - ); + const attributes = commandAttributes(command, state.language); Sentry.metrics.count("cli.command.invoked", 1, { attributes }); } catch (err) { logger.debug(`Could not send telemetry data: ${err}`); @@ -164,7 +173,8 @@ export async function startCommandTelemetry( /** Forgets the started run; tests run many commands in one process. */ export function resetCommandTelemetry(): void { - startedCommand = undefined; + state.startedCommand = undefined; + state.language = undefined; } /** @@ -183,7 +193,10 @@ export async function sendTelemetry( return; } - const attributes = commandAttributes(command, payload.language); + const attributes = commandAttributes( + command, + payload.language ?? state.language, + ); if (payload.error) { attributes.error_type = COMMAND_ERROR_TYPES.includes(payload.errorType) @@ -193,7 +206,10 @@ export async function sendTelemetry( return; } - if (startedCommand === undefined || startedCommand === command) { + if ( + state.startedCommand === undefined || + state.startedCommand === command + ) { Sentry.metrics.count("cli.command.completed", 1, { attributes }); } if (typeof payload.totalTime === "number") { From 2a876113fd40cab76a3b5974aef719af1586017b Mon Sep 17 00:00:00 2001 From: vincent de smet Date: Thu, 10 Sep 2026 18:36:32 +0700 Subject: [PATCH 11/17] chore(commons): state the bundle-copy direction of the shared telemetry store correctly --- packages/@cdktn/commons/src/telemetry.test.ts | 4 ++-- packages/@cdktn/commons/src/telemetry.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/@cdktn/commons/src/telemetry.test.ts b/packages/@cdktn/commons/src/telemetry.test.ts index b1974dd54..8196c1af3 100644 --- a/packages/@cdktn/commons/src/telemetry.test.ts +++ b/packages/@cdktn/commons/src/telemetry.test.ts @@ -306,8 +306,8 @@ describe("telemetry", () => { }); }); - // the bundle has one copy of this module per entry point: bin/cdktn.js - // captures the decision and starts the run, bin/cmds/handlers.js emits + // the bundle has one copy of this module per entry point: bin/cmds/handlers.js + // captures the decision and starts the run, bin/cdktn.js counts the failure describe("shared across module copies", () => { type Telemetry = typeof import("./telemetry"); let second: Telemetry; diff --git a/packages/@cdktn/commons/src/telemetry.ts b/packages/@cdktn/commons/src/telemetry.ts index 13becc162..536a6a455 100644 --- a/packages/@cdktn/commons/src/telemetry.ts +++ b/packages/@cdktn/commons/src/telemetry.ts @@ -68,7 +68,8 @@ type CommandTelemetryState = { language?: unknown; }; -// Set by the entrypoint's bundle copy, read by the handlers' copy. +// Shared by both bundle copies: the handlers' copy captures the decision and +// starts the run, the entrypoint's copy counts the failure. const state = processState( "cdktn.commandTelemetry", () => ({}), From 27e32a6bb7d08a86dd7be0c983a07a6c3e856c31 Mon Sep 17 00:00:00 2001 From: so0k Date: Tue, 22 Sep 2026 12:47:36 +0800 Subject: [PATCH 12/17] fix(commons): read a malformed consent flag as unset, not as an opt-out getUsageTelemetryConsent and shouldReportCrash now share parseConfig's normalizeConsentFlag, so a value such as 1, "yes" or null prompts interactively and falls back to the non-interactive defaults otherwise. Co-Authored-By: Claude Opus 5 (1M context) --- .../cli-core/src/lib/error-reporting.ts | 7 ++-- .../cli-core/src/test/error-reporting.test.ts | 38 ++++++++++++++++++- packages/@cdktn/commons/src/config.ts | 6 ++- packages/@cdktn/commons/src/telemetry.test.ts | 4 ++ packages/@cdktn/commons/src/telemetry.ts | 13 ++++--- 5 files changed, 55 insertions(+), 13 deletions(-) diff --git a/packages/@cdktn/cli-core/src/lib/error-reporting.ts b/packages/@cdktn/cli-core/src/lib/error-reporting.ts index 71141bb73..89aee3f60 100644 --- a/packages/@cdktn/cli-core/src/lib/error-reporting.ts +++ b/packages/@cdktn/cli-core/src/lib/error-reporting.ts @@ -10,6 +10,7 @@ import { startCommandTelemetry, collectDebugInformation, DISPLAY_VERSION, + normalizeConsentFlag, } from "@cdktn/commons"; import { logger } from "@cdktn/commons"; import * as path from "path"; @@ -25,16 +26,14 @@ export function shouldReportCrash( fs.readFileSync(path.resolve(projectPath, "cdktf.json"), "utf8"), ); - // tri-state: an absent flag means "unset" and triggers the + // tri-state: an absent or malformed flag means "unset" and triggers the // interactive consent prompt; outside a project (no readable // cdktf.json) crash reporting stays off if (!("sendCrashReports" in cdktfJson)) { return undefined; } - return typeof cdktfJson.sendCrashReports === "boolean" - ? cdktfJson.sendCrashReports - : cdktfJson.sendCrashReports === "true"; + return normalizeConsentFlag("sendCrashReports", cdktfJson.sendCrashReports); } catch (e) { logger.debug( `Error determining if crash reporting should be enabled, defaulting to false: ${e}`, diff --git a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts index 851515e58..27551434f 100644 --- a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts +++ b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts @@ -386,6 +386,37 @@ describe("consent gating (initializErrorReporting)", () => { expect(Sentry.init).not.toHaveBeenCalled(); }); + // A malformed flag is unset, not a silent opt-out. + it("malformed flags, interactive -> prompts for each and persists booleans", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: "yes", + sendUsageTelemetry: 1, + }); + setInteractive(true); + const crashPrompt = jest.fn().mockResolvedValue(false); + const usagePrompt = jest.fn().mockResolvedValue(true); + + await initializErrorReporting(crashPrompt, usagePrompt); + + expect(crashPrompt).toHaveBeenCalledTimes(1); + expect(usagePrompt).toHaveBeenCalledTimes(1); + expect(fs.readJsonSync(path.join(workdir, "cdktf.json"))).toMatchObject({ + sendCrashReports: false, + sendUsageTelemetry: true, + }); + }); + it("malformed flags, non-interactive -> usage on, crash off", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + sendCrashReports: null, + sendUsageTelemetry: "yes", + }); + setInteractive(false); + + await initializErrorReporting(); + + expect(isUsageTelemetryEnabled()).toBe(true); + await expect(initOptions().beforeSend({}, undefined)).resolves.toBeNull(); + }); // The resolved decision is captured for the whole run: commands that chdir // into another project (convert) must not re-read that project's flag. @@ -516,9 +547,12 @@ describe("shouldReportCrash tri-state", () => { [{ sendCrashReports: false }, false], [{ sendCrashReports: "true" }, true], [{ sendCrashReports: "false" }, false], - // undefined, not false, for an absent flag: that is what triggers the - // crash-consent prompt + // undefined, not false, for an absent or malformed flag: that is what + // triggers the crash-consent prompt [{}, undefined], + [{ sendCrashReports: "yes" }, undefined], + [{ sendCrashReports: 1 }, undefined], + [{ sendCrashReports: null }, undefined], ])("reads %j as %p", (config, expected) => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), config); expect(shouldReportCrash(workdir)).toBe(expected); diff --git a/packages/@cdktn/commons/src/config.ts b/packages/@cdktn/commons/src/config.ts index bd34d90ce..08e35f408 100644 --- a/packages/@cdktn/commons/src/config.ts +++ b/packages/@cdktn/commons/src/config.ts @@ -309,7 +309,11 @@ interface ConfigBase { const CONSENT_FLAGS = ["sendCrashReports", "sendUsageTelemetry"] as const; -function normalizeConsentFlag( +/** + * Tri-state read of a consent flag: a boolean or "true"/"false" string is a + * decision; any other value is treated as unset (`undefined`). + */ +export function normalizeConsentFlag( key: (typeof CONSENT_FLAGS)[number], value: unknown, ): boolean | undefined { diff --git a/packages/@cdktn/commons/src/telemetry.test.ts b/packages/@cdktn/commons/src/telemetry.test.ts index 8196c1af3..39e85cf12 100644 --- a/packages/@cdktn/commons/src/telemetry.test.ts +++ b/packages/@cdktn/commons/src/telemetry.test.ts @@ -482,6 +482,10 @@ describe("telemetry", () => { [{ sendUsageTelemetry: "true" }, true], [{ sendUsageTelemetry: "false" }, false], [{}, undefined], + // malformed is unset, not an opt-out + [{ sendUsageTelemetry: "yes" }, undefined], + [{ sendUsageTelemetry: 1 }, undefined], + [{ sendUsageTelemetry: null }, undefined], ])("reads %j as %p", (config, expected) => { fs.writeJsonSync(path.join(workdir, "cdktf.json"), config); expect(getUsageTelemetryConsent(workdir)).toBe(expected); diff --git a/packages/@cdktn/commons/src/telemetry.ts b/packages/@cdktn/commons/src/telemetry.ts index 536a6a455..8427e8985 100644 --- a/packages/@cdktn/commons/src/telemetry.ts +++ b/packages/@cdktn/commons/src/telemetry.ts @@ -1,5 +1,6 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 +import { normalizeConsentFlag } from "./config"; import * as Sentry from "@sentry/node"; import * as path from "path"; import * as fs from "fs-extra"; @@ -37,7 +38,8 @@ function readRawCdktfJson(projectPath: string): Record { /** * Reads the raw `sendUsageTelemetry` flag from `cdktf.json`. Returns - * `undefined` when the flag is unset or no readable `cdktf.json` exists; + * `undefined` when the flag is unset or malformed, or no readable + * `cdktf.json` exists; * `isUsageTelemetryEnabled` derives the effective state. */ export function getUsageTelemetryConsent( @@ -47,11 +49,10 @@ export function getUsageTelemetryConsent( if (!("sendUsageTelemetry" in cdktfJson)) { return undefined; } - // init templates render booleans as the strings "true"/"false"; a - // boolean-only check would opt every freshly init'ed project out - return typeof cdktfJson.sendUsageTelemetry === "boolean" - ? cdktfJson.sendUsageTelemetry - : cdktfJson.sendUsageTelemetry === "true"; + return normalizeConsentFlag( + "sendUsageTelemetry", + cdktfJson.sendUsageTelemetry, + ); } type CommandTelemetryState = { From c628e18458e31ac180103847f98e223e42c38c4f Mon Sep 17 00:00:00 2001 From: so0k Date: Tue, 22 Sep 2026 12:49:34 +0800 Subject: [PATCH 13/17] fix(cli): only prompt for consent when stdin is a terminal too canPrompt and isInteractiveTerminal now also require process.stdin.isTTY, so `cat main.tf | cdktn convert` no longer reads the piped HCL as the consent answer. Co-Authored-By: Claude Opus 5 (1M context) --- .../cli-core/src/lib/error-reporting.ts | 1 + .../cli-core/src/test/error-reporting.test.ts | 20 +++++++++ .../cmds/__tests__/handlers-convert.test.ts | 28 +++++++++++-- .../__tests__/check-environment.test.ts | 41 +++++++++++++++++++ .../src/bin/cmds/helper/check-environment.ts | 6 ++- 5 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 packages/cdktn-cli/src/bin/cmds/helper/__tests__/check-environment.test.ts diff --git a/packages/@cdktn/cli-core/src/lib/error-reporting.ts b/packages/@cdktn/cli-core/src/lib/error-reporting.ts index 89aee3f60..2887ead2e 100644 --- a/packages/@cdktn/cli-core/src/lib/error-reporting.ts +++ b/packages/@cdktn/cli-core/src/lib/error-reporting.ts @@ -95,6 +95,7 @@ export async function initializErrorReporting( // cdktf.json to persist the decision into; otherwise fall through to // the per-flag non-interactive defaults below. const canPrompt = + Boolean(process.stdin.isTTY) && Boolean(process.stdout.isTTY) && !ciInfo.isCI && !process.env.CI && diff --git a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts index 27551434f..723ffed54 100644 --- a/packages/@cdktn/cli-core/src/test/error-reporting.test.ts +++ b/packages/@cdktn/cli-core/src/test/error-reporting.test.ts @@ -58,6 +58,10 @@ const setInteractive = (interactive: boolean) => { value: interactive, configurable: true, }); + Object.defineProperty(process.stdin, "isTTY", { + value: interactive, + configurable: true, + }); if (interactive) { delete process.env.CI; ciInfoMock.isCI = false; @@ -76,6 +80,7 @@ function useReportingFixture() { CHECKPOINT_DISABLE: process.env.CHECKPOINT_DISABLE, }; const originalIsTTY = process.stdout.isTTY; + const originalStdinIsTTY = process.stdin.isTTY; beforeEach(() => { jest.clearAllMocks(); @@ -97,6 +102,10 @@ function useReportingFixture() { value: originalIsTTY, configurable: true, }); + Object.defineProperty(process.stdin, "isTTY", { + value: originalStdinIsTTY, + configurable: true, + }); for (const [key, value] of Object.entries(originalEnv)) { if (value === undefined) { delete process.env[key]; @@ -256,6 +265,17 @@ describe("consent gating (initializErrorReporting)", () => { }, true, ], + [ + "stdout TTY but stdin piped", + () => { + setInteractive(true); + Object.defineProperty(process.stdin, "isTTY", { + value: undefined, + configurable: true, + }); + }, + true, + ], [ "TTY but no cdktf.json (no-project command)", () => setInteractive(true), diff --git a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts index 5d4edaaf1..dbec16f01 100644 --- a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts +++ b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-convert.test.ts @@ -57,6 +57,7 @@ describe("convert consent", () => { let workdir: string; const originalCwd = process.cwd(); const originalIsTTY = process.stdout.isTTY; + const originalStdinIsTTY = process.stdin.isTTY; const originalEnv = { CI: process.env.CI, SENTRY_DSN: process.env.SENTRY_DSN, @@ -64,12 +65,18 @@ describe("convert consent", () => { }; let logSpy: jest.SpyInstance; - const setInteractive = (interactive: boolean) => { + const setTTY = (stdout: boolean | undefined, stdin: boolean | undefined) => { Object.defineProperty(process.stdout, "isTTY", { - value: interactive, + value: stdout, + configurable: true, + }); + Object.defineProperty(process.stdin, "isTTY", { + value: stdin, configurable: true, }); }; + const setInteractive = (interactive: boolean) => + setTTY(interactive, interactive); const cdktfJson = () => fs.readJsonSync(path.join(workdir, "cdktf.json")); @@ -97,7 +104,7 @@ describe("convert consent", () => { afterEach(() => { logSpy.mockRestore(); setUsageTelemetryEnabled(undefined); - setInteractive(originalIsTTY); + setTTY(originalIsTTY, originalStdinIsTTY); process.chdir(originalCwd); fs.removeSync(workdir); for (const [key, value] of Object.entries(originalEnv)) { @@ -175,4 +182,19 @@ describe("convert consent", () => { expect(cdktfJson()).not.toHaveProperty("sendUsageTelemetry"); expect(cdktfJson()).not.toHaveProperty("sendCrashReports"); }); + + it("does not prompt when the HCL is piped on stdin, and converts that input", async () => { + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + app: "npx ts-node main.ts", + }); + setTTY(true, undefined); + + await convert({ language: "typescript", provider: [] }); + + expect(mockCrashPrompt).not.toHaveBeenCalled(); + expect(mockUsagePrompt).not.toHaveBeenCalled(); + expect(cdktfJson()).not.toHaveProperty("sendUsageTelemetry"); + expect(mockConvert).toHaveBeenCalledWith("resource {}", expect.anything()); + }); }); diff --git a/packages/cdktn-cli/src/bin/cmds/helper/__tests__/check-environment.test.ts b/packages/cdktn-cli/src/bin/cmds/helper/__tests__/check-environment.test.ts new file mode 100644 index 000000000..af818b4bb --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/helper/__tests__/check-environment.test.ts @@ -0,0 +1,41 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import { isInteractiveTerminal } from "../check-environment"; + +describe("isInteractiveTerminal", () => { + const original = { + stdin: process.stdin.isTTY, + stdout: process.stdout.isTTY, + ci: process.env.CI, + }; + + const setTerminal = (stdin: boolean, stdout: boolean, ci?: string) => { + Object.defineProperty(process.stdin, "isTTY", { + value: stdin, + configurable: true, + }); + Object.defineProperty(process.stdout, "isTTY", { + value: stdout, + configurable: true, + }); + if (ci === undefined) { + delete process.env.CI; + } else { + process.env.CI = ci; + } + }; + + afterEach(() => { + setTerminal(original.stdin, original.stdout, original.ci); + }); + + it.each<[string, boolean, boolean, string | undefined, boolean]>([ + ["both terminals outside CI", true, true, undefined, true], + ["stdout terminal with piped stdin", false, true, undefined, false], + ["stdin terminal with piped stdout", true, false, undefined, false], + ["both terminals in CI", true, true, "true", false], + ])("%s -> %p", (_case, stdin, stdout, ci, expected) => { + setTerminal(stdin, stdout, ci); + expect(isInteractiveTerminal()).toBe(expected); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/helper/check-environment.ts b/packages/cdktn-cli/src/bin/cmds/helper/check-environment.ts index be687cfd5..84daa6dc4 100644 --- a/packages/cdktn-cli/src/bin/cmds/helper/check-environment.ts +++ b/packages/cdktn-cli/src/bin/cmds/helper/check-environment.ts @@ -149,5 +149,9 @@ export async function verifySimilarLibraryVersion() { } export function isInteractiveTerminal() { - return process.stdout.isTTY && !process.env.CI; + return ( + Boolean(process.stdin.isTTY) && + Boolean(process.stdout.isTTY) && + !process.env.CI + ); } From c9dc2e74c42b80ad13c1a060b9d74bac393e87b9 Mon Sep 17 00:00:00 2001 From: so0k Date: Tue, 22 Sep 2026 12:51:47 +0800 Subject: [PATCH 14/17] fix(cli): count watch, list, output and a no-op get as completed These runs emitted cli.command.invoked but no terminal event: their nested synth counts only as synth, and get returned early without providers or modules. Each handler now sends its own completion on success. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/handlers-completion.test.ts | 124 ++++++++++++++++++ packages/cdktn-cli/src/bin/cmds/handlers.ts | 5 + 2 files changed, 129 insertions(+) create mode 100644 packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts diff --git a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts new file mode 100644 index 000000000..a1a4d83c5 --- /dev/null +++ b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts @@ -0,0 +1,124 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as fs from "fs-extra"; +import * as os from "os"; +import * as path from "path"; +import * as Sentry from "@sentry/node"; +import { + Errors, + resetCommandTelemetry, + setUsageTelemetryEnabled, +} from "@cdktn/commons"; + +// Sentry is stubbed so reporting initializes and the run counts as invoked. +jest.mock("@sentry/node", () => ({ + init: jest.fn(), + getCurrentScope: jest.fn(() => ({ + setUser: jest.fn(), + setTag: jest.fn(), + setPropagationContext: jest.fn(), + setTransactionName: jest.fn(), + })), + addBreadcrumb: jest.fn(), + metrics: { count: jest.fn(), distribution: jest.fn() }, +})); +jest.mock("../helper/version-check", () => ({ + displayVersionMessage: jest.fn().mockResolvedValue(undefined), +})); +jest.mock("../helper/terraform-check", () => ({ + terraformCheck: jest.fn().mockResolvedValue(undefined), +})); +jest.mock("../helper/check-environment", () => ({ + ...jest.requireActual("../helper/check-environment"), + checkEnvironment: jest.fn().mockResolvedValue(undefined), + verifySimilarLibraryVersion: jest.fn().mockResolvedValue(undefined), +})); +jest.mock("../ui/watch", () => ({ + runWatch: jest.fn().mockResolvedValue(undefined), +})); +jest.mock("../ui/list", () => ({ + runList: jest.fn().mockResolvedValue(undefined), +})); +jest.mock("../ui/output", () => ({ + runOutput: jest.fn().mockResolvedValue(undefined), +})); +jest.mock("ci-info", () => ({ isCI: false, name: null })); + +import { get, list, output, watch } from "../handlers"; + +// A run that ends without an error counts exactly one cli.command.completed +// under its own command, whatever nested operation it drove. +describe("completion metric of handlers without an own emitter", () => { + let workdir: string; + const originalCwd = process.cwd(); + const originalEnv = { + SENTRY_DSN: process.env.SENTRY_DSN, + CHECKPOINT_DISABLE: process.env.CHECKPOINT_DISABLE, + }; + const count = Sentry.metrics.count as jest.Mock; + + const metricCalls = (name: string) => + count.mock.calls.filter(([metric]) => metric === name); + + beforeEach(() => { + workdir = fs.mkdtempSync(path.join(os.tmpdir(), "cdktn-completion-")); + process.chdir(workdir); + fs.writeJsonSync(path.join(workdir, "cdktf.json"), { + language: "typescript", + app: "npx ts-node main.ts", + sendCrashReports: false, + }); + process.env.SENTRY_DSN = "https://public@example.invalid/1"; + // the jest preset disables usage telemetry process-wide + delete process.env.CHECKPOINT_DISABLE; + setUsageTelemetryEnabled(undefined); + count.mockClear(); + }); + + afterEach(() => { + setUsageTelemetryEnabled(undefined); + resetCommandTelemetry(); + Errors.setScope("unknown"); + process.chdir(originalCwd); + fs.removeSync(workdir); + for (const [key, value] of Object.entries(originalEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + it.each<[string, () => Promise]>([ + ["watch", () => watch({ autoApprove: true })], + ["list", () => list({})], + ["output", () => output({})], + // no providers or modules: get returns before any generation + [ + "get", + () => + get({ output: ".gen", language: "typescript" as any, parallelism: 1 }), + ], + ])("%s counts invoked and completed once each", async (command, run) => { + Errors.setScope(command); + + await run(); + + for (const metric of ["cli.command.invoked", "cli.command.completed"]) { + expect(metricCalls(metric)).toEqual([ + [ + metric, + 1, + { + attributes: expect.objectContaining({ + command, + language: "typescript", + }), + }, + ], + ]); + } + expect(metricCalls("cli.command.error")).toHaveLength(0); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cmds/handlers.ts b/packages/cdktn-cli/src/bin/cmds/handlers.ts index c281e41b0..46993306a 100644 --- a/packages/cdktn-cli/src/bin/cmds/handlers.ts +++ b/packages/cdktn-cli/src/bin/cmds/handlers.ts @@ -353,6 +353,7 @@ export async function get(argv: { logger.warn( `WARNING: No providers or modules found in "cdktf.json" config file, therefore cdktn get does nothing.`, ); + await sendTelemetry("get", {}); return; } @@ -424,6 +425,7 @@ export async function list(argv: any) { await terraformCheck(); await runList({ outDir, synthCommand: command }); + await sendTelemetry("list", {}); } export async function login(argv: { tfeHostname: string }) { @@ -536,6 +538,8 @@ export async function watch(argv: any) { terraformParallelism, parallelism, }); + // runWatch resolves once the watch is stopped gracefully + await sendTelemetry("watch", {}); } export async function output(argv: any) { @@ -572,6 +576,7 @@ export async function output(argv: any) { skipSynth, skipProviderLock, }); + await sendTelemetry("output", {}); } export async function debug(argv: any) { From eb05bd4e58765a961dd1f04d7dde9a79c70d5e22 Mon Sep 17 00:00:00 2001 From: so0k Date: Tue, 22 Sep 2026 12:52:33 +0800 Subject: [PATCH 15/17] fix(cli): count provider add as invoked and completed like other commands providerAdd now initializes reporting itself and sends its completion, so it counts whether or not a nested get runs; before, it counted nothing without a get, and invoked but never completed with one. providerAdd and init flush telemetry before a nested get, whose re-initialization replaces the Sentry client holding the buffered invoked metric. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/handlers-completion.test.ts | 54 ++++++++++++++++++- packages/cdktn-cli/src/bin/cmds/handlers.ts | 10 ++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts index a1a4d83c5..42579c0b4 100644 --- a/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts +++ b/packages/cdktn-cli/src/bin/cmds/__tests__/handlers-completion.test.ts @@ -13,6 +13,7 @@ import { // Sentry is stubbed so reporting initializes and the run counts as invoked. jest.mock("@sentry/node", () => ({ init: jest.fn(), + flush: jest.fn().mockResolvedValue(true), getCurrentScope: jest.fn(() => ({ setUser: jest.fn(), setTag: jest.fn(), @@ -44,7 +45,17 @@ jest.mock("../ui/output", () => ({ })); jest.mock("ci-info", () => ({ isCI: false, name: null })); -import { get, list, output, watch } from "../handlers"; +const mockProviderAddLib = jest.fn(); +jest.mock("@cdktn/cli-core", () => ({ + ...jest.requireActual("@cdktn/cli-core"), + providerAdd: (...args: unknown[]) => mockProviderAddLib(...args), +})); +jest.mock("@cdktn/commons", () => ({ + ...jest.requireActual("@cdktn/commons"), + getPackageVersion: jest.fn().mockResolvedValue("0.23.0"), +})); + +import { get, list, output, providerAdd, watch } from "../handlers"; // A run that ends without an error counts exactly one cli.command.completed // under its own command, whatever nested operation it drove. @@ -56,6 +67,8 @@ describe("completion metric of handlers without an own emitter", () => { CHECKPOINT_DISABLE: process.env.CHECKPOINT_DISABLE, }; const count = Sentry.metrics.count as jest.Mock; + const init = Sentry.init as jest.Mock; + const flush = Sentry.flush as jest.Mock; const metricCalls = (name: string) => count.mock.calls.filter(([metric]) => metric === name); @@ -73,6 +86,8 @@ describe("completion metric of handlers without an own emitter", () => { delete process.env.CHECKPOINT_DISABLE; setUsageTelemetryEnabled(undefined); count.mockClear(); + init.mockClear(); + flush.mockClear(); }); afterEach(() => { @@ -121,4 +136,41 @@ describe("completion metric of handlers without an own emitter", () => { } expect(metricCalls("cli.command.error")).toHaveLength(0); }); + + // needsGet runs a nested get; its own completion must not count + it.each([false, true])( + "provider add (needsGet %p) counts invoked and completed once each", + async (needsGet) => { + mockProviderAddLib.mockResolvedValue(needsGet); + const log = jest.spyOn(console, "log").mockImplementation(() => {}); + Errors.setScope("provider add"); + + try { + await providerAdd({ provider: ["aws"], silent: true }); + } finally { + log.mockRestore(); + } + + for (const metric of ["cli.command.invoked", "cli.command.completed"]) { + expect(metricCalls(metric)).toEqual([ + [ + metric, + 1, + { + attributes: expect.objectContaining({ + command: "provider add", + }), + }, + ], + ]); + } + // the nested get replaces the client; invoked must be flushed first + if (needsGet) { + expect(init).toHaveBeenCalledTimes(2); + expect(flush.mock.invocationCallOrder[0]).toBeLessThan( + init.mock.invocationCallOrder[1], + ); + } + }, + ); }); diff --git a/packages/cdktn-cli/src/bin/cmds/handlers.ts b/packages/cdktn-cli/src/bin/cmds/handlers.ts index 46993306a..01028fbe0 100644 --- a/packages/cdktn-cli/src/bin/cmds/handlers.ts +++ b/packages/cdktn-cli/src/bin/cmds/handlers.ts @@ -14,6 +14,7 @@ import { Language, readConfigSync, sendTelemetry, + flushTelemetry, Errors, IsErrorType, logger, @@ -397,6 +398,8 @@ export async function init(argv: any) { "Local providers have been updated. Running cdktn get to update...", ); } + // get re-initializes Sentry; send this client's buffer before it is replaced + await flushTelemetry(); await get({ language, output: codeMakerOutput, @@ -653,6 +656,10 @@ export async function debug(argv: any) { } export async function providerAdd(argv: any) { + await initializErrorReporting( + askForCrashReportingConsent, + askForUsageTelemetryConsent, + ); const config = CdktfConfig.read(); const language = config.language; @@ -676,6 +683,8 @@ export async function providerAdd(argv: any) { console.log( "Local providers have been updated. Running cdktn get to update...", ); + // get re-initializes Sentry; send this client's buffer before it is replaced + await flushTelemetry(); await get({ language: language, output: config.codeMakerOutput, @@ -689,6 +698,7 @@ export async function providerAdd(argv: any) { "After adding this module to your imports, please run 'go mod tidy' to resolve newly added modules", ); } + await sendTelemetry("provider add", {}); } export async function providerUpgrade(argv: any) { From 3dc85882adc14b1f21661b1d7e52908f2392dc2f Mon Sep 17 00:00:00 2001 From: so0k Date: Tue, 22 Sep 2026 12:52:53 +0800 Subject: [PATCH 16/17] fix(cli): say why the usage-telemetry prompt asks for consent The prompt now states that usage data helps prioritize development. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts b/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts index 0c2db1197..55e89c5c4 100644 --- a/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts +++ b/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts @@ -16,7 +16,7 @@ export async function askForCrashReportingConsent() { export async function askForUsageTelemetryConsent() { return await confirm({ message: - "Do you want to send anonymous usage telemetry (command, language, timing) to the CDKTN team? Refer to https://cdktn.io/docs/telemetry for more information", + "Do you want to send anonymous usage telemetry (command, language, timing) to the CDKTN team? This enables the project to focus on what is actually used by the community to prioritize development. Refer to https://cdktn.io/docs/telemetry for more information", default: true, }); } From 03e1c523b7e30275dc818adf45723d7b4855d44f Mon Sep 17 00:00:00 2001 From: so0k Date: Tue, 22 Sep 2026 12:53:25 +0800 Subject: [PATCH 17/17] chore(cli): link crash-reporting consent to the telemetry page and fix the user-id note The crash-consent prompt now points at /docs/telemetry#crash-reporting like the --enable-crash-reporting flag, and the user-id file note reads "in order to inform". Co-Authored-By: Claude Opus 5 (1M context) --- packages/@cdktn/commons/src/identity.ts | 2 +- packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@cdktn/commons/src/identity.ts b/packages/@cdktn/commons/src/identity.ts index be1bf2bab..30510cb9a 100644 --- a/packages/@cdktn/commons/src/identity.ts +++ b/packages/@cdktn/commons/src/identity.ts @@ -60,7 +60,7 @@ export function getUserId(): string { path.resolve(homeDir(), "config.json"), "userId", true, - `This signature is a randomly generated UUID used to anonymously differentiate users in telemetry data order to inform product direction. + `This signature is a randomly generated UUID used to anonymously differentiate users in telemetry data in order to inform product direction. This signature is random, it is not based on any personally identifiable information. To create a new signature, you can simply delete this file at any time. See https://cdktn.io/docs/telemetry for more diff --git a/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts b/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts index 55e89c5c4..d1af8ade6 100644 --- a/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts +++ b/packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts @@ -8,7 +8,7 @@ import { confirm } from "@inquirer/prompts"; export async function askForCrashReportingConsent() { return await confirm({ message: - "Do you want to send crash reports to the CDKTN team? Refer to https://cdktn.io/docs/create-and-deploy/configuration-file#enable-crash-reporting-for-the-cli for more information", + "Do you want to send crash reports to the CDKTN team? Refer to https://cdktn.io/docs/telemetry#crash-reporting for more information", default: true, }); }