From 46114bb1f4702019a8873b9162d0a9f10763bc61 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 12 Mar 2021 12:27:49 +0100 Subject: [PATCH] fix(autoscaling): AutoScaling on percentile metrics doesn't work (#13366) AutoScaling on percentile metrics did not work because the `MetricAggregationType` was trying to be derived from the metric, and it can only be MIN, MAX or AVG. Figure out what the metric aggregation type does, default it to AVERAGE if no other suitable value can be determined, and also make it and the evaluation periods configurable while we're at it. Fixes #13144. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/step-scaling-policy.ts | 29 +++++-- .../test/test.step-scaling-policy.ts | 77 ++++++++++++++++++- .../lib/step-scaling-policy.ts | 29 +++++-- .../aws-autoscaling/test/scaling.test.ts | 67 +++++++++++++++- 4 files changed, 190 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-policy.ts b/packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-policy.ts index 417ecf34f1970..8b9f7b2644267 100644 --- a/packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-policy.ts +++ b/packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-policy.ts @@ -51,6 +51,25 @@ export interface BasicStepScalingPolicyProps { * @default No minimum scaling effect */ readonly minAdjustmentMagnitude?: number; + + /** + * How many evaluation periods of the metric to wait before triggering a scaling action + * + * Raising this value can be used to smooth out the metric, at the expense + * of slower response times. + * + * @default 1 + */ + readonly evaluationPeriods?: number; + + /** + * Aggregation to apply to all data points over the evaluation periods + * + * Only has meaning if `evaluationPeriods != 1`. + * + * @default - The statistic from the metric if applicable (MIN, MAX, AVERAGE), otherwise AVERAGE. + */ + readonly metricAggregationType?: MetricAggregationType; } export interface StepScalingPolicyProps extends BasicStepScalingPolicyProps { @@ -92,7 +111,7 @@ export class StepScalingPolicy extends CoreConstruct { this.lowerAction = new StepScalingAction(this, 'LowerPolicy', { adjustmentType, cooldown: props.cooldown, - metricAggregationType: aggregationTypeFromMetric(props.metric), + metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric), minAdjustmentMagnitude: props.minAdjustmentMagnitude, scalingTarget: props.scalingTarget, }); @@ -110,7 +129,7 @@ export class StepScalingPolicy extends CoreConstruct { metric: props.metric, alarmDescription: 'Lower threshold scaling alarm', comparisonOperator: cloudwatch.ComparisonOperator.LESS_THAN_OR_EQUAL_TO_THRESHOLD, - evaluationPeriods: 1, + evaluationPeriods: props.evaluationPeriods ?? 1, threshold, }); this.lowerAlarm.addAlarmAction(new StepScalingAlarmAction(this.lowerAction)); @@ -122,7 +141,7 @@ export class StepScalingPolicy extends CoreConstruct { this.upperAction = new StepScalingAction(this, 'UpperPolicy', { adjustmentType, cooldown: props.cooldown, - metricAggregationType: aggregationTypeFromMetric(props.metric), + metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric), minAdjustmentMagnitude: props.minAdjustmentMagnitude, scalingTarget: props.scalingTarget, }); @@ -140,7 +159,7 @@ export class StepScalingPolicy extends CoreConstruct { metric: props.metric, alarmDescription: 'Upper threshold scaling alarm', comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, - evaluationPeriods: 1, + evaluationPeriods: props.evaluationPeriods ?? 1, threshold, }); this.upperAlarm.addAlarmAction(new StepScalingAlarmAction(this.upperAction)); @@ -197,7 +216,7 @@ function aggregationTypeFromMetric(metric: cloudwatch.IMetric): MetricAggregatio case 'Maximum': return MetricAggregationType.MAXIMUM; default: - throw new Error(`Cannot only scale on 'Minimum', 'Maximum', 'Average' metrics, got ${statistic}`); + return MetricAggregationType.AVERAGE; } } diff --git a/packages/@aws-cdk/aws-applicationautoscaling/test/test.step-scaling-policy.ts b/packages/@aws-cdk/aws-applicationautoscaling/test/test.step-scaling-policy.ts index 4474cc6a46f58..fbcf70eb49f75 100644 --- a/packages/@aws-cdk/aws-applicationautoscaling/test/test.step-scaling-policy.ts +++ b/packages/@aws-cdk/aws-applicationautoscaling/test/test.step-scaling-policy.ts @@ -1,4 +1,4 @@ -import { expect, haveResource, SynthUtils } from '@aws-cdk/assert'; +import { expect, haveResource, haveResourceLike, SynthUtils } from '@aws-cdk/assert'; import * as cloudwatch from '@aws-cdk/aws-cloudwatch'; import * as cdk from '@aws-cdk/core'; import * as fc from 'fast-check'; @@ -152,6 +152,81 @@ export = { test.done(); }, + + 'step scaling from percentile metric'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const target = createScalableTarget(stack); + + // WHEN + target.scaleOnMetric('Tracking', { + metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }), + scalingSteps: [ + { upper: 0, change: -1 }, + { lower: 100, change: +1 }, + { lower: 500, change: +5 }, + ], + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::ApplicationAutoScaling::ScalingPolicy', { + PolicyType: 'StepScaling', + StepScalingPolicyConfiguration: { + AdjustmentType: 'ChangeInCapacity', + MetricAggregationType: 'Average', + }, + })); + expect(stack).to(haveResource('AWS::CloudWatch::Alarm', { + ComparisonOperator: 'GreaterThanOrEqualToThreshold', + EvaluationPeriods: 1, + AlarmActions: [ + { Ref: 'TargetTrackingUpperPolicy72CEFA77' }, + ], + ExtendedStatistic: 'p99', + MetricName: 'Metric', + Namespace: 'Test', + Threshold: 100, + })); + + test.done(); + }, + + 'step scaling with evaluation period configured'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const target = createScalableTarget(stack); + + // WHEN + target.scaleOnMetric('Tracking', { + metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }), + scalingSteps: [ + { upper: 0, change: -1 }, + { lower: 100, change: +1 }, + { lower: 500, change: +5 }, + ], + evaluationPeriods: 10, + metricAggregationType: appscaling.MetricAggregationType.MAXIMUM, + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::ApplicationAutoScaling::ScalingPolicy', { + PolicyType: 'StepScaling', + StepScalingPolicyConfiguration: { + AdjustmentType: 'ChangeInCapacity', + MetricAggregationType: 'Maximum', + }, + })); + expect(stack).to(haveResource('AWS::CloudWatch::Alarm', { + ComparisonOperator: 'GreaterThanOrEqualToThreshold', + EvaluationPeriods: 10, + ExtendedStatistic: 'p99', + MetricName: 'Metric', + Namespace: 'Test', + Threshold: 100, + })); + + test.done(); + }, }; /** diff --git a/packages/@aws-cdk/aws-autoscaling/lib/step-scaling-policy.ts b/packages/@aws-cdk/aws-autoscaling/lib/step-scaling-policy.ts index c3b51a892c222..307eaf525ae55 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/step-scaling-policy.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/step-scaling-policy.ts @@ -52,6 +52,25 @@ export interface BasicStepScalingPolicyProps { * @default No minimum scaling effect */ readonly minAdjustmentMagnitude?: number; + + /** + * How many evaluation periods of the metric to wait before triggering a scaling action + * + * Raising this value can be used to smooth out the metric, at the expense + * of slower response times. + * + * @default 1 + */ + readonly evaluationPeriods?: number; + + /** + * Aggregation to apply to all data points over the evaluation periods + * + * Only has meaning if `evaluationPeriods != 1`. + * + * @default - The statistic from the metric if applicable (MIN, MAX, AVERAGE), otherwise AVERAGE. + */ + readonly metricAggregationType?: MetricAggregationType; } export interface StepScalingPolicyProps extends BasicStepScalingPolicyProps { @@ -93,7 +112,7 @@ export class StepScalingPolicy extends CoreConstruct { this.lowerAction = new StepScalingAction(this, 'LowerPolicy', { adjustmentType: props.adjustmentType, cooldown: props.cooldown, - metricAggregationType: aggregationTypeFromMetric(props.metric), + metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric), minAdjustmentMagnitude: props.minAdjustmentMagnitude, autoScalingGroup: props.autoScalingGroup, }); @@ -111,7 +130,7 @@ export class StepScalingPolicy extends CoreConstruct { metric: props.metric, alarmDescription: 'Lower threshold scaling alarm', comparisonOperator: cloudwatch.ComparisonOperator.LESS_THAN_OR_EQUAL_TO_THRESHOLD, - evaluationPeriods: 1, + evaluationPeriods: props.evaluationPeriods ?? 1, threshold, }); this.lowerAlarm.addAlarmAction(new StepScalingAlarmAction(this.lowerAction)); @@ -123,7 +142,7 @@ export class StepScalingPolicy extends CoreConstruct { this.upperAction = new StepScalingAction(this, 'UpperPolicy', { adjustmentType: props.adjustmentType, cooldown: props.cooldown, - metricAggregationType: aggregationTypeFromMetric(props.metric), + metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric), minAdjustmentMagnitude: props.minAdjustmentMagnitude, autoScalingGroup: props.autoScalingGroup, }); @@ -141,7 +160,7 @@ export class StepScalingPolicy extends CoreConstruct { metric: props.metric, alarmDescription: 'Upper threshold scaling alarm', comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, - evaluationPeriods: 1, + evaluationPeriods: props.evaluationPeriods ?? 1, threshold, }); this.upperAlarm.addAlarmAction(new StepScalingAlarmAction(this.upperAction)); @@ -161,7 +180,7 @@ function aggregationTypeFromMetric(metric: cloudwatch.IMetric): MetricAggregatio case 'Maximum': return MetricAggregationType.MAXIMUM; default: - throw new Error(`Cannot only scale on 'Minimum', 'Maximum', 'Average' metrics, got ${statistic}`); + return MetricAggregationType.AVERAGE; } } diff --git a/packages/@aws-cdk/aws-autoscaling/test/scaling.test.ts b/packages/@aws-cdk/aws-autoscaling/test/scaling.test.ts index 5207897cd1786..5c1f8947b09bd 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/scaling.test.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/scaling.test.ts @@ -1,4 +1,4 @@ -import { expect, haveResource } from '@aws-cdk/assert'; +import { expect, haveResource, haveResourceLike } from '@aws-cdk/assert'; import * as cloudwatch from '@aws-cdk/aws-cloudwatch'; import * as ec2 from '@aws-cdk/aws-ec2'; import * as elbv2 from '@aws-cdk/aws-elasticloadbalancingv2'; @@ -277,6 +277,71 @@ nodeunitShim({ }, }); +test('step scaling from percentile metric', () => { + // GIVEN + const stack = new cdk.Stack(); + const fixture = new ASGFixture(stack, 'Fixture'); + + // WHEN + fixture.asg.scaleOnMetric('Tracking', { + metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }), + scalingSteps: [ + { upper: 0, change: -1 }, + { lower: 100, change: +1 }, + { lower: 500, change: +5 }, + ], + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::AutoScaling::ScalingPolicy', { + PolicyType: 'StepScaling', + MetricAggregationType: 'Average', + })); + expect(stack).to(haveResource('AWS::CloudWatch::Alarm', { + ComparisonOperator: 'GreaterThanOrEqualToThreshold', + EvaluationPeriods: 1, + AlarmActions: [ + { Ref: 'FixtureASGTrackingUpperPolicy27D4301F' }, + ], + ExtendedStatistic: 'p99', + MetricName: 'Metric', + Namespace: 'Test', + Threshold: 100, + })); +}); + +test('step scaling with evaluation period configured', () => { + // GIVEN + const stack = new cdk.Stack(); + const fixture = new ASGFixture(stack, 'Fixture'); + + // WHEN + fixture.asg.scaleOnMetric('Tracking', { + metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }), + scalingSteps: [ + { upper: 0, change: -1 }, + { lower: 100, change: +1 }, + { lower: 500, change: +5 }, + ], + evaluationPeriods: 10, + metricAggregationType: autoscaling.MetricAggregationType.MAXIMUM, + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::AutoScaling::ScalingPolicy', { + PolicyType: 'StepScaling', + MetricAggregationType: 'Maximum', + })); + expect(stack).to(haveResource('AWS::CloudWatch::Alarm', { + ComparisonOperator: 'GreaterThanOrEqualToThreshold', + EvaluationPeriods: 10, + ExtendedStatistic: 'p99', + MetricName: 'Metric', + Namespace: 'Test', + Threshold: 100, + })); +}); + class ASGFixture extends Construct { public readonly vpc: ec2.Vpc; public readonly asg: autoscaling.AutoScalingGroup;