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

Issue:
Allows users specify minimal sample size below which the alarm will be suppressed and considered unreliable
### Feature scope

Alarming

### Describe your suggested feature

The need for this feature is most visible when alarming on the AVERAGE, P90, P99... of some metric, which might contain sparse datapoints and hence decrease the reliability of the aggregated value.

Example:
You have an alarm on the P99 Latency metric. But during nights the TPS is going very low, hence you don't get enough records to reliably understand if there is an issue increasing latency or it's because of regular cache misses, retries, etc... If during 1 minute you got only a few calls to the service, the P99 latency won't be reliable reprepsentation of the health of the system. Hence we don't want to alarm on it.

With the current setup of the monitoring constructs, this is not easily possible. We have to create the composite alarms manually, add the actions, deal with the sample size metrics, etc...

By adding this to the library, we'll allow users to create alarms that are closer to their needs.

Definition of done:
- By adding a field to the alarms definition, the users can define minimal sample size after which they want the alarm to be enabled
- The change should be backwards compatible


Discussion / hints:
We were talking about adding a parameter to addAlarm properties, e.g. `addMinSampleCountCondition: number`; in order to enable more conditions in the future, we might also consider adding something like `addAlarmConditions: {minSampleCount: X}}`, allowing us to add e.g. composing with other alarms and so on

Fix patch:
diff --git a/API.md b/API.md
index ec526d23..5c8c1673 100644
--- a/API.md
+++ b/API.md
@@ -1983,6 +1983,7 @@ const addAlarmProps: AddAlarmProps = { ... }
 | <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.evaluateLowSampleCountPercentile">evaluateLowSampleCountPercentile</a></code> | <code>boolean</code> | Used only for alarms based on percentiles. |
 | <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.evaluationPeriods">evaluationPeriods</a></code> | <code>number</code> | Number of periods to consider when checking the number of breaching datapoints. |
 | <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.fillAlarmRange">fillAlarmRange</a></code> | <code>boolean</code> | Indicates whether the alarming range of values should be highlighted in the widget. |
+| <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.minMetricSamplesToAlarm">minMetricSamplesToAlarm</a></code> | <code>number</code> | Specifies how many samples (N) of the metric is needed to trigger the alarm. |
 | <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.overrideAnnotationColor">overrideAnnotationColor</a></code> | <code>string</code> | If specified, it modifies the final alarm annotation color. |
 | <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.overrideAnnotationLabel">overrideAnnotationLabel</a></code> | <code>string</code> | If specified, it modifies the final alarm annotation label. |
 | <code><a href="#cdk-monitoring-constructs.AddAlarmProps.property.overrideAnnotationVisibility">overrideAnnotationVisibility</a></code> | <code>boolean</code> | If specified, it modifies the final alarm annotation visibility. |
@@ -2243,6 +2244,26 @@ Indicates whether the alarming range of values should be highlighted in the widg
 
 ---
 
+##### `minMetricSamplesToAlarm`<sup>Optional</sup> <a name="minMetricSamplesToAlarm" id="cdk-monitoring-constructs.AddAlarmProps.property.minMetricSamplesToAlarm"></a>
+
+```typescript
+public readonly minMetricSamplesToAlarm: number;
+```
+
+- *Type:* number
+- *Default:* default behaviour - no condition on sample count will be added to the alarm
+
+Specifies how many samples (N) of the metric is needed to trigger the alarm.
+
+If this property is specified, an artificial composite alarm is created of the following:
+<ul>
+<li>The original alarm, created without this property being used; this alarm will have no actions set.</li>
+<li>A secondary alarm, which will monitor the same metric with the N (SampleCount) statistic, checking the sample count.</li>
+</ul>
+The newly created composite alarm will be returned as a result, and it will take the original alarm actions.
+
+---
+
 ##### `overrideAnnotationColor`<sup>Optional</sup> <a name="overrideAnnotationColor" id="cdk-monitoring-constructs.AddAlarmProps.property.overrideAnnotationColor"></a>
 
 ```typescript
@@ -2655,6 +2676,7 @@ const alarmAnnotationStrategyProps: AlarmAnnotationStrategyProps = { ... }
 | <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.fillAlarmRange">fillAlarmRange</a></code> | <code>boolean</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.metric">metric</a></code> | <code>aws-cdk-lib.aws_cloudwatch.Metric \| aws-cdk-lib.aws_cloudwatch.MathExpression</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.threshold">threshold</a></code> | <code>number</code> | *No description.* |
+| <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.minMetricSamplesToAlarm">minMetricSamplesToAlarm</a></code> | <code>number</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.overrideAnnotationColor">overrideAnnotationColor</a></code> | <code>string</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.overrideAnnotationLabel">overrideAnnotationLabel</a></code> | <code>string</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.overrideAnnotationVisibility">overrideAnnotationVisibility</a></code> | <code>boolean</code> | *No description.* |
@@ -2781,6 +2803,16 @@ public readonly threshold: number;
 
 ---
 
+##### `minMetricSamplesToAlarm`<sup>Optional</sup> <a name="minMetricSamplesToAlarm" id="cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.minMetricSamplesToAlarm"></a>
+
+```typescript
+public readonly minMetricSamplesToAlarm: number;
+```
+
+- *Type:* number
+
+---
+
 ##### `overrideAnnotationColor`<sup>Optional</sup> <a name="overrideAnnotationColor" id="cdk-monitoring-constructs.AlarmAnnotationStrategyProps.property.overrideAnnotationColor"></a>
 
 ```typescript
@@ -3383,7 +3415,7 @@ const alarmWithAnnotation: AlarmWithAnnotation = { ... }
 | <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.customTags">customTags</a></code> | <code>string[]</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.dedupeString">dedupeString</a></code> | <code>string</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.disambiguator">disambiguator</a></code> | <code>string</code> | *No description.* |
-| <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.alarm">alarm</a></code> | <code>aws-cdk-lib.aws_cloudwatch.Alarm</code> | *No description.* |
+| <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.alarm">alarm</a></code> | <code>aws-cdk-lib.aws_cloudwatch.AlarmBase</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.alarmDescription">alarmDescription</a></code> | <code>string</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.alarmLabel">alarmLabel</a></code> | <code>string</code> | *No description.* |
 | <code><a href="#cdk-monitoring-constructs.AlarmWithAnnotation.property.alarmName">alarmName</a></code> | <code>string</code> | *No description.* |
@@ -3448,10 +3480,10 @@ public readonly disambiguator: string;
 ##### `alarm`<sup>Required</sup> <a name="alarm" id="cdk-monitoring-constructs.AlarmWithAnnotation.property.alarm"></a>
 
 ```typescript
-public readonly alarm: Alarm;
+public readonly alarm: AlarmBase;
 ```
 
-- *Type:* aws-cdk-lib.aws_cloudwatch.Alarm
+- *Type:* aws-cdk-lib.aws_cloudwatch.AlarmBase
 
 ---
 
diff --git a/lib/common/alarm/AlarmFactory.ts b/lib/common/alarm/AlarmFactory.ts
index b2243902..40c62d3c 100644
--- a/lib/common/alarm/AlarmFactory.ts
+++ b/lib/common/alarm/AlarmFactory.ts
@@ -1,6 +1,6 @@
 import { Duration } from "aws-cdk-lib";
 import {
-  Alarm,
+  AlarmBase,
   AlarmRule,
   AlarmState,
   ComparisonOperator,
@@ -11,7 +11,11 @@ import {
 } from "aws-cdk-lib/aws-cloudwatch";
 import { Construct } from "constructs";
 
-import { MetricFactoryDefaults, MetricWithAlarmSupport } from "../metric";
+import {
+  MetricFactoryDefaults,
+  MetricStatistic,
+  MetricWithAlarmSupport,
+} from "../metric";
 import { removeBracketsWithDynamicLabels } from "../strings";
 import { AlarmNamingStrategy } from "./AlarmNamingStrategy";
 import { IAlarmActionStrategy } from "./IAlarmActionStrategy";
@@ -45,7 +49,7 @@ export interface AlarmMetadata {
  * Representation of an alarm with additional information.
  */
 export interface AlarmWithAnnotation extends AlarmMetadata {
-  readonly alarm: Alarm;
+  readonly alarm: AlarmBase;
   readonly alarmName: string;
   readonly alarmNameSuffix: string;
   readonly alarmLabel: string;
@@ -178,6 +182,18 @@ export interface AddAlarmProps {
    */
   readonly evaluateLowSampleCountPercentile?: boolean;
 
+  /**
+   * Specifies how many samples (N) of the metric is needed to trigger the alarm.
+   * If this property is specified, an artificial composite alarm is created of the following:
+   * <ul>
+   * <li>The original alarm, created without this property being used; this alarm will have no actions set.</li>
+   * <li>A secondary alarm, which will monitor the same metric with the N (SampleCount) statistic, checking the sample count.</li>
+   * </ul>
+   * The newly created composite alarm will be returned as a result, and it will take the original alarm actions.
+   * @default - default behaviour - no condition on sample count will be added to the alarm
+   */
+  readonly minMetricSamplesToAlarm?: number;
+
   /**
    * This allows user to attach custom values to this alarm, which can later be accessed from the "useCreatedAlarms" method.
    *
@@ -451,6 +467,8 @@ export class AlarmFactory {
     metric: MetricWithAlarmSupport,
     props: AddAlarmProps
   ): AlarmWithAnnotation {
+    // prepare the metric
+
     let adjustedMetric = metric;
     if (props.period) {
       // Adjust metric period for the alarm
@@ -462,6 +480,9 @@ export class AlarmFactory {
         label: removeBracketsWithDynamicLabels(adjustedMetric.label),
       });
     }
+
+    // prepare primary alarm properties
+
     const actionsEnabled = this.determineActionsEnabled(
       props.actionsEnabled,
       props.disambiguator
@@ -496,20 +517,64 @@ export class AlarmFactory {
       );
     }
 
-    const alarm = adjustedMetric.createAlarm(this.alarmScope, alarmName, {
+    // create primary alarm
+
+    const primaryAlarm = adjustedMetric.createAlarm(
+      this.alarmScope,
       alarmName,
-      alarmDescription,
-      threshold: props.threshold,
-      comparisonOperator: props.comparisonOperator,
-      treatMissingData: props.treatMissingData,
-      // default value (undefined) means "evaluate"
-      evaluateLowSampleCountPercentile: evaluateLowSampleCountPercentile
-        ? undefined
-        : "ignore",
-      datapointsToAlarm,
-      evaluationPeriods,
-      actionsEnabled,
-    });
+      {
+        alarmName,
+        alarmDescription,
+        threshold: props.threshold,
+        comparisonOperator: props.comparisonOperator,
+        treatMissingData: props.treatMissingData,
+        // default value (undefined) means "evaluate"
+        evaluateLowSampleCountPercentile: evaluateLowSampleCountPercentile
+          ? undefined
+          : "ignore",
+        datapointsToAlarm,
+        evaluationPeriods,
+        actionsEnabled,
+      }
+    );
+
+    let alarm: AlarmBase = primaryAlarm;
+
+    // create composite alarm for min metric samples (if defined)
+
+    if (props.minMetricSamplesToAlarm) {
+      const metricSampleCount = adjustedMetric.with({
+        statistic: MetricStatistic.N,
+      });
+      const noSamplesAlarm = metricSampleCount.createAlarm(
+        this.alarmScope,
+        `${alarmName}-NoSamples`,
+        {
+          alarmName: `${alarmName}-NoSamples`,
+          alarmDescription: `The metric (${adjustedMetric}) does not have enough samples to alarm. Must have at least ${props.minMetricSamplesToAlarm}.`,
+          threshold: props.minMetricSamplesToAlarm,
+          comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
+          treatMissingData: TreatMissingData.BREACHING,
+          datapointsToAlarm: 1,
+          evaluationPeriods: 1,
+          actionsEnabled,
+        }
+      );
+      alarm = new CompositeAlarm(this.alarmScope, `${alarmName}-WithSamples`, {
+        actionsEnabled,
+        compositeAlarmName: `${alarmName}-WithSamples`,
+        alarmDescription: this.joinDescriptionParts(
+          alarmDescription,
+          `Min number of samples to alarm: ${props.minMetricSamplesToAlarm}`
+        ),
+        alarmRule: AlarmRule.allOf(
+          AlarmRule.fromAlarm(primaryAlarm, AlarmState.ALARM),
+          AlarmRule.not(AlarmRule.fromAlarm(noSamplesAlarm, AlarmState.ALARM))
+        ),
+      });
+    }
+
+    // attach alarm actions
 
     action.addAlarmActions({
       alarm,
@@ -520,13 +585,16 @@ export class AlarmFactory {
       customParams: props.customParams ?? {},
     });
 
+    // create annotation for the primary alarm
+
     const annotation = this.createAnnotation({
-      alarm,
+      alarm: primaryAlarm,
       action,
       metric: adjustedMetric,
       evaluationPeriods,
       datapointsToAlarm,
       dedupeString,
+      minMetricSamplesToAlarm: props.minMetricSamplesToAlarm,
       fillAlarmRange: props.fillAlarmRange ?? false,
       overrideAnnotationColor: props.overrideAnnotationColor,
       overrideAnnotationLabel: props.overrideAnnotationLabel,
@@ -538,6 +606,8 @@ export class AlarmFactory {
       customParams: props.customParams ?? {},
     });
 
+    // return the final result
+
     return {
       alarm,
       action,
@@ -611,7 +681,7 @@ export class AlarmFactory {
       case CompositeAlarmOperator.OR:
         return AlarmRule.anyOf(...alarmRules);
       default:
-        throw new Error("Unsupported composite alarm operator: " + operator);
+        throw new Error(`Unsupported composite alarm operator: ${operator}`);
     }
   }
 
@@ -663,6 +733,10 @@ export class AlarmFactory {
       parts.push(`Documentation: ${documentationLink}`);
     }
 
+    return this.joinDescriptionParts(...parts);
+  }
+
+  protected joinDescriptionParts(...parts: string[]) {
     return parts.join(" \r\n");
   }
 
diff --git a/lib/common/alarm/IAlarmAnnotationStrategy.ts b/lib/common/alarm/IAlarmAnnotationStrategy.ts
index f7c96886..d99acce7 100644
--- a/lib/common/alarm/IAlarmAnnotationStrategy.ts
+++ b/lib/common/alarm/IAlarmAnnotationStrategy.ts
@@ -12,6 +12,7 @@ export interface AlarmAnnotationStrategyProps extends AlarmMetadata {
   readonly alarm: Alarm;
   readonly metric: MetricWithAlarmSupport;
   readonly comparisonOperator: ComparisonOperator;
+  readonly minMetricSamplesToAlarm?: number;
   readonly threshold: number;
   readonly datapointsToAlarm: number;
   readonly evaluationPeriods: number;


Test patch:
diff --git a/test/common/alarm/AlarmFactory.test.ts b/test/common/alarm/AlarmFactory.test.ts
index 8df6d0e3..3a5605bd 100644
--- a/test/common/alarm/AlarmFactory.test.ts
+++ b/test/common/alarm/AlarmFactory.test.ts
@@ -1,6 +1,7 @@
 import { Duration, Stack } from "aws-cdk-lib";
-import { Template } from "aws-cdk-lib/assertions";
+import { Capture, Template } from "aws-cdk-lib/assertions";
 import {
+  Alarm,
   ComparisonOperator,
   Metric,
   Shading,
@@ -193,9 +194,9 @@ test("addAlarm: period override is propagated to alarm metric", () => {
     alarmNameSuffix: "TwoHoursPeriod",
     period: Duration.hours(2),
   });
-  const alarm1hConfig = alarm1h.alarm.metric.toMetricConfig();
+  const alarm1hConfig = (alarm1h.alarm as Alarm).metric.toMetricConfig();
   expect(alarm1hConfig.metricStat?.period).toStrictEqual(Duration.hours(1));
-  const alarm2hConfig = alarm2h.alarm.metric.toMetricConfig();
+  const alarm2hConfig = (alarm2h.alarm as Alarm).metric.toMetricConfig();
   expect(alarm2hConfig.metricStat?.period).toStrictEqual(Duration.hours(2));
 });
 
@@ -252,6 +253,63 @@ test("addAlarm: annotation overrides are applied", () => {
   });
 });
 
+test("addAlarm: check created alarms when minMetricSamplesToAlarm is used", () => {
+  const stack = new Stack();
+  const factory = new AlarmFactory(stack, {
+    globalMetricDefaults,
+    globalAlarmDefaults,
+    localAlarmNamePrefix: "prefix",
+  });
+  factory.addAlarm(metric, {
+    ...props,
+    alarmNameSuffix: "none",
+    comparisonOperator: ComparisonOperator.LESS_THAN_THRESHOLD,
+    minMetricSamplesToAlarm: 42,
+  });
+  const template = Template.fromStack(stack);
+
+  template.hasResourceProperties("AWS::CloudWatch::Alarm", {
+    AlarmName: "DummyServiceAlarms-prefix-none",
+    MetricName: "DummyMetric1",
+    Statistic: "Average",
+  });
+  template.hasResourceProperties("AWS::CloudWatch::Alarm", {
+    AlarmName: "DummyServiceAlarms-prefix-none-NoSamples",
+    AlarmDescription:
+      "The metric (DummyMetric1) does not have enough samples to alarm. Must have at least 42.",
+    ComparisonOperator: "LessThanThreshold",
+    DatapointsToAlarm: 1,
+    EvaluationPeriods: 1,
+    MetricName: "DummyMetric1",
+    Statistic: "SampleCount",
+    Threshold: 42,
+    TreatMissingData: "breaching",
+  });
+  const alarmRuleCapture = new Capture();
+  template.hasResourceProperties("AWS::CloudWatch::CompositeAlarm", {
+    AlarmName: "DummyServiceAlarms-prefix-none-WithSamples",
+    AlarmRule: alarmRuleCapture,
+  });
+  const expectedPrimaryAlarmArn = {
+    "Fn::GetAtt": ["DummyServiceAlarmsprefixnoneF01556DA", "Arn"],
+  };
+  const expectedSecondaryAlarmArn = {
+    "Fn::GetAtt": ["DummyServiceAlarmsprefixnoneNoSamples414211DB", "Arn"],
+  };
+  expect(alarmRuleCapture.asObject()).toStrictEqual({
+    ["Fn::Join"]: [
+      "",
+      [
+        '(ALARM("',
+        expectedPrimaryAlarmArn,
+        '") AND (NOT (ALARM("',
+        expectedSecondaryAlarmArn,
+        '"))))',
+      ],
+    ],
+  });
+});
+
 test("addCompositeAlarm: snapshot for operator", () => {
   const stack = new Stack();
   const factory = new AlarmFactory(stack, {