From 1d630508f1686a4a1de0dc836d611ba605b4a867 Mon Sep 17 00:00:00 2001 From: nizar-lahlali Date: Thu, 28 May 2026 16:40:31 -0400 Subject: [PATCH 1/3] feat(cdk): add CloudWatch alarms for FanOut + ApprovalMetricsPublisher DLQs (#117) Adds ApproximateNumberOfMessagesVisible >= 1 alarms (5-min window, Maximum statistic) on both DLQs so poison-pill records don't silently accumulate. Each alarm includes a runbook pointer in alarmDescription and is exposed as a public property for future SNS topic wiring. --- .../approval-metrics-publisher-consumer.ts | 23 +++++++++++++++---- cdk/src/constructs/fanout-consumer.ts | 18 +++++++++++++++ ...pproval-metrics-publisher-consumer.test.ts | 15 ++++++++++++ cdk/test/constructs/fanout-consumer.test.ts | 20 ++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/cdk/src/constructs/approval-metrics-publisher-consumer.ts b/cdk/src/constructs/approval-metrics-publisher-consumer.ts index 01f754528..3d3d226cc 100644 --- a/cdk/src/constructs/approval-metrics-publisher-consumer.ts +++ b/cdk/src/constructs/approval-metrics-publisher-consumer.ts @@ -19,6 +19,7 @@ import * as path from 'path'; import { Duration, RemovalPolicy } from 'aws-cdk-lib'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; import { FilterCriteria, FilterRule, StartingPosition, Architecture, Runtime } from 'aws-cdk-lib/aws-lambda'; import { DynamoEventSource, SqsDlq } from 'aws-cdk-lib/aws-lambda-event-sources'; @@ -84,6 +85,7 @@ export interface ApprovalMetricsPublisherConsumerProps { export class ApprovalMetricsPublisherConsumer extends Construct { public readonly fn: lambda.NodejsFunction; public readonly dlq: sqs.Queue; + public readonly dlqAlarm: cloudwatch.Alarm; constructor(scope: Construct, id: string, props: ApprovalMetricsPublisherConsumerProps) { super(scope, id); @@ -93,10 +95,7 @@ export class ApprovalMetricsPublisherConsumer extends Construct { this.dlq = new sqs.Queue(this, 'ApprovalMetricsPublisherDlq', { // Persistent failures (malformed records the handler's // per-record try/catch throws on three times in a row) land - // here for operator inspection. Alarm wiring is deferred to - // Chunk 10 follow-ups — until a notification channel is wired - // to SNS, an alarm on ``ApproximateNumberOfMessagesVisible`` - // would fire into the void. + // here for operator inspection. retentionPeriod: Duration.days(14), enforceSSL: true, }); @@ -153,6 +152,22 @@ export class ApprovalMetricsPublisherConsumer extends Construct { filters: [agentMilestoneFilter], })); + // §11.5: alarm on DLQ depth so poison-pill records don't silently + // accumulate without operator visibility. + this.dlqAlarm = new cloudwatch.Alarm(this, 'DlqMessageAlarm', { + metric: this.dlq.metricApproximateNumberOfMessagesVisible({ + period: Duration.minutes(5), + statistic: 'Maximum', + }), + threshold: 1, + evaluationPeriods: 1, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, + alarmDescription: + 'ApprovalMetricsPublisher DLQ has at least one message; investigate poison records. ' + + 'Runbook: TODO — check CloudWatch Logs for the ApprovalMetricsPublisherFn error that caused the DLQ send.', + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + NagSuppressions.addResourceSuppressions(this.fn, [ { id: 'AwsSolutions-IAM4', diff --git a/cdk/src/constructs/fanout-consumer.ts b/cdk/src/constructs/fanout-consumer.ts index 0a2ddc447..71e8d4651 100644 --- a/cdk/src/constructs/fanout-consumer.ts +++ b/cdk/src/constructs/fanout-consumer.ts @@ -19,6 +19,7 @@ import * as path from 'path'; import { Duration, RemovalPolicy } from 'aws-cdk-lib'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; import * as iam from 'aws-cdk-lib/aws-iam'; import { StartingPosition, Architecture, Runtime } from 'aws-cdk-lib/aws-lambda'; @@ -106,6 +107,7 @@ export interface FanOutConsumerProps { export class FanOutConsumer extends Construct { public readonly fn: lambda.NodejsFunction; public readonly dlq: sqs.Queue; + public readonly dlqAlarm: cloudwatch.Alarm; constructor(scope: Construct, id: string, props: FanOutConsumerProps) { super(scope, id); @@ -185,6 +187,22 @@ export class FanOutConsumer extends Construct { reportBatchItemFailures: true, })); + // §11.5: alarm on DLQ depth so poison-pill records don't silently + // accumulate without operator visibility. + this.dlqAlarm = new cloudwatch.Alarm(this, 'DlqMessageAlarm', { + metric: this.dlq.metricApproximateNumberOfMessagesVisible({ + period: Duration.minutes(5), + statistic: 'Maximum', + }), + threshold: 1, + evaluationPeriods: 1, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, + alarmDescription: + 'FanOutConsumer DLQ has at least one message; investigate poison records. ' + + 'Runbook: TODO — check CloudWatch Logs for the FanOutFn error that caused the DLQ send.', + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + NagSuppressions.addResourceSuppressions(this.fn, [ { id: 'AwsSolutions-IAM4', diff --git a/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts b/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts index 227c8cd6a..fa98873b6 100644 --- a/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts +++ b/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts @@ -189,4 +189,19 @@ describe('ApprovalMetricsPublisherConsumer', () => { // expect exactly one Table in the synthesized stack. template.resourceCountIs('AWS::DynamoDB::Table', 1); }); + + test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (§11.5)', () => { + const { template } = createStack(); + + template.hasResourceProperties('AWS::CloudWatch::Alarm', { + MetricName: 'ApproximateNumberOfMessagesVisible', + Namespace: 'AWS/SQS', + Threshold: 1, + EvaluationPeriods: 1, + ComparisonOperator: 'GreaterThanOrEqualToThreshold', + TreatMissingData: 'notBreaching', + Statistic: 'Maximum', + Period: 300, + }); + }); }); diff --git a/cdk/test/constructs/fanout-consumer.test.ts b/cdk/test/constructs/fanout-consumer.test.ts index 175f15f8e..3c5791cbc 100644 --- a/cdk/test/constructs/fanout-consumer.test.ts +++ b/cdk/test/constructs/fanout-consumer.test.ts @@ -170,4 +170,24 @@ describe('FanOutConsumer', () => { expect(vars.TASK_TABLE_NAME).toBeUndefined(); } }); + + test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (§11.5)', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new FanOutConsumer(stack, 'FanOut', { + taskEventsTable: makeTaskEventsTable(stack), + }); + const template = Template.fromStack(stack); + + template.hasResourceProperties('AWS::CloudWatch::Alarm', { + MetricName: 'ApproximateNumberOfMessagesVisible', + Namespace: 'AWS/SQS', + Threshold: 1, + EvaluationPeriods: 1, + ComparisonOperator: 'GreaterThanOrEqualToThreshold', + TreatMissingData: 'notBreaching', + Statistic: 'Maximum', + Period: 300, + }); + }); }); From 1f0aa581e43b174dc713164629bebc155d08cfaf Mon Sep 17 00:00:00 2001 From: nizar-lahlali Date: Mon, 1 Jun 2026 10:15:09 -0400 Subject: [PATCH 2/3] feat(cdk): add CloudWatch alarms for FanOut + ApprovalMetricsPublisher DLQs (#117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ship DLQ-depth CloudWatch alarms (ApproximateNumberOfMessagesVisible >= 1, 5-min Maximum, treatMissingData NOT_BREACHING) for both stream consumers so poison-pill records no longer accumulate silently during the 14-day retention window. Alarms transition to ALARM state in the CloudWatch console without a notification action — SNS wiring is a bounded follow-up once an operational channel is provisioned. Also: - Update §11.5 in CEDAR_HITL_GATES.md to reflect the shipped decision - Narrow public type to cloudwatch.IAlarm (consumers only need addAlarmAction) - Add JSDoc matching the errorAlarm precedent in task-orchestrator.ts - Replace Runbook TODO with actionable diagnostic text and link to #117 - Reference issue #117 in code comments and test titles instead of §11.5 --- .../approval-metrics-publisher-consumer.ts | 8 ++++--- cdk/src/constructs/fanout-consumer.ts | 8 ++++--- ...pproval-metrics-publisher-consumer.test.ts | 2 +- cdk/test/constructs/fanout-consumer.test.ts | 2 +- docs/design/CEDAR_HITL_GATES.md | 21 ++++++++++++------- .../docs/architecture/Cedar-hitl-gates.md | 21 ++++++++++++------- 6 files changed, 38 insertions(+), 24 deletions(-) diff --git a/cdk/src/constructs/approval-metrics-publisher-consumer.ts b/cdk/src/constructs/approval-metrics-publisher-consumer.ts index 3d3d226cc..5efd1c823 100644 --- a/cdk/src/constructs/approval-metrics-publisher-consumer.ts +++ b/cdk/src/constructs/approval-metrics-publisher-consumer.ts @@ -85,7 +85,8 @@ export interface ApprovalMetricsPublisherConsumerProps { export class ApprovalMetricsPublisherConsumer extends Construct { public readonly fn: lambda.NodejsFunction; public readonly dlq: sqs.Queue; - public readonly dlqAlarm: cloudwatch.Alarm; + /** CloudWatch alarm that fires when the DLQ has at least one poison-pill record. */ + public readonly dlqAlarm: cloudwatch.IAlarm; constructor(scope: Construct, id: string, props: ApprovalMetricsPublisherConsumerProps) { super(scope, id); @@ -152,7 +153,7 @@ export class ApprovalMetricsPublisherConsumer extends Construct { filters: [agentMilestoneFilter], })); - // §11.5: alarm on DLQ depth so poison-pill records don't silently + // #117: alarm on DLQ depth so poison-pill records don't silently // accumulate without operator visibility. this.dlqAlarm = new cloudwatch.Alarm(this, 'DlqMessageAlarm', { metric: this.dlq.metricApproximateNumberOfMessagesVisible({ @@ -164,7 +165,8 @@ export class ApprovalMetricsPublisherConsumer extends Construct { comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, alarmDescription: 'ApprovalMetricsPublisher DLQ has at least one message; investigate poison records. ' + - 'Runbook: TODO — check CloudWatch Logs for the ApprovalMetricsPublisherFn error that caused the DLQ send.', + 'Check CloudWatch Logs for the ApprovalMetricsPublisherFn error that caused the DLQ send. ' + + 'See: https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/issues/117', treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); diff --git a/cdk/src/constructs/fanout-consumer.ts b/cdk/src/constructs/fanout-consumer.ts index 71e8d4651..c175749f4 100644 --- a/cdk/src/constructs/fanout-consumer.ts +++ b/cdk/src/constructs/fanout-consumer.ts @@ -107,7 +107,8 @@ export interface FanOutConsumerProps { export class FanOutConsumer extends Construct { public readonly fn: lambda.NodejsFunction; public readonly dlq: sqs.Queue; - public readonly dlqAlarm: cloudwatch.Alarm; + /** CloudWatch alarm that fires when the DLQ has at least one poison-pill record. */ + public readonly dlqAlarm: cloudwatch.IAlarm; constructor(scope: Construct, id: string, props: FanOutConsumerProps) { super(scope, id); @@ -187,7 +188,7 @@ export class FanOutConsumer extends Construct { reportBatchItemFailures: true, })); - // §11.5: alarm on DLQ depth so poison-pill records don't silently + // #117: alarm on DLQ depth so poison-pill records don't silently // accumulate without operator visibility. this.dlqAlarm = new cloudwatch.Alarm(this, 'DlqMessageAlarm', { metric: this.dlq.metricApproximateNumberOfMessagesVisible({ @@ -199,7 +200,8 @@ export class FanOutConsumer extends Construct { comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, alarmDescription: 'FanOutConsumer DLQ has at least one message; investigate poison records. ' + - 'Runbook: TODO — check CloudWatch Logs for the FanOutFn error that caused the DLQ send.', + 'Check CloudWatch Logs for the FanOutFn error that caused the DLQ send. ' + + 'See: https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/issues/117', treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); diff --git a/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts b/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts index fa98873b6..169d76557 100644 --- a/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts +++ b/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts @@ -190,7 +190,7 @@ describe('ApprovalMetricsPublisherConsumer', () => { template.resourceCountIs('AWS::DynamoDB::Table', 1); }); - test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (§11.5)', () => { + test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (#117)', () => { const { template } = createStack(); template.hasResourceProperties('AWS::CloudWatch::Alarm', { diff --git a/cdk/test/constructs/fanout-consumer.test.ts b/cdk/test/constructs/fanout-consumer.test.ts index 3c5791cbc..a72c322a1 100644 --- a/cdk/test/constructs/fanout-consumer.test.ts +++ b/cdk/test/constructs/fanout-consumer.test.ts @@ -171,7 +171,7 @@ describe('FanOutConsumer', () => { } }); - test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (§11.5)', () => { + test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (#117)', () => { const app = new App(); const stack = new Stack(app, 'TestStack'); new FanOutConsumer(stack, 'FanOut', { diff --git a/docs/design/CEDAR_HITL_GATES.md b/docs/design/CEDAR_HITL_GATES.md index df7159b34..aa22c9915 100644 --- a/docs/design/CEDAR_HITL_GATES.md +++ b/docs/design/CEDAR_HITL_GATES.md @@ -1585,21 +1585,26 @@ Extend `TaskDashboard` (`cdk/src/constructs/task-dashboard.ts`). These are read- Every `agent_milestone("approval_*")` event carries `trace_id` / `span_id`. A span `hitl.approval_wait` brackets the PreToolUse poll loop: `span.duration = decided_at - created_at`. `hitl.approval_race_loss` emitted when the agent's local timeout fired <5s before a late user decision (useful for tuning). -### 11.5 CloudWatch alarms — deferred (notification-channel gated) +### 11.5 CloudWatch alarms + +**DLQ-depth alarms (shipped):** CloudWatch alarms on `ApproximateNumberOfMessagesVisible >= 1` (5-min period, Maximum statistic, `treatMissingData: NOT_BREACHING`) are deployed for: + +- **FanOutConsumer DLQ** — poison-pill DynamoDB Stream records that failed three consecutive Lambda invocations. +- **ApprovalMetricsPublisher DLQ** — same failure mode for the metrics-publisher consumer. + +These alarms transition to `ALARM` state in CloudWatch and appear in the console/dashboard, providing operator visibility into silent record loss. They ship **without an `addAlarmAction` / SNS notification target** — operators must check the CloudWatch Alarms console or configure a subscription manually. This is an intentional intermediate step: alarm state is durable and queryable even without push notifications, and prevents poison records from accumulating silently for the full 14-day DLQ retention window. + +**Follow-up — notification channel wiring:** Once an operational notification channel (SNS topic → Slack / PagerDuty / email) is provisioned, add `alarm.addAlarmAction(new SnsAction(topic))` to both alarms. No metric or alarm restructuring is needed. + +**Additional alarms (not yet shipped):** The following remain deferred until the notification channel exists (alarm-without-action provides limited value for rate/latency conditions that require human triage): -Operator-facing CloudWatch alarms that would page on: - High approval-timeout rate (users not responding, notifications broken) - Tasks stuck in AWAITING_APPROVAL beyond `timeout_s + 60s` (reconciler failure) - High approval-write failure rate (DDB throttled or IAM drift) - Approval-gate cap hit (suspicious retry loop) -- Publisher / fanout DLQ non-empty (persistent consumer-side poison pills) - `MetricEmitSkipped` sustained > 0 (publisher schema mismatch — agent / publisher version skew) - `MetricsPublisherHeartbeat` flat-line (publisher pipeline broken) -…are **out of scope for v1** because the project does not yet have a notification channel (Slack / PagerDuty / SNS topic / email distribution list) configured for operational alerts. Adding alarms without a notification channel produces CloudWatch widgets that nobody sees — no safety benefit. - -**Plumbing status (post-Chunk 8):** the supporting metric data now flows as native CloudWatch metrics in namespace `ABCA/Cedar-HITL` via `ApprovalMetricsPublisherFn` (§11.3). Alarm wiring becomes a per-threshold `cloudwatch.Alarm` + `SnsAction`; no additional metric-extraction infra is needed. The remaining gap is the SNS topic + subscriber wiring itself — when that lands, the alarms above are a small bounded follow-up (not a multi-PR metrics build-out as they were pre-Chunk-8). - --- ## 12. Security model @@ -2070,7 +2075,7 @@ See §17.18 for the off-hours escalation future-work primitive, and §13.14 for **Future work — polish (tracked in §17):** - CLI inline streaming prompt (UX research first) - `approve --defer` / allowlist revocation (`bgagent revoke-approval`) -- CloudWatch alarm plumbing (§11.5) — deferred until an operational notification channel is available +- CloudWatch alarm SNS notification wiring (§11.5) — DLQ-depth alarms ship without an action target; add `SnsAction` once a notification channel is provisioned - More soft-deny policies in the default set based on real usage - Persistent recent-decision cache (if container-restart telemetry justifies it) - Persistent per-minute rate limit (if restart amplification becomes significant) diff --git a/docs/src/content/docs/architecture/Cedar-hitl-gates.md b/docs/src/content/docs/architecture/Cedar-hitl-gates.md index ab4061037..a9189f19c 100644 --- a/docs/src/content/docs/architecture/Cedar-hitl-gates.md +++ b/docs/src/content/docs/architecture/Cedar-hitl-gates.md @@ -1589,21 +1589,26 @@ Extend `TaskDashboard` (`cdk/src/constructs/task-dashboard.ts`). These are read- Every `agent_milestone("approval_*")` event carries `trace_id` / `span_id`. A span `hitl.approval_wait` brackets the PreToolUse poll loop: `span.duration = decided_at - created_at`. `hitl.approval_race_loss` emitted when the agent's local timeout fired <5s before a late user decision (useful for tuning). -### 11.5 CloudWatch alarms — deferred (notification-channel gated) +### 11.5 CloudWatch alarms + +**DLQ-depth alarms (shipped):** CloudWatch alarms on `ApproximateNumberOfMessagesVisible >= 1` (5-min period, Maximum statistic, `treatMissingData: NOT_BREACHING`) are deployed for: + +- **FanOutConsumer DLQ** — poison-pill DynamoDB Stream records that failed three consecutive Lambda invocations. +- **ApprovalMetricsPublisher DLQ** — same failure mode for the metrics-publisher consumer. + +These alarms transition to `ALARM` state in CloudWatch and appear in the console/dashboard, providing operator visibility into silent record loss. They ship **without an `addAlarmAction` / SNS notification target** — operators must check the CloudWatch Alarms console or configure a subscription manually. This is an intentional intermediate step: alarm state is durable and queryable even without push notifications, and prevents poison records from accumulating silently for the full 14-day DLQ retention window. + +**Follow-up — notification channel wiring:** Once an operational notification channel (SNS topic → Slack / PagerDuty / email) is provisioned, add `alarm.addAlarmAction(new SnsAction(topic))` to both alarms. No metric or alarm restructuring is needed. + +**Additional alarms (not yet shipped):** The following remain deferred until the notification channel exists (alarm-without-action provides limited value for rate/latency conditions that require human triage): -Operator-facing CloudWatch alarms that would page on: - High approval-timeout rate (users not responding, notifications broken) - Tasks stuck in AWAITING_APPROVAL beyond `timeout_s + 60s` (reconciler failure) - High approval-write failure rate (DDB throttled or IAM drift) - Approval-gate cap hit (suspicious retry loop) -- Publisher / fanout DLQ non-empty (persistent consumer-side poison pills) - `MetricEmitSkipped` sustained > 0 (publisher schema mismatch — agent / publisher version skew) - `MetricsPublisherHeartbeat` flat-line (publisher pipeline broken) -…are **out of scope for v1** because the project does not yet have a notification channel (Slack / PagerDuty / SNS topic / email distribution list) configured for operational alerts. Adding alarms without a notification channel produces CloudWatch widgets that nobody sees — no safety benefit. - -**Plumbing status (post-Chunk 8):** the supporting metric data now flows as native CloudWatch metrics in namespace `ABCA/Cedar-HITL` via `ApprovalMetricsPublisherFn` (§11.3). Alarm wiring becomes a per-threshold `cloudwatch.Alarm` + `SnsAction`; no additional metric-extraction infra is needed. The remaining gap is the SNS topic + subscriber wiring itself — when that lands, the alarms above are a small bounded follow-up (not a multi-PR metrics build-out as they were pre-Chunk-8). - --- ## 12. Security model @@ -2074,7 +2079,7 @@ See §17.18 for the off-hours escalation future-work primitive, and §13.14 for **Future work — polish (tracked in §17):** - CLI inline streaming prompt (UX research first) - `approve --defer` / allowlist revocation (`bgagent revoke-approval`) -- CloudWatch alarm plumbing (§11.5) — deferred until an operational notification channel is available +- CloudWatch alarm SNS notification wiring (§11.5) — DLQ-depth alarms ship without an action target; add `SnsAction` once a notification channel is provisioned - More soft-deny policies in the default set based on real usage - Persistent recent-decision cache (if container-restart telemetry justifies it) - Persistent per-minute rate limit (if restart amplification becomes significant) From 20c71ea52b5a67cf9489f0c07f51430f1a727df8 Mon Sep 17 00:00:00 2001 From: bgagent Date: Fri, 7 Aug 2026 10:51:31 -0400 Subject: [PATCH 3/3] fix(cdk): keep DLQ alarms as concrete Alarm type; consolidate FanOut alarm test (#117) - Revert dlqDepthAlarm/dlqAlarm public type from cloudwatch.IAlarm back to cloudwatch.Alarm. addAlarmAction is declared on Alarm, not the IAlarm interface, so the IAlarm widening actively blocked the future SNS wiring it claimed to enable. Matches the errorAlarm precedent in task-orchestrator.ts. - Fold the ComparisonOperator assertion into the existing FanOut alarm test and drop the redundant duplicate test (avoids an extra CDK synth, keeps the stronger queue-dimension + resourceCountIs guards). - Fix stale 'five times' -> 'three times' DLQ comment (retryAttempts: 3) and generalize an unverifiable merge-incident anecdote in the approval test. --- .../approval-metrics-publisher-consumer.ts | 7 +++++-- cdk/src/constructs/fanout-consumer.ts | 10 +++++---- ...pproval-metrics-publisher-consumer.test.ts | 5 ++--- cdk/test/constructs/fanout-consumer.test.ts | 21 +------------------ 4 files changed, 14 insertions(+), 29 deletions(-) diff --git a/cdk/src/constructs/approval-metrics-publisher-consumer.ts b/cdk/src/constructs/approval-metrics-publisher-consumer.ts index 157923829..0b60d3633 100644 --- a/cdk/src/constructs/approval-metrics-publisher-consumer.ts +++ b/cdk/src/constructs/approval-metrics-publisher-consumer.ts @@ -97,8 +97,11 @@ export interface ApprovalMetricsPublisherConsumerProps { export class ApprovalMetricsPublisherConsumer extends Construct { public readonly fn: lambda.NodejsFunction; public readonly dlq: sqs.Queue; - /** CloudWatch alarm that fires when the DLQ has at least one poison-pill record. */ - public readonly dlqAlarm: cloudwatch.IAlarm; + /** CloudWatch alarm that fires when the DLQ has at least one + * poison-pill record. Concrete {@link cloudwatch.Alarm} so an + * ``addAlarmAction`` (SNS) can be attached once a notification + * channel is provisioned (#117). */ + public readonly dlqAlarm: cloudwatch.Alarm; constructor(scope: Construct, id: string, props: ApprovalMetricsPublisherConsumerProps) { super(scope, id); diff --git a/cdk/src/constructs/fanout-consumer.ts b/cdk/src/constructs/fanout-consumer.ts index c65602de6..f232897e3 100644 --- a/cdk/src/constructs/fanout-consumer.ts +++ b/cdk/src/constructs/fanout-consumer.ts @@ -160,9 +160,11 @@ export class FanOutConsumer extends Construct { /** Fires when records land in the fan-out DLQ — a silent fan-out * outage (every Slack/GitHub/Linear notification failing) would * otherwise accumulate unnoticed for the queue's 14-day retention. - * Exposed as {@link cloudwatch.IAlarm} so a future consumer can call - * ``addAlarmAction`` once an SNS notification channel exists (#117). */ - public readonly dlqDepthAlarm: cloudwatch.IAlarm; + * Kept as the concrete {@link cloudwatch.Alarm} (matching the + * ``errorAlarm`` precedent in task-orchestrator.ts) so a future + * consumer can call ``addAlarmAction`` — declared on ``Alarm``, not + * the ``IAlarm`` interface — once an SNS channel exists (#117). */ + public readonly dlqDepthAlarm: cloudwatch.Alarm; constructor(scope: Construct, id: string, props: FanOutConsumerProps) { super(scope, id); @@ -171,7 +173,7 @@ export class FanOutConsumer extends Construct { this.dlq = new sqs.Queue(this, 'FanOutDlq', { // Persistent failures (e.g., dispatcher throws non-caught error - // five times in a row) land here for operator inspection. + // three times in a row) land here for operator inspection. retentionPeriod: Duration.days(DLQ_RETENTION_DAYS), enforceSSL: true, }); diff --git a/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts b/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts index 9d5b7042d..a6b09d2ff 100644 --- a/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts +++ b/cdk/test/constructs/approval-metrics-publisher-consumer.test.ts @@ -193,9 +193,8 @@ describe('ApprovalMetricsPublisherConsumer', () => { test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (#117)', () => { const { template } = createStack(); - // Exactly one alarm — guards against a future change accidentally - // duplicating the DLQ-depth alarm (the failure mode seen when this - // construct's near-identical FanOutConsumer twin merged with main). + // Exactly one alarm — guards against a future merge or refactor + // accidentally duplicating the DLQ-depth alarm on this construct. template.resourceCountIs('AWS::CloudWatch::Alarm', 1); template.hasResourceProperties('AWS::CloudWatch::Alarm', { MetricName: 'ApproximateNumberOfMessagesVisible', diff --git a/cdk/test/constructs/fanout-consumer.test.ts b/cdk/test/constructs/fanout-consumer.test.ts index 1f8268274..9a7b15149 100644 --- a/cdk/test/constructs/fanout-consumer.test.ts +++ b/cdk/test/constructs/fanout-consumer.test.ts @@ -152,6 +152,7 @@ describe('FanOutConsumer', () => { Period: 300, Threshold: 1, EvaluationPeriods: 1, + ComparisonOperator: 'GreaterThanOrEqualToThreshold', TreatMissingData: 'notBreaching', // The alarm must watch THIS construct's DLQ, not some other queue. Dimensions: Match.arrayWith([ @@ -266,24 +267,4 @@ describe('FanOutConsumer', () => { expect(vars.TASK_TABLE_NAME).toBeUndefined(); } }); - - test('creates a CloudWatch alarm on DLQ ApproximateNumberOfMessagesVisible (#117)', () => { - const app = new App(); - const stack = new Stack(app, 'TestStack'); - new FanOutConsumer(stack, 'FanOut', { - taskEventsTable: makeTaskEventsTable(stack), - }); - const template = Template.fromStack(stack); - - template.hasResourceProperties('AWS::CloudWatch::Alarm', { - MetricName: 'ApproximateNumberOfMessagesVisible', - Namespace: 'AWS/SQS', - Threshold: 1, - EvaluationPeriods: 1, - ComparisonOperator: 'GreaterThanOrEqualToThreshold', - TreatMissingData: 'notBreaching', - Statistic: 'Maximum', - Period: 300, - }); - }); });