fix(autoscaling): targetRequestsPerSecond is actually requests per …

…minute (#11457) Scaling to a request rate per instance was advertised to scale to a rate/second, but the underlying API was actually expecting a rate/minute. Upscale the value of `targetRequestsPerSecond` by a factor of 60 to make its name actually correct, deprecate it, and add a `targetRequestsPerMinute` property which should have been the original API. Fixes #11446. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Nov 13, 2020 · 39e277f · 39e277f
1 parent c71a4e9
commit 39e277f
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 4 deletions.
diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
@@ -10,6 +10,7 @@ import {
   Aws,
   CfnAutoScalingRollingUpdate, CfnCreationPolicy, CfnUpdatePolicy,
   Duration, Fn, IResource, Lazy, PhysicalName, Resource, Stack, Tags,
+  Token,
   Tokenization, withResolved,
 } from '@aws-cdk/core';
 import { Construct } from 'constructs';
@@ -765,10 +766,24 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
 
     const resourceLabel = `${this.albTargetGroup.firstLoadBalancerFullName}/${this.albTargetGroup.targetGroupFullName}`;
 
+    if ((props.targetRequestsPerMinute === undefined) === (props.targetRequestsPerSecond === undefined)) {
+      throw new Error('Specify exactly one of \'targetRequestsPerMinute\' or \'targetRequestsPerSecond\'');
+    }
+
+    let rpm: number;
+    if (props.targetRequestsPerSecond !== undefined) {
+      if (Token.isUnresolved(props.targetRequestsPerSecond)) {
+        throw new Error('\'targetRequestsPerSecond\' cannot be an unresolved value; use \'targetRequestsPerMinute\' instead.');
+      }
+      rpm = props.targetRequestsPerSecond * 60;
+    } else {
+      rpm = props.targetRequestsPerMinute!;
+    }
+
     const policy = new TargetTrackingScalingPolicy(this, `ScalingPolicy${id}`, {
       autoScalingGroup: this,
       predefinedMetric: PredefinedMetric.ALB_REQUEST_COUNT_PER_TARGET,
-      targetValue: props.targetRequestsPerSecond,
+      targetValue: rpm,
       resourceLabel,
       ...props,
     });
@@ -1603,8 +1618,17 @@ export interface NetworkUtilizationScalingProps extends BaseTargetTrackingProps
 export interface RequestCountScalingProps extends BaseTargetTrackingProps {
   /**
    * Target average requests/seconds on each instance
+   *
+   * @deprecated Use 'targetRequestsPerMinute' instead
+   * @default - Specify exactly one of 'targetRequestsPerSecond' and 'targetRequestsPerSecond'
+   */
+  readonly targetRequestsPerSecond?: number;
+
+  /**
+   * Target average requests/minute on each instance
+   * @default - Specify exactly one of 'targetRequestsPerSecond' and 'targetRequestsPerSecond'
    */
-  readonly targetRequestsPerSecond: number;
+  readonly targetRequestsPerMinute?: number;
 }
 
 /**

diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json
@@ -568,7 +568,7 @@
               ]
             }
           },
-          "TargetValue": 1
+          "TargetValue": 60
         }
       },
       "DependsOn": [

diff --git a/packages/@aws-cdk/aws-autoscaling/test/scaling.test.ts b/packages/@aws-cdk/aws-autoscaling/test/scaling.test.ts
@@ -75,7 +75,7 @@ nodeunitShim({
       test.done();
     },
 
-    'request count'(test: Test) {
+    'request count per second'(test: Test) {
       // GIVEN
       const stack = new cdk.Stack();
       const fixture = new ASGFixture(stack, 'Fixture');
@@ -99,6 +99,54 @@ nodeunitShim({
         ],
       };
 
+      expect(stack).to(haveResource('AWS::AutoScaling::ScalingPolicy', {
+        PolicyType: 'TargetTrackingScaling',
+        TargetTrackingConfiguration: {
+          TargetValue: 600,
+          PredefinedMetricSpecification: {
+            PredefinedMetricType: 'ALBRequestCountPerTarget',
+            ResourceLabel: {
+              'Fn::Join': ['', [
+                { 'Fn::Select': [1, arnParts] },
+                '/',
+                { 'Fn::Select': [2, arnParts] },
+                '/',
+                { 'Fn::Select': [3, arnParts] },
+                '/',
+                { 'Fn::GetAtt': ['ALBListenerTargetsGroup01D7716A', 'TargetGroupFullName'] },
+              ]],
+            },
+          },
+        },
+      }));
+
+      test.done();
+    },
+
+    'request count per minute'(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+      const fixture = new ASGFixture(stack, 'Fixture');
+      const alb = new elbv2.ApplicationLoadBalancer(stack, 'ALB', { vpc: fixture.vpc });
+      const listener = alb.addListener('Listener', { port: 80 });
+      listener.addTargets('Targets', {
+        port: 80,
+        targets: [fixture.asg],
+      });
+
+      // WHEN
+      fixture.asg.scaleOnRequestCount('ScaleRequest', {
+        targetRequestsPerMinute: 10,
+      });
+
+      // THEN
+      const arnParts = {
+        'Fn::Split': [
+          '/',
+          { Ref: 'ALBListener3B99FF85' },
+        ],
+      };
+
       expect(stack).to(haveResource('AWS::AutoScaling::ScalingPolicy', {
         PolicyType: 'TargetTrackingScaling',
         TargetTrackingConfiguration: {