cdklabs / cdk-monitoring-constructs

Easy-to-use CDK constructs for monitoring your AWS infrastructure
https://constructs.dev/packages/cdk-monitoring-constructs
Apache License 2.0
444 stars 56 forks source link

Incorrect ThresholdMetricId value when creating anomaly detection with math expression #425

Closed Laxenade closed 9 months ago

Laxenade commented 9 months ago

Version

5.4.2

Steps and/or minimal code example to reproduce

I'm trying to create an anomaly detection on a math expression, removing several layers of abstraction. I have a CustomMetricWithAnomalyDetection as shown below:

{
  alarmFriendlyName: 'Friendly Name',
  metric: new MathExpression(
    {
      expression: 'IF(m1 > 0, m2 * 100 / m1, 0)',
      usingMetrics: {
        m1: {
          metricName: 'Metric1',
          statistic: MetricStatistic.AVERAGE,
          metricPeriod: Duration.minutes(1),
          dimensionMap: {
            Key: 'Value'
          }
        },
        m2: {
          metricName: 'Metric2',
          statistic: MetricStatistic.AVERAGE,
          metricPeriod: Duration.minutes(1),
          dimensionMap: {
            Key: 'Value'
          }
        },
      }
      label: 'Label',
      period: Duration.minutes(1)
    }
  ),
  anomalyDetectionStandardDeviationToRender: 1,
  addAlarmOnAnomaly: {
    AnomalyDetected: {
      standardDeviationForAlarm: 1,
      alarmWhenAboveTheBand: true,
      alarmWhenBelowTheBand: true,
      ...otherProps
    }
  }
}

I could have missed a few keys here and there but the general idea is that I have a math expression for which I'd like to enable anomaly detection. However, during deployment, CloudFormation gives me an error, indicating:

Resource handler returned message: "Threshold metric expression must use ANOMALY_DETECTION_BAND function 

Looking at the generated template, I found that the ThresholdMetricId seems to be set incorrectly:

"Properties": {
   "Metrics": [
      {
         "Expression": "ANOMALY_DETECTION_BAND(alarm_39eb6b24e4ab,3)",
         "Id": "expr_1",
         "Label": "Band (stdev 3)",
         "ReturnData": true
      },
      {
         "Expression": "IF(m1 > 0, m2 * 100 / m1, 0)",
         "Id": "alarm_39eb6b24e4ab",
         "Label": "Label",
         "ReturnData": true
      },
      {
         "Id": "m1",
         ...more props omitted
      },
      {
         "Id": "m2",
         ...more props omitted
      }
   ],
   "ThresholdMetricId": "alarm_39eb6b24e4ab",
   "TreatMissingData": "breaching"
}

The correct ThresholdMetricId should be expr_1, but it is currently set to alarm_39eb6b24e4ab.

Expected behavior

ThresholdMetricId should be expr_1 since that's the expression that has the anomaly detection defined.

Actual behavior

ThresholdMetricId is alarm_39eb6b24e4ab instead.

Other details

Looking at the code https://github.com/cdklabs/cdk-monitoring-constructs/blob/465e388bf1418160143959ec18e2781f97fcae72/lib/common/metric/AnomalyDetectionMathExpression.ts#L37-L44

I think the problem is that it is assuming the last metric is always the anomaly detection expression which is clearly not true in this case.