Repository: cdklabs/cdk-monitoring-constructs (language: python)

Issue:
"Threshold metric expression must use ANOMALY_DETECTION_BAND function" when creating anomaly detection alarm on math expression
### Version

v3.0.0

### Steps and/or minimal code example to reproduce

1. Invoke `.monitorCustom()` function to monitor a custom metric.
2. Pass a math expression as a metric in metricGroup.
3. Set addAlarmOnAnomaly props for it.
4. Deploy the monitoring stack.
5. Failed to deploy with following error:
    > Threshold metric expression must use ANOMALY_DETECTION_BAND function.

### Expected behavior

Deploy successfully.

### Actual behavior

Failed to deploy.

### Other details

_No response_


Discussion / hints:
I ran into the same error on v5.0.0.
The root cause seems to be because of this code line: https://github.com/cdklabs/cdk-monitoring-constructs/blob/0d79e94c89dba85e5532a355717c199000505fec/lib/common/metric/AnomalyDetectionMathExpression.ts#L42.

All math expresion will have `ReturnData` to be `true`, while the CDK only expect one of them to be `true` and the others should be `false` per this doc: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudwatch-alarm-metricdataquery.html#cfn-cloudwatch-alarm-metricdataquery-returndata.
I just logged a similar bug https://github.com/cdklabs/cdk-monitoring-constructs/issues/425. Should have checked issues before cutting one but anyway.

I don't think `ReturnData` is the problem here, I was able to create an anomaly detection on a normal metric, despite `ReturnData` is `true` on both the expression and the metric, CFN and CW didn't complain. Though, I agree that when there are multiple expressions, only one should be marked as true.

```json
"Properties": {
   "Metrics": [
      {
         "Expression": "ANOMALY_DETECTION_BAND(alarm_a20137860f588,3)",
         "Id": "expr_1",
         "Label": "Band (stdev 3)",
         "ReturnData": true
      },
      {
         "Id": "alarm_a20137860f588",
         "ReturnData": true,
          ...Other props omitted
      }
   ],
   "ThresholdMetricId": "expr_1",
   "TreatMissingData": "breaching"
}
```



Fix patch:
diff --git a/lib/common/metric/AnomalyDetectionMathExpression.ts b/lib/common/metric/AnomalyDetectionMathExpression.ts
index a3b94feb..4e6f3eca 100644
--- a/lib/common/metric/AnomalyDetectionMathExpression.ts
+++ b/lib/common/metric/AnomalyDetectionMathExpression.ts
@@ -31,15 +31,29 @@ export class AnomalyDetectionMathExpression extends MathExpression {
   createAlarm(scope: Construct, id: string, props: CreateAlarmOptions): Alarm {
     const alarm = super.createAlarm(scope, id, props);
 
+    // `usingMetrics` of an anomaly detection alarm can only ever have one entry.
+    // Should the entry be a math expression, the math expression can have its own `usingMetrics`.
+    const finalExpressionId = Object.keys(this.usingMetrics)[0];
+
     // https://github.com/aws/aws-cdk/issues/10540#issuecomment-725222564
     const cfnAlarm = alarm.node.defaultChild as CfnAlarm;
     cfnAlarm.addPropertyDeletionOverride("Threshold");
     (cfnAlarm.metrics as CfnAlarm.MetricDataQueryProperty[]).forEach(
       (metric, index) => {
-        if (metric.expression) {
+        // To create an anomaly detection alarm, returned data should be set to true on two MetricDataQueryProperty(s):
+        // 1. The metric or math expression that is being evaluated for anomaly detection (eg. expr_1)
+        // 2. The actual expression of anomaly detection (eg. ANOMALY_DETECTION_BAND(expr_1, 1))
+        let returnData = false;
+
+        if (metric.expression?.includes("ANOMALY_DETECTION_BAND")) {
+          // thresholdMetricId is the ID of the ANOMALY_DETECTION_BAND function used as the threshold for the alarm.
           cfnAlarm.thresholdMetricId = metric.id;
+          returnData = true;
+        } else if (metric.id === finalExpressionId) {
+          returnData = true;
         }
-        cfnAlarm.addPropertyOverride(`Metrics.${index}.ReturnData`, true);
+
+        cfnAlarm.addPropertyOverride(`Metrics.${index}.ReturnData`, returnData);
       }
     );
 


Test patch:
diff --git a/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap b/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap
index 307b7f7a..975ebb96 100644
--- a/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap
+++ b/test/monitoring/custom/__snapshots__/CustomMonitoring.test.ts.snap
@@ -422,7 +422,7 @@ Object {
               "Period": 300,
               "Stat": "Average",
             },
-            "ReturnData": true,
+            "ReturnData": false,
           },
           Object {
             "Id": "m2",
@@ -434,10 +434,10 @@ Object {
               "Period": 300,
               "Stat": "Average",
             },
-            "ReturnData": true,
+            "ReturnData": false,
           },
         ],
-        "ThresholdMetricId": "alarm_39cd98a4dd0f0",
+        "ThresholdMetricId": "expr_1",
         "TreatMissingData": "missing",
       },
       "Type": "AWS::CloudWatch::Alarm",