fix(hosting): place CloudFront 5xx alarm in us-east-1 (#481) - #488
Merged
Merged
Conversation
AWS/CloudFront metrics are published only in us-east-1, and a CloudWatch
alarm can only evaluate a metric in its own region (confirmed by the
CloudWatch docs "Cross-Region functionality is not supported for alarms"
and rejected by aws-cdk-lib at synth). For any hosting stack outside
us-east-1 the CloudFront5xxRate alarm never received a datapoint and,
with treatMissingData: NOT_BREACHING, sat at OK forever instead of
alarming — a monitoring alarm that silently never fires.
Fix (two-topic design): off-region, defer the CloudFront alarm to a
dedicated hosting-owned us-east-1 support stack that owns its own SNS
topic. Its topic ARN is surfaced as MonitoringTopicArnUsEast1 (an output
of the support stack) for the operator to subscribe to, alongside the
regional MonitoringTopicArn. The regional alarms (SSR/image/DLQ) are
unchanged — their metrics are regional. In-region (us-east-1) behavior
is unchanged: single stack, alarm created locally.
New prop monitoring.cloudFrontAlarm: 'usEast1Stack' (default) | 'skip'.
The default requires env:{account,region} off-region (a cross-region
stack needs a concrete account); env-agnostic off-region synth throws
MonitoringErrorRequiredError with guidance. 'skip' emits a synth warning
and creates no second stack.
Notes:
- The us-east-1 alarm references the regional distribution id, which CDK
bridges with its standard cross-region export reader/writer custom
resources (added to both stacks automatically) — not a runtime
forwarder.
- Breaking-ish: the default now synthesizes a second CloudFormation stack
off-region and requires env:{account,region}.
Tests: MonitoringConstruct deferral unit tests; new
hosting_construct.cf_alarm_region.test.ts covering off-region two-stack
synth, env-required throw, skip warning, and in-region single-stack.
All 853 hosting tests pass.
🦋 Changeset detectedLatest commit: cfc17aa The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
… handling, core cloudFrontAlarm prop, split MonitoringStageRequiredError, lint
soberm
reviewed
Sep 4, 2026
Contributor
There was a problem hiding this comment.
Nice catch on the root cause. Off-region, the old CloudFront5xxRate alarm sat at OK forever (metric only in us-east-1 + treatMissingData: NOT_BREACHING) — a monitoring alarm that silently never fired. The deferral to a hosting-owned us-east-1 stack with its own topic is a clean approach, region/account gating reads correctly, and alarm-config parity with the original is exact.
I built the branch (Node 22, aws-cdk-lib 2.257.0) and ran hosting_construct.cf_alarm_region.test.ts — build green, 4/4 pass. Worth noting for future readers: the us-east-1 alarm references this.distribution.distributionId (an unresolved cross-stack token), and CDK auto-bridges that with CrossRegionExportWriter/ExportsReader custom resources on this CDK version without crossRegionReferences: true. That works — but it means the code comments claiming "plain string / no cross-region reference" are inaccurate (see inline).
The one thing I'd like a maintainer call on before merge: is breaking-by-default the behavior we want? The new 'usEast1Stack' default makes a previously-synthesizing env-agnostic off-region stack now throw MonitoringEnvRequiredError. The minor changeset is the correct label for that (0.x → minor = breaking), so semver is fine — this is purely a "do we want the default to break, or default to 'skip'?" product decision. Please also make the changeset state plainly that this is a breaking change requiring env off-region.
Remaining items are all non-blocking suggestions/nits inline. Recommend an off-region E2E destroy→deploy pass to confirm the new cross-region custom resources don't hit the known "cannot delete export in use" / CREATE-vs---revert-drift issues.
Closed
2 tasks
svidgen
requested changes
Sep 4, 2026
svidgen
left a comment
svidgen
left a comment
Contributor
There was a problem hiding this comment.
Small design with API BR review please. Details in slack.
…ion CloudFront alarm
Reworks the off-region CloudFront alarm fix around a monitoring.subscriptions
API instead of the two-topic BYO/operator-subscribes model.
- Replace monitoring.snsTopicArn (BYO topic) with monitoring.subscriptions,
restricted to endpoint subscriptions (EmailSubscription | UrlSubscription).
Each subscription is applied to BOTH the app-region topic and the us-east-1
CloudFront topic, so a developer subscribes once and both are covered.
Resource-target subscriptions (Lambda/SQS) are intentionally excluded for
now — applying them to the us-east-1 topic creates an unresolvable
cross-region reference from the app-region resource; the type prevents it at
compile time. Widening later (e.g. via a forwarder) is non-breaking.
- Replace the hosting.monitoringTopic attribute with
hosting.monitoring = { alarms, alarmTopics }.
- Off-region: always place the CloudFront alarm in a synthesized
<stackName>-CfMonitoring-<addr> us-east-1 stack with its own KMS-encrypted
topic; requires env:{account,region} (throws MonitoringEnvRequiredError
otherwise). No opt-out.
- Rename the us-east-1 topic construct id to AlarmTopicUsEast1 to avoid a
subscriber-id collision.
- Clear prior review nits: accurate cross-region distributionId docs,
unique support-stack id, output asserted by logical id + full alarm parity.
Breaking change (minor bump pre-1.0) for both @aws-blocks/hosting and
@aws-blocks/core.
Tests: hosting 857/857, core 747/747, both builds green.
…PI (#481) The hosting README's "Off-region CloudFront alarm parity" section still documented the removed cloudFrontAlarm: 'skip' | 'usEast1Stack' opt-out prop (a compile-error snippet) and the pre-suffix <stackName>-CfMonitoring stack name. Rewrite it to the shipped API: off-region placement is always on (no opt-out), the companion stack is <stackName>-CfMonitoring-<addr>, env:{account,region} is required off-region (throws MonitoringEnvRequiredError), and subscriptions are endpoint-only (Email/Url). Also add the subs import hint to the blocks README production-tips bullet.
soberm
reviewed
Sep 14, 2026
…count; address review (#481) Addresses soberm's review on the subscriptions revision. - Off-region + unresolved account no longer hard-throws MonitoringEnvRequiredError (which forced monitoring.enabled:false and dropped the working regional SSR/image/DLQ alarms). Instead skip ONLY the CloudFront alarm and emit a loud synth warning — a visible warning is not the silent-alarm bug #481 is about, and all other alarms are preserved. - Env-agnostic (unresolved region): warn that the locally-created CloudFront alarm will never fire if the app deploys outside us-east-1, instead of silently placing a dead alarm. - Fix dead in-region negative test assertion (exact name never matched the now-suffixed support-stack id) to scan by prefix. - Drop the no-op `{ ...props.monitoring }` spread in core. - `import type { Alarm }` (Biome useImportType); drop stale "BYO topic" parenthetical on the us-east-1 encryptionKey doc. - Changeset: document the two escape paths (central topic via monitoring.alarms; Lambda/SQS on the regional topic via alarmTopics) and the warn-and-skip behavior. - Add docs/DECISIONS.md D-006 recording the always-on + warn-and-skip decision and the cross-region-account synth constraint. Tests: hosting 857/857, core 747/747, both builds green.
…wording Reword the D-006/D-016 decision entry, the changeset, and the code/test comments I added for the off-region CloudFront alarm work for clarity and consistency. Renumber the decision entry to D-016 (D-006 and D-015 were already taken) and correct the changeset to describe the warn-and-skip behavior (it no longer hard-throws on an unresolved account). No code behavior change; comments and docs only.
- README off-region parity section described a MonitoringEnvRequiredError throw that the shipped code never raises; reword to the actual behavior (create-locally-and-warn for unresolved region; skip-CF-alarm-and-warn for unresolved account, keeping all other alarms). - Point the code, test, and changeset cross-references at D-016 (the decision was renamed from a colliding D-006, which is the unrelated getMockDataDir decision). - monitoring_construct module docstring: correct the stale "off by default" and bring-your-own-topic prose to match the shipped enabled-by-default default and single auto-created encrypted topic. - Re-indent the UsEast1MonitoringStack block to match its enclosing scope. No behavior change; comments and docs only.
soberm
approved these changes
Sep 14, 2026
svidgen
approved these changes
Sep 18, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #481.
Root cause
AWS/CloudFrontmetrics are published only in us-east-1, and a CloudWatch alarm can only evaluate a metric in its own region — confirmed by the CloudWatch docs ("Cross-Region functionality is not supported for alarms, so you cannot create an alarm in one Region that watches a metric in a different Region") and rejected by aws-cdk-lib at synth.new Metric({ region: 'us-east-1' })is honored for dashboards but throws for alarms.MonitoringConstructcreated theCloudFront5xxRatealarm in the hosting stack's region. For any stack outside us-east-1 the alarm never received a datapoint and, withtreatMissingData: NOT_BREACHING, sat atOKforever instead of alarming — a monitoring alarm that silently never fires.monitoring.enableddefaults totrue, so every off-region hosting stack was affected. (waf_construct.tsalready handles the same CloudFront/us-east-1 constraint; the alarm had no equivalent.)Fix — two-topic design
Off-region, the CloudFront alarm is deferred to a dedicated, hosting-owned us-east-1 support stack (
UsEast1MonitoringStack) that owns its own SNS topic. That topic's ARN is surfaced asMonitoringTopicArnUsEast1(an output of the support stack) for the operator to subscribe to, alongside the regionalMonitoringTopicArn. No forwarder Lambda and no runtime message forwarding.New prop
monitoring.cloudFrontAlarm: 'usEast1Stack' (default) | 'skip''usEast1Stack': create the support stack. Requiresenv: { account, region }off-region (a cross-region stack needs a concrete account). Env-agnostic off-region synth throwsHostingError('MonitoringEnvRequiredError')with resolution guidance.'skip': emit a synth warning, create no second stack (works env-agnostic).Breaking-ish notes for reviewers
env: { account, region }.Tests
monitoring_construct.test.ts: deferral unit tests (createCloudFrontAlarmLocally: false→ no CF alarm +cloudFrontAlarmDeferred, regional alarms still created).hosting_construct.cf_alarm_region.test.ts: off-region two-stack synth (CF alarm in the us-east-1 stack +MonitoringTopicArnUsEast1output, one SNS topic, no forwarder subscription); env-agnostic off-region →MonitoringEnvRequiredError;'skip'mode warning + no second stack; in-region single-stack with local alarm.Do not merge — opened for review.