From 3bc9447dc30181a7653c1e87bc16307c3d48d0cf Mon Sep 17 00:00:00 2001
From: Sajid Anwar <sajidanw@amazon.com>
Date: Mon, 28 Oct 2024 12:52:00 -0600
Subject: [PATCH] fix: Allow non-integral latency thresholds and durations

---
 .../monitoring/alarms/LatencyAlarmFactory.ts  |  8 +--
 test/common/alarm/LatencyAlarmFactory.test.ts | 67 +++++++++++++++++++
 2 files changed, 71 insertions(+), 4 deletions(-)
 create mode 100644 test/common/alarm/LatencyAlarmFactory.test.ts

diff --git a/lib/common/monitoring/alarms/LatencyAlarmFactory.ts b/lib/common/monitoring/alarms/LatencyAlarmFactory.ts
index 4753e5bd..04cf9a03 100644
--- a/lib/common/monitoring/alarms/LatencyAlarmFactory.ts
+++ b/lib/common/monitoring/alarms/LatencyAlarmFactory.ts
@@ -166,7 +166,7 @@ export class LatencyAlarmFactory {
         ComparisonOperator.GREATER_THAN_THRESHOLD,
       ...props,
       disambiguator,
-      threshold: props.maxLatency.toMilliseconds(),
+      threshold: props.maxLatency.toMilliseconds({ integral: false }),
       alarmNameSuffix,
       // we will dedupe any kind of latency issue to the same ticket
       alarmDedupeStringSuffix: this.alarmFactory
@@ -200,7 +200,7 @@ export class LatencyAlarmFactory {
         ComparisonOperator.GREATER_THAN_THRESHOLD,
       ...props,
       disambiguator,
-      threshold: props.maxLatency.toMilliseconds(),
+      threshold: props.maxLatency.toMilliseconds({ integral: false }),
       alarmNameSuffix,
       // we will dedupe any kind of latency issue to the same alarm
       alarmDedupeStringSuffix: this.alarmFactory
@@ -230,7 +230,7 @@ export class LatencyAlarmFactory {
         ComparisonOperator.GREATER_THAN_THRESHOLD,
       ...props,
       disambiguator,
-      threshold: props.maxDuration.toMilliseconds(),
+      threshold: props.maxDuration.toMilliseconds({ integral: false }),
       alarmNameSuffix,
       // we will dedupe any kind of latency issue to the same ticket
       alarmDedupeStringSuffix: this.alarmFactory
@@ -264,7 +264,7 @@ export class LatencyAlarmFactory {
         ComparisonOperator.GREATER_THAN_THRESHOLD,
       ...props,
       disambiguator,
-      threshold: props.maxDuration.toMilliseconds(),
+      threshold: props.maxDuration.toMilliseconds({ integral: false }),
       alarmNameSuffix,
       // we will dedupe any kind of latency issue to the same ticket
       alarmDedupeStringSuffix: this.alarmFactory
diff --git a/test/common/alarm/LatencyAlarmFactory.test.ts b/test/common/alarm/LatencyAlarmFactory.test.ts
new file mode 100644
index 00000000..a17c92c1
--- /dev/null
+++ b/test/common/alarm/LatencyAlarmFactory.test.ts
@@ -0,0 +1,67 @@
+import { Duration, Stack } from "aws-cdk-lib";
+import { Metric } from "aws-cdk-lib/aws-cloudwatch";
+import { Construct } from "constructs";
+
+import {
+  AlarmFactory,
+  AlarmFactoryDefaults,
+  LatencyAlarmFactory,
+  LatencyType,
+  MetricFactoryDefaults,
+  noopAction,
+} from "../../../lib";
+
+const stack = new Stack();
+const construct = new Construct(stack, "SampleConstruct");
+
+const globalMetricDefaults: MetricFactoryDefaults = {
+  namespace: "DummyNamespace",
+};
+const globalAlarmDefaults: AlarmFactoryDefaults = {
+  alarmNamePrefix: "DummyServiceAlarms",
+  actionsEnabled: true,
+  datapointsToAlarm: 6,
+  // we do not care about alarm actions in this test
+  action: noopAction(),
+};
+const factory = new AlarmFactory(construct, {
+  globalMetricDefaults,
+  globalAlarmDefaults,
+  localAlarmNamePrefix: "prefix",
+});
+
+const metric = new Metric({
+  metricName: "DummyMetric1",
+  namespace: "DummyCustomNamespace",
+  dimensionsMap: { CustomDimension: "CustomDimensionValue" },
+});
+
+const latencyAlarmFactory = new LatencyAlarmFactory(factory);
+
+test("addLatencyAlarm: non-integral millisecond thresholds do not throw error", () => {
+  latencyAlarmFactory.addLatencyAlarm(metric, LatencyType.P99, {
+    maxLatency: Duration.millis(0.5),
+  });
+});
+
+test("addIntegrationLatencyAlarm: non-integral millisecond thresholds do not throw error", () => {
+  latencyAlarmFactory.addIntegrationLatencyAlarm(metric, LatencyType.P99, {
+    maxLatency: Duration.millis(2.5),
+  });
+});
+
+test("addDurationAlarm: non-integral millisecond durations do not throw error", () => {
+  latencyAlarmFactory.addDurationAlarm(metric, LatencyType.P99, {
+    maxDuration: Duration.millis(0.5),
+  });
+});
+
+test("addJvmGarbageCollectionDurationAlarm: non-integral millisecond durations do not throw error", () => {
+  latencyAlarmFactory.addJvmGarbageCollectionDurationAlarm(
+    metric,
+    LatencyType.P99,
+    {
+      maxDuration: Duration.millis(2.5),
+    },
+  );
+});