From 05059e6fcf0481ec343ec8b564746944c77dbdd9 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:25:43 -0400 Subject: [PATCH 1/5] feat(cli): emit telemetry for the validate action Adds a VALIDATE telemetry event covering the validation phase of cdk validate (offline report collection and online CloudFormation validation, excluding synthesis), with counters for offline violations per severity, offlineWouldFailDeploy (offline validation found a report that would have failed cdk deploy), and onlineViolations. --- .../private/count-validation-results.ts | 30 +++++++ .../lib/toolkit/private/validation-report.ts | 9 ++ .../toolkit-lib/lib/toolkit/toolkit.ts | 4 +- .../toolkit/count-validation-results.test.ts | 83 +++++++++++++++++ packages/aws-cdk/lib/api-private.ts | 1 + packages/aws-cdk/lib/cli/cdk-toolkit.ts | 27 +++++- .../aws-cdk/lib/cli/io-host/cli-io-host.ts | 3 + .../aws-cdk/lib/cli/telemetry/messages.ts | 15 ++++ packages/aws-cdk/lib/cli/telemetry/schema.ts | 2 +- .../test/cli/io-host/cli-io-host.test.ts | 33 +++++++ ...ss_when_no_validation_report_exists.ndjson | 12 +-- ...n_validates_a_single_selected_stack.ndjson | 12 +-- ...e_even_when_no_violations_are_found.ndjson | 7 ++ ...age_with_offline_violation_counters.ndjson | 7 ++ ..._error_name_when_the_engine_crashes.ndjson | 6 ++ ...en_validation_report_has_violations.ndjson | 8 +- ...le_violations_from_multiple_plugins.ndjson | 8 +- .../aws-cdk/test/commands/validate.test.ts | 88 +++++++++++++++++++ 18 files changed, 333 insertions(+), 22 deletions(-) create mode 100644 packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts create mode 100644 packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts create mode 100644 packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson create mode 100644 packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_with_offline_violation_counters.ndjson create mode 100644 packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts new file mode 100644 index 000000000..92902f0de --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts @@ -0,0 +1,30 @@ +import { ONLINE_VALIDATION_PLUGIN_NAME } from './validation-report'; +import type { ValidateResult } from '../../actions/validate'; +import type { IMessageSpan } from '../../api/io/private/span'; + +/** + * Add counters describing the outcome of a validate run to the given span + * + * Offline violations (policy plugin reports and construct annotations read + * from the cloud assembly) are counted per severity. An offline report with a + * 'failure' conclusion is the exact condition that makes deploy-like actions + * throw (see `throwIfValidationFailures`), so `offlineWouldFailDeploy` records + * that offline validation caught an error before a deployment attempt. + */ +export function countValidationResults(span: IMessageSpan, result: ValidateResult) { + const offline = result.pluginReports.filter((r) => r.pluginName !== ONLINE_VALIDATION_PLUGIN_NAME); + const online = result.pluginReports.filter((r) => r.pluginName === ONLINE_VALIDATION_PLUGIN_NAME); + + for (const report of offline) { + for (const violation of report.violations) { + span.incCounter(`offlineViolations:${violation.severity}`); + } + } + + span.incCounter('onlineViolations', sum(online.map((r) => r.violations.length))); + span.incCounter('offlineWouldFailDeploy', offline.some((r) => r.conclusion === 'failure') ? 1 : 0); +} + +function sum(xs: number[]) { + return xs.reduce((a, b) => a + b, 0); +} diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts index a1a9eeb47..f68ba4f20 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts @@ -12,6 +12,15 @@ import type { MinimumSeverity } from '../types'; const VALIDATION_REPORT_FILE = 'validation-report.json'; +/** + * The plugin name under which online (CloudFormation change set) validation results are reported. + * + * All other plugin names in a unified validation report are offline sources: + * policy validation plugins and construct annotations, both read from the + * cloud assembly. + */ +export const ONLINE_VALIDATION_PLUGIN_NAME = 'CloudFormation'; + /** * The name of the plugin that emits construct annotations into the validation report. * diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 0cb2e0355..664e98a77 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -116,7 +116,7 @@ import { formatErrorMessage, formatExpressStabilizationWarning, formatTime, obsc import { pLimit } from '../util/concurrency'; import { createIgnoreMatcher } from '../util/glob-matcher'; import { promiseWithResolvers } from '../util/promises'; -import { combineConclusions, obtainUnifiedValidationReport, throwIfValidationFailures } from './private/validation-report'; +import { combineConclusions, obtainUnifiedValidationReport, ONLINE_VALIDATION_PLUGIN_NAME, throwIfValidationFailures } from './private/validation-report'; export interface ToolkitOptions { /** @@ -765,7 +765,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { } return { - pluginName: 'CloudFormation', + pluginName: ONLINE_VALIDATION_PLUGIN_NAME, conclusion: 'failure', violations, }; diff --git a/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts new file mode 100644 index 000000000..86b17ff2e --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts @@ -0,0 +1,83 @@ +import type { PluginReportJson } from '@aws-cdk/cloud-assembly-schema'; +import type { ValidateResult } from '../../lib/actions/validate'; +import type { IMessageSpan } from '../../lib/api/io/private/span'; +import { countValidationResults } from '../../lib/toolkit/private/count-validation-results'; + +let span: IMessageSpan; +let counters: Record; + +beforeEach(() => { + counters = {}; + span = { + incCounter: (name: string, delta: number = 1) => { + counters[name] = (counters[name] ?? 0) + delta; + }, + } as IMessageSpan; +}); + +function report(pluginName: string, conclusion: 'success' | 'failure', severities: string[]): PluginReportJson { + return { + pluginName, + conclusion, + violations: severities.map((severity) => ({ + ruleName: 'some-rule', + description: 'some description', + severity: severity as any, + violatingConstructs: [], + })), + }; +} + +function result(...pluginReports: PluginReportJson[]): ValidateResult { + return { + conclusion: pluginReports.some((r) => r.conclusion === 'failure') ? 'failure' : 'success', + pluginReports, + }; +} + +test('counts offline violations per severity', () => { + countValidationResults(span, result( + report('SomePlugin', 'failure', ['error', 'error', 'warning']), + report('Construct Annotations', 'success', ['warning', 'info']), + )); + + expect(counters).toEqual({ + 'offlineViolations:error': 2, + 'offlineViolations:warning': 2, + 'offlineViolations:info': 1, + 'onlineViolations': 0, + 'offlineWouldFailDeploy': 1, + }); +}); + +test('online violations are counted separately from offline severities', () => { + countValidationResults(span, result( + report('CloudFormation', 'failure', ['fatal', 'fatal']), + )); + + expect(counters).toEqual({ + onlineViolations: 2, + offlineWouldFailDeploy: 0, + }); +}); + +test('offlineWouldFailDeploy is 0 when offline reports succeed', () => { + countValidationResults(span, result( + report('SomePlugin', 'success', ['warning']), + )); + + expect(counters).toEqual({ + 'offlineViolations:warning': 1, + 'onlineViolations': 0, + 'offlineWouldFailDeploy': 0, + }); +}); + +test('no reports produce zero counters', () => { + countValidationResults(span, result()); + + expect(counters).toEqual({ + onlineViolations: 0, + offlineWouldFailDeploy: 0, + }); +}); diff --git a/packages/aws-cdk/lib/api-private.ts b/packages/aws-cdk/lib/api-private.ts index 2a2d8106d..942e56444 100644 --- a/packages/aws-cdk/lib/api-private.ts +++ b/packages/aws-cdk/lib/api-private.ts @@ -12,4 +12,5 @@ export * from '../../@aws-cdk/toolkit-lib/lib/api/tags/private'; export * from '../../@aws-cdk/toolkit-lib/lib/private/activity-printer'; export * from '../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/borrowed-assembly'; export * from '../../@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results'; +export * from '../../@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results'; export { throwIfValidationFailures } from '../../@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report'; diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 6030293d5..fb68c8070 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -14,7 +14,7 @@ import { CliIoHost } from './io-host'; import type { Configuration } from './user-configuration'; import { PROJECT_CONFIG } from './user-configuration'; import type { ActionLessRequest, IMessageSpan, IoHelper } from '../../lib/api-private'; -import { asIoHelper, cfnApi, createIgnoreMatcher, formatExpressStabilizationWarning, IO, tagsForStack, throwIfValidationFailures } from '../../lib/api-private'; +import { asIoHelper, cfnApi, countValidationResults, createIgnoreMatcher, formatExpressStabilizationWarning, IO, tagsForStack, throwIfValidationFailures } from '../../lib/api-private'; import type { AssetBuildNode, AssetPublishNode, Concurrency, MarkerNode, StackNode, WorkGraph, WorkGraphActions } from '../api'; import { CloudWatchLogEventMonitor, @@ -644,8 +644,29 @@ export class CdkToolkit { return this.validateWatch(validateOptions); } - const result = await this.toolkit.validate(this.props.cloudExecutable, validateOptions); - return result.conclusion === 'failure' ? 1 : 0; + // Synthesize before starting the VALIDATE span, so the span measures only + // the validation phase (offline report collection and, unless disabled, + // online CloudFormation validation). Synthesis is reported as its own + // SYNTH event; the assembly is cached, so the synthesis inside + // `toolkit.validate()` below is a cache hit. + await this.props.cloudExecutable.synthesize(); + + // The span is ended even if the engine crashes, so telemetry always + // records that a validation was started. + const validateSpan = await this.ioHost.asIoHelper().span(CLI_PRIVATE_SPAN.VALIDATE).begin({}); + let error: ErrorDetails | undefined; + try { + const result = await this.toolkit.validate(this.props.cloudExecutable, validateOptions); + countValidationResults(validateSpan, result); + return result.conclusion === 'failure' ? 1 : 0; + } catch (e: any) { + error = { + name: cdkCliErrorName(e), + }; + throw e; + } finally { + await validateSpan.end({ error }); + } } /** diff --git a/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts b/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts index 6f1f7b547..733dd4a26 100644 --- a/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts +++ b/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts @@ -1165,6 +1165,9 @@ function eventFromMessage(msg: IoMessage): TelemetryEvent | undefined { if (CLI_PRIVATE_IO.CDK_CLI_I3003.is(msg)) { return eventResult('ASSET', msg); } + if (CLI_PRIVATE_IO.CDK_CLI_I4001.is(msg)) { + return eventResult('VALIDATE', msg); + } // Hotswap lives in the cdk-toolkit so it cannot be a CDK_CLI error code. // Instead we reuse the existing Hotswap span. if (IO.CDK_TOOLKIT_I5410.is(msg)) { diff --git a/packages/aws-cdk/lib/cli/telemetry/messages.ts b/packages/aws-cdk/lib/cli/telemetry/messages.ts index 00f33b65a..625781df6 100644 --- a/packages/aws-cdk/lib/cli/telemetry/messages.ts +++ b/packages/aws-cdk/lib/cli/telemetry/messages.ts @@ -59,6 +59,16 @@ export const CLI_PRIVATE_IO = { description: 'Finished asset building and publishing', interface: 'EventResult', }), + CDK_CLI_I4000: make.trace({ + code: 'CDK_CLI_I4000', + description: 'Validation has started', + interface: 'EventStart', + }), + CDK_CLI_I4001: make.trace({ + code: 'CDK_CLI_I4001', + description: 'Validation has finished', + interface: 'EventResult', + }), }; /** @@ -85,4 +95,9 @@ export const CLI_PRIVATE_SPAN = { start: CLI_PRIVATE_IO.CDK_CLI_I3002, end: CLI_PRIVATE_IO.CDK_CLI_I3003, }, + VALIDATE: { + name: 'Validation', + start: CLI_PRIVATE_IO.CDK_CLI_I4000, + end: CLI_PRIVATE_IO.CDK_CLI_I4001, + }, } satisfies Record>; diff --git a/packages/aws-cdk/lib/cli/telemetry/schema.ts b/packages/aws-cdk/lib/cli/telemetry/schema.ts index c2affc98d..e60edac09 100644 --- a/packages/aws-cdk/lib/cli/telemetry/schema.ts +++ b/packages/aws-cdk/lib/cli/telemetry/schema.ts @@ -25,7 +25,7 @@ interface SessionEvent { readonly command: Command; } -export type EventType = 'SYNTH' | 'INVOKE' | 'DEPLOY' | 'HOTSWAP' | 'ASSET'; +export type EventType = 'SYNTH' | 'INVOKE' | 'DEPLOY' | 'HOTSWAP' | 'ASSET' | 'VALIDATE'; export type State = 'ABORTED' | 'FAILED' | 'SUCCEEDED'; interface Event extends SessionEvent { readonly state: State; diff --git a/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts b/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts index 1e6b95b86..8d82c3e90 100644 --- a/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts +++ b/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts @@ -781,6 +781,39 @@ describe('CliIoHost', () => { })); }); + test('emit telemetry on VALIDATE event', async () => { + // Create a message that should trigger telemetry using the actual message code + const message: IoMessage = { + time: new Date(), + level: 'trace', + action: 'validate', + code: 'CDK_CLI_I4001', + message: 'telemetry message', + data: { + duration: 123, + counters: { + 'offlineViolations:error': 2, + 'offlineWouldFailDeploy': 1, + 'onlineViolations': 0, + }, + }, + }; + + // Send the notification + await telemetryIoHost.notify(message); + + // Verify that the emit method was called with the correct parameters + expect(telemetryEmitSpy).toHaveBeenCalledWith(expect.objectContaining({ + eventType: 'VALIDATE', + duration: 123, + counters: { + 'offlineViolations:error': 2, + 'offlineWouldFailDeploy': 1, + 'onlineViolations': 0, + }, + })); + }); + test('do not emit telemetry on non telemetry codes', async () => { // Create a message that should trigger telemetry using the actual message code const message: IoMessage = { diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson index 9388eb1c0..89358b246 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson @@ -1,5 +1,7 @@ -{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} -{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":3,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} -{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} +{"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson index 9388eb1c0..89358b246 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson @@ -1,5 +1,7 @@ -{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} -{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":3,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} -{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} +{"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson new file mode 100644 index 000000000..89358b246 --- /dev/null +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson @@ -0,0 +1,7 @@ +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} +{"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_with_offline_violation_counters.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_with_offline_violation_counters.ndjson new file mode 100644 index 000000000..609d3ca6e --- /dev/null +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_with_offline_violation_counters.ndjson @@ -0,0 +1,7 @@ +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"error","code":"CDK_TOOLKIT_E9600","message":"ERROR S3 Buckets must not be publicly accessible (TestPlugin)\n Test-Stack-A-Display-Name/MyBucket/Resource (MyBucket)\n Acknowledge with 'TestPlugin::no-public-buckets'"} +{"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson new file mode 100644 index 000000000..d989c066c --- /dev/null +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson @@ -0,0 +1,6 @@ +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_failure_when_validation_report_has_violations.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_failure_when_validation_report_has_violations.ndjson index 17b16301d..609d3ca6e 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_failure_when_validation_report_has_violations.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_failure_when_validation_report_has_violations.ndjson @@ -1,5 +1,7 @@ {"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} {"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} -{"seq":3,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} -{"seq":4,"type":"notify","action":"validate","level":"error","code":"CDK_TOOLKIT_E9600","message":"ERROR S3 Buckets must not be publicly accessible (TestPlugin)\n Test-Stack-A-Display-Name/MyBucket/Resource (MyBucket)\n Acknowledge with 'TestPlugin::no-public-buckets'"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"error","code":"CDK_TOOLKIT_E9600","message":"ERROR S3 Buckets must not be publicly accessible (TestPlugin)\n Test-Stack-A-Display-Name/MyBucket/Resource (MyBucket)\n Acknowledge with 'TestPlugin::no-public-buckets'"} +{"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_multiple_violations_from_multiple_plugins.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_multiple_violations_from_multiple_plugins.ndjson index 0ad412ccb..b46112b2e 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_multiple_violations_from_multiple_plugins.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/with_violations_reports_multiple_violations_from_multiple_plugins.ndjson @@ -1,5 +1,7 @@ {"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} {"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} -{"seq":3,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} -{"seq":4,"type":"notify","action":"validate","level":"error","code":"CDK_TOOLKIT_E9600","message":"ERROR S3 Buckets must not be publicly accessible (SecurityPlugin)\n Test-Stack-A-Display-Name/MyBucket/Resource (MyBucket)\n Acknowledge with 'SecurityPlugin::no-public-buckets'\n\nWARNING All resources must have cost allocation tags (CostPlugin)\n Test-Stack-B/MyTopic/Resource (MyTopic)\n Acknowledge with 'CostPlugin::require-cost-tags'"} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":5,"type":"notify","action":"validate","level":"error","code":"CDK_TOOLKIT_E9600","message":"ERROR S3 Buckets must not be publicly accessible (SecurityPlugin)\n Test-Stack-A-Display-Name/MyBucket/Resource (MyBucket)\n Acknowledge with 'SecurityPlugin::no-public-buckets'\n\nWARNING All resources must have cost allocation tags (CostPlugin)\n Test-Stack-B/MyTopic/Resource (MyTopic)\n Acknowledge with 'CostPlugin::require-cost-tags'"} +{"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/validate.test.ts b/packages/aws-cdk/test/commands/validate.test.ts index 372a8f6d5..1a6f268f8 100644 --- a/packages/aws-cdk/test/commands/validate.test.ts +++ b/packages/aws-cdk/test/commands/validate.test.ts @@ -152,6 +152,94 @@ describe('with violations', () => { }); }); +describe('telemetry', () => { + // Remove the spies installed by these tests; the file-level `resetAllMocks` + // would otherwise strip the passthrough implementation from `ioHost.notify` + // and break tests that run later (test order is randomized). + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('emits a VALIDATE span end message with offline violation counters', async () => { + const assembly = await cloudExecutable.synthesize(); + await fs.writeJSON(path.join(assembly.directory, 'validation-report.json'), { + version: '1.0.0', + pluginReports: [{ + pluginName: 'TestPlugin', + conclusion: 'failure', + violations: [{ + ruleName: 'no-public-buckets', + description: 'S3 Buckets must not be publicly accessible', + severity: 'error', + violatingConstructs: [{ + constructPath: 'Test-Stack-A-Display-Name/MyBucket/Resource', + cloudFormationResource: { + templatePath: 'Test-Stack-A.template.json', + logicalId: 'MyBucket', + }, + }], + }], + }], + }); + + const notifySpy = jest.spyOn(ioHost, 'notify'); + await toolkit.validate({ + stacks: { patterns: [], strategy: StackSelectionStrategy.ALL_STACKS }, + online: false, + }); + + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + code: 'CDK_CLI_I4001', + data: expect.objectContaining({ + duration: expect.any(Number), + counters: { + 'offlineViolations:error': 1, + 'offlineWouldFailDeploy': 1, + 'onlineViolations': 0, + }, + }), + })); + }); + + test('emits a VALIDATE span end message even when no violations are found', async () => { + const notifySpy = jest.spyOn(ioHost, 'notify'); + await toolkit.validate({ + stacks: { patterns: [], strategy: StackSelectionStrategy.ALL_STACKS }, + online: false, + }); + + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + code: 'CDK_CLI_I4001', + data: expect.objectContaining({ + duration: expect.any(Number), + counters: { + onlineViolations: 0, + offlineWouldFailDeploy: 0, + }, + }), + })); + }); + + test('ends the VALIDATE span with the error name when the engine crashes', async () => { + // The CLI synthesizes (and caches) the assembly before the VALIDATE span + // begins, so failing `produce()` crashes the engine inside the span. + jest.spyOn(cloudExecutable, 'produce').mockRejectedValue(new Error('engine exploded')); + + const notifySpy = jest.spyOn(ioHost, 'notify'); + await expect(toolkit.validate({ + stacks: { patterns: [], strategy: StackSelectionStrategy.ALL_STACKS }, + online: false, + })).rejects.toThrow('engine exploded'); + + expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + code: 'CDK_CLI_I4001', + data: expect.objectContaining({ + error: { name: 'UnknownError' }, + }), + })); + }); +}); + describe('stack selection', () => { test('validates a single selected stack', async () => { const exitCode = await toolkit.validate({ From d35801762bf9af9825b22397e0e775987019e4fb Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:13:19 -0400 Subject: [PATCH 2/5] fix(toolkit-lib): classify online validation reports by source, not plugin name pluginName is a plugin-supplied string, so an offline policy plugin named 'CloudFormation' would have been counted as online telemetry. Track the online report explicitly on ValidateResult.onlineReports and split the counters by reference identity instead. --- .../toolkit-lib/lib/actions/validate/index.ts | 9 ++++ .../private/count-validation-results.ts | 10 +++-- .../lib/toolkit/private/validation-report.ts | 9 ---- .../toolkit-lib/lib/toolkit/toolkit.ts | 7 ++- .../toolkit/count-validation-results.test.ts | 43 +++++++++++++++---- 5 files changed, 56 insertions(+), 22 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts index dc7f88111..e0f388a8d 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts @@ -41,6 +41,15 @@ export interface ValidateResult { * Reports from each validation plugin */ readonly pluginReports: PluginReportJson[]; + + /** + * The subset of `pluginReports` produced by online (CloudFormation change + * set) validation, as opposed to offline sources: policy validation plugins + * and construct annotations, both read from the cloud assembly. + * + * @default - no online validation was performed + */ + readonly onlineReports?: PluginReportJson[]; } export type { PolicyValidationReportJson, PolicyValidationReportConclusion, PluginReportJson }; diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts index 92902f0de..3b88b9b5a 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts @@ -1,4 +1,3 @@ -import { ONLINE_VALIDATION_PLUGIN_NAME } from './validation-report'; import type { ValidateResult } from '../../actions/validate'; import type { IMessageSpan } from '../../api/io/private/span'; @@ -10,10 +9,15 @@ import type { IMessageSpan } from '../../api/io/private/span'; * 'failure' conclusion is the exact condition that makes deploy-like actions * throw (see `throwIfValidationFailures`), so `offlineWouldFailDeploy` records * that offline validation caught an error before a deployment attempt. + * + * Online reports are identified by reference via `onlineReports`, not by + * plugin name: `pluginName` is a plugin-supplied string, so an offline + * policy plugin may carry any name. */ export function countValidationResults(span: IMessageSpan, result: ValidateResult) { - const offline = result.pluginReports.filter((r) => r.pluginName !== ONLINE_VALIDATION_PLUGIN_NAME); - const online = result.pluginReports.filter((r) => r.pluginName === ONLINE_VALIDATION_PLUGIN_NAME); + const onlineSet = new Set(result.onlineReports ?? []); + const offline = result.pluginReports.filter((r) => !onlineSet.has(r)); + const online = result.pluginReports.filter((r) => onlineSet.has(r)); for (const report of offline) { for (const violation of report.violations) { diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts index f68ba4f20..a1a9eeb47 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts @@ -12,15 +12,6 @@ import type { MinimumSeverity } from '../types'; const VALIDATION_REPORT_FILE = 'validation-report.json'; -/** - * The plugin name under which online (CloudFormation change set) validation results are reported. - * - * All other plugin names in a unified validation report are offline sources: - * policy validation plugins and construct annotations, both read from the - * cloud assembly. - */ -export const ONLINE_VALIDATION_PLUGIN_NAME = 'CloudFormation'; - /** * The name of the plugin that emits construct annotations into the validation report. * diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 664e98a77..48dfcf9e8 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -116,7 +116,7 @@ import { formatErrorMessage, formatExpressStabilizationWarning, formatTime, obsc import { pLimit } from '../util/concurrency'; import { createIgnoreMatcher } from '../util/glob-matcher'; import { promiseWithResolvers } from '../util/promises'; -import { combineConclusions, obtainUnifiedValidationReport, ONLINE_VALIDATION_PLUGIN_NAME, throwIfValidationFailures } from './private/validation-report'; +import { combineConclusions, obtainUnifiedValidationReport, throwIfValidationFailures } from './private/validation-report'; export interface ToolkitOptions { /** @@ -692,6 +692,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { const stacks = await assembly.selectStacksV2(selectStacks); const reports = await obtainUnifiedValidationReport(assembly, stacks); + const onlineReports: PluginReportJson[] = []; // Online validation: submit templates to CloudFormation for early validation if (options.online ?? true) { @@ -700,6 +701,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { const onlineReport = await this.validateOnline(ioHelper, stacks, deployments); if (onlineReport) { reports.push(onlineReport); + onlineReports.push(onlineReport); } } @@ -709,6 +711,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { conclusion: combineConclusions(reports), title: undefined, pluginReports: reports, + onlineReports, }; if (!hasAnyViolations) { @@ -765,7 +768,7 @@ export class Toolkit extends CloudAssemblySourceBuilder { } return { - pluginName: ONLINE_VALIDATION_PLUGIN_NAME, + pluginName: 'CloudFormation', conclusion: 'failure', violations, }; diff --git a/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts index 86b17ff2e..fd3bfc3e7 100644 --- a/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts @@ -28,18 +28,20 @@ function report(pluginName: string, conclusion: 'success' | 'failure', severitie }; } -function result(...pluginReports: PluginReportJson[]): ValidateResult { +function result(offlineReports: PluginReportJson[], onlineReports: PluginReportJson[] = []): ValidateResult { + const pluginReports = [...offlineReports, ...onlineReports]; return { conclusion: pluginReports.some((r) => r.conclusion === 'failure') ? 'failure' : 'success', pluginReports, + onlineReports, }; } test('counts offline violations per severity', () => { - countValidationResults(span, result( + countValidationResults(span, result([ report('SomePlugin', 'failure', ['error', 'error', 'warning']), report('Construct Annotations', 'success', ['warning', 'info']), - )); + ])); expect(counters).toEqual({ 'offlineViolations:error': 2, @@ -51,9 +53,9 @@ test('counts offline violations per severity', () => { }); test('online violations are counted separately from offline severities', () => { - countValidationResults(span, result( + countValidationResults(span, result([], [ report('CloudFormation', 'failure', ['fatal', 'fatal']), - )); + ])); expect(counters).toEqual({ onlineViolations: 2, @@ -61,10 +63,35 @@ test('online violations are counted separately from offline severities', () => { }); }); +test('an offline plugin named CloudFormation is still counted as offline', () => { + countValidationResults(span, result([ + report('CloudFormation', 'failure', ['error']), + ])); + + expect(counters).toEqual({ + 'offlineViolations:error': 1, + 'onlineViolations': 0, + 'offlineWouldFailDeploy': 1, + }); +}); + +test('reports without onlineReports on the result are all counted as offline', () => { + countValidationResults(span, { + conclusion: 'failure', + pluginReports: [report('CloudFormation', 'failure', ['error'])], + }); + + expect(counters).toEqual({ + 'offlineViolations:error': 1, + 'onlineViolations': 0, + 'offlineWouldFailDeploy': 1, + }); +}); + test('offlineWouldFailDeploy is 0 when offline reports succeed', () => { - countValidationResults(span, result( + countValidationResults(span, result([ report('SomePlugin', 'success', ['warning']), - )); + ])); expect(counters).toEqual({ 'offlineViolations:warning': 1, @@ -74,7 +101,7 @@ test('offlineWouldFailDeploy is 0 when offline reports succeed', () => { }); test('no reports produce zero counters', () => { - countValidationResults(span, result()); + countValidationResults(span, result([])); expect(counters).toEqual({ onlineViolations: 0, From e938ff38898255dacb159c37f6ac07d85698f9f6 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:45:11 -0400 Subject: [PATCH 3/5] fix(cli): address review findings on validate telemetry - Remove the pre-synthesis before the VALIDATE span: it made the user-visible 'Synthesis time' message report the cache hit (~0s) instead of the real duration, and it moved app-crash-during-synth outside the span. Synthesis now runs inside the VALIDATE span; the SYNTH event (instrumented in CloudExecutable) stays accurate. - Extract wouldFailDeploy() from throwIfValidationFailures and use it for the offlineWouldFailDeploy counter, instead of re-deriving only the default failAt branch. - Only set ValidateResult.onlineReports when online validation ran, so undefined distinguishes 'skipped' from 'ran clean' as documented. - Deduplicate sum() into util/arrays instead of a third private copy. --- .../toolkit-lib/lib/actions/validate/index.ts | 5 ++- .../lib/api/work-graph/work-graph.ts | 10 +---- .../toolkit/private/count-assembly-results.ts | 5 +-- .../private/count-validation-results.ts | 20 +++++----- .../lib/toolkit/private/validation-report.ts | 40 ++++++++++++------- .../toolkit-lib/lib/toolkit/toolkit.ts | 8 ++-- .../@aws-cdk/toolkit-lib/lib/util/arrays.ts | 7 ++++ packages/aws-cdk/lib/cli/cdk-toolkit.ts | 13 +++--- ...ss_when_no_validation_report_exists.ndjson | 8 ++-- ...n_validates_a_single_selected_stack.ndjson | 8 ++-- ...e_even_when_no_violations_are_found.ndjson | 8 ++-- ..._error_name_when_the_engine_crashes.ndjson | 10 ++--- .../aws-cdk/test/commands/validate.test.ts | 4 +- 13 files changed, 73 insertions(+), 73 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts index e0f388a8d..7e34b01b9 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/validate/index.ts @@ -47,7 +47,10 @@ export interface ValidateResult { * set) validation, as opposed to offline sources: policy validation plugins * and construct annotations, both read from the cloud assembly. * - * @default - no online validation was performed + * Contains the same object references as `pluginReports`. An empty array + * means online validation ran and found no problems. + * + * @default - online validation was skipped */ readonly onlineReports?: PluginReportJson[]; } diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts b/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts index 93344d83d..f3d1d7cf8 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/work-graph/work-graph.ts @@ -1,7 +1,7 @@ import type { WorkNode, StackNode, AssetBuildNode, AssetPublishNode, MarkerNode } from './work-graph-types'; import { DeploymentState } from './work-graph-types'; import { ToolkitError } from '../../toolkit/toolkit-error'; -import { parallelPromises } from '../../util'; +import { parallelPromises, sum } from '../../util'; import type { IoHelper } from '../io/private'; export type Concurrency = number | Record; @@ -416,14 +416,6 @@ export interface WorkGraphActions { marker: (markerNode: MarkerNode) => Promise; } -function sum(xs: number[]) { - let ret = 0; - for (const x of xs) { - ret += x; - } - return ret; -} - function retainOnly(xs: A[], pred: (x: A) => boolean) { xs.splice(0, xs.length, ...xs.filter(pred)); } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts index 64d0fede5..197167245 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-assembly-results.ts @@ -1,6 +1,7 @@ import type * as cxapi from '@aws-cdk/cloud-assembly-api'; import { SynthesisMessageLevel } from '@aws-cdk/cloud-assembly-api'; import type { IMessageSpan } from '../../api/io/private/span'; +import { sum } from '../../util'; export function countAssemblyResults(span: IMessageSpan, assembly: cxapi.CloudAssembly) { const stacksRecursively = assembly.stacksRecursively; @@ -21,10 +22,6 @@ export function countAssemblyResults(span: IMessageSpan, assembly: cxapi.Cl } } -function sum(xs: number[]) { - return xs.reduce((a, b) => a + b, 0); -} - /** * Well-known and agreed-upon value between aws-cdk-lib and the toolkit * diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts index 3b88b9b5a..a05627b10 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts @@ -1,23 +1,25 @@ +import { wouldFailDeploy } from './validation-report'; import type { ValidateResult } from '../../actions/validate'; import type { IMessageSpan } from '../../api/io/private/span'; +import { sum } from '../../util'; /** * Add counters describing the outcome of a validate run to the given span * * Offline violations (policy plugin reports and construct annotations read - * from the cloud assembly) are counted per severity. An offline report with a - * 'failure' conclusion is the exact condition that makes deploy-like actions - * throw (see `throwIfValidationFailures`), so `offlineWouldFailDeploy` records - * that offline validation caught an error before a deployment attempt. + * from the cloud assembly) are counted per severity. `offlineWouldFailDeploy` + * records whether the offline reports fail `wouldFailDeploy` at the default + * 'error' threshold; a deploy run with `--strict` or `--ignore-errors` moves + * that threshold, so this counter approximates the default deploy behavior. * * Online reports are identified by reference via `onlineReports`, not by * plugin name: `pluginName` is a plugin-supplied string, so an offline * policy plugin may carry any name. */ export function countValidationResults(span: IMessageSpan, result: ValidateResult) { - const onlineSet = new Set(result.onlineReports ?? []); + const online = result.onlineReports ?? []; + const onlineSet = new Set(online); const offline = result.pluginReports.filter((r) => !onlineSet.has(r)); - const online = result.pluginReports.filter((r) => onlineSet.has(r)); for (const report of offline) { for (const violation of report.violations) { @@ -26,9 +28,5 @@ export function countValidationResults(span: IMessageSpan, result: Validate } span.incCounter('onlineViolations', sum(online.map((r) => r.violations.length))); - span.incCounter('offlineWouldFailDeploy', offline.some((r) => r.conclusion === 'failure') ? 1 : 0); -} - -function sum(xs: number[]) { - return xs.reduce((a, b) => a + b, 0); + span.incCounter('offlineWouldFailDeploy', wouldFailDeploy(offline, 'error') ? 1 : 0); } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts index a1a9eeb47..18ac9e3ae 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/private/validation-report.ts @@ -88,26 +88,36 @@ export async function throwIfValidationFailures( const result: ValidateResult = { conclusion, pluginReports }; await ioHelper.notify(hostMessageFromValidation(process.cwd(), result)); + 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; + } + + const error = AssemblyError.withStacks('Synthesis finished with errors', stacks.stackArtifacts); + error.attachSynthesisErrorCode('AnnotationErrors'); + throw error; +} + +/** + * Whether the given validation reports make a deploy-like action fail at the given severity threshold + * + * This is the exact predicate applied by `throwIfValidationFailures`. + */ +export function wouldFailDeploy(pluginReports: PluginReportJson[], failAt: MinimumSeverity): boolean { switch (failAt) { case 'error': - if (conclusion === 'failure') { - const error = AssemblyError.withStacks('Synthesis finished with errors', stacks.stackArtifacts); - error.attachSynthesisErrorCode('AnnotationErrors'); - throw error; - } - break; + return combineConclusions(pluginReports) === 'failure'; case 'warn': - // if we're failing at 'warn', then both warnings and errors cause failure, so the initial conclusion is correct - if (conclusion === 'failure' || hasWarnings(pluginReports)) { - const error = AssemblyError.withStacks('Synthesis finished with warnings (--strict mode)', stacks.stackArtifacts); - error.attachSynthesisErrorCode('StrictAnnotationWarnings'); - throw error; - } - - break; + // if we're failing at 'warn', then both warnings and errors cause failure + return combineConclusions(pluginReports) === 'failure' || hasWarnings(pluginReports); case 'none': // if we're not failing at all, then the conclusion is always success - break; + return false; } } diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts index 48dfcf9e8..1f27f9d6a 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts @@ -692,17 +692,15 @@ export class Toolkit extends CloudAssemblySourceBuilder { const stacks = await assembly.selectStacksV2(selectStacks); const reports = await obtainUnifiedValidationReport(assembly, stacks); - const onlineReports: PluginReportJson[] = []; // Online validation: submit templates to CloudFormation for early validation + let onlineReports: PluginReportJson[] | undefined; if (options.online ?? true) { const deployments = await this.deploymentsForAction('validate'); const onlineReport = await this.validateOnline(ioHelper, stacks, deployments); - if (onlineReport) { - reports.push(onlineReport); - onlineReports.push(onlineReport); - } + onlineReports = onlineReport ? [onlineReport] : []; + reports.push(...onlineReports); } const hasAnyViolations = reports.some(report => report.violations && report.violations.length > 0); diff --git a/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts b/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts index 268d1c09e..a7006503a 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/util/arrays.ts @@ -12,6 +12,13 @@ export function flatten(xs: T[][]): T[] { return Array.prototype.concat.apply([], xs); } +/** + * Sum a list of numbers + */ +export function sum(xs: number[]): number { + return xs.reduce((a, b) => a + b, 0); +} + /** * Partition a collection by removing and returning all elements that match a predicate * diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index fb68c8070..20072e9b2 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -644,14 +644,11 @@ export class CdkToolkit { return this.validateWatch(validateOptions); } - // Synthesize before starting the VALIDATE span, so the span measures only - // the validation phase (offline report collection and, unless disabled, - // online CloudFormation validation). Synthesis is reported as its own - // SYNTH event; the assembly is cached, so the synthesis inside - // `toolkit.validate()` below is a cache hit. - await this.props.cloudExecutable.synthesize(); - - // The span is ended even if the engine crashes, so telemetry always + // The VALIDATE span wraps the whole action, including the synthesis + // performed inside `toolkit.validate()`. Synthesis is also reported as + // its own SYNTH event (instrumented in CloudExecutable), so telemetry + // consumers can subtract it from the VALIDATE duration. The span is + // ended even if the app crashes during synthesis, so telemetry always // records that a validation was started. const validateSpan = await this.ioHost.asIoHelper().span(CLI_PRIVATE_SPAN.VALIDATE).begin({}); let error: ErrorDetails | undefined; diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson index 89358b246..d70f69152 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/no_violations_reports_success_when_no_validation_report_exists.ndjson @@ -1,7 +1,7 @@ -{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} -{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} -{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} {"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} {"seq":5,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} {"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson index 89358b246..d70f69152 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/stack_selection_validates_a_single_selected_stack.ndjson @@ -1,7 +1,7 @@ -{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} -{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} -{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} {"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} {"seq":5,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} {"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson index 89358b246..d70f69152 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_emits_a_VALIDATE_span_end_message_even_when_no_violations_are_found.ndjson @@ -1,7 +1,7 @@ -{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} -{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} -{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} {"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} {"seq":5,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I9600","message":"Validation did not find any problems."} {"seq":6,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson index d989c066c..cc81883b9 100644 --- a/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson +++ b/packages/aws-cdk/test/commands/__io_snapshots__/validate/telemetry_ends_the_VALIDATE_span_with_the_error_name_when_the_engine_crashes.ndjson @@ -1,6 +1,4 @@ -{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1000","message":"Starting Synthesis ..."} -{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I1001","message":"\n✨ Synthesis time: \n"} -{"seq":2,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} -{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} -{"seq":4,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} -{"seq":5,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} +{"seq":0,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4000","message":"Starting Validation ..."} +{"seq":1,"type":"notify","action":"validate","level":"trace","code":"CDK_TOOLKIT_I1001","message":"Starting Synthesis ..."} +{"seq":2,"type":"notify","action":"validate","level":"info","code":"CDK_TOOLKIT_I1000","message":"✨ Synthesis time: "} +{"seq":3,"type":"notify","action":"validate","level":"trace","code":"CDK_CLI_I4001","message":"\n✨ Validation time: \n"} diff --git a/packages/aws-cdk/test/commands/validate.test.ts b/packages/aws-cdk/test/commands/validate.test.ts index 1a6f268f8..51dad308f 100644 --- a/packages/aws-cdk/test/commands/validate.test.ts +++ b/packages/aws-cdk/test/commands/validate.test.ts @@ -221,8 +221,8 @@ describe('telemetry', () => { }); test('ends the VALIDATE span with the error name when the engine crashes', async () => { - // The CLI synthesizes (and caches) the assembly before the VALIDATE span - // begins, so failing `produce()` crashes the engine inside the span. + // Synthesis happens inside the VALIDATE span, so a CDK app that crashes + // during synth (modeled by a failing `produce()`) still ends the span. jest.spyOn(cloudExecutable, 'produce').mockRejectedValue(new Error('engine exploded')); const notifySpy = jest.spyOn(ioHost, 'notify'); From 7d1548d09cc6c011745f2502a601e8fc1d924474 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:15:52 -0400 Subject: [PATCH 4/5] test(cli-integ): cdk validate emits VALIDATE telemetry event Uses the validate-app fixture (SecurityPlugin + construct annotation) to assert the VALIDATE event and all four counter kinds in the telemetry file, and asserts the batch POST succeeds against the real endpoint, which doubles as an end-to-end check that the telemetry backend accepts the VALIDATE event type. --- .../cdk-validate-telemetry.integtest.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts diff --git a/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts new file mode 100644 index 000000000..150a38cf8 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts @@ -0,0 +1,50 @@ +import * as path from 'path'; +import * as fs from 'fs-extra'; +import { integTest, withSpecificFixture } from '../../lib'; + +integTest( + 'cdk validate emits VALIDATE telemetry event with violation counters', + withSpecificFixture('validate-app', async (fixture) => { + const telemetryFile = path.join(fixture.integTestDir, `telemetry-validate-${Date.now()}.json`); + + // --no-online keeps the run deterministic; onlineViolations is asserted to be 0. + const output = await fixture.cdk( + ['--unstable=validate', 'validate', fixture.fullStackName('validate'), '--no-online', `--telemetry-file=${telemetryFile}`], + { + verboseLevel: 3, // trace mode + allowErrExit: true, // violations make validate exit non-zero + }, + ); + + // The endpoint sink POSTs the whole event batch to the real telemetry + // endpoint, which validates it against a request schema. This passes only + // once the backend accepts the VALIDATE event type. + expect(output).toContain('Telemetry Sent Successfully'); + + const json = fs.readJSONSync(telemetryFile); + expect(json).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + event: expect.objectContaining({ + state: 'SUCCEEDED', + eventType: 'VALIDATE', + }), + // The app's single S3 bucket makes SecurityPlugin report one violation + // each of fatal/error/warning/cost-optimization severity, plus one + // construct annotation warning. The plugin failure is what would have + // failed a deploy. + counters: expect.objectContaining({ + 'offlineViolations:fatal': 1, + 'offlineViolations:error': 1, + 'offlineViolations:warning': 2, + 'offlineViolations:cost-optimization': 1, + 'onlineViolations': 0, + 'offlineWouldFailDeploy': 1, + }), + }), + ]), + ); + + fs.unlinkSync(telemetryFile); + }), +); From ac3270b37826b15aaf9e27e2719a4e329748dfae Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:57:33 -0400 Subject: [PATCH 5/5] test(cli-integ): don't pin the library-version-dependent severity counter key Newer aws-cdk-lib normalizes the fixture plugin's non-standard 'cost-optimization' severity to 'custom', so assert the stable counter keys plus the total offline violation count instead of that key. --- .../cdk-validate-telemetry.integtest.ts | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts index 150a38cf8..6f1e07277 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts @@ -22,29 +22,32 @@ integTest( expect(output).toContain('Telemetry Sent Successfully'); const json = fs.readJSONSync(telemetryFile); - expect(json).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - event: expect.objectContaining({ - state: 'SUCCEEDED', - eventType: 'VALIDATE', - }), - // The app's single S3 bucket makes SecurityPlugin report one violation - // each of fatal/error/warning/cost-optimization severity, plus one - // construct annotation warning. The plugin failure is what would have - // failed a deploy. - counters: expect.objectContaining({ - 'offlineViolations:fatal': 1, - 'offlineViolations:error': 1, - 'offlineViolations:warning': 2, - 'offlineViolations:cost-optimization': 1, - 'onlineViolations': 0, - 'offlineWouldFailDeploy': 1, - }), - }), - ]), + const validateEvent = json.find((e: any) => e.event?.eventType === 'VALIDATE'); + expect(validateEvent).toBeDefined(); + expect(validateEvent.event.state).toEqual('SUCCEEDED'); + + // The app's single S3 bucket makes SecurityPlugin report one violation each + // of fatal/error/warning/cost-optimization severity, plus one construct + // annotation warning. The plugin failure is what would have failed a deploy. + expect(validateEvent.counters).toEqual( + expect.objectContaining({ + 'offlineViolations:fatal': 1, + 'offlineViolations:error': 1, + 'offlineViolations:warning': 2, + 'onlineViolations': 0, + 'offlineWouldFailDeploy': 1, + }), ); + // The plugin's non-standard 'cost-optimization' severity is reported under + // a library-version-dependent key ('offlineViolations:cost-optimization' + // on older aws-cdk-lib, 'offlineViolations:custom' on newer), so assert + // the total offline violation count instead of that key. + const totalOfflineViolations = Object.entries(validateEvent.counters) + .filter(([key]) => key.startsWith('offlineViolations:')) + .reduce((acc, [, value]) => acc + Number(value), 0); + expect(totalOfflineViolations).toEqual(5); + fs.unlinkSync(telemetryFile); }), );