-
Notifications
You must be signed in to change notification settings - Fork 4k
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
AutoScalingGroup.scaleOnMetric with MathExpression as metric created invalid template #5776
Labels
@aws-cdk/aws-autoscaling
Related to Amazon EC2 Auto Scaling
bug
This issue is a bug.
in-progress
This issue is being actively worked on.
p1
Comments
leocurrie
added
bug
This issue is a bug.
needs-triage
This issue or PR still needs to be triaged.
labels
Jan 13, 2020
rix0rrr
pushed a commit
that referenced
this issue
Jan 14, 2020
It was not possible to use `MathExpression` objects in AutoScaling, because the `Alarm` interface has a bug: it takes "override" arguments like "period" and "statistic" that could and should have already been encoded into the `Metric` object passed to the alarm. Subsequently, AutoScaling used this ill-advised feature to force the period of metrics to 1 minute. This caused an invalid render for math expressions and was probably ill-advised to begin with. We should be respecting the customer's period on the metric they pass in. Fixes #5776. BREAKING CHANGE: AutoScaling by using `scaleOnMetric` will no longer force the alarm period to 1 minute, but use the period from the Metric object instead (5 minutes by default). Use `metric.with({ period: Duration.minute(1) })` to create a high-frequency scaling policy.
rix0rrr
pushed a commit
that referenced
this issue
Jan 14, 2020
It was not possible to use `MathExpression` objects in AutoScaling, because the `Alarm` interface has a bug: it takes "override" arguments like "period" and "statistic" that could and should have already been encoded into the `Metric` object passed to the alarm. Subsequently, AutoScaling used this ill-advised feature to force the period of metrics to 1 minute. This caused an invalid render for math expressions and was probably ill-advised to begin with. We should be respecting the customer's period on the metric they pass in. Fixes #5776. BREAKING CHANGE: AutoScaling by using `scaleOnMetric` will no longer force the alarm period to 1 minute, but use the period from the Metric object instead (5 minutes by default). Use `metric.with({ period: Duration.minute(1) })` to create a high-frequency scaling policy.
HI @leocurrie, thanks for reporting this issue. We are actively working on this. |
SomayaB
added
in-progress
This issue is being actively worked on.
and removed
needs-triage
This issue or PR still needs to be triaged.
labels
Jan 15, 2020
mergify bot
pushed a commit
that referenced
this issue
Jan 16, 2020
It was not possible to use `MathExpression` objects in AutoScaling, because the `Alarm` interface has a bug: it takes "override" arguments like "period" and "statistic" that could and should have already been encoded into the `Metric` object passed to the alarm. Subsequently, AutoScaling used this ill-advised feature to force the period of metrics to 1 minute. This caused an invalid render for math expressions and was probably ill-advised to begin with. We should be respecting the customer's period on the metric they pass in. Fixes #5776. BREAKING CHANGE: AutoScaling by using `scaleOnMetric` will no longer force the alarm period to 1 minute, but use the period from the Metric object instead (5 minutes by default). Use `metric.with({ period: Duration.minute(1) })` to create a high-frequency scaling policy.
Just to clarify, is this now usable in CFN for target tracking, or only for step scaling? From the doc, seems to suggest no. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
@aws-cdk/aws-autoscaling
Related to Amazon EC2 Auto Scaling
bug
This issue is a bug.
in-progress
This issue is being actively worked on.
p1
When attempting to create an Autoscaling Group that uses a Math Expression as the scaling metric, CDK will generate a template that is rejected by CloudFormation.
Reproduction Steps
Starting from the Application Load Balancer Typescript example (https://github.com/aws-samples/aws-cdk-examples/blob/master/typescript/application-load-balancer/index.ts), add the following:
(I'm not suggesting that this is a useful metric! It's just an example)
run cdk stack synth to generate the stack.
Note in the generated output, the generated
ASGMyScalingMetricUpperAlarm
ASGMyScalingMetricLowerAlarm
both include thePeriod
property, which according to the docs, should not be supplied for Math expression metrics: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cw-alarm.html#cfn-cloudwatch-alarms-periodWhen deploying the stack, the upper and lower alarms will fail to be created with an error.
Error Log
--
Environment
Other
Period
property from the alarms resolves the problem.This is 🐛 Bug Report
The text was updated successfully, but these errors were encountered: