fix(lambda): wrong dimension for metrics on lambda versions #20836
Conversation
|
|
varju
force-pushed
the
fix-14248-version-metric
branch
from
4bffea7
to
e4c292c
Compare
varju
force-pushed
the
fix-14248-version-metric
branch
from
e4c292c
to
7e7be76
Compare
|
@Mergifyio update |
✅ Branch has been successfully updated |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| @@ -231,12 +231,12 @@ export class Version extends QualifiedFunctionBase implements IVersion { | |||
| public metric(metricName: string, props: cloudwatch.MetricOptions = {}): cloudwatch.Metric { | |||
| // Metrics on Aliases need the "bare" function name, and the alias' ARN, this differs from the base behavior. | |||
| return super.metric(metricName, { | |||
| dimensions: { | |||
| dimensionsMap: { | |||
There was a problem hiding this comment.
I'm not sure I understand why the name of this was changed.
There was a problem hiding this comment.
I changed this to fix a deprecation warning, because MetricProps now prefers dimensionMap instead of the older parameter.
There was a problem hiding this comment.
Ah, good catch.
TheRealAmazonKendra
changed the title
There was a problem hiding this comment.
OK, so I think that the code in this change is good, however, I have a couple of concerns here:
- This change should have broken an integration test somewhere, which means we're either missing integration test coverage here or that the output isn't correct. I suspect the former. Please update/add an integration test to make sure this functionality is covered.
- Our changelog is automatically generated from the PR title. Can you please make the title more specific to what is wrong so that it's more helpful to those reading the changelog?
Thanks for fixing this bug, we appreciate your work on this!
| @@ -231,12 +231,12 @@ export class Version extends QualifiedFunctionBase implements IVersion { | |||
| public metric(metricName: string, props: cloudwatch.MetricOptions = {}): cloudwatch.Metric { | |||
| // Metrics on Aliases need the "bare" function name, and the alias' ARN, this differs from the base behavior. | |||
There was a problem hiding this comment.
This comment line is now incorrect with this change. Please make sure it gets updated.
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
This is a partial fix for #14248, addressing the incorrect dimension used when building metrics for Lambda Version resources.
This does not address the second issue in that ticket, where the Lambda version is always being used, even when working with aliases. Presumably fixing that would reintroduce the circular dependency issue with CodeDeploy that's mentioned in the comments.
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license