feat(cli): emit telemetry for the validate action by iankhou · Pull Request #1864 · aws/aws-cdk-cli

iankhou · 2026-08-20T14:26:59Z

Description

cdk synth, cdk deploy, and cdk validate commands currently emit telemetry with the following information:

Commands perform synth under the hood and emit a SYNTH eventType
The SYNTH eventType subsumes the time taken for offline validation

What we don't know:

What ratio/how many runs of cdk validate catch fatal issues that would have failed deployment?
When is online validation being run, and how long does it take?
How often does online validation (not findings, the engine itself) fail?

This change adds:

Counters to the SYNTH eventType, (offlineValidationWarnings, offlineWouldFailDeploy); this reads the validation report with best effort, and is different from the "warnings" counter, which reports construct annotations
A VALIDATE_ONLINE eventType that fires when online validation is run, and contains counters onlineViolations and online:stacksIncomplete (indicates the number of stacks for which we could not run online validation)
A failure mode for VALIDATE_ONLINE when it cannot complete online validation on any stacks

Testing

Unit tests
Sent some payloads to validate that the backend takes our new eventType VALIDATE_ONLINE

Payloads test the following cases:

VALIDATE_ONLINE with onlineViolations + online:stacksIncomplete counters (tests this change)
VALIDATE_ONLINE event failure
SYNTH with offlineWouldFailDeploy + offlineValidationWarnings counters
SYNTH without additional counters (regression test)

Not included

We have separately enabled our Telemetry backend to accept VALIDATE_ONLINE events.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

github-actions · 2026-08-20T14:27:12Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

…at would have failed deploy

Add an offlineValidationWarnings counter alongside the existing warnings counter, counting warning-severity violations from the policy validation report (validation-report.json). Construct annotation warnings are excluded so they are not double-counted with the warnings counter. The report is now read once via offlineValidationSummary, which returns both offlineWouldFailDeploy and offlineValidationWarnings.

…output The CLI now dispatches telemetry from a background process, so its own output no longer prints "Telemetry Sent Successfully" — only that the batch was dispatched. Drop that stale assertion and verify the SYNTH event counters via the --telemetry-file contents, which is what this test is about. Real endpoint delivery stays covered by cdk-telemetry-reaches-the-endpoint.integtest.ts.

The offline validation counters run inside countAssemblyResults on every synth (cdk ls/diff/synth/deploy), and Manifest.loadValidationReport schema-validates the file, so a malformed validation-report.json would throw and fail the command outright. Wrap the report read so telemetry stays best-effort: on any read or schema error, behave as if there were no report (wouldFailDeploy still reflects error-level construct annotations; offlineValidationWarnings is 0).

…esult onlineReports was [] both when online validation passed cleanly and when it could not run for any stack (bad credentials), so a programmatic consumer was told "clean" when nothing was validated. The field was only emitted (return value + I9600/E9600 payloads), never read internally: the online violation counter uses the local variable, and every online report is already present in pluginReports. Drop the field rather than ship an ambiguous contract. Online findings remain available via pluginReports; a dedicated online-status API can be added later if a consumer needs the online/offline split.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…tup failure If setup failed before validateOnline ran (e.g. credential resolution in deploymentsForAction), the VALIDATE_ONLINE event was marked failed but online:stacksIncomplete stayed 0 — inaccurate precisely for an engine-wide failure, and inconsistent with the every-stack-failed path which reports stackCount. Track the incomplete count in _validate, initialized to the selected stack count and overwritten only once validateOnline returns, so a pre-validation failure records every selected stack as incomplete. validateOnline no longer touches the span (it just returns the count); countOnlineValidationResults emits online:stacksIncomplete from that single value.

Copilot

Copilot review overview

🔵 Needs a closer look

Diagnostic failures and report-only construct warnings currently produce inaccurate telemetry counters.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Count selected stacks as incomplete on pre-validation failure
Initialize online:stacksIncomplete to zero on successful validation

Previously missed (2)

In code that hasn't changed since last review

Count report-only construct annotations without duplicating legacy warnings

packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts:65

Construct annotations can exist only in validation-report.json, rather than in stack.messages (the report-only mode is handled in validation-report.ts:29-55, and the repository fixture has such an annotation at test/_fixtures/stack-with-multi-plugin-validation/cdk.out/validation-report.json:116-138). Filtering them here means neither the existing warnings counter nor offlineValidationWarnings records those warnings. Count report-backed construct annotations in warnings, while avoiding duplicates for legacy assemblies.

Treat error-diagnosing results as incomplete stack validation

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts:797

diagnoseChangeSet() converts diagnostic exceptions into a resolved Diagnosis.errorDiagnosing(...) (stack-diagnoser.ts:138-143), so those failures never enter this catch and the stack is reported as successfully validated. Handle diagnosis.type === 'error-diagnosing' as an incomplete stack (and add coverage for that result) so online:stacksIncomplete and the all-stacks-failed event state remain accurate.

Construct annotations live in stack.messages for legacy assemblies but in the validation report (Construct Annotations plugin) in report-only mode. The warnings counter only read messages and offlineValidationWarnings excludes the annotation plugin, so report-only annotation warnings were counted by neither. Fold the report's Construct Annotations warnings into the warnings counter. Legacy assemblies have no such report entry and report-only ones have no message warnings, so the two sources are mutually exclusive and are not double-counted.

… stacks createValidationChangeSet resolves a failed diagnosis into an 'error-diagnosing' result instead of throwing, so it never reached the catch and the stack was treated as validated. That undercounted online:stacksIncomplete and could miss the all-stacks-failed state. Handle diagnosis.type === 'error-diagnosing' like a thrown failure: count the stack as incomplete and emit the W9602 warning.

The engine-wide failure state (onlineError = OnlineValidationIncomplete) was not exercised: the error-diagnosing test only checked the counter, so removing the assignment would still pass. Assert the I9604 payload carries error.name === 'OnlineValidationIncomplete' when every stack is incomplete, and add a partially-incomplete case (one of two stacks fails) that asserts no error is set.

iankhou · 2026-09-28T19:37:02Z

Copilot review overview

🔵 Needs a closer look

Offline counters currently include violations from unselected stacks, producing inaccurate per-command telemetry.

Review effort: Balanced Findings: 1
Open (1)

Previously missed (1)

The counters are on the whole assembly, like existing counters on the SYNTH eventType. Stack selection happens on the stack assembly (after synthesis).

rix0rrr · 2026-09-29T06:42:02Z

+   *
+   * @default - no counters
+   */
+  readonly counters?: Record<string, number>;


Is this necessary? I'm pretty sure we have a counters facility built-in to the whole messages system already?

rix0rrr · 2026-09-29T06:51:58Z

+  if (!wouldFailDeploy(pluginReports, failAt)) {
+    return;
+  }
+
+  if (failAt === 'warn') {
+    const error = AssemblyError.withStacks('Synthesis finished with warnings (--strict mode)', stacks.stackArtifacts);
+    error.attachSynthesisErrorCode('StrictAnnotationWarnings');
+    throw error;
+  }


This is not necessarily true. Just because we WOULD fail if there were warnings, doesn't mean there WERE warnings.

I also don't like wouldFailDeploy as a name because we're not making any statement about whether the deployment would fail or not: we are just checking whether there are errors and we fail at errors, or if there are warnings and we fail at warnings.

Don't combine 2 bits of information here (wouldFailDeploy: boolean and failAt: FailureReason) to make a decision. Have a bit of logic make a decision for you and encode that decision into a single value, then act based on that.

// If any of the variants had associated data here we would do a union of objects with a 'type' field, for this case strings are sufficient type ValidationAction = 'fail-errors' | 'fail-strict-warnings' | 'continue'; function decideValidationAction(...): ValidationAction { ... } switch (decideValidationAction(pluginReports, failAt) { case 'fail-errors': // ... case 'fail-strict-warnings': // ... case 'continue': break; }

Type this with your own hands, don't let AI do it. Get this pattern ingrained in your mind.

rix0rrr · 2026-09-29T06:55:59Z

+  // Online validation lives in toolkit-lib, so (like hotswap) it cannot use a
+  // CDK_CLI code. We translate the toolkit-lib VALIDATE_ONLINE span end message
+  // into the telemetry event instead. Its `error` is a raw `Error`, so we
+  // convert it to a telemetry error name rather than passing it through.
+  if (IO.CDK_TOOLKIT_I9604.is(msg)) {
+    return {
+      eventType: 'VALIDATE_ONLINE',
+      duration: msg.data.duration,
+      ...(msg.data.error ? { error: { name: cdkCliErrorName(msg.data.error) } } : {}),
+      counters: msg.data.counters,
+    };
+  }


Why does this not use eventResult? Or a function like it?

This comment block should go at the top of the function, if there are different classes of event pairs that get translated into telemetry differently. There should be common rules governing this that are explained, not an ad-hoc "oh this is like that other one" comment line.

iankhou temporarily deployed to automation August 20, 2026 14:27 — with GitHub Actions Inactive

iankhou had a problem deploying to automation August 20, 2026 14:27 — with GitHub Actions Failure

github-actions Bot added the p2 label Aug 20, 2026

aws-cdk-automation requested a review from a team August 20, 2026 14:27

iankhou temporarily deployed to no-approval August 20, 2026 14:27 — with GitHub Actions Inactive

iankhou temporarily deployed to run-tests August 20, 2026 14:30 — with GitHub Actions Inactive

iankhou temporarily deployed to run-tests August 24, 2026 19:19 — with GitHub Actions Inactive

iankhou changed the title ~~feat(cli): DO NOT MERGE emit telemetry for the validate action~~ feat(cli): emit telemetry for the validate action Aug 24, 2026

iankhou temporarily deployed to no-approval August 25, 2026 14:45 — with GitHub Actions Inactive

iankhou temporarily deployed to run-tests August 25, 2026 14:49 — with GitHub Actions Inactive

This comment was marked as resolved.

Sign in to view

iankhou added 6 commits September 25, 2026 10:39

VALIDATE_ONLINE event and counters for offline validation in synth th…

5b3b764

…at would have failed deploy

Merge branch 'main' into telemetry-validate-event

228bf7e

This comment was marked as resolved.

Sign in to view

iankhou and others added 2 commits September 28, 2026 13:51

Fix countOnlineValidationResults function syntax

110fc03

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Sep 28, 2026

View reviewed changes

iankhou added 3 commits September 28, 2026 14:50

Merge branch 'main' into telemetry-validate-event

84d61c3

This comment was marked as outdated.

Sign in to view

This comment was marked as low quality.

Sign in to view

rix0rrr requested changes Sep 29, 2026

View reviewed changes

Navigation Menu

feat(cli): emit telemetry for the validate action - #1864

feat(cli): emit telemetry for the validate action#1864
iankhou wants to merge 12 commits into
mainfrom
telemetry-validate-event

iankhou commented Aug 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as low quality.

Uh oh!

iankhou commented Sep 28, 2026 •

edited

Loading

Copilot review overview

🔵 Needs a closer look

Uh oh!

rix0rrr Sep 29, 2026

Uh oh!

rix0rrr Sep 29, 2026

Uh oh!

rix0rrr Sep 29, 2026

Uh oh!

This branch had an error being deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

iankhou commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Not included

Uh oh!

github-actions Bot commented Aug 20, 2026

Dependency Review

Scanned Files

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as low quality.

Uh oh!

iankhou commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Copilot review overview

🔵 Needs a closer look

Uh oh!

rix0rrr Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

rix0rrr Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

rix0rrr Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

This branch had an error being deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iankhou commented Aug 20, 2026 •

edited

Loading

iankhou commented Sep 28, 2026 •

edited

Loading