-
Notifications
You must be signed in to change notification settings - Fork 119
feat(cli): emit telemetry for the validate action #1864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
iankhou
wants to merge
6
commits into
main
Choose a base branch
from
telemetry-validate-event
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
05059e6
feat(cli): emit telemetry for the validate action
iankhou d358017
fix(toolkit-lib): classify online validation reports by source, not p…
iankhou e938ff3
fix(cli): address review findings on validate telemetry
iankhou 9c82088
Merge branch 'main' into telemetry-validate-event
iankhou 7d1548d
test(cli-integ): cdk validate emits VALIDATE telemetry event
iankhou ac3270b
test(cli-integ): don't pin the library-version-dependent severity cou…
iankhou File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
53 changes: 53 additions & 0 deletions
53
...aws-cdk-testing/cli-integ/tests/telemetry-integ-tests/cdk-validate-telemetry.integtest.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| 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); | ||
| 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); | ||
| }), | ||
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 32 additions & 0 deletions
32
packages/@aws-cdk/toolkit-lib/lib/toolkit/private/count-validation-results.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| 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. `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<any>, result: ValidateResult) { | ||
| const online = result.onlineReports ?? []; | ||
| const onlineSet = new Set(online); | ||
| const offline = result.pluginReports.filter((r) => !onlineSet.has(r)); | ||
|
|
||
| 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', wouldFailDeploy(offline, 'error') ? 1 : 0); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
110 changes: 110 additions & 0 deletions
110
packages/@aws-cdk/toolkit-lib/test/toolkit/count-validation-results.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| 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<any>; | ||
| let counters: Record<string, number>; | ||
|
|
||
| beforeEach(() => { | ||
| counters = {}; | ||
| span = { | ||
| incCounter: (name: string, delta: number = 1) => { | ||
| counters[name] = (counters[name] ?? 0) + delta; | ||
| }, | ||
| } as IMessageSpan<any>; | ||
| }); | ||
|
|
||
| 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(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([ | ||
| 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('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([ | ||
| 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, | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.