Repository navigation
Conversation
github-actions
Bot
added
beginning-contributor
GitHub Actions
Waiting
aws-cdk-automation
left a comment
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.
|
Maintainer assistance requested: I cannot run the real AWS integration deployment because no AWS test environment is available to me. As permitted by the contribution guide, could a maintainer please run the integration test and generate/commit the deployment-backed snapshots?
Local validation is recorded in the PR description. The real AWS test has not been run and its deployment-backed snapshot is missing; I have not used dry-run or manually fabricated snapshots. I am marking this PR ready for code review with that limitation explicit. This is a request to complete the required validation, not an integration-test exemption or a claim that the PR is ready to merge. |
|
Automated review A maintainer will still review this — treat the notes below as a starting point. 📌 Commit This PR fixes a long-standing bug in the Lambda 🔴 1 blocking · 🟡 0 recommended · ⚪ 0 optional Files with findings (1)
Generated automatically. React 👍 or 👎 to tell us whether this review helped, so we can improve these reviews. |
![maintainer-for-aws[bot]](https://avatars.githubusercontent.com/b/1290?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the review summary comment for the overview; the notes below are inline.
| import { App, Stack } from 'aws-cdk-lib'; | ||
| import * as lambda from 'aws-cdk-lib/aws-lambda'; | ||
| import { ExpectedResult, IntegTest } from '@aws-cdk/integ-tests-alpha'; | ||
|
|
||
| const app = new App({ context: { '@aws-cdk/aws-lambda:versionMetrics': true } }); | ||
| const stack = new Stack(app, 'VersionMetricDimensions'); | ||
| const fn = new lambda.Function(stack, 'Function', { | ||
| runtime: lambda.Runtime.NODEJS_LATEST, | ||
| handler: 'index.handler', | ||
| code: lambda.Code.fromInline('exports.handler = async () => {};'), | ||
| }); | ||
| const version = fn.currentVersion; | ||
| const alarm = version.metricErrors().createAlarm(stack, 'Alarm', { | ||
| threshold: 1, | ||
| evaluationPeriods: 1, | ||
| }); | ||
| const test = new IntegTest(app, 'VersionMetricDimensionsTest', { testCases: [stack] }); | ||
| test.assertions.awsApiCall('CloudWatch', 'describeAlarms', { | ||
| AlarmNames: [alarm.alarmName], | ||
| }).expect(ExpectedResult.objectLike({ | ||
| MetricAlarms: [{ | ||
| Dimensions: [ | ||
| { Name: 'FunctionName', Value: fn.functionName }, | ||
| { Name: 'Resource', Value: `${fn.functionName}:${version.version}` }, | ||
| ], | ||
| }], | ||
| })); |
There was a problem hiding this comment.
🔴 Blocking — This new integration test ships without its companion integ.version-metrics.js.snapshot/ directory. The snapshot is the synthesized Cloud Assembly that the test suite diffs against; without it the integ test has nothing to compare to and cannot run, so the committed test does not prove the template deploys or that the alarm's FunctionName/Resource dimensions are actually published as expected (INTEGRATION_TESTS.md#what-are-cdk-integration-tests). The PR body notes the author could not deploy and asks a maintainer to run it, confirming this gap.
Suggested change: Run the integ test against a real AWS account (yarn integ --update-on-failed integ.version-metrics.js) and commit the generated integ.version-metrics.js.snapshot/ directory as part of this PR.
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
2 similar comments
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
This branch is waiting to be deployed
2 participants
Issue # (if applicable)
Closes #14248.
Description of changes
With @aws-cdk/aws-lambda:versionMetrics enabled, Version metric helpers use FunctionName=function-name and Resource=function-name:version. The flag preserves the current unconfigured behavior: the inherited dimensionsMap takes precedence over the old dimensions object, so the actual baseline is a qualified FunctionName dimension without Resource. With the flag enabled, explicit dimensionsMap/dimensions overrides are respected, with dimensionsMap taking precedence.
Addresses the Resource-name bug identified by the maintainer in #14248. ExecutedVersion selection behind an alias remains a separate feature, as requested in that issue; callers can explicitly supply those dimensions. Adds flag matrix, override precedence, integration source and migration documentation.
Validation: independent branch Lambda version and feature-flag tests passed (148 tests). The generated feature-flag report and recommended defaults contain only this new flag. Changed library/integration files passed ESLint; integration source passed strict TypeScript consumer compilation against emitted declarations. The five batch changes passed one combined JSII compilation (0 errors, 0 warnings). No new TypeScript README snippet was introduced.
Deployment validation: AWS credentials are unavailable in this environment. Integration source is included; no dry-run snapshot was committed.
Maintainer assistance requested: real AWS integration validation
This PR is ready for code review, but deployment validation is incomplete. I do not have an AWS account/environment available to run the real integration deployment. Local validation is documented above; it does not replace AWS deployment validation.
Following the CDK contribution guidance for contributors who cannot run integration tests, could a maintainer please run the integration test below in an AWS test environment and generate/commit the deployment-backed snapshots?
packages/@aws-cdk-testing/framework-integ/test/aws-lambda/test/integ.version-metrics.tsNo real AWS deployment has been performed, and no dry-run or manually fabricated snapshot has been committed. The required deployment-backed snapshot is still missing, so the integration snapshot checks cannot yet pass. This request is for help completing the validation, not for waiving it; the PR should not be merged until the required validation and review are complete.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license