fix(AlarmFactory)!: respect disambiguatorAction alarm defaults for composite alarms

lib/common/alarm/AlarmFactory.ts

@@ -585,7 +585,6 @@ export class AlarmFactory {
     props: AddAlarmProps,
   ): AlarmWithAnnotation {
     // adjust the metric
-
     const metricAdjuster = props.metricAdjuster
       ? CompositeMetricAdjuster.of(
           props.metricAdjuster,
@@ -602,7 +601,6 @@ export class AlarmFactory {
     let alarmMetric: MetricWithAlarmSupport = adjustedMetric;
 
     // prepare primary alarm properties
-
     const actionsEnabled = this.determineActionsEnabled(
       props.actionsEnabled,
       props.disambiguator,
@@ -646,7 +644,6 @@ export class AlarmFactory {
     }
 
     // apply metric math for minimum metric samples
-
     if (props.minSampleCountToEvaluateDatapoint) {
       let label: string = `${adjustedMetric}`;
       let metricExpression: string;
@@ -701,7 +698,6 @@ export class AlarmFactory {
     }
 
     // create primary alarm
-
     const primaryAlarm = alarmMetric.createAlarm(this.alarmScope, alarmName, {
       alarmName,
       alarmDescription,
@@ -721,7 +717,6 @@ export class AlarmFactory {
 
     // create composite alarm for min metric samples (if defined)
     // deprecated in favour of minSampleCountToEvaluateDatapoint
-
     if (
       !props.minSampleCountToEvaluateDatapoint &&
       props.minMetricSamplesToAlarm
@@ -758,18 +753,16 @@ export class AlarmFactory {
     }
 
     // attach alarm actions
-
     action.addAlarmActions({
       alarm,
       action,
       dedupeString,
       disambiguator: props.disambiguator,
-      customTags: props.customTags ?? [],
-      customParams: props.customParams ?? {},
+      customTags: props.customTags,
+      customParams: props.customParams,
     });
 
     // create annotation for the primary alarm
-
     const annotation = this.createAnnotation({
       alarm: primaryAlarm,
       action,
@@ -792,7 +785,6 @@ export class AlarmFactory {
     });
 
     // return the final result
-
     return {
       alarm,
       action,
@@ -826,18 +818,20 @@ export class AlarmFactory {
     props: AddCompositeAlarmProps,
   ): CompositeAlarm {
     const actionsEnabled = this.determineActionsEnabled(
-      props?.actionsEnabled,
-      props?.disambiguator,
+      props.actionsEnabled,
+      props.disambiguator,
+    );
+    const action = this.determineAction(
+      props.disambiguator,
+      props.actionOverride,
     );
-    const action =
-      props.actionOverride ?? this.globalAlarmDefaults.action ?? noopAction();
     const namingInput = { alarmNameSuffix: "Composite", ...props };
     const alarmName = this.alarmNamingStrategy.getName(namingInput);
     const alarmDescription = this.generateDescription(
-      props?.alarmDescription ?? "Composite alarm",
-      props?.alarmDescriptionOverride,
-      props?.runbookLink,
-      props?.documentationLink,
+      props.alarmDescription ?? "Composite alarm",
+      props.alarmDescriptionOverride,
+      props.runbookLink,
+      props.documentationLink,
     );
     const dedupeString = this.alarmNamingStrategy.getDedupeString(namingInput);
     const alarmRule = this.determineCompositeAlarmRule(alarms, props);
@@ -847,18 +841,18 @@ export class AlarmFactory {
       alarmDescription,
       alarmRule,
       actionsEnabled,
-      actionsSuppressor: props?.actionsSuppressor,
-      actionsSuppressorExtensionPeriod: props?.actionsSuppressorExtensionPeriod,
-      actionsSuppressorWaitPeriod: props?.actionsSuppressorWaitPeriod,
+      actionsSuppressor: props.actionsSuppressor,
+      actionsSuppressorExtensionPeriod: props.actionsSuppressorExtensionPeriod,
+      actionsSuppressorWaitPeriod: props.actionsSuppressorWaitPeriod,
     });
 
     action.addAlarmActions({
       alarm,
       action,
       dedupeString,
-      disambiguator: props?.disambiguator,
-      customTags: props?.customTags,
-      customParams: props?.customParams,
+      disambiguator: props.disambiguator,
+      customTags: props.customTags,
+      customParams: props.customParams,
     });
 
     return alarm;
@@ -908,7 +902,7 @@ export class AlarmFactory {
       return actionOverride;
     }
 
-    // Default by disambiugator
+    // Default by disambiguator
     if (
       disambiguator &&
       this.globalAlarmDefaults.disambiguatorAction?.[disambiguator]

test/common/alarm/AlarmFactory.test.ts

@@ -9,6 +9,11 @@ import {
   Shading,
   TreatMissingData,
 } from "aws-cdk-lib/aws-cloudwatch";
+import {
+  determineLatestNodeRuntime,
+  Function,
+  InlineCode,
+} from "aws-cdk-lib/aws-lambda";
 import { Topic } from "aws-cdk-lib/aws-sns";
 import { Construct } from "constructs";
 
@@ -25,15 +30,15 @@ import {
   MetricStatistic,
   multipleActions,
   noopAction,
+  notifySns,
   SnsAlarmActionStrategy,
+  triggerLambda,
 } from "../../../lib";
 
 const stack = new Stack();
 const construct = new Construct(stack, "SampleConstruct");
 
-const snsAction = new SnsAlarmActionStrategy({
-  onAlarmTopic: new Topic(stack, "Dummy2"),
-});
+const snsAction = notifySns(new Topic(stack, "Dummy2"));
 
 class SampleAlarmActionStrategy implements IAlarmActionStrategy {
   readonly prop = "Sample";
@@ -564,10 +569,6 @@ test("addCompositeAlarm: snapshot for operator", () => {
     globalAlarmDefaults: globalAlarmDefaultsWithDisambiguator,
     localAlarmNamePrefix: "prefix",
   });
-  const metric = new Metric({
-    namespace: "DummyNamespace",
-    metricName: "DummyMetric",
-  });
   const alarm1 = factory.addAlarm(metric, {
     alarmNameSuffix: "Alarm1",
     alarmDescription: "Testing alarm 1",
@@ -596,6 +597,67 @@ test("addCompositeAlarm: snapshot for operator", () => {
   expect(Template.fromStack(stack)).toMatchSnapshot();
 });
 
+test("addCompositeAlarm: with actions", () => {
+  const stack = new Stack();
+  const factory = new AlarmFactory(stack, {
+    globalMetricDefaults,
+    globalAlarmDefaults: {
+      alarmNamePrefix: "DummyServiceAlarms",
+      actionsEnabled: true,
+      datapointsToAlarm: 6,
+      action: notifySns(new Topic(stack, "DefaultTopic")),
+      disambiguatorAction: {
+        Critical: notifySns(new Topic(stack, "CriticalTopic")),
+        Warning: notifySns(new Topic(stack, "WarningTopic")),
+      },
+    },
+    localAlarmNamePrefix: "prefix",
+  });
+  const alarm = factory.addAlarm(metric, {
+    alarmNameSuffix: "Alarm",
+    alarmDescription: "Testing alarm",
+    threshold: 1,
+    comparisonOperator: ComparisonOperator.GREATER_THAN_THRESHOLD,
+    treatMissingData: TreatMissingData.MISSING,
+  });
+
+  // Should use action defined in disambiguatorAction
+  factory.addCompositeAlarm([alarm], {
+    disambiguator: "Critical",
+  });
+
+  // Should ignore action defined in disambiguatorAction in favor of actionOverride
+  factory.addCompositeAlarm([alarm], {
+    disambiguator: "Warning",
+    actionOverride: triggerLambda(
+      new Function(stack, "FunctionAction", {
+        runtime: determineLatestNodeRuntime(stack),
+        code: InlineCode.fromInline("{}"),
+        handler: "Dummy::handler",
+      }),
+    ),
+  });
+
+  // Should use default action
+  factory.addCompositeAlarm([alarm], {
+    disambiguator: "Unknown",
+  });
+
+  const template = Template.fromStack(stack);
+  template.hasResourceProperties("AWS::CloudWatch::CompositeAlarm", {
+    AlarmName: "DummyServiceAlarms-prefix-Composite-Critical",
+    AlarmActions: [{ Ref: "CriticalTopic48D98EE4" }],
+  });
+  template.hasResourceProperties("AWS::CloudWatch::CompositeAlarm", {
+    AlarmName: "DummyServiceAlarms-prefix-Composite-Warning",
+    AlarmActions: [{ "Fn::GetAtt": ["FunctionActionD082C8F7", "Arn"] }],
+  });
+  template.hasResourceProperties("AWS::CloudWatch::CompositeAlarm", {
+    AlarmName: "DummyServiceAlarms-prefix-Composite-Unknown",
+    AlarmActions: [{ Ref: "DefaultTopicC4DC856F" }],
+  });
+});
+
 test("addCompositeAlarm: snapshot for suppressor alarm props", () => {
   const stack = new Stack();
   const factory = new AlarmFactory(stack, {