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

Issue:
LatencyAlarmFactory doesn't allow for non-integral latency thresholds
### Version

8.3.3

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

Add a latency alarm with a `maxLatency` value of `Duration.millis(/* some non-integral number */)`.

```js
latencyAlarmFactory.addLatencyAlarm(metric, LatencyType.P99, { maxLatency: Duration.millis(0.5) });
```

### Expected behavior

Latency alarm and annotation are created in CloudWatch with the provided duration value.

### Actual behavior

Synthesis fails with a duration conversion exception: `0.5 must be a whole number of millis.`

### Other details

This issue prevents using non-integral or sub-millisecond latency thresholds, even though CloudWatch supports it. The root cause is that the `Duration#toMilliseconds` method from the CDK core library defaults to requiring an integral value, but the option is there to enable non-integral values.

I'll be submitting a PR shortly for this. 🙂


Discussion / hints:

Fix patch:
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


Test patch:
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),
+    },
+  );
+});