[so0k]

Every usage metric now carries the environment it ran in: the installed Terraform or OpenTofu binary and version, the target versions the project declares, the OS and architecture, and the project's package manager. This answers which binaries, platforms and package managers people actually run. Part of #48.

What changes

• The base attribute set of every metric grows by the attributes below (commons/src/telemetry.ts). No new metrics are added.
• commons/src/terraform.ts probes the binary with <binary> version, lazily, at most once per process, shared across bundle copies via process state. The probe is killed after 1500 ms. A failed or timed-out probe reports binary: unknown; a binary that cannot be spawned reports missing. This replaces the terraform version -json call that ran at module import in every commons copy regardless of consent; with usage telemetry off, metrics trigger no probe.
• cdktn debug uses the same probe and prints "terraform": null when it fails.
• cli-core captures the project's target attributes once at init from the project that is running, so later metrics in the run carry the same values.
• Values are validated before they become attributes: versions are reduced to MAJOR.MINOR.PATCH and sent only for a recognised product; ranges are canonical semver or the literal invalid. No path, binary location or free text is forwarded.
• The package manager is detected once at command start in the project the run started in, and is omitted when it cannot be determined. Only members of a closed set are sent.

Attribute	Value	Source
os, arch	process.platform, process.arch	Node
binary	terraform, opentofu, unknown, missing	version probe
binary_version	MAJOR.MINOR.PATCH, only for terraform or opentofu	version probe
target_terraform, target_opentofu	canonical semver range, else invalid	targetVersions in cdktf.json, else the CLI defaults
targets_declared	boolean	cdktf.json has a targetVersions block
validate_installed_binary	boolean	validateInstalledBinary === true in cdktf.json
package_manager	npm, pnpm, yarn, yarn-berry (TypeScript); pip, pipenv (Python); maven, gradle (Java); nuget; go	TypeScript: detectNodePackageManager from #422. Python: pipenv if the app command uses it, else pip. Java: build.gradle present, else maven. C# and Go: constant

All are added to every metric that carries the base attributes (command, ci, language, from #411), except cli.error, which keeps only type and command.

Not in this PR

• A shell wrapper named terraform that runs the real binary as a child is killed only at the wrapper, so the probe can wait for the child to close stdout. Shims that exec or symlink (tfenv, asdf, mise) are unaffected. Fixing it means changing exec in commons/src/util.ts.

Testing

• Unit: the sorted attribute key set of cli.command.invoked is pinned, so an extra attribute fails; target attributes captured at init appear on later metrics.
• Unit with fixture binaries: the probe runs once, a hanging binary resolves unknown at the timeout, a missing binary resolves missing, an unrecognised product sends no version.
• Table-driven validation of versions and ranges, including prerelease and build text.
• Package manager: each detected value appears on invoked and on the terminal metric; it is absent outside a project, and a detector result outside the closed set is dropped.

Stack: 4 of 6 — #409 → #410 → #411 → #412 → #413 → #414

[jsteinich]

Reviewed as part of the full tele/s1–tele/s6 stack. Breaking the errors.ts → terraform.ts cycle so telemetry.ts can be imported from errors.ts is a nice bit of untangling, and the process-global Symbol.for probe key is the right answer to the "bundle carries several copies of this module" problem.

One real issue and one smaller one.

The 1500 ms bound doesn't bound what the docstring says it does

telemetry.ts:149:

 * Binary attributes from the version probe, bounded so a hung binary never
 * delays the command; a timed-out probe reports `binary: "unknown"`.

The Promise.race bounds the attribute, not the process. exec in commons/src/util.ts uses spawn with no unref() and no kill-on-timeout, so a hung terraform version leaves a live child handle on the event loop: the race resolves at 1500 ms, the metric gets binary: "unknown", and then the CLI sits there until the child exits. beforeExit won't fire while that handle is active, so createBeforeExitFlush doesn't run either — the command hangs at exit rather than degrading gracefully.

Either kill the child when the timer fires, or unref() the probe's child so it can't hold the loop open. Worth fixing because "the version probe hangs" is exactly the scenario the timeout was written for.

The probe now runs on commands that never needed the binary

Related but separate: getBinaryAttributes() is awaited unconditionally from sendTelemetry (telemetry.ts:229), so init, get, convert and synth now spawn terraform version purely to populate a metric attribute. It's memoised per process via the Symbol.for key so it's one spawn, not many, and it's correctly behind the isUsageTelemetryEnabled() early-return — but it's still a new subprocess on commands that previously touched no binary at all, and on the failure path in reportFailure it can add up to 1500 ms before the process exits.

Not necessarily wrong, but worth a deliberate decision: is binary / binary_version valuable enough on an init or convert metric to justify spawning for it? Restricting the probe to the commands that already shell out would remove both this and most of the exposure above.

Minor: fabricated error prose as a success value

terraform.ts:71-77:

.catch(
  (err) =>
    `Error: Usage Error: Unknown: Error loading terraform version ${err}`,
);

Resolving to a string that impersonates the old Errors.Usage output is a smell — the "Error: Usage Error: " prefix now exists only to make cdktn debug print something that looks like it used to. Since the only consumer is debug.ts:407 (which already does ?? null), returning undefined and letting debug.ts render the failure message would be clearer and wouldn't leave a fake error string floating around for someone to pattern-match on later.

(The Promise → function change itself is fine — all in-repo consumers are updated.)

[jsteinich]

Re-reviewed at 7adca3969. One small new point, from the processState helper that landed in #411 below this layer.

terraform.ts:35 still hand-rolls the same globalThis pattern the helper now exists for:

const PROBE_KEY = Symbol.for("cdktn.terraformCli");

errors.ts was converted to processState("cdktn.errorScope", ...) in the same round, so this is the last remaining copy. Worth folding in for consistency while the layer is still open — it also means the "one spawn per process across bundle copies" guarantee is expressed the same way as the other two pieces of cross-copy state, rather than looking like an unrelated mechanism.

[so0k]

@jsteinich on "the probe now runs on commands that never needed the binary":

The probe isn't new work for those commands. Before this layer, @cdktn/commons/src/terraform.ts ran terraform version -json at import, once per commons copy. That's 3× on convert, debug, synth, list and provider list, and 1× on --version, with or without consent. terraformCheck() added terraform -v on 11 commands. With this layer, consent off runs zero probes (only terraformCheck's -v), and consent on runs one, shared by every commons copy. convert goes from 3 to 0. The hang case also improves: no orphaned processes once the probe is killed at 1500 ms, and debug is bounded where it previously waited indefinitely.

full probe results on binary runs before and after this PR (from agent session):

[jsteinich]

Thanks — you're right about the spawn counts and I'll withdraw that half of the finding. I'd missed that the old terraform.ts exported terraformVersion as a module-scope exec(...), so it ran at import rather than on use. Per-copy at import is strictly worse than once-per-process on demand, and gating it behind consent means consent-off now runs zero. That's a net improvement, not a regression.

Two corrections to the record, both small.

There is no kill. grep -nE "\.kill\(|unref\(|AbortController" over packages/@cdktn/commons/src returns nothing, and getBinaryAttributes' finally block is clearTimeout(timer) — which clears the timer, not the child. Promise.race resolves the attribute at 1500 ms; the spawn from exec is untouched and still holds a handle.

My "hangs at exit" framing is superseded — by #410, not by this layer. A hung probe no longer delays the CLI because runCli now ends the success path with an explicit process.exit(process.exitCode ?? 0) after the flush and drainStdio, and process.exit does not wait on live handles. So the docstring's "bounded so a hung binary never delays the command" is true today, but for a reason that lives in another PR rather than anything in this function.

What's left is much milder than I first said: a genuinely hung binary leaves one orphaned child after the CLI exits, reparented and harmless in the normal case. Not worth blocking on. Either add the .kill() the docstring implies, or reword it to say the attribute is bounded and the process exit is what actually caps the wait — I'd slightly prefer the latter, since it keeps the honest version of the guarantee next to the code that relies on it.

(The PROBE_KEY point from my other comment still stands independently — errors.ts moved to processState, terraform.ts:35 is the last hand-rolled copy.)

[so0k]

Thanks for the re-review, and for the two corrections — both are right, and I went with the kill rather than the reworded docstring.

The bound now reaches the child — fix(commons): bound the terraform version probe with exec's timeout. versionOutput() passes { timeout: PROBE_TIMEOUT_MS } (1500, named once) to exec, which forwards it to spawn, so the child is killed with SIGTERM and the probe settles on its own. The Promise.race, its timer and the timeoutMs parameter are gone from getBinaryAttributes, and its docstring is now just "Binary attributes from the version probe" — no guarantee stated where it isn't implemented. terraformCli() reports missing only for a spawn error with code === "ENOENT"; a timed-out or otherwise failed version reports unknown.

Measured against real fixtures: a binary that hangs resolves { name: "unknown" } after ~1505 ms with no child left behind (pgrep clean), and an empty PATH resolves { name: "missing" } in 2 ms.

And you're right that #410's explicit process.exit is what actually caps the wait today — with the kill in place, the orphan goes away too, so neither the docstring nor the behaviour leans on the other PR any more.

One known gap, small enough that I left it: if terraform on PATH is a shell wrapper that runs the real binary as a child rather than execing it, the kill only reaches the wrapper; its child keeps stdout open and exec() settles on close, so the probe still waits for it (measured ~11 s). Closing that means changing exec in util.ts to settle on exit or to kill the process group, which is wider than this layer. tfenv, asdf and mise shims all exec or symlink, so they aren't affected.

Fabricated error string — fix(commons): report no terraform version in debug when the probe fails. terraformVersion() resolves to undefined on failure, debug.ts keeps its ?? null, and cdktn debug prints "terraform": null.

PROBE_KEY — refactor(commons): keep the terraform probe in process state. The probe now uses processState("cdktn.terraformCli", ...), like cdktn.errorScope, so the hand-rolled symbol is gone. Same key, so it's still one spawn per process across the bundle copies, and seedTerraformCliProbeForTests seeds and clears through the same object.

Also in this push: the consent prompt now lists what this layer adds — "(command, language, timing, Terraform/OpenTofu binary and version, target versions, OS and architecture)".

[jsteinich]

Heads-up on a rebase artifact rather than a code finding: this layer currently reverts #411's consent-prompt fix, and the diff against its stated base renders that revert as if it were intentional.

What the diff shows

packages/cdktn-cli/src/bin/cmds/helper/error-reporting.ts in this branch carries the old 334-character field-list prompt:

Do you want to send anonymous usage telemetry (command, language, timing, Terraform/OpenTofu binary and version, target versions, OS and architecture) to the CDKTN team? …

ad45ff8da ("describe usage telemetry by category in the consent prompt") replaced that on tele/s3-sentry-usage-metrics with the 238-character category + negative-guarantee version. git merge-base --is-ancestor ad45ff8da says:

branch	contains the fix
tele/s3-sentry-usage-metrics	yes
tele/s4-binary-target-attrs	no
tele/s5-stack-metrics	no
tele/s6-telemetry-e2e	yes

So s4 and s5 predate the fix and still carry the field-list commit that s6's lineage dropped. packages/cdktn-cli/src/bin/cmds/init.ts on s4 is byte-identical to its pre-fix state, which confirms the branch point rather than a deliberate change.

Why it is easy to miss

The chain is no longer linear. Checking each link against its stated base:

main -> s1 : behind (main moved to 81da16486)
s1   -> s2 : intact
s2   -> s3 : broken
s3   -> s4 : broken
s4   -> s5 : intact
s5   -> s6 : broken

merge-base(s3, s4) and merge-base(s5, s6) are both 705e5b939, so those branches were each rebased onto main independently and at different times; each now carries its own copy of the lower layers rather than descending from the branch GitHub names as its base. The bottom (s1→s2) and the top (s6) both look correct in isolation, and only the middle is stale — which is exactly why the commit list reads fine.

It is not only the prompt, either: s3 carries feat(cli): add a typescript-pnpm init template and deprecate Yarn (#422) and s4 does not, which is why dependency-manager.ts, node-package-manager.ts and the typescript templates also differ between the two.

Suggested fix

Rebase s4 onto the current s3, then s5 onto s4, then s6 onto s5, dropping s4's field-list prompt commit on the way (s6's lineage already has). Worth re-running the ancestry check afterwards rather than trusting the PR view — GitHub will show a diff against a base the branch no longer descends from without any indication that it has diverged.

No action needed on the code in this layer itself; the binary/target/platform work is unaffected.

[so0k]

@jsteinich that was a snapshot taken between two pushes of a cascade. Re-checked on the current remote branches: each of the six links contains the branch below it, the field-list prompt commit is gone from this layer, and #422 is in every layer's base. No code change here for it.

Since that review this layer also gained package_manager (closed set, detected once at command start); the description is updated.