aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

CDK:Alarm constructor statistic prop doesn't use Cfn extendedStatistic for IQM #28812

Open victor-xplore opened 9 months ago

victor-xplore commented 9 months ago

Describe the bug

When creating an alarm with IQM, eg

  new Alarm(..., {
    ...,
    metric: new Metric({
      ...,
      statistic: Stats.IQM,
    }),
  })

Expected Behavior

"ExtendedStatistic": "IQM",

Current Behavior

"Statistic": "IQM",

which is not valid

Reproduction Steps

import { Stack, type StackProps } from "aws-cdk-lib";
import { Alarm, ComparisonOperator, Metric, Stats } from "aws-cdk-lib/aws-cloudwatch";
import { Construct } from "constructs";

export class TestStack extends Stack {
  constructor(scope: Construct, props?: StackProps) {
    super(scope, "test-stack", props);

    new Alarm(this, "MockAlarm", {
      evaluationPeriods: 1,
      threshold: 0,
      comparisonOperator: ComparisonOperator.GREATER_THAN_THRESHOLD,
      metric: new Metric({
        namespace: "MOCK",
        metricName: "TEST",
        statistic: Stats.IQM,
      }),
    });
  }
}

Possible Solution

No response

Additional Information/Context

This used to work on cdk 2.45.0

CDK CLI Version

2.122.0 (build 7e77e02)

Framework Version

No response

Node.js Version

v20.9.0

OS

Ubuntu 23.10

Language

TypeScript

Language Version

5.3.3

Other information

No response

pahud commented 9 months ago

Thanks for the report. Do you have any document or official sample for IQM with ExtendedStatistic? This is probably something we need fix but we have to make sure it's relevant in the document.

victor-xplore commented 9 months ago

@pahud The cloud formation docs do not seem to specify a list of options.

The cli docs do have this list:

If you specify ExtendedStatistic , the following are valid values:

  • p90
  • tm90
  • tc90
  • ts90
  • wm90
  • IQM
  • PR(n :m ) where n and m are values of the metric
  • TC(X %:X %) where X is between 10 and 90 inclusive.
  • TM(X %:X %) where X is between 10 and 90 inclusive.
  • TS(X %:X %) where X is between 10 and 90 inclusive.
  • WM(X %:X %) where X is between 10 and 90 inclusive.

I'm not sure what you are looking for exactly.

It was in ExtendedStatistic before I updated this old stack used which used cdk 2.45.0, after the update to 2.122.0 cdk put it on Statistic instead which fails in cloud formation:

Resource handler returned message: "1 validation error detected: Value 'IQM' at 'statistic' failed to satisfy constraint: Member must satisfy enum value set: [Maximum, SampleCount, Sum, Minimum, Average] (Service: CloudWatch, Status Code: 400, Request ID: 7eeec91f-609a-4279-a1a3-9143513d4a5f)" (RequestToken: 21022ab1-d7dc-e51b-3c41-29b9bad78738, HandlerErrorCode: InvalidRequest)

I tried some older cdk versions but they also behaved the same as 2.122.0, I think the oldest I checked was 2.90.0

If you are worried that cloud-formation/cloud-watch has stopped supporting IQM, then I guess cdk should either remove the IQM value in stats, or produce ExtendedStatistic: TM(25%:75%) when used.