Add production observability baseline with routed alarms - #910
Merged
Conversation
The stack had 13 CloudWatch alarms and none of them notified anybody. Three
constructs carried a comment saying so. Two of those alarms were worse than
silent: they watched metric names that exist in no CloudWatch namespace, so they
sat in INSUFFICIENT_DATA from the day they were created, which an operator reads
as healthy.
Every alarm now publishes to one SNS topic. 77 alarms, zero unrouted.
Routing is structural, not conventional. AlarmFactory attaches AlarmActions and
OKActions as a consequence of being used at all, so an unrouted alarm requires
deliberately bypassing it, and a source-level test fails the build if anyone
calls new cloudwatch.Alarm() directly. The previous gap was not carelessness —
the broken form was the shorter one.
Verified against the live account rather than documentation:
- The AgentCore alarms used namespace `bedrock-agentcore` with InvocationCount /
InvocationErrors / InvocationLatency. That namespace is real but holds only the
OpenTelemetry/Strands application metrics; those three names exist nowhere.
Corrected to AWS/Bedrock-AgentCore with the verified Resource + Operation +
Name dimensions, split so SystemErrors (AWS's fault) is separate from
UserErrors (ours), plus a new throttle alarm.
- The latency threshold of 30s sat BELOW the observed maximum. Measured over 14
days, turns average 3.0-4.5s with daily peaks to 24.4s, because the chat path
is SSE and the runtime does not finish a request until the stream closes.
Floors now default to 120s. AgentCore Latency is in milliseconds while ALB
TargetResponseTime is in seconds — a 1000x trap in either direction, so both
units were confirmed with get-metric-statistics.
- DynamoDB has never throttled: ReadThrottleEvents, WriteThrottleEvents and
SystemErrors all had zero metric streams, since every table is on-demand.
Account-level UserErrors had live data with nothing watching it. That traded 78
alarms on signals that have never fired for 27 that include one firing today.
- No Cognito failure alarm and no Browser alarm. AWS/Cognito on the ESSENTIALS
feature plan publishes only success metrics, and Browser has zero streams.
Alarming on either would recreate the permanently-green alarm this change
exists to remove. Both omissions are asserted by tests.
New coverage: ALB (6), ECS service (3), DynamoDB (27), Lambda (21), AI path (9),
AgentCore Runtime (4). Plus a {prefix}-platform-health dashboard ordered by
triage rather than service inventory, which links to the two existing dashboards
instead of restating them — keeping the stack at exactly 3, the CloudWatch free
ceiling.
Configuration is 18 single scalars with cost-conscious defaults. No
config.production branching: this repo is forked by many institutions, so a fork
with one environment should not reason about a production boolean and a fork with
three should not be limited to two. Per-environment values live in the forker's
deployment config. Enforced by test. The clearest case is X-Ray sampling, which
was fixedRate 1.0 for any fork that never set production — a recorded trace for
every agent invocation at $5/million. Now 1% by default.
Log retention becomes one configured value across all 15 groups, replacing 14
hardcoded literals (13 ONE_WEEK plus one ONE_MONTH that differed silently). Two
gaps needed more than per-site edits: the AgentCore Runtime's group is created by
the service rather than CloudFormation, so an AwsCustomResource calls
PutRetentionPolicy on it; and CDK creates groups for its own machinery that
default to 731 days and are declared nowhere here, so a LogRetentionAspect
rewrites every group in the tree. The second was found by diffing a real cdk
synth — unit tests missed it because a bare cdk.App lacks the cdk.json feature
flags that materialise those groups.
Resource budget: 382 of CloudFormation's 500-resource limit, up from 308. This is
a deliberate single-stack architecture with nowhere to spill, so the DynamoDB
allocation was decided by measurement rather than by covering every documented
metric. A guard test fails above 460.
Subscriptions are deliberately not infrastructure-as-code. Several teams need to
hear about failures and their membership changes far more often than the
infrastructure does; requiring a PR and a deploy to add one address is how a
notification list goes stale and stops being trusted. A test asserts zero
subscription resources exist so the decision cannot be quietly reversed. The
required post-deploy step is documented in step-05-verify.
Verification: real cdk synth produces 382 resources, 77 alarms with 0 empty
AlarmActions, 15 log groups all at the configured retention. Full suite 781 tests
across 40 suites, sharded 4 ways. Guards proven non-vacuous by planting a file
violating all three rules and confirming three tests fail.
Comment density in the new constructs ran 33-56%, and the platform.yml env block had 22 lines of prose above 18 variables. Most of it explained things a reader already knows or restated decisions that belong in the steering doc. Removed ~980 net comment lines. What stayed is the set of facts someone would otherwise get wrong: CloudWatch cannot publish to an alias/aws/sns-encrypted topic and needs GenerateDataKey* rather than just Decrypt; CloudWatch caps math-expression alarms at 10 metrics while CDK defaults to 14 operations; AgentCore Latency is milliseconds where ALB TargetResponseTime is seconds; Code Interpreter publishes Resource as a bare id where Memory and Gateway use ARNs; UnHealthyHostCount and RunningTaskCount stop being published rather than reporting zero; CDK's own provider log groups default to 731 days. Constructs now sit at 11-27% and tests at 4-17%. No behaviour change. Full suite 781 tests across 40 suites, sharded 4 ways.
Merged
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
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.
The stack had 13 CloudWatch alarms and none of them notified anybody. Three constructs carried a comment saying so. Two of those alarms were worse than silent: they watched metric names that exist in no CloudWatch namespace, so they sat in INSUFFICIENT_DATA from the day they were created, which an operator reads as healthy.
Every alarm now publishes to one SNS topic. 77 alarms, zero unrouted.
Routing is structural, not conventional. AlarmFactory attaches AlarmActions and OKActions as a consequence of being used at all, so an unrouted alarm requires deliberately bypassing it, and a source-level test fails the build if anyone calls new cloudwatch.Alarm() directly. The previous gap was not carelessness — the broken form was the shorter one.
Verified against the live account rather than documentation:
The AgentCore alarms used namespace
bedrock-agentcorewith InvocationCount / InvocationErrors / InvocationLatency. That namespace is real but holds only the OpenTelemetry/Strands application metrics; those three names exist nowhere. Corrected to AWS/Bedrock-AgentCore with the verified Resource + Operation + Name dimensions, split so SystemErrors (AWS's fault) is separate from UserErrors (ours), plus a new throttle alarm.The latency threshold of 30s sat BELOW the observed maximum. Measured over 14 days, turns average 3.0-4.5s with daily peaks to 24.4s, because the chat path is SSE and the runtime does not finish a request until the stream closes. Floors now default to 120s. AgentCore Latency is in milliseconds while ALB TargetResponseTime is in seconds — a 1000x trap in either direction, so both units were confirmed with get-metric-statistics.
DynamoDB has never throttled: ReadThrottleEvents, WriteThrottleEvents and SystemErrors all had zero metric streams, since every table is on-demand. Account-level UserErrors had live data with nothing watching it. That traded 78 alarms on signals that have never fired for 27 that include one firing today.
No Cognito failure alarm and no Browser alarm. AWS/Cognito on the ESSENTIALS feature plan publishes only success metrics, and Browser has zero streams. Alarming on either would recreate the permanently-green alarm this change exists to remove. Both omissions are asserted by tests.
New coverage: ALB (6), ECS service (3), DynamoDB (27), Lambda (21), AI path (9), AgentCore Runtime (4). Plus a {prefix}-platform-health dashboard ordered by triage rather than service inventory, which links to the two existing dashboards instead of restating them — keeping the stack at exactly 3, the CloudWatch free ceiling.
Configuration is 18 single scalars with cost-conscious defaults. No config.production branching: this repo is forked by many institutions, so a fork with one environment should not reason about a production boolean and a fork with three should not be limited to two. Per-environment values live in the forker's deployment config. Enforced by test. The clearest case is X-Ray sampling, which was fixedRate 1.0 for any fork that never set production — a recorded trace for every agent invocation at $5/million. Now 1% by default.
Log retention becomes one configured value across all 15 groups, replacing 14 hardcoded literals (13 ONE_WEEK plus one ONE_MONTH that differed silently). Two gaps needed more than per-site edits: the AgentCore Runtime's group is created by the service rather than CloudFormation, so an AwsCustomResource calls PutRetentionPolicy on it; and CDK creates groups for its own machinery that default to 731 days and are declared nowhere here, so a LogRetentionAspect rewrites every group in the tree. The second was found by diffing a real cdk synth — unit tests missed it because a bare cdk.App lacks the cdk.json feature flags that materialise those groups.
Resource budget: 382 of CloudFormation's 500-resource limit, up from 308. This is a deliberate single-stack architecture with nowhere to spill, so the DynamoDB allocation was decided by measurement rather than by covering every documented metric. A guard test fails above 460.
Subscriptions are deliberately not infrastructure-as-code. Several teams need to hear about failures and their membership changes far more often than the infrastructure does; requiring a PR and a deploy to add one address is how a notification list goes stale and stops being trusted. A test asserts zero subscription resources exist so the decision cannot be quietly reversed. The required post-deploy step is documented in step-05-verify.
Verification: real cdk synth produces 382 resources, 77 alarms with 0 empty AlarmActions, 15 log groups all at the configured retention. Full suite 781 tests across 40 suites, sharded 4 ways. Guards proven non-vacuous by planting a file violating all three rules and confirming three tests fail.