diff --git a/.github/docs/deploy/step-05-verify.md b/.github/docs/deploy/step-05-verify.md index 364f9b56e..5ed01dd52 100644 --- a/.github/docs/deploy/step-05-verify.md +++ b/.github/docs/deploy/step-05-verify.md @@ -102,6 +102,67 @@ In order of likelihood: --- +### 6. Subscribe to Platform Alarms (required — not automated) + +The deploy creates one SNS topic that **every** CloudWatch alarm in the stack +publishes to. It has no subscribers until you add them, so until you do this step +the alarms change colour in the console and tell nobody. + +This is deliberately not infrastructure-as-code. Several teams usually need to +hear about failures, and their membership changes far more often than the +infrastructure does — requiring a pull request, a review, and a CloudFormation +deploy to add one email address is how a notification list goes stale and stops +being trusted. Subscribing is a one-line command that touches no code. + +Find the topic and subscribe: + +```bash +PREFIX="your-project-prefix" # the same CDK_PROJECT_PREFIX you deployed with + +TOPIC=$(aws ssm get-parameter \ + --name "/${PREFIX}/observability/alarm-topic-arn" \ + --query Parameter.Value --output text) + +aws sns subscribe \ + --topic-arn "$TOPIC" \ + --protocol email \ + --notification-endpoint platform-team@example.edu +``` + +AWS sends a confirmation email; the subscription is inactive until the recipient +clicks the link. Repeat for each address or distribution list. + +Other useful protocols: + +| Protocol | Use for | +|---|---| +| `email` | A team distribution list. Simplest, and enough for most forks. | +| `https` | PagerDuty, Opsgenie, ServiceNow, or any webhook receiver. | +| `sms` | Genuine paging. Costs per message. | +| `lambda` | Custom routing, e.g. severity-based fan-out or Slack formatting. | + +Verify it took: + +```bash +aws sns list-subscriptions-by-topic --topic-arn "$TOPIC" \ + --query 'Subscriptions[].[Protocol,Endpoint,SubscriptionArn]' --output table +``` + +A `SubscriptionArn` of `PendingConfirmation` means the email has not been +confirmed yet. + +Then open the health dashboard — `{PREFIX}-platform-health` in the CloudWatch +console — and confirm the alarm-status row is populated and green. Row 1 tells +you whether traffic is being served, row 2 tells you why, and row 3 lists every +alarm's current state. + +> **Note on latency alarms:** the chat path uses server-sent events, so response +> times of tens of seconds are normal for a healthy agent turn. Latency alarms are +> deliberately set at 120 seconds. A *drop* in latency can actually mean turns are +> failing early. + +--- + ## You're Done! Your AgentCore Public Stack is deployed and running. Here's what you have: diff --git a/.github/workflows/platform.yml b/.github/workflows/platform.yml index f9bd1c17c..37d1f6298 100644 --- a/.github/workflows/platform.yml +++ b/.github/workflows/platform.yml @@ -207,6 +207,26 @@ jobs: # (default 100). Leave unset to take those defaults. CDK_MANAGED_KB_STORAGE_ALARM_GB: ${{ vars.CDK_MANAGED_KB_STORAGE_ALARM_GB }} CDK_MANAGED_KB_DAILY_COST_ALARM_USD: ${{ vars.CDK_MANAGED_KB_DAILY_COST_ALARM_USD }} + # Observability. All optional — unset means the default in config.ts. + # XRAY_SAMPLING_RATE is a rate (0.0-1.0), not a percentage. + CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED: ${{ vars.CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED }} + CDK_OBSERVABILITY_LOG_RETENTION_DAYS: ${{ vars.CDK_OBSERVABILITY_LOG_RETENTION_DAYS }} + CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD }} + CDK_OBSERVABILITY_ALB_P99_LATENCY_MS: ${{ vars.CDK_OBSERVABILITY_ALB_P99_LATENCY_MS }} + CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS: ${{ vars.CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS }} + CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD }} + CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD }} + CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT: ${{ vars.CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT }} + CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD }} + CDK_OBSERVABILITY_ECS_CPU_PERCENT: ${{ vars.CDK_OBSERVABILITY_ECS_CPU_PERCENT }} + CDK_OBSERVABILITY_ECS_MEMORY_PERCENT: ${{ vars.CDK_OBSERVABILITY_ECS_MEMORY_PERCENT }} + CDK_OBSERVABILITY_XRAY_SAMPLING_RATE: ${{ vars.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE }} + CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR: ${{ vars.CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR }} + CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS: ${{ vars.CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS }} + CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED: ${{ vars.CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED }} + CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD }} + CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD }} + CDK_OBSERVABILITY_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD: ${{ vars.CDK_OBSERVABILITY_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD }} # Secrets AWS_ROLE_ARN: ${{ secrets.AWS_ROLE_ARN }} AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} diff --git a/.kiro/steering/observability.md b/.kiro/steering/observability.md new file mode 100644 index 000000000..6205c7819 --- /dev/null +++ b/.kiro/steering/observability.md @@ -0,0 +1,317 @@ +--- +inclusion: fileMatch +fileMatchPattern: ["infrastructure/lib/constructs/observability/*", "infrastructure/test/observability-*"] +--- + +# Observability + +Every CloudWatch alarm in `PlatformStack` publishes to one SNS topic. This doc +covers the rules that keep that true, the gotchas that silently break alarms, and +what to do when one fires. + +## 0. The failure this system was built to prevent + +Before this work the stack had 13 alarms and **not one of them notified anybody**. +Three separate constructs carried a comment saying so. Two of those alarms were +worse than silent: they watched metric names that exist in no CloudWatch +namespace at all, so they sat in `INSUFFICIENT_DATA` from the day they were +created — which an operator reads as *healthy*. + +Both failures share a shape: **nothing errored.** The alarm deployed, evaluated, +and turned green. Every rule below exists because this domain fails quietly, and +quiet failure has to be caught by structure or by a test, never by remembering. + +## 1. Never call `new cloudwatch.Alarm()` — use `AlarmFactory` + +```typescript +const alarms = new AlarmFactory(this, config, props.alarmTopic); + +alarms.alarm('MyAlarmLogicalId', { + name: 'my-service-errors', // NOT alarmName; prefix is applied for you + alarmDescription: 'What broke, and what the first response should be', + metric: someMetric, + threshold: config.observability.someThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, +}); +``` + +The factory attaches both `AlarmActions` and `OKActions` as a consequence of +being used at all. `new cloudwatch.Alarm()` produces a console-only alarm that +looks completely finished, which is why the rule is enforced by a source-level +test rather than convention: + +- `observability-alarm-routing.test.ts` fails if any file under `lib/` calls the + constructor directly (except the factory itself), and fails if any alarm in the + synthesized template lacks actions. + +Use `expressionAlarm()` for metric math so the routing guarantee survives. + +## 2. No `config.production` in observability code + +This repo is forked by many institutions. A fork with one environment should not +have to reason about a `production` boolean, and a fork with three should not be +limited to two. Every tunable is a **single scalar** in `ObservabilityConfig`, +and per-environment differences live in the forker's own deployment config +(GitHub Variables scoped to a GitHub Environment) — not in a ternary here. + +Enforced: `observability-alarm-routing.test.ts` fails on any `config.production` +under `lib/constructs/observability/`. + +Adding a tunable means touching five places. Miss the third and the flag is +accepted then silently ignored: + +1. `OBSERVABILITY_DEFAULT_*` constant in `config.ts`, with the reasoning inline +2. field on `ObservabilityConfig` +3. loader entry using the full precedence chain (see §3) +4. `scripts/common/load-env.sh` → `build_cdk_context_params()` +5. job-level `env:` in `.github/workflows/platform.yml` + +Defaults are **cost-conscious**: they are what a fork inherits when it configures +nothing, so they are the cheapest setting that still leaves alerting useful. +Diagnostic depth is opt-in. The X-Ray sampling default is the clearest case — it +was `1.0` for any fork that never set `production`, meaning a recorded trace for +every single agent invocation at $5/million. + +## 3. The flat dotted context key (this has bitten the repo three times) + +`--context observability.logRetentionDays=90` sets the **flat** key +`context['observability.logRetentionDays']`. It does **not** build a nested +object. Reading only the nested form accepts the operator's flag and ignores it. + +```typescript +logRetentionDays: + parseIntEnv(process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS) + ?? parseIntEnv(scope.node.tryGetContext('observability.logRetentionDays')) // ← flat + ?? scope.node.tryGetContext('observability')?.logRetentionDays // ← nested + ?? OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS, +``` + +Use `parseFloatEnv` for fractional values. `parseIntEnv('0.05')` is `0`, which +would switch X-Ray sampling off entirely rather than setting it to 5%. + +## 4. Gotchas that produce silently-broken alarms + +### The SNS topic must use a customer-managed KMS key + +CloudWatch **cannot** publish to a topic encrypted with the AWS-managed +`alias/aws/sns` key: the publish is made by the `cloudwatch.amazonaws.com` +service principal, and an AWS-managed key's policy cannot be edited to grant it +`kms:GenerateDataKey*`. The alarm goes to ALARM, the console shows it firing, and +the notification is dropped. + +`kms:Decrypt` alone is **not enough** — SNS envelope encryption has the +*publisher* generate the data key, so `GenerateDataKey*` is required too. + +### Dimensions must come from real resources + +A `CPUUtilization` alarm with no dimensions is a valid CloudWatch alarm that +silently averages every ECS service in the account. Same for ALB metrics. Always +derive dimensions from the CDK resource (`service.metricCpuUtilization()`, +`targetGroup.metrics.*`, `table.metric()`), never from a name string. + +### Metric-math alarms cap at 10 metrics + +CloudWatch rejects an alarm whose math expression contains more than 10 +individual metrics. CDK's `table.metricSystemErrorsForOperations()` defaults to +**all 14** DynamoDB operations and throws `TooManyMetricsInMathExpression` at +synth. Pass an explicit operations list. + +Also: CDK deprecates `metricThrottledRequests()` and `metricSystemErrors()` as +returning invalid metrics. Use `table.metric('ReadThrottleEvents')` etc. + +### Don't pass `label` to a metric used in an alarm + +It forces CDK to render the alarm as a `Metrics[]` array instead of flat +`Namespace`/`MetricName`/`ExtendedStatistic` properties. CloudWatch labels +percentile series adequately on its own. + +### Units differ between services + +| Metric | Unit | Threshold handling | +|---|---|---| +| AgentCore `Latency` | **Milliseconds** | use `agentCoreLatencyMs` directly | +| ALB `TargetResponseTime` | **Seconds** | divide by 1000 | + +Both verified with `get-metric-statistics`. Getting this wrong is a 1000x error +in either direction, and neither direction fails loudly. + +## 5. Streaming makes latency a weak signal + +The chat path is SSE. The ALB does not consider a request complete until the +stream closes, so `TargetResponseTime` and AgentCore `Latency` are legitimately +tens of seconds for a healthy turn — and a sudden *drop* can mean turns are +failing early. + +Measured over 14 days in dev: average turn **3.0–4.5s**, daily maxima **16.7s, +16.9s, 24.4s**. The original alarm threshold was 30s, i.e. *below* the observed +maximum, so a healthy long turn could trip it. Latency floors default to 120s. + +Reliable signals on this path are the discrete ones: 5xx counts, unhealthy hosts, +rejected connections, throttles. + +## 6. `treatMissingData` is a per-metric decision + +Never defaulted by the factory, because both answers are correct somewhere: + +- **`NOT_BREACHING`** for error and throttle counts. A service that is not + failing publishes nothing, so absent data is the healthy state. +- **`BREACHING`** for `UnHealthyHostCount` and `RunningTaskCount`. These stop + being published when the service is at zero or deleted. `NOT_BREACHING` would + leave the alarm silent during a total outage — the exact case it exists for. + +## 7. Verify metrics exist before alarming on them + +Documentation is not sufficient evidence that a metric is published, and an alarm +on a non-existent metric is indistinguishable from a healthy one. + +```bash +aws cloudwatch list-metrics --namespace "AWS/Bedrock-AgentCore" \ + | jq -r '.Metrics | group_by(.MetricName) | .[] | + "\(.[0].MetricName) dims=\(map(.Dimensions|map(.Name)|sort)|unique|tostring)"' +``` + +Known facts from that sweep, all pinned by tests: + +- Runtime metrics live in `AWS/Bedrock-AgentCore` (hyphenated), dimensioned + `Resource` + `Operation=InvokeAgentRuntime` + `Name={runtimeName}::DEFAULT`. + The lowercase `bedrock-agentcore` namespace is real but holds only the + OpenTelemetry/Strands *application* metrics. +- **Memory and Gateway publish `Resource` as a full ARN; Code Interpreter + publishes a bare ID.** Passing an ARN for Code Interpreter yields an alarm that + matches nothing. +- `AWS/Cognito` on the **ESSENTIALS** feature plan publishes *only* success + metrics. There is no sign-in failure or throttle metric to alarm on — failure + and threat metrics require the **Plus** plan. The auth-path failure signal is + the token-enrichment Lambda's `Errors` metric instead. +- AgentCore Browser has zero metric streams (provisioned, unused). + +A metric absent from `list-metrics` may simply never have fired. Alarming on it +is still correct — `NOT_BREACHING` keeps it quiet until the first occurrence. + +## 8. Resource budget + +CloudFormation caps a stack at **500 resources**, and this is a deliberate +single-stack architecture with nowhere to spill. Alarms are the largest +discretionary consumer. + +Measured: **308** resources before this work, **~370** after. A guard in +`observability-dynamodb-alarms.test.ts` fails the build above 460, so the ceiling +surfaces while there is still room to react rather than as a failed deploy. + +When budget matters, decide with data rather than by covering every documented +metric. The DynamoDB allocation was cut from 78 alarms to 27 because a live sweep +showed `ReadThrottleEvents`, `WriteThrottleEvents` and `SystemErrors` had **zero +streams** — all tables are on-demand, so throttling had never occurred — while +account-level `UserErrors` had real data nobody was watching. + +Dashboards: the first **3 are free**, then $3/month each. The stack is at exactly +3, which is why the platform dashboard links to the other two instead of +restating their widgets. + +## 9. Log retention + +One value, `observability.logRetentionDays`, applied through +`logRetentionFor(config)`. A source guard fails the build if any construct +hardcodes `retention: logs.RetentionDays.*`. + +The AgentCore Runtime's log group is created by the **AgentCore service**, not +CloudFormation, so a CDK `LogGroup` cannot set its retention — declaring one +would collide on create or manage a second, empty group. An `AwsCustomResource` +calls `logs:PutRetentionPolicy` instead. That API is idempotent *and* creates the +group if absent, which matters on a first deploy when the runtime exists but has +never been invoked. There is deliberately **no `onDelete`**: removing the +retention policy on teardown would revert the group to "keep forever", which is +the cost problem it fixes. + +## 10. Subscriptions are not infrastructure-as-code + +The topic is created by CDK; **subscribers are not**. Several teams need to hear +about failures and their membership changes far more often than the +infrastructure does. Requiring a PR, a review, and a CloudFormation deploy to add +one address is how a notification list goes stale and stops being trusted. + +```bash +TOPIC=$(aws ssm get-parameter --name "/${PREFIX}/observability/alarm-topic-arn" \ + --query Parameter.Value --output text) +aws sns subscribe --topic-arn "$TOPIC" \ + --protocol email --notification-endpoint team@example.edu +``` + +A test asserts zero `AWS::SNS::Subscription` resources exist, so this decision +cannot be quietly reversed. + +## 11. Runbook — first response by alarm + +| Alarm | What it means | First action | +|---|---|---| +| `alb-unhealthy-hosts` | Targets failing health checks, **or none reporting** | `aws ecs describe-services` — are tasks running at all? Then app-api logs. | +| `alb-elb-5xx` | The load balancer itself could not serve | Almost always no healthy target. Check the alarm above first. | +| `alb-target-5xx` | App is reachable and erroring | app-api logs. This is application code. | +| `alb-rejected-connections` | ALB connection limit hit | Users were turned away *before* reaching the app, so nothing is in app logs. Check request volume. | +| `app-api-running-tasks-low` | Fewer tasks than desired | Task failing to start: check stopped-task reason, image pull, subnet IPs. | +| `app-api-memory-high` | Sustained memory pressure | Fargate **kills** a task that exhausts memory. Raise memory or find the leak. | +| `agentcore-system-errors` | AgentCore's fault | Escalate to AWS. Not application code. | +| `agentcore-high-error-rate` | `UserErrors` — our requests are malformed | Recent inference-api deploy? Check payload shape and IAM. | +| `agentcore-throttles` | At the TPS or session quota | Request a quota increase. Will not self-resolve. | +| `agentcore-high-latency` | p99 above 120s | Genuinely hung, not merely slow — 24s is a normal maximum here. | +| `bedrock-tpm-quota-usage` | **Leading** indicator | Request a quota increase *now*, before throttling starts. | +| `bedrock-invocation-throttles` | At a model's TPM/RPM quota | Users see chats that never respond. Quota increase. | +| `agentcore-memory-*` | Memory hot path failing | Users experience an agent that has forgotten the conversation. | +| `agentcore-gateway-*` | MCP calls failing at the gateway | Agents lose tool access. Check gateway targets. | +| `ddb-*-throttle` | Named table throttling | On-demand, so this is a hot partition or an account limit. Compare `ReadThrottleEvents` vs `WriteThrottleEvents` on that table. | +| `ddb-user-errors` | DynamoDB rejecting our requests (4xx) | Application code misusing the API. Account-wide, so use CloudTrail or app logs to find the caller. | +| `lambda-token-enrichment-errors` | **Silent** degradation | Handler is fail-open: logins still work, but MCP tools are losing user-identity claims. | +| `dlq-kb-ingestion-not-empty` | Work accepted then failed every retry | Will **not** self-clear. Inspect, fix, then replay or drain. | +| `prompt-cache-session-partial-miss` | One conversation re-writing its prefix every turn | Use the "Sessions by partial-miss waste" widget to find which session. | + +Start at the **`{prefix}-platform-health`** dashboard: row 1 says whether traffic +is being served, row 2 says why, row 3 shows every alarm's current state. + +## 12. Boise State's own profile + +The committed defaults are what a **fork** should inherit. Boise State authors +this platform and does far more diagnostic work than any deployer of it, so our +values differ — and they live in GitHub Variables scoped to a GitHub Environment, +never in committed code. That separation is what lets both be right at once. + +`scripts/observability/set-bsu-overrides.sh --env ` +applies them. It is **not run by CI**, makes no AWS changes, and requires +confirmation, because it mutates shared repository configuration. Add `--dry-run` +to print the plan (no `gh` auth needed). + +| Field | OSS default | BSU dev | BSU prod | +|---|---|---|---| +| `xraySamplingRate` | `0.01` | `0.5` | `0.1` | +| `xraySamplingReservoir` | `1` | `5` | `2` | +| `xrayInsightsNotifications` | `false` | `true` | `true` | +| `agentCoreApplicationLogsEnabled` | `false` | `true` | **`false`** | +| `logRetentionDays` | `30` | `14` | `90` | +| `agentCoreErrorThreshold` | `10` | `5` | `5` | +| `lambdaErrorThreshold` | `5` | `1` | `3` | +| `albTarget5xxThreshold` | `10` | `5` | `5` | +| `dynamoThrottleThreshold` | `10` | `1` | default | +| `ecsCpuPercent` / `ecsMemoryPercent` | `80` / `85` | default | `75` / `80` | +| `promptCacheAvoidableMissThreshold` | `10` | `5` | default | +| `promptCacheWastedUsdThreshold` | `1` | `0.5` | default | + +Two choices worth understanding rather than copying: + +- **Dev traces at 50%, prod at 10%.** Not a mistake. X-Ray bills per trace + recorded, and prod traffic is orders of magnitude larger, so 10% of prod is far + more traces than 50% of dev. Dev is where we debug; prod is where we pay. +- **`agentCoreApplicationLogsEnabled` stays OFF in production.** Those records + carry every user's prompt and the model's response verbatim — the + highest-volume log source available and a genuine PII surface. Enable it + temporarily for a specific investigation, then turn it back off. + +Retention inverts between the two for the same reason latency floors are high: +dev keeps 14 days because dev noise is not worth a month, prod keeps 90 because a +real incident review reaches back weeks. + +**Verifying a variable took effect.** The synth log prints a line beginning +`Observability:` with the *resolved* values. If it disagrees with the variable you +set, the value is not reaching `--context` — check the deploy job's **job-level** +`env:` block, since `vars.*` in a workflow-level `env:` silently resolves to an +empty string. diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f158e59..9f9f86a6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,40 @@ All notable changes to this project are documented in this file. Format follows For narrative release notes written for operators and product owners, see [RELEASE_NOTES.md](RELEASE_NOTES.md). +## [Unreleased] + +Production observability baseline. Every CloudWatch alarm in the stack now publishes to a single SNS topic — before this, the stack had 13 alarms and **none of them notified anybody**, and two of those watched metric names that exist in no CloudWatch namespace, so they had sat in `INSUFFICIENT_DATA` since creation and read as healthy. **Requires a CDK deploy**, and one manual step after it: subscribe your team to the new alarm topic (see [step-05-verify](.github/docs/deploy/step-05-verify.md#6-subscribe-to-platform-alarms-required--not-automated)). Subscriptions are deliberately not infrastructure-as-code. + +### 🚀 Added + +- **Single SNS alarm topic** (`{prefix}-alarms`) that every alarm routes to, encrypted with a customer-managed KMS key. The CMK is required, not a preference: CloudWatch cannot publish to a topic encrypted with the AWS-managed `alias/aws/sns` key, and that failure is silent — the alarm fires, the console shows it, the notification is dropped. Topic ARN published to SSM at `/{prefix}/observability/alarm-topic-arn` and as a CfnOutput +- **`AlarmFactory`** — the only sanctioned way to create an alarm. Attaches `AlarmActions` *and* `OKActions` as a consequence of being used, so an unrouted alarm now requires deliberately bypassing it. A source-level test fails the build if any file under `lib/` calls `new cloudwatch.Alarm()` directly +- **ALB alarms** (6): ELB 5xx, target 5xx, unhealthy hosts, target connection errors, rejected connections, and a streaming-aware p99 latency floor +- **ECS service alarms** (3): CPU, memory, and running-task-count below desired +- **DynamoDB alarms** (27): a combined read+write throttle alarm per table naming that table, plus one account-level `UserErrors` alarm +- **Lambda alarms** (21): errors and throttles across every runtime function, including `artifact-render`, `rag-ingestion` and the four kb-migration functions which previously had none, plus dead-letter-queue depth on the kb-ingestion DLQ +- **AI-path alarms** (9): Bedrock invocation throttles, server errors and `EstimatedTPMQuotaUsage` (the only *leading* indicator in the set — visible before throttling starts), AgentCore Memory hot-path errors and throttles, Gateway MCP errors and throttles, Code Interpreter session errors and concurrent-session count +- **`{prefix}-platform-health` dashboard** — one pane answering "is the platform healthy right now": traffic and errors, then saturation, then every alarm's current state. Links to the two existing dashboards rather than restating them, which keeps the stack at exactly 3 (CloudWatch's free ceiling) +- **`observability` configuration section** — 18 single-scalar tunables with `CDK_OBSERVABILITY_*` overrides, cost-conscious defaults, and validation that rejects retention values CloudWatch does not accept and X-Ray sampling rates given as percentages +- `.kiro/steering/observability.md` — the gotchas that silently break alarms, plus a first-response runbook for every alarm + +### 🐛 Fixed + +- **The two AgentCore Runtime alarms were watching metrics that do not exist.** They used namespace `bedrock-agentcore` with `InvocationCount` / `InvocationErrors` / `InvocationLatency`. Verified against the live account: that namespace is real but holds only the OpenTelemetry/Strands *application* metrics, and those three names exist in no namespace at all. Corrected to `AWS/Bedrock-AgentCore` with the verified `Resource` + `Operation` + `Name` dimension set, and split into four alarms — `SystemErrors` (AWS's fault) separated from `UserErrors` (ours), plus a new throttle alarm +- **The AgentCore latency alarm would have fired on healthy traffic.** Its 30-second threshold sat *below* the observed maximum: measured over 14 days, average turns run 3.0–4.5s with daily maxima reaching 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 +- **The `agentcore-observability` dashboard's token-usage widget was always empty** — `InputTokens`/`OutputTokens` do not exist, and the token metrics that do exist in that namespace are Memory-strategy counters dimensioned by `StrategyId`, not model tokens. Removed; the header now points at the prompt-cache dashboard for real token accounting +- **X-Ray recorded a trace for every single agent invocation** in any deployment that never set `production` — `fixedRate` was `1.0` with a 50/sec reservoir on that branch, at $5 per million traces recorded. Now a single configured value defaulting to 1% with a 1/sec reservoir +- **The AgentCore Runtime's log group had no retention policy and grew forever.** It is created by the AgentCore service rather than CloudFormation, so a CDK `LogGroup` cannot set it. An `AwsCustomResource` calls `logs:PutRetentionPolicy` instead — idempotent, and it creates the group if the runtime has not yet been invoked + +### ⚠️ Changed + +- **Log retention is one configured value** (`observability.logRetentionDays`, default 30) applied to all 14 log groups through `logRetentionFor(config)`. Previously every construct hardcoded `ONE_WEEK`, except AgentCore Memory which used `ONE_MONTH` — differing silently rather than deliberately. A source guard fails the build on any hardcoded `RetentionDays` +- **No `config.production` branching in observability code.** This repo is forked by many institutions: a fork with one environment should not have to reason about a `production` boolean, and a fork with three should not be limited to two. Per-environment differences now live in the forker's deployment config as single values. Enforced by test + +### 📚 Docs + +- Deploy guide gains a required post-deploy step for subscribing to the alarm topic, with protocol options and verification + ## [1.16.0] - 2026-08-28 Minor release on knowledge bases, the marketplace review flow, and fine-tuning. The Bedrock Managed Knowledge Base migration lands **inert** — every managed-KB flag is default OFF, and the owner-facing upgrade card only appears where `CDK_MANAGED_KB_MIGRATION_ENABLED=true`. Marketplace admins can finally read, test-drive and decline a submission instead of approving on a name and a category alone. Fine-tuning was unreachable in every deployed environment and now isn't — it becomes **reachable by default** on this deploy, with `CDK_FINE_TUNING_ENABLED=false` as the kill switch. Two live RAG behaviours change regardless of flags: queries clamp at 10,000 characters on both backends, and the document-status filter now fails **closed**. **Requires a CDK deploy** — `platform.yml` (set `CDK_TAG_ENVIRONMENT` first), then `backend.yml` (two new kb-migration jobs), then `frontend-deploy.yml`. diff --git a/infrastructure/lib/config.ts b/infrastructure/lib/config.ts index 8cc8e19de..b724d4cde 100644 --- a/infrastructure/lib/config.ts +++ b/infrastructure/lib/config.ts @@ -66,6 +66,7 @@ export interface AppConfig { * AppConfig by hand does not have to know this feature exists. */ tokenExchange?: TokenExchangeConfig; + observability: ObservabilityConfig; appVersion: string; tags: { [key: string]: string }; } @@ -422,6 +423,85 @@ export interface TokenExchangeConfig { clientId: string; } +// Observability defaults. Tuned for cost: these are what a fork inherits when it +// configures nothing. See .kiro/steering/observability.md. + +/** Retention for every log group in the stack. */ +export const OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS = 30; + +/** X-Ray sampling rate, 0.0-1.0. Billed per trace recorded, so keep it low. */ +export const OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE = 0.01; + +/** Traces per second recorded before the sampling rate applies. */ +export const OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RESERVOIR = 1; + +/** ALB target 5xx per 5-minute period. */ +export const OBSERVABILITY_DEFAULT_ALB_TARGET_5XX_THRESHOLD = 10; + +/** p99 latency floor (ms). High because the chat path is SSE: a healthy turn + * runs for seconds and peaks around 25s, so a tight threshold only makes noise. */ +export const OBSERVABILITY_DEFAULT_P99_LATENCY_MS = 120_000; + +/** AgentCore Runtime errors per 5-minute period. */ +export const OBSERVABILITY_DEFAULT_AGENTCORE_ERROR_THRESHOLD = 10; + +/** Lambda errors per 5-minute period. */ +export const OBSERVABILITY_DEFAULT_LAMBDA_ERROR_THRESHOLD = 5; + +/** Lambda duration alarm as a percentage of the function's own timeout. */ +export const OBSERVABILITY_DEFAULT_LAMBDA_DURATION_PERCENT_OF_TIMEOUT = 80; + +/** DynamoDB throttle events per 5-minute period. */ +export const OBSERVABILITY_DEFAULT_DYNAMO_THROTTLE_THRESHOLD = 10; + +/** ECS service CPU / memory utilisation alarm thresholds (percent). */ +export const OBSERVABILITY_DEFAULT_ECS_CPU_PERCENT = 80; +export const OBSERVABILITY_DEFAULT_ECS_MEMORY_PERCENT = 85; + +/** Avoidable prompt-cache misses per 5-minute period. */ +export const OBSERVABILITY_DEFAULT_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD = 10; + +/** Dollars of fleet prompt-cache waste per 5-minute period. */ +export const OBSERVABILITY_DEFAULT_PROMPT_CACHE_WASTED_USD_THRESHOLD = 1; + +/** Cumulative partial-miss waste for one session, in dollars. A fleet sum + * cannot see a single conversation re-writing its prefix every turn. */ +export const OBSERVABILITY_DEFAULT_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD = 5; + +/** + * Observability configuration. + * + * Precedence per field: CDK_OBSERVABILITY_* env var, then the flat dotted + * context key, then a nested `observability` object, then the default constant. + */ +export interface ObservabilityConfig { + /** Create the SNS alarm topic and route every alarm to it. */ + alarmTopicEnabled: boolean; + logRetentionDays: number; + albTarget5xxThreshold: number; + /** ALB p99 TargetResponseTime floor, in ms. */ + albP99LatencyMs: number; + /** AgentCore Runtime p99 Latency floor, in ms. */ + agentCoreLatencyMs: number; + agentCoreErrorThreshold: number; + lambdaErrorThreshold: number; + lambdaDurationPercentOfTimeout: number; + dynamoThrottleThreshold: number; + ecsCpuPercent: number; + ecsMemoryPercent: number; + + promptCacheAvoidableMissThreshold: number; + promptCacheWastedUsdThreshold: number; + promptCacheSessionWastedUsdThreshold: number; + + xraySamplingRate: number; + xraySamplingReservoir: number; + xrayInsightsNotifications: boolean; + /** AgentCore APPLICATION_LOGS vended delivery. Off by default: the records + * carry full prompts and responses, so it is both high-volume and PII. */ + agentCoreApplicationLogsEnabled: boolean; +} + /** * Load and validate configuration from CDK context * @param scope The CDK construct scope @@ -782,6 +862,102 @@ export function loadConfig(scope: cdk.App): AppConfig { clientId: tokenExchangeClientId, } : undefined, + // Same precedence as managedKb above. The flat dotted read at step 2 is + // load-bearing: `--context observability.x=y` sets context['observability.x'], + // it does NOT build a nested object. + observability: { + alarmTopicEnabled: + parseBooleanEnv(process.env.CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED) + ?? parseBooleanEnv(scope.node.tryGetContext('observability.alarmTopicEnabled')) + ?? scope.node.tryGetContext('observability')?.alarmTopicEnabled + ?? true, + logRetentionDays: + parseIntEnv(process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS) + ?? parseIntEnv(scope.node.tryGetContext('observability.logRetentionDays')) + ?? scope.node.tryGetContext('observability')?.logRetentionDays + ?? OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS, + albTarget5xxThreshold: + parseIntEnv(process.env.CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD) + ?? parseIntEnv(scope.node.tryGetContext('observability.albTarget5xxThreshold')) + ?? scope.node.tryGetContext('observability')?.albTarget5xxThreshold + ?? OBSERVABILITY_DEFAULT_ALB_TARGET_5XX_THRESHOLD, + albP99LatencyMs: + parseIntEnv(process.env.CDK_OBSERVABILITY_ALB_P99_LATENCY_MS) + ?? parseIntEnv(scope.node.tryGetContext('observability.albP99LatencyMs')) + ?? scope.node.tryGetContext('observability')?.albP99LatencyMs + ?? OBSERVABILITY_DEFAULT_P99_LATENCY_MS, + agentCoreLatencyMs: + parseIntEnv(process.env.CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS) + ?? parseIntEnv(scope.node.tryGetContext('observability.agentCoreLatencyMs')) + ?? scope.node.tryGetContext('observability')?.agentCoreLatencyMs + ?? OBSERVABILITY_DEFAULT_P99_LATENCY_MS, + agentCoreErrorThreshold: + parseIntEnv(process.env.CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD) + ?? parseIntEnv(scope.node.tryGetContext('observability.agentCoreErrorThreshold')) + ?? scope.node.tryGetContext('observability')?.agentCoreErrorThreshold + ?? OBSERVABILITY_DEFAULT_AGENTCORE_ERROR_THRESHOLD, + lambdaErrorThreshold: + parseIntEnv(process.env.CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD) + ?? parseIntEnv(scope.node.tryGetContext('observability.lambdaErrorThreshold')) + ?? scope.node.tryGetContext('observability')?.lambdaErrorThreshold + ?? OBSERVABILITY_DEFAULT_LAMBDA_ERROR_THRESHOLD, + lambdaDurationPercentOfTimeout: + parseIntEnv(process.env.CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT) + ?? parseIntEnv(scope.node.tryGetContext('observability.lambdaDurationPercentOfTimeout')) + ?? scope.node.tryGetContext('observability')?.lambdaDurationPercentOfTimeout + ?? OBSERVABILITY_DEFAULT_LAMBDA_DURATION_PERCENT_OF_TIMEOUT, + dynamoThrottleThreshold: + parseIntEnv(process.env.CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD) + ?? parseIntEnv(scope.node.tryGetContext('observability.dynamoThrottleThreshold')) + ?? scope.node.tryGetContext('observability')?.dynamoThrottleThreshold + ?? OBSERVABILITY_DEFAULT_DYNAMO_THROTTLE_THRESHOLD, + ecsCpuPercent: + parseIntEnv(process.env.CDK_OBSERVABILITY_ECS_CPU_PERCENT) + ?? parseIntEnv(scope.node.tryGetContext('observability.ecsCpuPercent')) + ?? scope.node.tryGetContext('observability')?.ecsCpuPercent + ?? OBSERVABILITY_DEFAULT_ECS_CPU_PERCENT, + ecsMemoryPercent: + parseIntEnv(process.env.CDK_OBSERVABILITY_ECS_MEMORY_PERCENT) + ?? parseIntEnv(scope.node.tryGetContext('observability.ecsMemoryPercent')) + ?? scope.node.tryGetContext('observability')?.ecsMemoryPercent + ?? OBSERVABILITY_DEFAULT_ECS_MEMORY_PERCENT, + promptCacheAvoidableMissThreshold: + parseIntEnv(process.env.CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD) + ?? parseIntEnv(scope.node.tryGetContext('observability.promptCacheAvoidableMissThreshold')) + ?? scope.node.tryGetContext('observability')?.promptCacheAvoidableMissThreshold + ?? OBSERVABILITY_DEFAULT_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD, + promptCacheWastedUsdThreshold: + parseFloatEnv(process.env.CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD) + ?? parseFloatEnv(scope.node.tryGetContext('observability.promptCacheWastedUsdThreshold')) + ?? scope.node.tryGetContext('observability')?.promptCacheWastedUsdThreshold + ?? OBSERVABILITY_DEFAULT_PROMPT_CACHE_WASTED_USD_THRESHOLD, + promptCacheSessionWastedUsdThreshold: + parseFloatEnv(process.env.CDK_OBSERVABILITY_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD) + ?? parseFloatEnv(scope.node.tryGetContext('observability.promptCacheSessionWastedUsdThreshold')) + ?? scope.node.tryGetContext('observability')?.promptCacheSessionWastedUsdThreshold + ?? OBSERVABILITY_DEFAULT_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD, + // parseFloatEnv: parseIntEnv turns 0.05 into 0, disabling sampling. + xraySamplingRate: + parseFloatEnv(process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE) + ?? parseFloatEnv(scope.node.tryGetContext('observability.xraySamplingRate')) + ?? scope.node.tryGetContext('observability')?.xraySamplingRate + ?? OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE, + xraySamplingReservoir: + parseIntEnv(process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR) + ?? parseIntEnv(scope.node.tryGetContext('observability.xraySamplingReservoir')) + ?? scope.node.tryGetContext('observability')?.xraySamplingReservoir + ?? OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RESERVOIR, + xrayInsightsNotifications: + parseBooleanEnv(process.env.CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS) + ?? parseBooleanEnv(scope.node.tryGetContext('observability.xrayInsightsNotifications')) + ?? scope.node.tryGetContext('observability')?.xrayInsightsNotifications + ?? false, + agentCoreApplicationLogsEnabled: + parseBooleanEnv(process.env.CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED) + ?? parseBooleanEnv(scope.node.tryGetContext('observability.agentCoreApplicationLogsEnabled')) + ?? scope.node.tryGetContext('observability')?.agentCoreApplicationLogsEnabled + ?? false, + }, tags: { ...(scope.node.tryGetContext('tags') || {}), // `--context tags.Environment=dev` sets the FLAT dotted key @@ -824,6 +1000,14 @@ export function loadConfig(scope: cdk.App): AppConfig { console.log(` Retain Data on Delete: ${config.retainDataOnDelete}`); console.log(` Manage DNS Records: ${config.manageDnsRecords}`); console.log(` App Version: ${config.appVersion}`); + // Printed so a deploy log shows which values actually took effect. + console.log( + ` Observability: alarmTopic=${config.observability.alarmTopicEnabled}` + + ` logRetentionDays=${config.observability.logRetentionDays}` + + ` xraySamplingRate=${config.observability.xraySamplingRate}` + + ` xrayReservoir=${config.observability.xraySamplingReservoir}` + + ` agentCoreAppLogs=${config.observability.agentCoreApplicationLogsEnabled}` + ); // Validate configuration validateConfig(config); @@ -876,6 +1060,23 @@ function parseIntEnv(value: string | undefined): number | undefined { return isNaN(parsed) ? undefined : parsed; } +/** + * Parse a floating-point environment/context value. + * + * Separate from parseIntEnv because the fractional observability tunables + * (notably the X-Ray sampling rate) round to 0 under parseInt — "0.05" would + * become 0 and switch sampling off entirely rather than setting it to 5%. + * Returns undefined for unset/empty/invalid input so nullish coalescing can + * fall through to a context value or default. + */ +function parseFloatEnv(value: string | undefined): number | undefined { + if (value === undefined || value === '') { + return undefined; + } + const parsed = parseFloat(value); + return isNaN(parsed) ? undefined : parsed; +} + /** * Parse a JSON object of string->string from an environment variable. * @@ -1061,6 +1262,45 @@ function validateConfig(config: AppConfig): void { // and the respective certificate ARNs for a real deployment. Synth and // tests proceed without them (constructs handle the undefined case by // falling back to CloudFront default domains). + + // ── Observability ── + // CloudWatch Logs accepts only a fixed set of retention values; an arbitrary + // number is rejected at deploy time, long after CI has gone green. + const validRetentionDays = [ + 1, 3, 5, 7, 14, 30, 60, 90, 120, 150, 180, 365, 400, 545, 731, 1096, + 1827, 2192, 2557, 2922, 3288, 3653, + ]; + if (!validRetentionDays.includes(config.observability.logRetentionDays)) { + throw new Error( + `Invalid observability.logRetentionDays: ${config.observability.logRetentionDays}. ` + + `CloudWatch Logs accepts only: ${validRetentionDays.join(', ')}. ` + + `Set CDK_OBSERVABILITY_LOG_RETENTION_DAYS to one of those values.` + ); + } + + // A rate, not a percentage: 5 instead of 0.05 is a 100x cost error. + const rate = config.observability.xraySamplingRate; + if (rate < 0 || rate > 1) { + throw new Error( + `Invalid observability.xraySamplingRate: ${rate}. ` + + `Expected a rate between 0.0 and 1.0 (e.g. 0.05 for 5%), not a percentage. ` + + `X-Ray bills per trace recorded, so a value above 1.0 is rejected rather ` + + `than clamped.` + ); + } + + const percentFields: Array<[string, number]> = [ + ['ecsCpuPercent', config.observability.ecsCpuPercent], + ['ecsMemoryPercent', config.observability.ecsMemoryPercent], + ['lambdaDurationPercentOfTimeout', config.observability.lambdaDurationPercentOfTimeout], + ]; + for (const [name, value] of percentFields) { + if (value <= 0 || value > 100) { + throw new Error( + `Invalid observability.${name}: ${value}. Expected a percentage between 1 and 100.` + ); + } + } } /** diff --git a/infrastructure/lib/constructs/agentcore/memory-construct.ts b/infrastructure/lib/constructs/agentcore/memory-construct.ts index 8ccb0218c..43016f902 100644 --- a/infrastructure/lib/constructs/agentcore/memory-construct.ts +++ b/infrastructure/lib/constructs/agentcore/memory-construct.ts @@ -6,6 +6,7 @@ import * as ssm from 'aws-cdk-lib/aws-ssm'; import { Construct } from 'constructs'; import { AppConfig, getResourceName } from '../../config'; +import { logRetentionFor } from '../observability/log-retention'; export interface AgentCoreMemoryConstructProps { config: AppConfig; @@ -108,7 +109,7 @@ export class AgentCoreMemoryConstruct extends Construct { // that prefix. This is a service constraint, not a naming choice. const memoryLogsLogGroup = new logs.LogGroup(this, 'MemoryLogsLogGroup', { logGroupName: `/aws/vendedlogs/bedrock-agentcore/memory/${config.projectPrefix}`, - retention: logs.RetentionDays.ONE_MONTH, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); const memoryLogsSource = new logs.CfnDeliverySource(this, 'MemoryLogsSource', { diff --git a/infrastructure/lib/constructs/app-api/app-api-service-construct.ts b/infrastructure/lib/constructs/app-api/app-api-service-construct.ts index 56bc3c40d..fde7427fe 100644 --- a/infrastructure/lib/constructs/app-api/app-api-service-construct.ts +++ b/infrastructure/lib/constructs/app-api/app-api-service-construct.ts @@ -13,6 +13,7 @@ import { AppConfig, getResourceName } from '../../config'; import { resolveAppApiParams, buildAppApiEnvironment } from './app-api-environment'; import { PlatformComputeRefs } from '../platform-compute-refs'; import { grantAppApiPermissions } from './app-api-iam-grants'; +import { logRetentionFor } from '../observability/log-retention'; export interface AppApiServiceConstructProps { config: AppConfig; @@ -82,6 +83,8 @@ export interface AppApiServiceConstructProps { */ export class AppApiServiceConstruct extends Construct { public readonly ecsService: ecs.FargateService; + /** Exposed so alarms bind to the real target-group dimensions. */ + public readonly targetGroup: elbv2.ApplicationTargetGroup; constructor(scope: Construct, id: string, props: AppApiServiceConstructProps) { super(scope, id); @@ -122,7 +125,7 @@ export class AppApiServiceConstruct extends Construct { // Auto-generated log group name (no `logGroupName` set) so a // failed-deploy orphan can't collide with a redeploy. const logGroup = new logs.LogGroup(this, 'AppApiLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -242,7 +245,7 @@ export class AppApiServiceConstruct extends Construct { }); // ── Target group ── - const targetGroup = new elbv2.ApplicationTargetGroup(this, 'AppApiTargetGroup', { + const targetGroup = this.targetGroup = new elbv2.ApplicationTargetGroup(this, 'AppApiTargetGroup', { vpc, targetGroupName: getResourceName(config, 'app-api-tg'), port: 8000, diff --git a/infrastructure/lib/constructs/artifacts/artifact-render-lambda-construct.ts b/infrastructure/lib/constructs/artifacts/artifact-render-lambda-construct.ts index 739a3c422..5c8ac98e7 100644 --- a/infrastructure/lib/constructs/artifacts/artifact-render-lambda-construct.ts +++ b/infrastructure/lib/constructs/artifacts/artifact-render-lambda-construct.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig } from '../../config'; +import { logRetentionFor } from '../observability/log-retention'; export interface ArtifactRenderLambdaConstructProps { config: AppConfig; @@ -79,7 +80,7 @@ export class ArtifactRenderLambdaConstruct extends Construct { // Auto-generated log group name (no `logGroupName`) so a // failed-deploy orphan can't collide with a redeploy. const renderLogGroup = new logs.LogGroup(this, 'RenderFunctionLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); diff --git a/infrastructure/lib/constructs/identity/token-enrichment-construct.ts b/infrastructure/lib/constructs/identity/token-enrichment-construct.ts index 435d437c1..db7489e23 100644 --- a/infrastructure/lib/constructs/identity/token-enrichment-construct.ts +++ b/infrastructure/lib/constructs/identity/token-enrichment-construct.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig, getResourceName } from '../../config'; +import { logRetentionFor } from '../observability/log-retention'; export interface TokenEnrichmentConstructProps { config: AppConfig; @@ -62,7 +63,7 @@ export class TokenEnrichmentConstruct extends Construct { // orphan can't collide with a redeploy. Short retention — these logs are // only useful for diagnosing a misconfigured claim map. const logGroup = new logs.LogGroup(this, 'TokenEnrichmentLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); diff --git a/infrastructure/lib/constructs/inference-api/inference-agentcore-construct.ts b/infrastructure/lib/constructs/inference-api/inference-agentcore-construct.ts index c2ffd542a..5a4a86d2c 100644 --- a/infrastructure/lib/constructs/inference-api/inference-agentcore-construct.ts +++ b/infrastructure/lib/constructs/inference-api/inference-agentcore-construct.ts @@ -4,11 +4,14 @@ import * as ecr_assets from 'aws-cdk-lib/aws-ecr-assets'; import * as ssm from 'aws-cdk-lib/aws-ssm'; import * as iam from 'aws-cdk-lib/aws-iam'; import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as cr from 'aws-cdk-lib/custom-resources'; +import * as sns from 'aws-cdk-lib/aws-sns'; import * as xray from 'aws-cdk-lib/aws-xray'; import * as bedrock from 'aws-cdk-lib/aws-bedrockagentcore'; import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig, getResourceName, getTruncatedResourceName, applyStandardTags, buildCorsOrigins } from '../../config'; +import { AlarmFactory } from '../observability/alarm-factory'; import { PlatformComputeRefs } from '../platform-compute-refs'; import { createRuntimeExecutionRole, @@ -47,6 +50,7 @@ export interface InferenceAgentCoreConstructProps { browserArn: string; /** AgentCore Browser ID — same provenance as browserArn. */ browserId: string; + alarmTopic?: sns.ITopic; } /** @@ -77,6 +81,9 @@ export class InferenceAgentCoreConstruct extends Construct { * stack query the group that has data in it. */ public readonly runtimeLogGroupName: string; + /** The `Name` dimension on every runtime metric: `{runtimeName}::DEFAULT`. + * Shared with the dashboard so both bind identically. */ + public readonly runtimeMetricName: string; constructor(scope: Construct, id: string, props: InferenceAgentCoreConstructProps) { super(scope, id); @@ -272,8 +279,12 @@ export class InferenceAgentCoreConstruct extends Construct { // Single CDK-Managed AgentCore Runtime with Cognito JWT Authorizer // ============================================================ + // Also the basis of the CloudWatch `Name` dimension on every runtime + // metric, so both derive from one expression. + const agentRuntimeName = getResourceName(config, 'agentcore_runtime').replace(/-/g, '_'); + this.runtime = new bedrock.CfnRuntime(this, 'AgentCoreRuntime', { - agentRuntimeName: getResourceName(config, 'agentcore_runtime').replace(/-/g, '_'), + agentRuntimeName, agentRuntimeArtifact: { containerConfiguration: { containerUri: inferenceApiImageUri, @@ -467,12 +478,55 @@ export class InferenceAgentCoreConstruct extends Construct { // "no errors" / "no traffic" rather than as a broken query. Removing it also // drops a retention policy that never applied to anything. // - // ⚠️ Retention on the real group is therefore unmanaged (service-created - // groups can't take a CDK retention setting), which is a live cost item — + // ⚠️ Retention on the real group cannot be set with a CDK `LogGroup` + // construct, because the group is created by the AgentCore service rather + // than by CloudFormation — declaring one here would either collide on + // create or manage a second, empty group. Left unmanaged, it grows forever: // dev alone carries several such groups in the hundreds of MB. Tracked as a - // W5 follow-up in docs/one-pagers/cost-effectiveness-roadmap.md. + // W5 follow-up in docs/one-pagers/cost-effectiveness-roadmap.md, closed by + // the custom resource below. this.runtimeLogGroupName = `/aws/bedrock-agentcore/runtimes/${this.runtime.attrAgentRuntimeId}-DEFAULT`; + this.runtimeMetricName = `${agentRuntimeName}::DEFAULT`; + + // PutRetentionPolicy is idempotent and creates the group if absent, which + // matters before the runtime's first invocation. No onDelete: dropping the + // policy on teardown would revert the group to "keep forever". + const runtimeLogRetention = new cr.AwsCustomResource(this, 'RuntimeLogRetention', { + onCreate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: this.runtimeLogGroupName, + retentionInDays: config.observability.logRetentionDays, + }, + // Embeds the value so CFN re-invokes when it changes. + physicalResourceId: cr.PhysicalResourceId.of( + `${this.runtimeLogGroupName}-retention-${config.observability.logRetentionDays}`, + ), + }, + onUpdate: { + service: 'CloudWatchLogs', + action: 'putRetentionPolicy', + parameters: { + logGroupName: this.runtimeLogGroupName, + retentionInDays: config.observability.logRetentionDays, + }, + physicalResourceId: cr.PhysicalResourceId.of( + `${this.runtimeLogGroupName}-retention-${config.observability.logRetentionDays}`, + ), + }, + policy: cr.AwsCustomResourcePolicy.fromStatements([ + new iam.PolicyStatement({ + actions: ['logs:PutRetentionPolicy', 'logs:CreateLogGroup'], + resources: [ + `arn:aws:logs:${config.awsRegion}:${config.awsAccount}:log-group:${this.runtimeLogGroupName}:*`, + ], + }), + ]), + installLatestAwsSdk: false, + }); + runtimeLogRetention.node.addDependency(this.runtime); // NOTE: X-Ray TransactionSearchConfig is an account-level singleton. // It cannot be created via CloudFormation if it already exists. @@ -501,8 +555,12 @@ export class InferenceAgentCoreConstruct extends Construct { samplingRule: { ruleName: getTruncatedResourceName(config, 32, 'ac-sampling'), priority: 100, - fixedRate: config.production ? 0.05 : 1.0, - reservoirSize: config.production ? 5 : 50, + // Single configured values, not a production ternary. The old + // non-production branch was fixedRate 1.0 / reservoir 50 — a recorded + // trace for EVERY agent invocation, at $5 per million traces, inherited + // by any fork that never set `production`. Defaults are now 0.01 / 1. + fixedRate: config.observability.xraySamplingRate, + reservoirSize: config.observability.xraySamplingReservoir, serviceName: '*', serviceType: '*', host: '*', @@ -522,7 +580,7 @@ export class InferenceAgentCoreConstruct extends Construct { filterExpression: 'annotation.gen_ai_system = "strands-agents" OR service(id(name: "bedrock-agentcore", type: "AWS::BedrockAgentCore"))', insightsConfiguration: { insightsEnabled: true, - notificationsEnabled: config.production, + notificationsEnabled: config.observability.xrayInsightsNotifications, }, }); @@ -535,75 +593,72 @@ export class InferenceAgentCoreConstruct extends Construct { defaultInterval: cdk.Duration.hours(3), }); - const agentCoreNamespace = 'bedrock-agentcore'; - - const invocationCountMetric = new cloudwatch.Metric({ - namespace: agentCoreNamespace, - metricName: 'InvocationCount', - statistic: 'Sum', - period: cdk.Duration.minutes(5), - }); - - const invocationErrorMetric = new cloudwatch.Metric({ - namespace: agentCoreNamespace, - metricName: 'InvocationErrors', - statistic: 'Sum', - period: cdk.Duration.minutes(5), - }); - - const latencyP50Metric = new cloudwatch.Metric({ - namespace: agentCoreNamespace, - metricName: 'InvocationLatency', - statistic: 'p50', - period: cdk.Duration.minutes(5), - }); - - const latencyP90Metric = new cloudwatch.Metric({ - namespace: agentCoreNamespace, - metricName: 'InvocationLatency', - statistic: 'p90', - period: cdk.Duration.minutes(5), - }); - - const latencyP99Metric = new cloudwatch.Metric({ - namespace: agentCoreNamespace, - metricName: 'InvocationLatency', - statistic: 'p99', - period: cdk.Duration.minutes(5), - }); - - const inputTokensMetric = new cloudwatch.Metric({ + // Namespace and metric names verified with `aws cloudwatch list-metrics`. + // The lowercase `bedrock-agentcore` namespace exists but holds only the + // OpenTelemetry/Strands application metrics, and the names this used before + // (InvocationCount / InvocationErrors / InvocationLatency) exist nowhere — + // so both alarms had sat in INSUFFICIENT_DATA since creation. Pinned by test. + // + // Every stream here is dimensioned; an undimensioned metric matches nothing. + const agentCoreNamespace = 'AWS/Bedrock-AgentCore'; + + // A four-dimension variant adding ComputeType=MicroVM also exists; not used, + // since that is an implementation detail an alarm should not depend on. + const runtimeDimensions = { + Resource: this.runtime.attrAgentRuntimeArn, + Operation: 'InvokeAgentRuntime', + Name: this.runtimeMetricName, + }; + + // No `label`: it forces CDK to render the alarm as a Metrics[] array rather + // than flat Namespace/MetricName properties. + const runtimeMetric = ( + metricName: string, + statistic: string, + ) => new cloudwatch.Metric({ namespace: agentCoreNamespace, - metricName: 'InputTokens', - statistic: 'Sum', + metricName, + dimensionsMap: runtimeDimensions, + statistic, period: cdk.Duration.minutes(5), }); - const outputTokensMetric = new cloudwatch.Metric({ + const invocationsMetric = runtimeMetric('Invocations', 'Sum'); + const systemErrorsMetric = runtimeMetric('SystemErrors', 'Sum'); + const userErrorsMetric = runtimeMetric('UserErrors', 'Sum'); + const throttlesMetric = runtimeMetric('Throttles', 'Sum'); + const sessionsMetric = runtimeMetric('Sessions', 'Sum'); + const latencyP50Metric = runtimeMetric('Latency', 'p50'); + const latencyP90Metric = runtimeMetric('Latency', 'p90'); + const latencyP99Metric = runtimeMetric('Latency', 'p99'); + + // `Sessions` is a cumulative creation counter; this is the live gauge. + const activeSessionsMetric = new cloudwatch.Metric({ namespace: agentCoreNamespace, - metricName: 'OutputTokens', - statistic: 'Sum', + metricName: 'ActiveSessionCount', + dimensionsMap: { Service: 'AgentCore.Runtime' }, + statistic: 'Maximum', period: cdk.Duration.minutes(5), }); dashboard.addWidgets( new cloudwatch.TextWidget({ - markdown: `# AgentCore Runtime Observability\n**Project:** ${config.projectPrefix} | **Region:** ${config.awsRegion}`, + markdown: `# AgentCore Runtime Observability\n**Project:** ${config.projectPrefix} | **Region:** ${config.awsRegion} | **Namespace:** \`${agentCoreNamespace}\`\n\nLLM token usage and prompt-cache efficiency live on the **${getResourceName(config, 'prompt-cache-observability')}** dashboard — the token metrics in this namespace are Memory-strategy counters, not model tokens.`, width: 24, - height: 1, + height: 2, }), ); dashboard.addWidgets( new cloudwatch.GraphWidget({ - title: 'Invocation Count & Errors', - left: [invocationCountMetric], - right: [invocationErrorMetric], + title: 'Invocations & Errors', + left: [invocationsMetric], + right: [systemErrorsMetric, userErrorsMetric, throttlesMetric], width: 12, height: 6, }), new cloudwatch.GraphWidget({ - title: 'Invocation Latency (p50 / p90 / p99)', + title: 'Invocation Latency (p50 / p90 / p99) — SSE, so seconds are normal', left: [latencyP50Metric, latencyP90Metric, latencyP99Metric], width: 12, height: 6, @@ -612,8 +667,9 @@ export class InferenceAgentCoreConstruct extends Construct { dashboard.addWidgets( new cloudwatch.GraphWidget({ - title: 'Token Usage (Input / Output)', - left: [inputTokensMetric, outputTokensMetric], + title: 'Sessions created vs currently active', + left: [sessionsMetric], + right: [activeSessionsMetric], width: 12, height: 6, }), @@ -635,21 +691,53 @@ export class InferenceAgentCoreConstruct extends Construct { // Observability: CloudWatch Alarms // ============================================================ - new cloudwatch.Alarm(this, 'AgentCoreHighErrorRateAlarm', { - alarmName: getResourceName(config, 'agentcore-high-error-rate'), - alarmDescription: 'AgentCore Runtime invocation error rate exceeded threshold', - metric: invocationErrorMetric, - threshold: config.production ? 10 : 50, + const alarms = new AlarmFactory(this, config, props.alarmTopic); + + // Split by blame: SystemErrors means escalate to AWS, UserErrors means our + // request was wrong. + alarms.alarm('AgentCoreSystemErrorAlarm', { + name: 'agentcore-system-errors', + alarmDescription: + 'AgentCore Runtime returned server-side errors — AWS-side fault, not application code', + metric: systemErrorsMetric, + threshold: config.observability.agentCoreErrorThreshold, evaluationPeriods: 3, comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); - new cloudwatch.Alarm(this, 'AgentCoreHighLatencyAlarm', { - alarmName: getResourceName(config, 'agentcore-high-latency'), + // Original logical id and name retained so CFN updates in place. + alarms.alarm('AgentCoreHighErrorRateAlarm', { + name: 'agentcore-high-error-rate', + alarmDescription: + 'AgentCore Runtime returned client-side (user) errors above threshold — malformed requests, missing permissions, or rejected payloads', + metric: userErrorsMetric, + threshold: config.observability.agentCoreErrorThreshold, + evaluationPeriods: 3, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // Threshold 0: a throttle is unambiguous and does not self-correct. + alarms.alarm('AgentCoreThrottleAlarm', { + name: 'agentcore-throttles', + alarmDescription: + 'AgentCore Runtime is throttling invocations — the account is at its TPS or session quota, which needs a quota increase rather than a retry', + metric: throttlesMetric, + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.alarm('AgentCoreHighLatencyAlarm', { + name: 'agentcore-high-latency', alarmDescription: 'AgentCore Runtime p99 latency exceeded threshold', metric: latencyP99Metric, - threshold: 30000, // 30 seconds + // Milliseconds here, unlike the ALB's TargetResponseTime which is seconds. + // Measured turns average 3-4.5s and peak near 25s, so the previous 30s + // threshold sat just above normal. + threshold: config.observability.agentCoreLatencyMs, evaluationPeriods: 3, comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, diff --git a/infrastructure/lib/constructs/kb-sync/kb-sync-construct.ts b/infrastructure/lib/constructs/kb-sync/kb-sync-construct.ts index 343e965f3..f721e7f5f 100644 --- a/infrastructure/lib/constructs/kb-sync/kb-sync-construct.ts +++ b/infrastructure/lib/constructs/kb-sync/kb-sync-construct.ts @@ -7,12 +7,15 @@ import * as iam from 'aws-cdk-lib/aws-iam'; import * as lambda from 'aws-cdk-lib/aws-lambda'; import * as logs from 'aws-cdk-lib/aws-logs'; import * as s3 from 'aws-cdk-lib/aws-s3'; +import * as sns from 'aws-cdk-lib/aws-sns'; import * as ssm from 'aws-cdk-lib/aws-ssm'; import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig } from '../../config'; +import { AlarmFactory } from '../observability/alarm-factory'; import { grantManagedKbDocumentDeletion } from '../managed-kb/managed-kb-role-construct'; +import { logRetentionFor } from '../observability/log-retention'; export interface KbSyncConstructProps { config: AppConfig; @@ -28,6 +31,7 @@ export interface KbSyncConstructProps { * MUST be the same identity app-api/inference-api use. */ workloadIdentityName: string; + alarmTopic?: sns.ITopic; } /** @@ -84,7 +88,7 @@ export class KbSyncConstruct extends Construct { ); const workerLogGroup = new logs.LogGroup(this, 'KbSyncWorkerLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -114,7 +118,7 @@ export class KbSyncConstruct extends Construct { }); const dispatcherLogGroup = new logs.LogGroup(this, 'KbSyncDispatcherLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -232,17 +236,16 @@ export class KbSyncConstruct extends Construct { }); this.scheduleRule.addTarget(new targets.LambdaFunction(this.dispatcherLambda)); - // Error visibility (no SNS wiring in this stack yet; alarms are - // dashboard/console signals). - new cloudwatch.Alarm(this, 'KbSyncDispatcherErrorAlarm', { - alarmName: `${config.projectPrefix}-kb-sync-dispatcher-errors`, + const alarms = new AlarmFactory(this, config, props.alarmTopic); + alarms.alarm('KbSyncDispatcherErrorAlarm', { + name: 'kb-sync-dispatcher-errors', metric: this.dispatcherLambda.metricErrors({ period: cdk.Duration.minutes(15) }), threshold: 1, evaluationPeriods: 2, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); - new cloudwatch.Alarm(this, 'KbSyncWorkerErrorAlarm', { - alarmName: `${config.projectPrefix}-kb-sync-worker-errors`, + alarms.alarm('KbSyncWorkerErrorAlarm', { + name: 'kb-sync-worker-errors', metric: this.workerLambda.metricErrors({ period: cdk.Duration.minutes(15) }), threshold: 3, evaluationPeriods: 2, diff --git a/infrastructure/lib/constructs/managed-kb/kb-migration-construct.ts b/infrastructure/lib/constructs/managed-kb/kb-migration-construct.ts index 9419b550d..0a0b6ad6c 100644 --- a/infrastructure/lib/constructs/managed-kb/kb-migration-construct.ts +++ b/infrastructure/lib/constructs/managed-kb/kb-migration-construct.ts @@ -7,13 +7,16 @@ import * as iam from 'aws-cdk-lib/aws-iam'; import * as lambda from 'aws-cdk-lib/aws-lambda'; import * as logs from 'aws-cdk-lib/aws-logs'; import * as s3 from 'aws-cdk-lib/aws-s3'; +import * as sns from 'aws-cdk-lib/aws-sns'; import * as sqs from 'aws-cdk-lib/aws-sqs'; import * as ssm from 'aws-cdk-lib/aws-ssm'; import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig, getResourceName } from '../../config'; +import { AlarmFactory } from '../observability/alarm-factory'; import { ManagedKbRoleConstruct, managedKbMetricNamespace } from './managed-kb-role-construct'; +import { logRetentionFor } from '../observability/log-retention'; /** * Tag keys every runtime-created Managed_KB carries (Requirement 20.11). @@ -115,6 +118,7 @@ export interface KbMigrationConstructProps { * left unattached there, waiting for these Lambda roles. */ managedKbRole: ManagedKbRoleConstruct; + alarmTopic?: sns.ITopic; } /** @@ -245,7 +249,7 @@ export class KbMigrationConstruct extends Construct { // ── Worker ── const workerLogGroup = new logs.LogGroup(this, 'KbMigrationWorkerLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -272,7 +276,7 @@ export class KbMigrationConstruct extends Construct { // ── Dispatcher ── const dispatcherLogGroup = new logs.LogGroup(this, 'KbMigrationDispatcherLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -301,7 +305,7 @@ export class KbMigrationConstruct extends Construct { // ── Reconciler ── const reconcilerLogGroup = new logs.LogGroup(this, 'KbMigrationReconcilerLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -340,7 +344,7 @@ export class KbMigrationConstruct extends Construct { }); const ingestionConsumerLogGroup = new logs.LogGroup(this, 'KbIngestionConsumerLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -568,8 +572,10 @@ export class KbMigrationConstruct extends Construct { // then be worth nothing when it finally had something to say. const namespace = managedKbMetricNamespace(config); - new cloudwatch.Alarm(this, 'ManagedKbTotalStorageAlarm', { - alarmName: getResourceName(config, 'managed-kb-total-storage'), + const alarms = new AlarmFactory(this, config, props.alarmTopic); + + alarms.alarm('ManagedKbTotalStorageAlarm', { + name: 'managed-kb-total-storage', alarmDescription: 'Fleet-wide managed knowledge base storage exceeded the configured GB threshold', metric: new cloudwatch.Metric({ @@ -584,8 +590,8 @@ export class KbMigrationConstruct extends Construct { treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); - new cloudwatch.Alarm(this, 'ManagedKbCountAlarm', { - alarmName: getResourceName(config, 'managed-kb-count'), + alarms.alarm('ManagedKbCountAlarm', { + name: 'managed-kb-count', alarmDescription: 'Managed knowledge base count reached 80% of the 10,000 per-account quota — a quota increase takes lead time', metric: new cloudwatch.Metric({ @@ -611,8 +617,8 @@ export class KbMigrationConstruct extends Construct { // `AmazonBedrockAgentCore`, so keying on `AmazonBedrock` misses it // entirely and keying on the service code alone blends it into the // AgentCore Runtime memory line (Requirement 22.7). - new cloudwatch.Alarm(this, 'ManagedKbDailyCostAlarm', { - alarmName: getResourceName(config, 'managed-kb-daily-cost'), + alarms.alarm('ManagedKbDailyCostAlarm', { + name: 'managed-kb-daily-cost', alarmDescription: 'Daily Knowledge-Base usagetype cost exceeded the configured USD threshold', metric: new cloudwatch.Metric({ @@ -633,8 +639,8 @@ export class KbMigrationConstruct extends Construct { // that crashed and will be cleaned up, whereas the same finding three // days running means the delete saga is leaking and every leaked // knowledge base is still billing. - new cloudwatch.Alarm(this, 'ManagedKbOrphansAlarm', { - alarmName: getResourceName(config, 'managed-kb-orphans'), + alarms.alarm('ManagedKbOrphansAlarm', { + name: 'managed-kb-orphans', alarmDescription: 'Reconciler reported orphaned managed knowledge bases on three consecutive runs — the delete saga is leaking', metric: new cloudwatch.Metric({ diff --git a/infrastructure/lib/constructs/observability/ai-path-alarms-construct.ts b/infrastructure/lib/constructs/observability/ai-path-alarms-construct.ts new file mode 100644 index 000000000..78bbeb73d --- /dev/null +++ b/infrastructure/lib/constructs/observability/ai-path-alarms-construct.ts @@ -0,0 +1,271 @@ +import * as cdk from 'aws-cdk-lib'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import { Construct } from 'constructs'; + +import { AppConfig } from '../../config'; +import { AlarmFactory, ALARM_PERIOD } from './alarm-factory'; + +/** The namespace AgentCore publishes service metrics to. */ +const AGENTCORE_NAMESPACE = 'AWS/Bedrock-AgentCore'; +/** The namespace bedrock-runtime inference publishes to. */ +const BEDROCK_NAMESPACE = 'AWS/Bedrock'; + +export interface AiPathAlarmsConstructProps { + config: AppConfig; + /** AgentCore Memory ARN. The `Resource` dimension value is the full ARN. */ + memoryArn: string; + /** AgentCore Gateway ARN. Also a full ARN in the `Resource` dimension. */ + gatewayArn: string; + /** + * AgentCore Code Interpreter **ID**, not ARN. + * + * Not an oversight — see the class docstring. Code Interpreter publishes a + * bare id in `Resource` where Memory and Gateway publish ARNs. + */ + codeInterpreterId: string; + /** Platform alarm topic. Undefined leaves these alarms console-only. */ + alarmTopic?: sns.ITopic; +} + +/** + * Alarms for the managed AI services a chat turn depends on. These failures + * present as application bugs — a Bedrock throttle reaches the user as a chat + * that never responds, a Memory error as an agent that has forgotten the + * conversation. + * + * Dimension values were enumerated with `aws cloudwatch list-metrics`, which + * surfaced one asymmetry: Memory and Gateway publish `Resource` as a full ARN, + * but Code Interpreter publishes a bare id. + * + * Memory and Code Interpreter publish a stream per API operation, so those alarms + * sum operations via metric math (CloudWatch caps that at 10 metrics). + * + * NOT alarmed: Cognito, because AWS/Cognito on the ESSENTIALS feature plan + * publishes only success metrics (failure metrics need Plus) — the auth-path + * signal is the token-enrichment Lambda instead. And Browser, which has no metric + * streams. + */ +export class AiPathAlarmsConstruct extends Construct { + constructor(scope: Construct, id: string, props: AiPathAlarmsConstructProps) { + super(scope, id); + + const { config, memoryArn, gatewayArn, codeInterpreterId } = props; + const alarms = new AlarmFactory(this, config, props.alarmTopic); + const errorThreshold = config.observability.agentCoreErrorThreshold; + + /** An AgentCore metric for one resource + operation. */ + const acMetric = ( + metricName: string, + resource: string, + operation: string, + ) => new cloudwatch.Metric({ + namespace: AGENTCORE_NAMESPACE, + metricName, + dimensionsMap: { Resource: resource, Operation: operation }, + statistic: 'Sum', + period: ALARM_PERIOD, + }); + + /** Sum one metric across several operations for a single resource. */ + const sumAcrossOperations = ( + metricName: string, + resource: string, + operations: string[], + ): cloudwatch.IMetric => { + const usingMetrics: Record = {}; + operations.forEach((op, i) => { + usingMetrics[`op${i}`] = acMetric(metricName, resource, op); + }); + return new cloudwatch.MathExpression({ + expression: operations.map((_, i) => `op${i}`).join(' + '), + usingMetrics, + period: ALARM_PERIOD, + }); + }; + + // ============================================================ + // Bedrock inference + // ============================================================ + + const bedrockMetric = (metricName: string, statistic = 'Sum') => new cloudwatch.Metric({ + namespace: BEDROCK_NAMESPACE, + metricName, + // Account-wide roll-up. A per-ModelId variant exists, but models are + // managed through the admin UI at runtime so a synth-time set would drift. + statistic, + period: ALARM_PERIOD, + }); + + // Had no metric streams when verified — never fired, rather than absent. + // NOT_BREACHING keeps it quiet until the first occurrence. + alarms.alarm('BedrockThrottleAlarm', { + name: 'bedrock-invocation-throttles', + alarmDescription: + 'Bedrock is throttling model invocations — the account is at a model TPM/RPM ' + + 'quota. Users see chats that never respond. Needs a quota increase or less ' + + 'traffic, not a code fix.', + metric: bedrockMetric('InvocationThrottles'), + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.alarm('BedrockServerErrorAlarm', { + name: 'bedrock-invocation-server-errors', + alarmDescription: + 'Bedrock returned server-side errors on model invocation — AWS-side fault, not ' + + 'application code.', + metric: bedrockMetric('InvocationServerErrors'), + threshold: errorThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // The only leading indicator here: quota usage climbs before throttling. + alarms.alarm('BedrockQuotaUsageAlarm', { + name: 'bedrock-tpm-quota-usage', + alarmDescription: + 'Estimated Bedrock tokens-per-minute quota usage is high. This is the leading ' + + 'indicator for bedrock-invocation-throttles — acting on it means requesting a ' + + 'quota increase before users see failures rather than after.', + metric: bedrockMetric('EstimatedTPMQuotaUsage', 'Maximum'), + threshold: 80, + evaluationPeriods: 3, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // AgentCore Memory + // ============================================================ + + // Extraction and Consolidation are excluded: async background strategies + // whose failure does not break a live turn. + const MEMORY_HOT_PATH = [ + 'CreateEvent', + 'RetrieveMemoryRecords', + 'GetMemoryRecord', + 'ListEvents', + 'GetMemory', + ]; + + alarms.expressionAlarm('MemorySystemErrorAlarm', { + name: 'agentcore-memory-system-errors', + alarmDescription: + 'AgentCore Memory returned server-side errors on the conversation hot path ' + + '(CreateEvent / RetrieveMemoryRecords / GetMemoryRecord / ListEvents / ' + + 'GetMemory). Users experience this as an agent that has forgotten the ' + + 'conversation.', + expression: sumAcrossOperations('SystemErrors', memoryArn, MEMORY_HOT_PATH), + threshold: errorThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.expressionAlarm('MemoryThrottleAlarm', { + name: 'agentcore-memory-throttles', + alarmDescription: + 'AgentCore Memory is throttling hot-path requests — turns are failing to persist ' + + 'or to retrieve context. Needs a quota review.', + expression: sumAcrossOperations('Throttles', memoryArn, MEMORY_HOT_PATH), + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // AgentCore Gateway (MCP tools) + // ============================================================ + + // The roll-up across MCP methods; a per-Method set would multiply with + // every tool the gateway exposes. + const gatewayDimensions = { + Resource: gatewayArn, + Operation: 'InvokeGateway', + Protocol: 'MCP', + }; + + alarms.alarm('GatewaySystemErrorAlarm', { + name: 'agentcore-gateway-system-errors', + alarmDescription: + 'AgentCore Gateway returned server-side errors on MCP calls — tool invocations ' + + 'are failing for reasons outside the tool Lambda itself.', + metric: new cloudwatch.Metric({ + namespace: AGENTCORE_NAMESPACE, + metricName: 'SystemErrors', + dimensionsMap: gatewayDimensions, + statistic: 'Sum', + period: ALARM_PERIOD, + }), + threshold: errorThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.alarm('GatewayThrottleAlarm', { + name: 'agentcore-gateway-throttles', + alarmDescription: + 'AgentCore Gateway is throttling MCP calls — agents will lose tool access.', + metric: new cloudwatch.Metric({ + namespace: AGENTCORE_NAMESPACE, + metricName: 'Throttles', + dimensionsMap: gatewayDimensions, + statistic: 'Sum', + period: ALARM_PERIOD, + }), + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // AgentCore Code Interpreter + // ============================================================ + + // `Resource` is a bare id here, NOT an ARN as it is for Memory and Gateway. + // An ARN would match no stream and the alarm would stay green. + alarms.expressionAlarm('CodeInterpreterSystemErrorAlarm', { + name: 'agentcore-code-interpreter-system-errors', + alarmDescription: + 'AgentCore Code Interpreter returned server-side errors on session start, ' + + 'invoke, or stop. Users experience this as charts and data analysis silently ' + + 'failing to appear.', + expression: sumAcrossOperations('SystemErrors', codeInterpreterId, [ + 'StartCodeInterpreterSession', + 'InvokeCodeInterpreter', + 'StopCodeInterpreterSession', + ]), + threshold: errorThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // Account-level gauge (only a Service dimension), matching the quota it + // consumes. + alarms.alarm('CodeInterpreterActiveSessionAlarm', { + name: 'agentcore-code-interpreter-active-sessions', + alarmDescription: + 'Concurrent AgentCore Code Interpreter sessions are unusually high — approaching ' + + 'the account session quota, past which new sessions are refused.', + metric: new cloudwatch.Metric({ + namespace: AGENTCORE_NAMESPACE, + metricName: 'ActiveSessionCount', + dimensionsMap: { Service: 'AgentCore.CodeInterpreter' }, + statistic: 'Maximum', + period: cdk.Duration.minutes(5), + }), + threshold: 50, + evaluationPeriods: 3, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + } +} diff --git a/infrastructure/lib/constructs/observability/alarm-factory.ts b/infrastructure/lib/constructs/observability/alarm-factory.ts new file mode 100644 index 000000000..30188eb6f --- /dev/null +++ b/infrastructure/lib/constructs/observability/alarm-factory.ts @@ -0,0 +1,80 @@ +import * as cdk from 'aws-cdk-lib'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as cloudwatchActions from 'aws-cdk-lib/aws-cloudwatch-actions'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import { Construct } from 'constructs'; + +import { AppConfig, getResourceName } from '../../config'; + +/** Alarm props, with `name` (unprefixed) in place of `alarmName`. */ +export interface RoutedAlarmProps extends Omit { + name: string; +} + +/** + * Creates alarms wired to the SNS topic. + * + * Use this instead of `new cloudwatch.Alarm()` — an alarm without actions looks + * finished but notifies nobody, so routing is made a property of the tool rather + * than something to remember. Enforced by observability-alarm-routing.test.ts. + * + * `treatMissingData` is deliberately not defaulted; see + * .kiro/steering/observability.md. + */ +export class AlarmFactory { + constructor( + private readonly scope: Construct, + private readonly config: AppConfig, + /** Undefined when alarmTopicEnabled is false; alarms stay console-only. */ + private readonly topic?: sns.ITopic, + ) {} + + public alarm(id: string, props: RoutedAlarmProps): cloudwatch.Alarm { + const { name, ...rest } = props; + + const alarm = new cloudwatch.Alarm(this.scope, id, { + ...rest, + alarmName: getResourceName(this.config, name), + }); + + if (this.topic) { + const action = new cloudwatchActions.SnsAction(this.topic); + alarm.addAlarmAction(action); + // Also notify on recovery, so nobody has to check the console to find out + // whether the condition cleared. + alarm.addOkAction(action); + } + + return alarm; + } + + /** Same routing guarantee, for metric-math alarms. */ + public expressionAlarm( + id: string, + props: Omit & { expression: cloudwatch.IMetric }, + ): cloudwatch.Alarm { + const { expression, ...rest } = props; + return this.alarm(id, { ...rest, metric: expression }); + } +} + +/** Shared period for count-based alarms; thresholds are chosen against it. */ +export const ALARM_PERIOD = cdk.Duration.minutes(5); + +/** + * Every alarm beneath `scope`, for the dashboard's alarm-status widget. + * + * Discovered by walking the tree rather than passed in, so an alarm added later + * cannot go missing from the dashboard. Call after all alarms are constructed. + */ +export function collectAlarms(scope: Construct): cloudwatch.Alarm[] { + const found: cloudwatch.Alarm[] = []; + const visit = (node: Construct) => { + for (const child of node.node.children) { + if (child instanceof cloudwatch.Alarm) found.push(child); + visit(child as Construct); + } + }; + visit(scope); + return found; +} diff --git a/infrastructure/lib/constructs/observability/alarm-topic-construct.ts b/infrastructure/lib/constructs/observability/alarm-topic-construct.ts new file mode 100644 index 000000000..55ee1d2b7 --- /dev/null +++ b/infrastructure/lib/constructs/observability/alarm-topic-construct.ts @@ -0,0 +1,111 @@ +import * as cdk from 'aws-cdk-lib'; +import * as iam from 'aws-cdk-lib/aws-iam'; +import * as kms from 'aws-cdk-lib/aws-kms'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import * as ssm from 'aws-cdk-lib/aws-ssm'; +import { Construct } from 'constructs'; + +import { AppConfig, getResourceName } from '../../config'; + +export interface AlarmTopicConstructProps { + config: AppConfig; +} + +/** + * The SNS topic every alarm publishes to. + * + * Subscriptions are intentionally absent — teams subscribe out-of-band so adding + * a recipient needs no deploy. The ARN is published to SSM and as a CfnOutput. + * + * The key must be customer-managed: CloudWatch cannot publish to a topic + * encrypted with alias/aws/sns, because that key's policy cannot be edited to + * grant the service principal kms:GenerateDataKey*. The failure is silent — the + * alarm fires and the notification is dropped. + */ +export class AlarmTopicConstruct extends Construct { + /** The topic every alarm action targets. */ + public readonly topic: sns.Topic; + + /** CMK encrypting the topic. Exposed for tests and for any future publisher + * that needs an explicit grant. */ + public readonly key: kms.Key; + + constructor(scope: Construct, id: string, props: AlarmTopicConstructProps) { + super(scope, id); + + const { config } = props; + + // ============================================================ + // CMK + // ============================================================ + + this.key = new kms.Key(this, 'AlarmTopicKey', { + alias: getResourceName(config, 'alarm-topic-key'), + description: 'Encrypts the platform alarm SNS topic.', + enableKeyRotation: true, + // Not getRemovalPolicy(config): this wraps in-flight notifications only, + // so retaining it would strand a billable key with nothing to decrypt. + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + + // Decrypt alone is not enough: SNS envelope encryption has the publisher + // generate the data key, so GenerateDataKey* is required too. + this.key.addToResourcePolicy( + new iam.PolicyStatement({ + sid: 'AllowCloudWatchAlarmsToPublishToEncryptedTopic', + effect: iam.Effect.ALLOW, + principals: [new iam.ServicePrincipal('cloudwatch.amazonaws.com')], + actions: ['kms:GenerateDataKey*', 'kms:Decrypt'], + resources: ['*'], + conditions: { + StringEquals: { 'aws:SourceAccount': config.awsAccount }, + }, + }), + ); + + // ============================================================ + // Topic + // ============================================================ + + this.topic = new sns.Topic(this, 'AlarmTopic', { + topicName: getResourceName(config, 'alarms'), + displayName: `${config.projectPrefix} platform alarms`, + masterKey: this.key, + enforceSSL: true, + }); + + this.topic.addToResourcePolicy( + new iam.PolicyStatement({ + sid: 'AllowCloudWatchAlarmsToPublish', + effect: iam.Effect.ALLOW, + principals: [new iam.ServicePrincipal('cloudwatch.amazonaws.com')], + actions: ['sns:Publish'], + resources: [this.topic.topicArn], + conditions: { + StringEquals: { 'aws:SourceAccount': config.awsAccount }, + }, + }), + ); + + // ============================================================ + // Discovery + // ============================================================ + + new ssm.StringParameter(this, 'AlarmTopicArnParam', { + parameterName: `/${config.projectPrefix}/observability/alarm-topic-arn`, + stringValue: this.topic.topicArn, + description: + 'SNS topic ARN for all platform CloudWatch alarms. Subscribe teams to ' + + 'this topic out-of-band: aws sns subscribe --topic-arn ' + + '--protocol email --notification-endpoint you@example.edu', + }); + + new cdk.CfnOutput(this, 'AlarmTopicArn', { + value: this.topic.topicArn, + description: + 'SNS topic for platform alarms. Subscriptions are intentionally not ' + + 'managed by CDK — subscribe with `aws sns subscribe`.', + exportName: `${config.projectPrefix}-AlarmTopicArn`, + }); + } +} diff --git a/infrastructure/lib/constructs/observability/alb-alarms-construct.ts b/infrastructure/lib/constructs/observability/alb-alarms-construct.ts new file mode 100644 index 000000000..7af9f728e --- /dev/null +++ b/infrastructure/lib/constructs/observability/alb-alarms-construct.ts @@ -0,0 +1,146 @@ +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import { Construct } from 'constructs'; + +import { AppConfig } from '../../config'; +import { AlarmFactory, ALARM_PERIOD } from './alarm-factory'; + +export interface AlbAlarmsConstructProps { + config: AppConfig; + /** The application load balancer. Alarm dimensions are taken from it rather + * than constructed by hand. */ + loadBalancer: elbv2.IApplicationLoadBalancer; + /** The app-api target group. */ + targetGroup: elbv2.IApplicationTargetGroup; + /** Platform alarm topic. Undefined leaves these alarms console-only. */ + alarmTopic?: sns.ITopic; +} + +/** + * Front-door alarms. + * + * The chat path is SSE, so TargetResponseTime is legitimately tens of seconds and + * latency is a weak signal in both directions — the discrete metrics (5xx counts, + * unhealthy hosts, rejected connections) are the reliable ones. + * + * ELB 5xx and target 5xx are separate alarms because the first response differs: + * "is anything running" versus "read the application logs". + */ +export class AlbAlarmsConstruct extends Construct { + constructor(scope: Construct, id: string, props: AlbAlarmsConstructProps) { + super(scope, id); + + const { config, loadBalancer, targetGroup } = props; + const alarms = new AlarmFactory(this, config, props.alarmTopic); + const obs = config.observability; + + // ============================================================ + // Errors + // ============================================================ + + alarms.alarm('AlbElb5xxAlarm', { + name: 'alb-elb-5xx', + alarmDescription: + 'ALB returned 5xx responses of its own (not the application) — usually no healthy target to route to', + metric: loadBalancer.metrics.httpCodeElb( + elbv2.HttpCodeElb.ELB_5XX_COUNT, + { period: ALARM_PERIOD, statistic: 'Sum' }, + ), + threshold: obs.albTarget5xxThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.alarm('AlbTarget5xxAlarm', { + name: 'alb-target-5xx', + alarmDescription: + 'App API returned 5xx responses through the ALB — the service is reachable but failing', + metric: targetGroup.metrics.httpCodeTarget( + elbv2.HttpCodeTarget.TARGET_5XX_COUNT, + { period: ALARM_PERIOD, statistic: 'Sum' }, + ), + threshold: obs.albTarget5xxThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // Availability + // ============================================================ + + // BREACHING: UnHealthyHostCount stops being published entirely when no + // targets are registered, so absent data is the outage, not health. + alarms.alarm('AlbUnhealthyHostAlarm', { + name: 'alb-unhealthy-hosts', + alarmDescription: + 'One or more App API targets are failing their health check, or no targets are reporting at all', + metric: targetGroup.metrics.unhealthyHostCount({ + period: ALARM_PERIOD, + statistic: 'Maximum', + }), + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.BREACHING, + }); + + alarms.alarm('AlbTargetConnectionErrorAlarm', { + name: 'alb-target-connection-errors', + alarmDescription: + 'ALB could not establish connections to App API targets — network path or security group fault', + metric: loadBalancer.metrics.targetConnectionErrorCount({ + period: ALARM_PERIOD, + statistic: 'Sum', + }), + threshold: obs.albTarget5xxThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // Saturation + // ============================================================ + + // Threshold 0: a rejected connection never reaches the app, so it appears + // in no application log. + alarms.alarm('AlbRejectedConnectionAlarm', { + name: 'alb-rejected-connections', + alarmDescription: + 'ALB rejected connections after reaching its limit — users were turned away before reaching the application, so nothing appears in application logs', + metric: loadBalancer.metrics.rejectedConnectionCount({ + period: ALARM_PERIOD, + statistic: 'Sum', + }), + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // Latency (streaming-aware — see the class docstring) + // ============================================================ + + alarms.alarm('AlbTargetLatencyAlarm', { + name: 'alb-target-p99-latency', + alarmDescription: + 'App API p99 response time through the ALB exceeded the configured floor. ' + + 'NOTE: the chat path is SSE, so a healthy agent turn legitimately takes ' + + 'tens of seconds — this floor is set high on purpose and a breach means ' + + 'requests are hanging, not merely slow.', + metric: targetGroup.metrics.targetResponseTime({ + period: ALARM_PERIOD, + statistic: 'p99', + }), + // CloudWatch reports this metric in SECONDS; config is in ms. + threshold: obs.albP99LatencyMs / 1000, + evaluationPeriods: 3, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + } +} diff --git a/infrastructure/lib/constructs/observability/dynamodb-alarms-construct.ts b/infrastructure/lib/constructs/observability/dynamodb-alarms-construct.ts new file mode 100644 index 000000000..361168d15 --- /dev/null +++ b/infrastructure/lib/constructs/observability/dynamodb-alarms-construct.ts @@ -0,0 +1,99 @@ +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import { Construct } from 'constructs'; + +import { AppConfig } from '../../config'; +import { AlarmFactory, ALARM_PERIOD } from './alarm-factory'; + +export interface AlarmedTable { + /** Unprefixed table name, used in the alarm name so a notification names the + * table. */ + name: string; + table: dynamodb.ITable; +} + +export interface DynamoDbAlarmsConstructProps { + config: AppConfig; + tables: AlarmedTable[]; + alarmTopic?: sns.ITopic; +} + +/** + * Throttle alarms per table, plus one account-level request-error alarm. + * + * Read and write throttles share one alarm rather than getting one each: all + * tables are on-demand, and a live check found zero metric streams for + * ReadThrottleEvents, WriteThrottleEvents and SystemErrors — none has ever + * fired. The alarm description names both metrics so the read-vs-write + * distinction is still recoverable. + * + * SystemErrors is not alarmed per table for the same reason; account-level + * UserErrors is, because it had real data and nothing watching it. UserErrors is + * published account-wide only, with no TableName dimension. + */ +export class DynamoDbAlarmsConstruct extends Construct { + constructor(scope: Construct, id: string, props: DynamoDbAlarmsConstructProps) { + super(scope, id); + + const { config, tables } = props; + const alarms = new AlarmFactory(this, config, props.alarmTopic); + const threshold = config.observability.dynamoThrottleThreshold; + + for (const { name, table } of tables) { + const id = name + .split('-') + .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) + .join(''); + + // table.metric(), not metricThrottledRequests() — CDK deprecates the + // latter as returning an invalid metric. + const readThrottles = table.metric('ReadThrottleEvents', { + period: ALARM_PERIOD, + statistic: 'Sum', + }); + const writeThrottles = table.metric('WriteThrottleEvents', { + period: ALARM_PERIOD, + statistic: 'Sum', + }); + + alarms.expressionAlarm(`${id}ThrottleAlarm`, { + name: `ddb-${name}-throttle`, + alarmDescription: + `DynamoDB throttling on ${name}. Compare ReadThrottleEvents and ` + + `WriteThrottleEvents on this table: reads point at a query pattern or a hot ` + + `partition being read, writes at a hot key or a write burst. On-demand table, ` + + `so this is a partition hot spot or an account limit, not under-provisioning.`, + expression: new cloudwatch.MathExpression({ + expression: 'reads + writes', + usingMetrics: { reads: readThrottles, writes: writeThrottles }, + period: ALARM_PERIOD, + }), + threshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + } + + // DynamoDB 4xx across all tables: validation failures, missing keys, + // malformed requests — application code misusing the API. + alarms.alarm('DynamoDbUserErrorAlarm', { + name: 'ddb-user-errors', + alarmDescription: + 'DynamoDB rejected requests account-wide (4xx: validation, missing key, ' + + 'malformed request). Application code, not an AWS fault. No dimensions are ' + + 'available, so use CloudTrail or application logs to find the caller.', + metric: new cloudwatch.Metric({ + namespace: 'AWS/DynamoDB', + metricName: 'UserErrors', + statistic: 'Sum', + period: ALARM_PERIOD, + }), + threshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + } +} diff --git a/infrastructure/lib/constructs/observability/ecs-service-alarms-construct.ts b/infrastructure/lib/constructs/observability/ecs-service-alarms-construct.ts new file mode 100644 index 000000000..d8275815b --- /dev/null +++ b/infrastructure/lib/constructs/observability/ecs-service-alarms-construct.ts @@ -0,0 +1,97 @@ +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as ecs from 'aws-cdk-lib/aws-ecs'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import { Construct } from 'constructs'; + +import { AppConfig } from '../../config'; +import { AlarmFactory, ALARM_PERIOD } from './alarm-factory'; + +export interface EcsServiceAlarmsConstructProps { + config: AppConfig; + /** The app-api Fargate service. Alarm dimensions are derived from it. */ + service: ecs.FargateService; + /** Desired task count, for the "fewer tasks running than asked for" alarm. */ + desiredCount: number; + /** Platform alarm topic. Undefined leaves these alarms console-only. */ + alarmTopic?: sns.ITopic; +} + +/** + * Saturation and capacity alarms for the app-api service. + * + * Uses the service's own metric helpers so ClusterName + ServiceName come from + * the resource — a dimension-less AWS/ECS alarm silently averages every service + * in the account. + * + * RunningTaskCount complements the ALB's UnHealthyHostCount: that one catches + * tasks running but failing health checks, this one catches tasks not running. + */ +export class EcsServiceAlarmsConstruct extends Construct { + constructor(scope: Construct, id: string, props: EcsServiceAlarmsConstructProps) { + super(scope, id); + + const { config, service, desiredCount } = props; + const alarms = new AlarmFactory(this, config, props.alarmTopic); + const obs = config.observability; + + // ============================================================ + // Saturation + // ============================================================ + + alarms.alarm('AppApiCpuAlarm', { + name: 'app-api-cpu-high', + alarmDescription: + 'App API service CPU utilisation is sustained above the configured percentage', + metric: service.metricCpuUtilization({ + period: ALARM_PERIOD, + statistic: 'Average', + }), + threshold: obs.ecsCpuPercent, + evaluationPeriods: 3, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.alarm('AppApiMemoryAlarm', { + name: 'app-api-memory-high', + alarmDescription: + 'App API service memory utilisation is sustained above the configured percentage — a Fargate task that exhausts memory is killed, not throttled', + metric: service.metricMemoryUtilization({ + period: ALARM_PERIOD, + statistic: 'Average', + }), + threshold: obs.ecsMemoryPercent, + evaluationPeriods: 3, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + // ============================================================ + // Capacity + // ============================================================ + + // BREACHING: a service at zero tasks stops publishing rather than + // publishing zero. LESS_THAN desiredCount, so autoscaling up never trips it. + alarms.alarm('AppApiRunningTaskAlarm', { + name: 'app-api-running-tasks-low', + alarmDescription: + `Fewer than the desired ${desiredCount} App API task(s) are running — tasks are failing to start or being killed, which the ALB health-check alarm would not catch`, + metric: new cloudwatch.Metric({ + // Container Insights: no service.metric* helper, so dimensions come + // explicitly from the service resource. + namespace: 'ECS/ContainerInsights', + metricName: 'RunningTaskCount', + dimensionsMap: { + ClusterName: service.cluster.clusterName, + ServiceName: service.serviceName, + }, + period: ALARM_PERIOD, + statistic: 'Minimum', + }), + threshold: desiredCount, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.LESS_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.BREACHING, + }); + } +} diff --git a/infrastructure/lib/constructs/observability/lambda-alarms-construct.ts b/infrastructure/lib/constructs/observability/lambda-alarms-construct.ts new file mode 100644 index 000000000..6464a2f17 --- /dev/null +++ b/infrastructure/lib/constructs/observability/lambda-alarms-construct.ts @@ -0,0 +1,119 @@ +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as lambda from 'aws-cdk-lib/aws-lambda'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import * as sqs from 'aws-cdk-lib/aws-sqs'; +import { Construct } from 'constructs'; + +import { AppConfig } from '../../config'; +import { AlarmFactory, ALARM_PERIOD } from './alarm-factory'; + +/** One Lambda to alarm on, with the short name used in the alarm name. */ +export interface AlarmedFunction { + /** Unprefixed short name, e.g. 'artifact-render'. */ + name: string; + fn: lambda.IFunction; + /** Error threshold override. Defaults to `observability.lambdaErrorThreshold`. */ + errorThreshold?: number; + /** Skip the error alarm: this function's own construct defines one with a + * tuned threshold (kb-sync and scheduled-runs use 1 for dispatchers, 3 for + * workers). */ + throttleOnly?: boolean; +} + +/** A dead-letter queue whose depth should be alarmed. */ +export interface AlarmedDlq { + name: string; + queue: sqs.IQueue; +} + +export interface LambdaAlarmsConstructProps { + config: AppConfig; + functions: AlarmedFunction[]; + /** Dead-letter queues to watch. Any message here is work that was lost. */ + dlqs?: AlarmedDlq[]; + /** Platform alarm topic. Undefined leaves these alarms console-only. */ + alarmTopic?: sns.ITopic; +} + +/** + * Error and throttle alarms for the stack's Lambdas, plus DLQ depth. + * + * No duration alarms: a function that exceeds its timeout is killed and records + * an Errors datapoint, so the failure that matters is already covered. + * + * token-enrichment is worth alarming precisely because its handler is fail-open — + * an error means MCP tools silently lose user-identity claims rather than a + * visible login failure. + * + * rag-cors-updater is excluded: a deploy-time custom resource whose failure fails + * the CloudFormation deploy directly. + */ +export class LambdaAlarmsConstruct extends Construct { + constructor(scope: Construct, id: string, props: LambdaAlarmsConstructProps) { + super(scope, id); + + const { config, functions, dlqs = [] } = props; + const alarms = new AlarmFactory(this, config, props.alarmTopic); + const defaultErrorThreshold = config.observability.lambdaErrorThreshold; + + /** 'artifact-render' -> 'ArtifactRender' */ + const toId = (name: string) => name + .split('-') + .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) + .join(''); + + for (const { name, fn, errorThreshold, throttleOnly } of functions) { + const id = toId(name); + + if (!throttleOnly) alarms.alarm(`${id}ErrorAlarm`, { + name: `lambda-${name}-errors`, + alarmDescription: + `${name} Lambda is returning errors. Includes invocations killed for ` + + `exceeding their timeout.`, + metric: fn.metricErrors({ period: ALARM_PERIOD, statistic: 'Sum' }), + threshold: errorThreshold ?? defaultErrorThreshold, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + // A function that is not invoked publishes nothing, and most of these + // are event-driven and idle for long stretches. + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + + alarms.alarm(`${id}ThrottleAlarm`, { + name: `lambda-${name}-throttles`, + alarmDescription: + `${name} Lambda invocations are being throttled — concurrency exhausted. ` + + `A reserved-concurrency or account-limit problem, not a code problem.`, + metric: fn.metricThrottles({ period: ALARM_PERIOD, statistic: 'Sum' }), + threshold: 0, + evaluationPeriods: 2, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + } + + // ============================================================ + // Dead-letter queues + // ============================================================ + + for (const { name, queue } of dlqs) { + // Threshold 0 and one evaluation period: DLQ messages persist until + // drained or replayed, so this must not self-clear. + alarms.alarm(`${toId(name)}DlqDepthAlarm`, { + name: `dlq-${name}-not-empty`, + alarmDescription: + `The ${name} dead-letter queue is not empty — work was accepted and then ` + + `failed every retry. These messages persist until drained or replayed, so ` + + `this alarm does not clear on its own.`, + metric: queue.metricApproximateNumberOfMessagesVisible({ + period: ALARM_PERIOD, + statistic: 'Maximum', + }), + threshold: 0, + evaluationPeriods: 1, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + } + } +} diff --git a/infrastructure/lib/constructs/observability/log-retention.ts b/infrastructure/lib/constructs/observability/log-retention.ts new file mode 100644 index 000000000..de1eb554c --- /dev/null +++ b/infrastructure/lib/constructs/observability/log-retention.ts @@ -0,0 +1,73 @@ +import * as cdk from 'aws-cdk-lib'; +import * as logs from 'aws-cdk-lib/aws-logs'; +import { IConstruct } from 'constructs'; + +import { AppConfig } from '../../config'; + +/** + * CloudWatch's accepted retention values. + * + * Mapped explicitly because `logs.RetentionDays` is a string-valued enum, so a + * number cannot be cast into it. + */ +const RETENTION_BY_DAYS: Record = { + 1: logs.RetentionDays.ONE_DAY, + 3: logs.RetentionDays.THREE_DAYS, + 5: logs.RetentionDays.FIVE_DAYS, + 7: logs.RetentionDays.ONE_WEEK, + 14: logs.RetentionDays.TWO_WEEKS, + 30: logs.RetentionDays.ONE_MONTH, + 60: logs.RetentionDays.TWO_MONTHS, + 90: logs.RetentionDays.THREE_MONTHS, + 120: logs.RetentionDays.FOUR_MONTHS, + 150: logs.RetentionDays.FIVE_MONTHS, + 180: logs.RetentionDays.SIX_MONTHS, + 365: logs.RetentionDays.ONE_YEAR, + 400: logs.RetentionDays.THIRTEEN_MONTHS, + 545: logs.RetentionDays.EIGHTEEN_MONTHS, + 731: logs.RetentionDays.TWO_YEARS, + 1096: logs.RetentionDays.THREE_YEARS, + 1827: logs.RetentionDays.FIVE_YEARS, + 2192: logs.RetentionDays.SIX_YEARS, + 2557: logs.RetentionDays.SEVEN_YEARS, + 2922: logs.RetentionDays.EIGHT_YEARS, + 3288: logs.RetentionDays.NINE_YEARS, + 3653: logs.RetentionDays.TEN_YEARS, +}; + +/** Retention for every log group in this stack. */ +export function logRetentionFor(config: AppConfig): logs.RetentionDays { + const days = config.observability.logRetentionDays; + const retention = RETENTION_BY_DAYS[days]; + if (!retention) { + throw new Error( + `Invalid observability.logRetentionDays: ${days}. ` + + `CloudWatch accepts only: ${Object.keys(RETENTION_BY_DAYS).join(', ')}.`, + ); + } + return retention; +} + +/** + * Forces the configured retention onto every log group, including ones CDK + * creates for its own machinery (the AwsCustomResource and BucketDeployment + * provider Lambdas both default to 731 days and are declared nowhere here). + * + * The per-site logRetentionFor() calls are kept for legibility; this catches what + * they cannot see. + */ +export class LogRetentionAspect implements cdk.IAspect { + private readonly retentionInDays: number; + + constructor(config: AppConfig) { + // Validate via the same helper rather than silently leaving CDK's default. + logRetentionFor(config); + this.retentionInDays = config.observability.logRetentionDays; + } + + public visit(node: IConstruct): void { + if (node instanceof logs.CfnLogGroup) { + node.retentionInDays = this.retentionInDays; + } + } +} diff --git a/infrastructure/lib/constructs/observability/platform-dashboard-construct.ts b/infrastructure/lib/constructs/observability/platform-dashboard-construct.ts new file mode 100644 index 000000000..74cdf09a8 --- /dev/null +++ b/infrastructure/lib/constructs/observability/platform-dashboard-construct.ts @@ -0,0 +1,211 @@ +import * as cdk from 'aws-cdk-lib'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as ecs from 'aws-cdk-lib/aws-ecs'; +import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2'; +import { Construct } from 'constructs'; + +import { AppConfig, getResourceName } from '../../config'; +import { ALARM_PERIOD } from './alarm-factory'; + +export interface PlatformDashboardConstructProps { + config: AppConfig; + loadBalancer: elbv2.IApplicationLoadBalancer; + targetGroup: elbv2.IApplicationTargetGroup; + service: ecs.FargateService; + /** AgentCore Runtime ARN, for the runtime metric dimensions. */ + runtimeArn: string; + /** `{agentRuntimeName}::DEFAULT`, the runtime metrics' `Name` dimension. */ + runtimeMetricName: string; + /** Every alarm in the stack, for the alarm-status widget. */ + alarms: cloudwatch.IAlarm[]; +} + +/** + * The on-call dashboard: "is the platform healthy right now". + * + * Rows follow triage order — is traffic being served, then why not, then which + * alarms are already firing. + * + * Links to the two existing dashboards rather than restating their widgets, which + * keeps the stack at three (CloudWatch charges $3/month beyond that). + */ +export class PlatformDashboardConstruct extends Construct { + public readonly dashboard: cloudwatch.Dashboard; + + constructor(scope: Construct, id: string, props: PlatformDashboardConstructProps) { + super(scope, id); + + const { + config, loadBalancer, targetGroup, service, runtimeArn, runtimeMetricName, alarms, + } = props; + + const agentCoreNamespace = 'AWS/Bedrock-AgentCore'; + const runtimeDimensions = { + Resource: runtimeArn, + Operation: 'InvokeAgentRuntime', + Name: runtimeMetricName, + }; + + const runtimeMetric = (metricName: string) => new cloudwatch.Metric({ + namespace: agentCoreNamespace, + metricName, + dimensionsMap: runtimeDimensions, + statistic: 'Sum', + period: ALARM_PERIOD, + }); + + this.dashboard = new cloudwatch.Dashboard(this, 'PlatformDashboard', { + dashboardName: getResourceName(config, 'platform-health'), + defaultInterval: cdk.Duration.hours(3), + }); + + // ============================================================ + // Header + // ============================================================ + + this.dashboard.addWidgets( + new cloudwatch.TextWidget({ + markdown: [ + `# ${config.projectPrefix} — Platform Health`, + '', + `**Region:** ${config.awsRegion} | ` + + `**Alarms route to:** \`${getResourceName(config, 'alarms')}\` (SNS)`, + '', + '**Drill-downs:** ' + + `[AgentCore Runtime detail](/cloudwatch/home?region=${config.awsRegion}#dashboards:name=${getResourceName(config, 'agentcore-observability')}) · ` + + `[Prompt cache & token economics](/cloudwatch/home?region=${config.awsRegion}#dashboards:name=${getResourceName(config, 'prompt-cache-observability')})`, + '', + '_The chat path is SSE, so response times of tens of seconds are normal ' + + 'and a sudden DROP in latency can mean turns are failing early._', + ].join('\n'), + width: 24, + height: 4, + }), + ); + + // ============================================================ + // Row 1 — is traffic being served? + // ============================================================ + + this.dashboard.addWidgets( + new cloudwatch.GraphWidget({ + title: 'Front door — requests vs 5xx', + left: [ + loadBalancer.metrics.requestCount({ period: ALARM_PERIOD, statistic: 'Sum' }), + ], + right: [ + loadBalancer.metrics.httpCodeElb( + elbv2.HttpCodeElb.ELB_5XX_COUNT, { period: ALARM_PERIOD, statistic: 'Sum' }, + ), + targetGroup.metrics.httpCodeTarget( + elbv2.HttpCodeTarget.TARGET_5XX_COUNT, { period: ALARM_PERIOD, statistic: 'Sum' }, + ), + ], + width: 8, + height: 6, + }), + new cloudwatch.GraphWidget({ + title: 'Agent — invocations vs errors & throttles', + left: [runtimeMetric('Invocations')], + right: [ + runtimeMetric('SystemErrors'), + runtimeMetric('UserErrors'), + runtimeMetric('Throttles'), + ], + width: 8, + height: 6, + }), + new cloudwatch.GraphWidget({ + title: 'Capacity — running tasks vs unhealthy targets', + left: [ + new cloudwatch.Metric({ + namespace: 'ECS/ContainerInsights', + metricName: 'RunningTaskCount', + dimensionsMap: { + ClusterName: service.cluster.clusterName, + ServiceName: service.serviceName, + }, + statistic: 'Minimum', + period: ALARM_PERIOD, + }), + ], + right: [ + targetGroup.metrics.unhealthyHostCount({ period: ALARM_PERIOD, statistic: 'Maximum' }), + ], + width: 8, + height: 6, + }), + ); + + // ============================================================ + // Row 2 — saturation: why is it unhealthy? + // ============================================================ + + this.dashboard.addWidgets( + new cloudwatch.GraphWidget({ + title: 'App API saturation (CPU / memory %)', + left: [ + service.metricCpuUtilization({ period: ALARM_PERIOD, statistic: 'Average' }), + service.metricMemoryUtilization({ period: ALARM_PERIOD, statistic: 'Average' }), + ], + leftYAxis: { min: 0, max: 100 }, + width: 8, + height: 6, + }), + new cloudwatch.GraphWidget({ + title: 'Bedrock quota headroom (TPM %) — leading indicator', + left: [ + new cloudwatch.Metric({ + namespace: 'AWS/Bedrock', + metricName: 'EstimatedTPMQuotaUsage', + statistic: 'Maximum', + period: ALARM_PERIOD, + }), + ], + leftYAxis: { min: 0, max: 100 }, + right: [ + new cloudwatch.Metric({ + namespace: 'AWS/Bedrock', + metricName: 'InvocationThrottles', + statistic: 'Sum', + period: ALARM_PERIOD, + }), + ], + width: 8, + height: 6, + }), + new cloudwatch.GraphWidget({ + title: 'Data layer — DynamoDB request errors', + left: [ + new cloudwatch.Metric({ + namespace: 'AWS/DynamoDB', + metricName: 'UserErrors', + statistic: 'Sum', + period: ALARM_PERIOD, + }), + ], + width: 8, + height: 6, + }), + ); + + // ============================================================ + // Row 3 — what is already known to be broken? + // ============================================================ + + this.dashboard.addWidgets( + new cloudwatch.AlarmStatusWidget({ + title: `All platform alarms (${alarms.length})`, + alarms, + width: 24, + height: 8, + }), + ); + + new cdk.CfnOutput(this, 'PlatformDashboardName', { + value: this.dashboard.dashboardName, + description: 'Single-pane platform health dashboard', + exportName: `${config.projectPrefix}-PlatformDashboard`, + }); + } +} diff --git a/infrastructure/lib/constructs/observability/prompt-cache-observability-construct.ts b/infrastructure/lib/constructs/observability/prompt-cache-observability-construct.ts index f013d7991..161a9b2cb 100644 --- a/infrastructure/lib/constructs/observability/prompt-cache-observability-construct.ts +++ b/infrastructure/lib/constructs/observability/prompt-cache-observability-construct.ts @@ -1,11 +1,14 @@ import * as cdk from 'aws-cdk-lib'; import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as sns from 'aws-cdk-lib/aws-sns'; import { Construct } from 'constructs'; import { AppConfig, getResourceName } from '../../config'; +import { AlarmFactory } from './alarm-factory'; export interface PromptCacheObservabilityConstructProps { config: AppConfig; + alarmTopic?: sns.ITopic; /** * The log group the AgentCore Runtime actually writes to, from * `InferenceAgentCoreConstruct.runtimeLogGroupName`. @@ -40,11 +43,8 @@ export interface PromptCacheObservabilityConstructProps { * per-session drill-down counterpart is the cost-anatomy admin endpoint * (`GET /admin/costs/sessions/{id}/calls`). * - * Alarms are console-only — no SNS topics exist in the stack yet (same - * posture as kb-sync and scheduled-runs). They use NOT_BREACHING for - * missing data because the `PROMPT_CACHE_OBSERVABILITY_ENABLED=false` - * kill switch (or simply zero traffic) makes the metrics absent - * entirely. + * NOT_BREACHING on missing data: the PROMPT_CACHE_OBSERVABILITY_ENABLED=false + * kill switch, or simply no traffic, makes the metrics absent entirely. */ export class PromptCacheObservabilityConstruct extends Construct { constructor( @@ -225,46 +225,44 @@ export class PromptCacheObservabilityConstruct extends Construct { ); // ============================================================ - // Alarms (console-only; no SNS wiring yet) + // Alarms // ============================================================ + const alarms = new AlarmFactory(this, config, props.alarmTopic); + // AvoidableMiss is the nominated alarm target (see emf.py): a // prefix-stability regression flips a large share of calls to // `miss_avoidable`, showing up as a step change in this sum. - new cloudwatch.Alarm(this, 'PromptCacheAvoidableMissAlarm', { - alarmName: getResourceName(config, 'prompt-cache-avoidable-miss'), + alarms.alarm('PromptCacheAvoidableMissAlarm', { + name: 'prompt-cache-avoidable-miss', alarmDescription: 'Avoidable prompt-cache misses exceeded threshold — likely a prompt-prefix stability regression', metric: avoidableMissMetric, - threshold: config.production ? 10 : 50, + threshold: config.observability.promptCacheAvoidableMissThreshold, evaluationPeriods: 3, comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); - // Session-level accumulation (#833 PR-1). A fleet sum never notices one - // conversation spending $0.43 a turn for five days — the incident this - // alarm exists for would have tripped it on day 2, at ~10 turns. The - // metric is the session's cumulative partial-miss waste and the statistic - // is Maximum, so this reads as "a session at or over $5 of partial-miss - // waste was active in the last 24h"; it clears once that session stops. - new cloudwatch.Alarm(this, 'PromptCacheSessionPartialMissAlarm', { - alarmName: getResourceName(config, 'prompt-cache-session-partial-miss'), + // Maximum, not Sum: reads as "a session over the threshold was active in + // the last 24h". A fleet sum cannot see one conversation doing this (#833). + alarms.alarm('PromptCacheSessionPartialMissAlarm', { + name: 'prompt-cache-session-partial-miss', alarmDescription: - 'A single session accumulated more than $5 of partial-miss cache waste — one conversation is re-writing its prefix every turn (see the "Sessions by partial-miss waste" dashboard widget for which)', + 'A single session accumulated more than the configured partial-miss cache waste — one conversation is re-writing its prefix every turn (see the "Sessions by partial-miss waste" dashboard widget for which)', metric: sessionPartialMissUsdMetric, - threshold: 5, + threshold: config.observability.promptCacheSessionWastedUsdThreshold, evaluationPeriods: 1, comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); - new cloudwatch.Alarm(this, 'PromptCacheWastedUsdAlarm', { - alarmName: getResourceName(config, 'prompt-cache-wasted-usd'), + alarms.alarm('PromptCacheWastedUsdAlarm', { + name: 'prompt-cache-wasted-usd', alarmDescription: 'Dollars wasted on prompt-cache re-writes of already-cached prefix bytes (avoidable + partial misses) exceeded threshold', metric: wastedUsdMetric, - threshold: config.production ? 1 : 5, + threshold: config.observability.promptCacheWastedUsdThreshold, evaluationPeriods: 3, comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, diff --git a/infrastructure/lib/constructs/rag-ingestion/rag-ingestion-lambda-construct.ts b/infrastructure/lib/constructs/rag-ingestion/rag-ingestion-lambda-construct.ts index 5059de76b..73ac1a766 100644 --- a/infrastructure/lib/constructs/rag-ingestion/rag-ingestion-lambda-construct.ts +++ b/infrastructure/lib/constructs/rag-ingestion/rag-ingestion-lambda-construct.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig } from '../../config'; +import { logRetentionFor } from '../observability/log-retention'; export interface RagIngestionLambdaConstructProps { config: AppConfig; @@ -87,7 +88,7 @@ export class RagIngestionLambdaConstruct extends Construct { } = props; const ingestionLogGroup = new logs.LogGroup(this, 'RagIngestionLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); diff --git a/infrastructure/lib/constructs/scheduled-runs/scheduled-runs-construct.ts b/infrastructure/lib/constructs/scheduled-runs/scheduled-runs-construct.ts index ad9a7d14d..eaf1d88b3 100644 --- a/infrastructure/lib/constructs/scheduled-runs/scheduled-runs-construct.ts +++ b/infrastructure/lib/constructs/scheduled-runs/scheduled-runs-construct.ts @@ -8,11 +8,14 @@ import * as iam from 'aws-cdk-lib/aws-iam'; import * as lambda from 'aws-cdk-lib/aws-lambda'; import * as logs from 'aws-cdk-lib/aws-logs'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; +import * as sns from 'aws-cdk-lib/aws-sns'; import * as ssm from 'aws-cdk-lib/aws-ssm'; import * as path from 'path'; import { Construct } from 'constructs'; import { AppConfig } from '../../config'; +import { AlarmFactory } from '../observability/alarm-factory'; +import { logRetentionFor } from '../observability/log-retention'; export interface ScheduledRunsConstructProps { config: AppConfig; @@ -38,6 +41,7 @@ export interface ScheduledRunsConstructProps { * only HTTP dependency (via run_agent_headless). */ inferenceApiRuntimeEndpointUrl: string; cognitoRegion: string; + alarmTopic?: sns.ITopic; } /** @@ -121,7 +125,7 @@ export class ScheduledRunsConstruct extends Construct { }; const workerLogGroup = new logs.LogGroup(this, 'ScheduledRunsWorkerLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -148,7 +152,7 @@ export class ScheduledRunsConstruct extends Construct { }); const dispatcherLogGroup = new logs.LogGroup(this, 'ScheduledRunsDispatcherLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); @@ -282,17 +286,16 @@ export class ScheduledRunsConstruct extends Construct { }); this.scheduleRule.addTarget(new targets.LambdaFunction(this.dispatcherLambda)); - // Error visibility (no SNS wiring in this stack yet; alarms are - // dashboard/console signals). - new cloudwatch.Alarm(this, 'ScheduledRunsDispatcherErrorAlarm', { - alarmName: `${config.projectPrefix}-scheduled-runs-dispatcher-errors`, + const alarms = new AlarmFactory(this, config, props.alarmTopic); + alarms.alarm('ScheduledRunsDispatcherErrorAlarm', { + name: 'scheduled-runs-dispatcher-errors', metric: this.dispatcherLambda.metricErrors({ period: cdk.Duration.minutes(5) }), threshold: 1, evaluationPeriods: 2, treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, }); - new cloudwatch.Alarm(this, 'ScheduledRunsWorkerErrorAlarm', { - alarmName: `${config.projectPrefix}-scheduled-runs-worker-errors`, + alarms.alarm('ScheduledRunsWorkerErrorAlarm', { + name: 'scheduled-runs-worker-errors', metric: this.workerLambda.metricErrors({ period: cdk.Duration.minutes(5) }), threshold: 3, evaluationPeriods: 2, diff --git a/infrastructure/lib/constructs/spa/rag-cors-updater-construct.ts b/infrastructure/lib/constructs/spa/rag-cors-updater-construct.ts index fb288fcdb..74b031b9f 100644 --- a/infrastructure/lib/constructs/spa/rag-cors-updater-construct.ts +++ b/infrastructure/lib/constructs/spa/rag-cors-updater-construct.ts @@ -6,6 +6,7 @@ import * as s3 from 'aws-cdk-lib/aws-s3'; import { Construct } from 'constructs'; import { AppConfig } from '../../config'; +import { logRetentionFor } from '../observability/log-retention'; export interface RagCorsUpdaterConstructProps { config: AppConfig; @@ -46,8 +47,7 @@ export class RagCorsUpdaterConstruct extends Construct { ) { super(scope, id); - const { config: _config, frontendUrl, documentsBucket } = props; - void _config; + const { config, frontendUrl, documentsBucket } = props; const ragDocumentsBucketName = documentsBucket.bucketName; const ragDocumentsBucketArn = documentsBucket.bucketArn; @@ -55,7 +55,7 @@ export class RagCorsUpdaterConstruct extends Construct { // Auto-generated log group name — see ArtifactRenderLambdaConstruct // for the same pattern + rationale. const updateCorsLogGroup = new logs.LogGroup(this, 'UpdateRagCorsFnLogGroup', { - retention: logs.RetentionDays.ONE_WEEK, + retention: logRetentionFor(config), removalPolicy: cdk.RemovalPolicy.DESTROY, }); diff --git a/infrastructure/lib/platform-stack.ts b/infrastructure/lib/platform-stack.ts index 2c6cc4fc2..581608b76 100644 --- a/infrastructure/lib/platform-stack.ts +++ b/infrastructure/lib/platform-stack.ts @@ -7,9 +7,11 @@ import * as ec2 from 'aws-cdk-lib/aws-ec2'; import * as ecs from 'aws-cdk-lib/aws-ecs'; import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2'; import * as kms from 'aws-cdk-lib/aws-kms'; +import * as lambda from 'aws-cdk-lib/aws-lambda'; import * as s3 from 'aws-cdk-lib/aws-s3'; import * as s3n from 'aws-cdk-lib/aws-s3-notifications'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; +import * as sns from 'aws-cdk-lib/aws-sns'; import { CfnResource } from 'aws-cdk-lib'; import { Construct } from 'constructs'; @@ -75,6 +77,15 @@ import { McpSandboxBucketConstruct } from './constructs/mcp-sandbox/mcp-sandbox- import { McpSandboxDistributionConstruct } from './constructs/mcp-sandbox/mcp-sandbox-distribution-construct'; // Observability (cross-service dashboards + alarms) +import { AlarmTopicConstruct } from './constructs/observability/alarm-topic-construct'; +import { AlbAlarmsConstruct } from './constructs/observability/alb-alarms-construct'; +import { DynamoDbAlarmsConstruct } from './constructs/observability/dynamodb-alarms-construct'; +import { AiPathAlarmsConstruct } from './constructs/observability/ai-path-alarms-construct'; +import { collectAlarms } from './constructs/observability/alarm-factory'; +import { LogRetentionAspect } from './constructs/observability/log-retention'; +import { PlatformDashboardConstruct } from './constructs/observability/platform-dashboard-construct'; +import { LambdaAlarmsConstruct } from './constructs/observability/lambda-alarms-construct'; +import { EcsServiceAlarmsConstruct } from './constructs/observability/ecs-service-alarms-construct'; import { PromptCacheObservabilityConstruct } from './constructs/observability/prompt-cache-observability-construct'; // Fine-tuning (data half lives in Platform) @@ -128,6 +139,10 @@ export interface PlatformStackProps extends cdk.StackProps { * a stack to avoid a circular dependency). */ export class PlatformStack extends cdk.Stack { + /** SNS topic every alarm publishes to. Undefined when alarmTopicEnabled is + * false, leaving alarms console-only. */ + public readonly alarmTopic?: sns.ITopic; + // ── Network public readonly vpc: ec2.IVpc; public readonly alb: elbv2.IApplicationLoadBalancer; @@ -244,6 +259,14 @@ export class PlatformStack extends cdk.Stack { // ── Internal handles for the two-step wiring methods private readonly _config: AppConfig; + + // Created in the constructor, alarmed in wireCompute(). + private _kbSync?: KbSyncConstruct; + private _gatewayArn!: string; + private _artifactRenderFunction!: lambda.IFunction; + private _ragIngestionFunction!: lambda.IFunction; + private _kbMigration?: KbMigrationConstruct; + private _tokenEnrichment?: TokenEnrichmentConstruct; private readonly _spaBucketConstruct: SpaBucketConstruct; private readonly _mcpSandboxBucketConstruct: McpSandboxBucketConstruct; private readonly _artifactsDataConstruct: ArtifactsDataConstruct; @@ -256,6 +279,18 @@ export class PlatformStack extends cdk.Stack { this._config = config; applyStandardTags(this, config); + // Covers log groups CDK creates for its own machinery, which default to + // 731 days and are declared nowhere in this codebase. + cdk.Aspects.of(this).add(new LogRetentionAspect(config)); + + // ============================================================ + // Observability routing + // ============================================================ + // First, so every later construct can take the reference. + this.alarmTopic = config.observability.alarmTopicEnabled + ? new AlarmTopicConstruct(this, 'AlarmTopic', { config }).topic + : undefined; + // ============================================================ // Network // ============================================================ @@ -325,12 +360,14 @@ export class PlatformStack extends cdk.Stack { // depends on the Cognito Essentials feature plan — a deliberate opt-in. The // handler is fail-open (returns the event unchanged on any error), so the // worst case is "claim not added", never "login blocked". + let tokenEnrichment: TokenEnrichmentConstruct | undefined; if (config.mcpIdentity.tokenEnrichment?.enabled) { - new TokenEnrichmentConstruct(this, 'TokenEnrichment', { + tokenEnrichment = new TokenEnrichmentConstruct(this, 'TokenEnrichment', { config, userPool: cognitoConstruct.userPool, }); } + this._tokenEnrichment = tokenEnrichment; const artifactRenderToken = new ArtifactRenderTokenSecretConstruct( this, @@ -435,6 +472,7 @@ export class PlatformStack extends cdk.Stack { vectorIndexName: this.ragVectorIndexName, }, ); + this._ragIngestionFunction = ragIngestion.lambda; this.ragDocumentsBucket.addEventNotification( s3.EventType.OBJECT_CREATED, @@ -447,7 +485,8 @@ export class PlatformStack extends cdk.Stack { // (dispatcher + worker Lambdas + EventBridge rate rule; inert // unless config.kbSync.enabled) // ============================================================ - new KbSyncConstruct(this, 'KbSync', { + this._kbSync = new KbSyncConstruct(this, 'KbSync', { + alarmTopic: this.alarmTopic, config, assistantsTable: this.ragAssistantsTable, documentsBucket: this.ragDocumentsBucket, @@ -487,7 +526,8 @@ export class PlatformStack extends cdk.Stack { // anyone reading back the service-role ARN from an SSM parameter // this same stack publishes. // ============================================================ - new KbMigrationConstruct(this, 'KbMigration', { + this._kbMigration = new KbMigrationConstruct(this, 'KbMigration', { + alarmTopic: this.alarmTopic, config, assistantsTable: this.ragAssistantsTable, documentsBucket: this.ragDocumentsBucket, @@ -589,6 +629,7 @@ export class PlatformStack extends cdk.Stack { frameAncestors: this.artifactsFrameAncestors, }, ); + this._artifactRenderFunction = artifactRenderLambda.renderFunction; const artifactsDistribution = new ArtifactsDistributionConstruct( this, @@ -647,11 +688,12 @@ export class PlatformStack extends cdk.Stack { // Targets are managed out-of-band by mcp-servers' own deploy. // Cognito refs are passed explicitly (not via SSM) because the pool is a // sibling in this same stack — see AgentCoreGatewayConstructProps. - new AgentCoreGatewayConstruct(this, 'AgentCoreGateway', { + const agentCoreGateway = new AgentCoreGatewayConstruct(this, 'AgentCoreGateway', { config, userPool: cognitoConstruct.userPool, bffAppClient: cognitoConstruct.bffAppClient, }); + this._gatewayArn = agentCoreGateway.gateway.attrGatewayArn; // ============================================================ // MCP sandbox edge (always-on; bucket+dist; everything is wired @@ -815,6 +857,7 @@ export class PlatformStack extends cdk.Stack { }; const inferenceApi = new InferenceAgentCoreConstruct(this, 'InferenceApi', { + alarmTopic: this.alarmTopic, config: this._config, refs, memoryArn: this.agentCoreMemoryArn, @@ -830,6 +873,7 @@ export class PlatformStack extends cdk.Stack { // other platform-level observability because it needs the runtime's log // group name from the construct above. new PromptCacheObservabilityConstruct(this, 'PromptCacheObservability', { + alarmTopic: this.alarmTopic, config: this._config, runtimeLogGroupName: inferenceApi.runtimeLogGroupName, }); @@ -845,7 +889,7 @@ export class PlatformStack extends cdk.Stack { privateSubnetIdsString: sagemakerPrivateSubnetIds, }); - new AppApiServiceConstruct(this, 'AppApi', { + const appApi = new AppApiServiceConstruct(this, 'AppApi', { config: this._config, refs, agentCoreMemoryArn: this.agentCoreMemoryArn, @@ -857,6 +901,21 @@ export class PlatformStack extends cdk.Stack { sagemakerPrivateSubnetIds, }); + // After AppApiServiceConstruct: these bind to its target group and service. + new AlbAlarmsConstruct(this, 'AlbAlarms', { + config: this._config, + loadBalancer: this.alb, + targetGroup: appApi.targetGroup, + alarmTopic: this.alarmTopic, + }); + + new EcsServiceAlarmsConstruct(this, 'EcsServiceAlarms', { + config: this._config, + service: appApi.ecsService, + desiredCount: this._config.appApi.desiredCount, + alarmTopic: this.alarmTopic, + }); + // ============================================================ // Scheduled runs — the F3 scheduled trigger's engine (dispatcher + // worker Lambdas + EventBridge rate rule; inert unless @@ -864,7 +923,8 @@ export class PlatformStack extends cdk.Stack { // so it lands here in wireCompute() rather than the constructor // (unlike KbSyncConstruct, which has no such dependency). // ============================================================ - new ScheduledRunsConstruct(this, 'ScheduledRuns', { + const scheduledRuns = new ScheduledRunsConstruct(this, 'ScheduledRuns', { + alarmTopic: this.alarmTopic, config: this._config, sessionsMetadataTable: this.sessionsMetadataTable, bffSessionsTable: this.bffSessionsTable, @@ -874,5 +934,93 @@ export class PlatformStack extends cdk.Stack { inferenceApiRuntimeEndpointUrl: inferenceApi.runtimeEndpointUrl, cognitoRegion: this._config.awsRegion, }); + + new LambdaAlarmsConstruct(this, 'LambdaAlarms', { + config: this._config, + alarmTopic: this.alarmTopic, + functions: [ + { name: 'artifact-render', fn: this._artifactRenderFunction }, + { name: 'rag-ingestion', fn: this._ragIngestionFunction }, + ...(this._tokenEnrichment + ? [{ name: 'token-enrichment', fn: this._tokenEnrichment.enrichmentFunction }] + : []), + ...(this._kbMigration + ? [ + { name: 'kb-migration-dispatcher', fn: this._kbMigration.dispatcherLambda }, + { name: 'kb-migration-worker', fn: this._kbMigration.workerLambda }, + { name: 'kb-migration-reconciler', fn: this._kbMigration.reconcilerLambda }, + { name: 'kb-ingestion-consumer', fn: this._kbMigration.ingestionConsumerLambda }, + ] + : []), + ...(this._kbSync + ? [ + { name: 'kb-sync-dispatcher', fn: this._kbSync.dispatcherLambda, throttleOnly: true }, + { name: 'kb-sync-worker', fn: this._kbSync.workerLambda, throttleOnly: true }, + ] + : []), + { name: 'scheduled-runs-dispatcher', fn: scheduledRuns.dispatcherLambda, throttleOnly: true }, + { name: 'scheduled-runs-worker', fn: scheduledRuns.workerLambda, throttleOnly: true }, + ], + dlqs: this._kbMigration + ? [{ name: 'kb-ingestion', queue: this._kbMigration.ingestionConsumerDlq }] + : [], + }); + + // Code Interpreter takes an ID while the others take ARNs — that asymmetry + // is in AWS's metric emission. See the construct docstring. + new AiPathAlarmsConstruct(this, 'AiPathAlarms', { + config: this._config, + alarmTopic: this.alarmTopic, + memoryArn: this.agentCoreMemoryArn, + gatewayArn: this._gatewayArn, + codeInterpreterId: this.agentCoreCodeInterpreterId, + }); + + // A test asserts this list covers every AWS::DynamoDB::Table in the + // template, so a new table without an alarm fails CI. + new DynamoDbAlarmsConstruct(this, 'DynamoDbAlarms', { + config: this._config, + alarmTopic: this.alarmTopic, + tables: [ + { name: 'voice-ticket-replay', table: this.voiceTicketReplayTable }, + { name: 'oauth-providers', table: this.oauthProvidersTable }, + { name: 'oauth-user-tokens', table: this.oauthUserTokensTable }, + { name: 'auth-providers', table: this.authProvidersTable }, + { name: 'oidc-state', table: this.oidcStateTable }, + { name: 'bff-sessions', table: this.bffSessionsTable }, + { name: 'users', table: this.usersTable }, + { name: 'app-roles', table: this.appRolesTable }, + { name: 'api-keys', table: this.apiKeysTable }, + { name: 'user-quotas', table: this.userQuotasTable }, + { name: 'quota-events', table: this.quotaEventsTable }, + { name: 'audit-log', table: this.auditLogTable }, + { name: 'sessions-metadata', table: this.sessionsMetadataTable }, + { name: 'user-cost-summary', table: this.userCostSummaryTable }, + { name: 'system-cost-rollup', table: this.systemCostRollupTable }, + { name: 'managed-models', table: this.managedModelsTable }, + { name: 'user-settings', table: this.userSettingsTable }, + { name: 'user-menu-links', table: this.userMenuLinksTable }, + { name: 'system-prompts', table: this.systemPromptsTable }, + { name: 'shared-conversations', table: this.sharedConversationsTable }, + { name: 'user-file-uploads', table: this.fileUploadTable }, + { name: 'rag-assistants', table: this.ragAssistantsTable }, + { name: 'user-artifacts', table: this.artifactsTable }, + { name: 'memory-spaces', table: this.memorySpacesTable }, + { name: 'fine-tuning-jobs', table: this.fineTuningJobsTable }, + { name: 'fine-tuning-access', table: this.fineTuningAccessTable }, + ], + }); + + // Last: collectAlarms() walks the construct tree, so every alarm must + // already exist. + new PlatformDashboardConstruct(this, 'PlatformDashboard', { + config: this._config, + loadBalancer: this.alb, + targetGroup: appApi.targetGroup, + service: appApi.ecsService, + runtimeArn: inferenceApi.runtime.attrAgentRuntimeArn, + runtimeMetricName: inferenceApi.runtimeMetricName, + alarms: collectAlarms(this), + }); } } diff --git a/infrastructure/test/config.test.ts b/infrastructure/test/config.test.ts index 23fa54b48..99c00c78a 100644 --- a/infrastructure/test/config.test.ts +++ b/infrastructure/test/config.test.ts @@ -1,5 +1,17 @@ import * as cdk from 'aws-cdk-lib'; -import { loadConfig, AppConfig } from '../lib/config'; +import { loadConfig, AppConfig, + OBSERVABILITY_DEFAULT_AGENTCORE_ERROR_THRESHOLD, + OBSERVABILITY_DEFAULT_ALB_TARGET_5XX_THRESHOLD, + OBSERVABILITY_DEFAULT_DYNAMO_THROTTLE_THRESHOLD, + OBSERVABILITY_DEFAULT_ECS_CPU_PERCENT, + OBSERVABILITY_DEFAULT_ECS_MEMORY_PERCENT, + OBSERVABILITY_DEFAULT_LAMBDA_DURATION_PERCENT_OF_TIMEOUT, + OBSERVABILITY_DEFAULT_LAMBDA_ERROR_THRESHOLD, + OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS, + OBSERVABILITY_DEFAULT_P99_LATENCY_MS, + OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE, + OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RESERVOIR, +} from '../lib/config'; /** * Unit Tests for RAG Ingestion Configuration @@ -62,6 +74,37 @@ function clearManagedKbEnv(): void { } } +/** + * Scrubbed before AND after every test: these assert the defaults, so a leaked + * value would make a "defaults to X" assertion pass while reading an override. + */ +const OBSERVABILITY_ENV_KEYS = [ + 'CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED', + 'CDK_OBSERVABILITY_LOG_RETENTION_DAYS', + 'CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD', + 'CDK_OBSERVABILITY_ALB_P99_LATENCY_MS', + 'CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS', + 'CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD', + 'CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD', + 'CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT', + 'CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD', + 'CDK_OBSERVABILITY_ECS_CPU_PERCENT', + 'CDK_OBSERVABILITY_ECS_MEMORY_PERCENT', + 'CDK_OBSERVABILITY_XRAY_SAMPLING_RATE', + 'CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR', + 'CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS', + 'CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED', + 'CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD', + 'CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD', + 'CDK_OBSERVABILITY_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD', +] as const; + +function clearObservabilityEnv(): void { + for (const key of OBSERVABILITY_ENV_KEYS) { + delete process.env[key]; + } +} + describe('RAG Ingestion Configuration', () => { let app: cdk.App; let originalEnv: NodeJS.ProcessEnv; @@ -74,6 +117,7 @@ describe('RAG Ingestion Configuration', () => { // Hermetic start: drop any RAG keys a prior test may have leaked. clearRagEnv(); clearManagedKbEnv(); + clearObservabilityEnv(); // Create a fresh CDK app for each test app = new cdk.App(); @@ -137,6 +181,7 @@ describe('RAG Ingestion Configuration', () => { // the original environment object. clearRagEnv(); clearManagedKbEnv(); + clearObservabilityEnv(); process.env = originalEnv; }); @@ -1446,3 +1491,313 @@ describe('RAG Ingestion Configuration', () => { }); }); }); + +// ============================================================ +// Observability Configuration +// ============================================================ + +/** + * Defaults are asserted against their exported constants, and every field is + * checked through the FLAT dotted context key — `--context observability.x=y` + * sets context['observability.x'] and does NOT build a nested object, a trap + * that has already cost this repo twice. + */ +describe('Observability Configuration', () => { + let app: cdk.App; + let originalEnv: NodeJS.ProcessEnv; + + /** Minimum context for loadConfig() to reach the observability section. */ + function setRequiredContext(a: cdk.App): void { + a.node.setContext('projectPrefix', 'test-project'); + a.node.setContext('awsRegion', 'us-east-1'); + a.node.setContext('awsAccount', '123456789012'); + a.node.setContext('vpcCidr', '10.0.0.0/16'); + a.node.setContext('frontend', { cloudFrontPriceClass: 'PriceClass_100' }); + a.node.setContext('appApi', { + cpu: 256, memory: 512, desiredCount: 1, maxCapacity: 4, + }); + a.node.setContext('ragIngestion', { + lambdaMemorySize: 10240, + lambdaTimeout: 900, + embeddingModel: 'amazon.titan-embed-text-v2', + vectorDimension: 1024, + vectorDistanceMetric: 'cosine', + }); + } + + beforeEach(() => { + originalEnv = { ...process.env }; + process.env = { ...originalEnv }; + clearObservabilityEnv(); + app = new cdk.App(); + setRequiredContext(app); + }); + + afterEach(() => { + clearObservabilityEnv(); + process.env = originalEnv; + }); + + describe('cost-conscious defaults', () => { + test('log retention defaults to the exported constant', () => { + expect(loadConfig(app).observability.logRetentionDays).toBe( + OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS, + ); + }); + + // Was fixedRate 1.0 for any fork that never set `production`. + test('X-Ray sampling defaults to 1%, not 100%', () => { + const { xraySamplingRate } = loadConfig(app).observability; + expect(xraySamplingRate).toBe(OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE); + expect(xraySamplingRate).toBeLessThanOrEqual(0.05); + }); + + test('X-Ray reservoir defaults to 1 trace/sec', () => { + expect(loadConfig(app).observability.xraySamplingReservoir).toBe( + OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RESERVOIR, + ); + }); + + test('AgentCore APPLICATION_LOGS default to OFF', () => { + expect(loadConfig(app).observability.agentCoreApplicationLogsEnabled).toBe(false); + }); + + test('X-Ray Insights notifications default to OFF', () => { + expect(loadConfig(app).observability.xrayInsightsNotifications).toBe(false); + }); + + test('alarm topic defaults to ON', () => { + expect(loadConfig(app).observability.alarmTopicEnabled).toBe(true); + }); + + test('latency floors are streaming-aware, well above a normal agent turn', () => { + const obs = loadConfig(app).observability; + expect(obs.agentCoreLatencyMs).toBe(OBSERVABILITY_DEFAULT_P99_LATENCY_MS); + expect(obs.albP99LatencyMs).toBe(OBSERVABILITY_DEFAULT_P99_LATENCY_MS); + expect(obs.agentCoreLatencyMs).toBeGreaterThan(30_000); + }); + + test('threshold and percentage defaults match their constants', () => { + const obs = loadConfig(app).observability; + expect(obs.albTarget5xxThreshold).toBe(OBSERVABILITY_DEFAULT_ALB_TARGET_5XX_THRESHOLD); + expect(obs.agentCoreErrorThreshold).toBe(OBSERVABILITY_DEFAULT_AGENTCORE_ERROR_THRESHOLD); + expect(obs.lambdaErrorThreshold).toBe(OBSERVABILITY_DEFAULT_LAMBDA_ERROR_THRESHOLD); + expect(obs.lambdaDurationPercentOfTimeout).toBe( + OBSERVABILITY_DEFAULT_LAMBDA_DURATION_PERCENT_OF_TIMEOUT, + ); + expect(obs.dynamoThrottleThreshold).toBe(OBSERVABILITY_DEFAULT_DYNAMO_THROTTLE_THRESHOLD); + expect(obs.ecsCpuPercent).toBe(OBSERVABILITY_DEFAULT_ECS_CPU_PERCENT); + expect(obs.ecsMemoryPercent).toBe(OBSERVABILITY_DEFAULT_ECS_MEMORY_PERCENT); + }); + }); + + describe('environment variable overrides', () => { + test('CDK_OBSERVABILITY_LOG_RETENTION_DAYS reaches config', () => { + process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS = '90'; + expect(loadConfig(app).observability.logRetentionDays).toBe(90); + }); + + // parseInt('0.25') is 0, which would switch sampling off entirely. + test('fractional X-Ray sampling rate survives parsing', () => { + process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE = '0.25'; + expect(loadConfig(app).observability.xraySamplingRate).toBe(0.25); + }); + + test('booleans parse from env', () => { + process.env.CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED = 'false'; + process.env.CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED = 'true'; + const obs = loadConfig(app).observability; + expect(obs.alarmTopicEnabled).toBe(false); + expect(obs.agentCoreApplicationLogsEnabled).toBe(true); + }); + + test('every numeric field is settable from its env var', () => { + process.env.CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD = '1'; + process.env.CDK_OBSERVABILITY_ALB_P99_LATENCY_MS = '2000'; + process.env.CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS = '3000'; + process.env.CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD = '4'; + process.env.CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD = '5'; + process.env.CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT = '60'; + process.env.CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD = '7'; + process.env.CDK_OBSERVABILITY_ECS_CPU_PERCENT = '65'; + process.env.CDK_OBSERVABILITY_ECS_MEMORY_PERCENT = '70'; + process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR = '9'; + + const obs = loadConfig(app).observability; + expect(obs.albTarget5xxThreshold).toBe(1); + expect(obs.albP99LatencyMs).toBe(2000); + expect(obs.agentCoreLatencyMs).toBe(3000); + expect(obs.agentCoreErrorThreshold).toBe(4); + expect(obs.lambdaErrorThreshold).toBe(5); + expect(obs.lambdaDurationPercentOfTimeout).toBe(60); + expect(obs.dynamoThrottleThreshold).toBe(7); + expect(obs.ecsCpuPercent).toBe(65); + expect(obs.ecsMemoryPercent).toBe(70); + expect(obs.xraySamplingReservoir).toBe(9); + }); + }); + + describe('flat dotted context key (what --context actually sets)', () => { + test('flat dotted key is honoured for a number', () => { + app.node.setContext('observability.logRetentionDays', '90'); + expect(loadConfig(app).observability.logRetentionDays).toBe(90); + }); + + test('flat dotted key is honoured for a fractional rate', () => { + app.node.setContext('observability.xraySamplingRate', '0.5'); + expect(loadConfig(app).observability.xraySamplingRate).toBe(0.5); + }); + + test('flat dotted key is honoured for a boolean', () => { + app.node.setContext('observability.agentCoreApplicationLogsEnabled', 'true'); + app.node.setContext('observability.promptCacheAvoidableMissThreshold', '22'); + app.node.setContext('observability.promptCacheWastedUsdThreshold', '2.5'); + app.node.setContext('observability.promptCacheSessionWastedUsdThreshold', '23'); + expect(loadConfig(app).observability.agentCoreApplicationLogsEnabled).toBe(true); + }); + + test('every field is reachable via its flat dotted key', () => { + app.node.setContext('observability.alarmTopicEnabled', 'false'); + app.node.setContext('observability.logRetentionDays', '7'); + app.node.setContext('observability.albTarget5xxThreshold', '11'); + app.node.setContext('observability.albP99LatencyMs', '12'); + app.node.setContext('observability.agentCoreLatencyMs', '13'); + app.node.setContext('observability.agentCoreErrorThreshold', '14'); + app.node.setContext('observability.lambdaErrorThreshold', '15'); + app.node.setContext('observability.lambdaDurationPercentOfTimeout', '16'); + app.node.setContext('observability.dynamoThrottleThreshold', '17'); + app.node.setContext('observability.ecsCpuPercent', '18'); + app.node.setContext('observability.ecsMemoryPercent', '19'); + app.node.setContext('observability.xraySamplingRate', '0.2'); + app.node.setContext('observability.xraySamplingReservoir', '21'); + app.node.setContext('observability.xrayInsightsNotifications', 'true'); + app.node.setContext('observability.agentCoreApplicationLogsEnabled', 'true'); + app.node.setContext('observability.promptCacheAvoidableMissThreshold', '22'); + app.node.setContext('observability.promptCacheWastedUsdThreshold', '2.5'); + app.node.setContext('observability.promptCacheSessionWastedUsdThreshold', '23'); + + expect(loadConfig(app).observability).toEqual({ + alarmTopicEnabled: false, + logRetentionDays: 7, + albTarget5xxThreshold: 11, + albP99LatencyMs: 12, + agentCoreLatencyMs: 13, + agentCoreErrorThreshold: 14, + lambdaErrorThreshold: 15, + lambdaDurationPercentOfTimeout: 16, + dynamoThrottleThreshold: 17, + ecsCpuPercent: 18, + ecsMemoryPercent: 19, + xraySamplingRate: 0.2, + xraySamplingReservoir: 21, + xrayInsightsNotifications: true, + agentCoreApplicationLogsEnabled: true, + promptCacheAvoidableMissThreshold: 22, + promptCacheWastedUsdThreshold: 2.5, + promptCacheSessionWastedUsdThreshold: 23, + }); + }); + }); + + describe('nested context object (cdk.context.json)', () => { + test('nested object is honoured', () => { + app.node.setContext('observability', { + logRetentionDays: 365, + xraySamplingRate: 0.1, + alarmTopicEnabled: false, + }); + const obs = loadConfig(app).observability; + expect(obs.logRetentionDays).toBe(365); + expect(obs.xraySamplingRate).toBe(0.1); + expect(obs.alarmTopicEnabled).toBe(false); + }); + + test('unset fields in a nested object still take their defaults', () => { + app.node.setContext('observability', { logRetentionDays: 365 }); + const obs = loadConfig(app).observability; + expect(obs.logRetentionDays).toBe(365); + expect(obs.xraySamplingRate).toBe(OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE); + }); + }); + + describe('precedence: env > flat dotted context > nested context > default', () => { + test('env beats both context forms', () => { + process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS = '7'; + app.node.setContext('observability.logRetentionDays', '90'); + app.node.setContext('observability', { logRetentionDays: 365 }); + expect(loadConfig(app).observability.logRetentionDays).toBe(7); + }); + + test('flat dotted context beats nested context', () => { + app.node.setContext('observability.logRetentionDays', '90'); + app.node.setContext('observability', { logRetentionDays: 365 }); + expect(loadConfig(app).observability.logRetentionDays).toBe(90); + }); + + // An unset GitHub Actions variable arrives as the empty string. + test('empty env var falls through to the default', () => { + process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS = ''; + process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE = ''; + const obs = loadConfig(app).observability; + expect(obs.logRetentionDays).toBe(OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS); + expect(obs.xraySamplingRate).toBe(OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE); + }); + + // false is a legitimate value, not "absent". + test('an explicit false is not overwritten by the ON default', () => { + process.env.CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED = 'false'; + expect(loadConfig(app).observability.alarmTopicEnabled).toBe(false); + }); + }); + + describe('validation', () => { + test('rejects a retention value CloudWatch does not accept', () => { + process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS = '45'; + expect(() => loadConfig(app)).toThrow(/logRetentionDays/); + }); + + test('accepts every documented CloudWatch retention value', () => { + for (const days of [1, 7, 30, 90, 365, 3653]) { + const freshApp = new cdk.App(); + setRequiredContext(freshApp); + process.env.CDK_OBSERVABILITY_LOG_RETENTION_DAYS = String(days); + expect(loadConfig(freshApp).observability.logRetentionDays).toBe(days); + } + }); + + // 5 instead of 0.05 is a 100x cost error; reject rather than clamp. + test('rejects an X-Ray sampling rate given as a percentage', () => { + process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE = '5'; + expect(() => loadConfig(app)).toThrow(/xraySamplingRate/); + }); + + test('rejects a negative X-Ray sampling rate', () => { + process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE = '-0.1'; + expect(() => loadConfig(app)).toThrow(/xraySamplingRate/); + }); + + test('accepts the boundary sampling rates 0.0 and 1.0', () => { + for (const rate of ['0', '1']) { + const freshApp = new cdk.App(); + setRequiredContext(freshApp); + process.env.CDK_OBSERVABILITY_XRAY_SAMPLING_RATE = rate; + expect(() => loadConfig(freshApp)).not.toThrow(); + } + }); + + test('rejects out-of-range percentages', () => { + const cases: Array<[string, string]> = [ + ['CDK_OBSERVABILITY_ECS_CPU_PERCENT', '101'], + ['CDK_OBSERVABILITY_ECS_MEMORY_PERCENT', '0'], + ['CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT', '150'], + ]; + for (const [key, value] of cases) { + const freshApp = new cdk.App(); + setRequiredContext(freshApp); + clearObservabilityEnv(); + process.env[key] = value; + expect(() => loadConfig(freshApp)).toThrow(/Expected a percentage/); + } + }); + }); +}); diff --git a/infrastructure/test/helpers/mock-config.ts b/infrastructure/test/helpers/mock-config.ts index 992dd1773..750812254 100644 --- a/infrastructure/test/helpers/mock-config.ts +++ b/infrastructure/test/helpers/mock-config.ts @@ -11,6 +11,20 @@ import { AppConfig, MANAGED_KB_ELEVATED_PER_OWNER_BYTES, MANAGED_KB_PER_KB_CEILING_BYTES, MANAGED_KB_RETENTION_WINDOW_DAYS, + OBSERVABILITY_DEFAULT_AGENTCORE_ERROR_THRESHOLD, + OBSERVABILITY_DEFAULT_ALB_TARGET_5XX_THRESHOLD, + OBSERVABILITY_DEFAULT_DYNAMO_THROTTLE_THRESHOLD, + OBSERVABILITY_DEFAULT_ECS_CPU_PERCENT, + OBSERVABILITY_DEFAULT_ECS_MEMORY_PERCENT, + OBSERVABILITY_DEFAULT_LAMBDA_DURATION_PERCENT_OF_TIMEOUT, + OBSERVABILITY_DEFAULT_LAMBDA_ERROR_THRESHOLD, + OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS, + OBSERVABILITY_DEFAULT_P99_LATENCY_MS, + OBSERVABILITY_DEFAULT_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD, + OBSERVABILITY_DEFAULT_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD, + OBSERVABILITY_DEFAULT_PROMPT_CACHE_WASTED_USD_THRESHOLD, + OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE, + OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RESERVOIR, } from '../../lib/config'; /** Default mock account and region used across all tests. */ @@ -43,6 +57,35 @@ export function createMockConfig(overrides: Partial = {}): AppConfig maxCapacity: 2, }, inferenceApi: {}, + // Observability mirrors the shipped OSS defaults rather than hardcoded + // literals, so a change to a default is exercised by every existing test + // instead of silently diverging from what a fork actually deploys. + // alarmTopicEnabled is ON here because routing is the point of the feature: + // the "every alarm has an action" guard must run against the default shape. + observability: { + alarmTopicEnabled: true, + logRetentionDays: OBSERVABILITY_DEFAULT_LOG_RETENTION_DAYS, + albTarget5xxThreshold: OBSERVABILITY_DEFAULT_ALB_TARGET_5XX_THRESHOLD, + albP99LatencyMs: OBSERVABILITY_DEFAULT_P99_LATENCY_MS, + agentCoreLatencyMs: OBSERVABILITY_DEFAULT_P99_LATENCY_MS, + agentCoreErrorThreshold: OBSERVABILITY_DEFAULT_AGENTCORE_ERROR_THRESHOLD, + lambdaErrorThreshold: OBSERVABILITY_DEFAULT_LAMBDA_ERROR_THRESHOLD, + lambdaDurationPercentOfTimeout: + OBSERVABILITY_DEFAULT_LAMBDA_DURATION_PERCENT_OF_TIMEOUT, + dynamoThrottleThreshold: OBSERVABILITY_DEFAULT_DYNAMO_THROTTLE_THRESHOLD, + ecsCpuPercent: OBSERVABILITY_DEFAULT_ECS_CPU_PERCENT, + ecsMemoryPercent: OBSERVABILITY_DEFAULT_ECS_MEMORY_PERCENT, + promptCacheAvoidableMissThreshold: + OBSERVABILITY_DEFAULT_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD, + promptCacheWastedUsdThreshold: + OBSERVABILITY_DEFAULT_PROMPT_CACHE_WASTED_USD_THRESHOLD, + promptCacheSessionWastedUsdThreshold: + OBSERVABILITY_DEFAULT_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD, + xraySamplingRate: OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RATE, + xraySamplingReservoir: OBSERVABILITY_DEFAULT_XRAY_SAMPLING_RESERVOIR, + xrayInsightsNotifications: false, + agentCoreApplicationLogsEnabled: false, + }, ragIngestion: { lambdaMemorySize: 3008, lambdaTimeout: 900, diff --git a/infrastructure/test/observability-agentcore-alarms.test.ts b/infrastructure/test/observability-agentcore-alarms.test.ts new file mode 100644 index 000000000..8dca826d7 --- /dev/null +++ b/infrastructure/test/observability-agentcore-alarms.test.ts @@ -0,0 +1,168 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; + +/** + * Pins the AgentCore metric binding, which was previously wrong: namespace + * `bedrock-agentcore` with InvocationCount / InvocationErrors / + * InvocationLatency, none of which exist. Both alarms had been in + * INSUFFICIENT_DATA since creation, reading as healthy. + */ +describe('AgentCore Runtime alarms — verified metric binding', () => { + const NAMESPACE = 'AWS/Bedrock-AgentCore'; + let template: Template; + let alarms: Record; + + function byName(name: string): any { + const full = `${MOCK_PREFIX}-${name}`; + const found = Object.values(alarms).find((a) => a.Properties.AlarmName === full); + expect(found).toBeDefined(); + return found; + } + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + alarms = template.findResources('AWS::CloudWatch::Alarm'); + }); + + const AGENTCORE_ALARMS = [ + 'agentcore-system-errors', + 'agentcore-high-error-rate', + 'agentcore-throttles', + 'agentcore-high-latency', + ]; + + it('creates the four runtime alarms', () => { + for (const name of AGENTCORE_ALARMS) byName(name); + }); + + it('uses the AWS/Bedrock-AgentCore namespace', () => { + for (const name of AGENTCORE_ALARMS) { + expect(byName(name).Properties.Namespace).toBe(NAMESPACE); + } + }); + + // Whole-template, so a dashboard widget cannot reintroduce them either. + it('no alarm or dashboard references the non-existent metric names', () => { + const whole = JSON.stringify(template.toJSON()); + for (const dead of ['InvocationCount', 'InvocationErrors', 'InvocationLatency']) { + expect(whole).not.toContain(dead); + } + // The bare lowercase namespace must not appear as a metric namespace. It is + // a real namespace, but it holds OTEL application metrics, not these. + for (const alarm of Object.values(alarms)) { + expect((alarm as any).Properties.Namespace).not.toBe('bedrock-agentcore'); + } + }); + + it('alarms on the verified metric names', () => { + expect(byName('agentcore-system-errors').Properties.MetricName).toBe('SystemErrors'); + expect(byName('agentcore-high-error-rate').Properties.MetricName).toBe('UserErrors'); + expect(byName('agentcore-throttles').Properties.MetricName).toBe('Throttles'); + expect(byName('agentcore-high-latency').Properties.MetricName).toBe('Latency'); + }); + + // Every stream here is dimensioned; an undimensioned metric matches nothing. + it('binds the three-dimension runtime set on every alarm', () => { + for (const name of AGENTCORE_ALARMS) { + const dims = byName(name).Properties.Dimensions; + const keys = dims.map((d: any) => d.Name).sort(); + expect(keys).toEqual(['Name', 'Operation', 'Resource']); + + const operation = dims.find((d: any) => d.Name === 'Operation'); + expect(operation.Value).toBe('InvokeAgentRuntime'); + + // Resource is the runtime ARN, resolved from the real resource. + const resource = dims.find((d: any) => d.Name === 'Resource'); + expect(JSON.stringify(resource.Value)).toMatch(/Fn::GetAtt|Ref/); + + // Name is {runtimeName}::DEFAULT, matching the endpoint qualifier used + // for the runtime's log group. + const nameDim = dims.find((d: any) => d.Name === 'Name'); + expect(JSON.stringify(nameDim.Value)).toContain('::DEFAULT'); + } + }); + + it('does not bind the ComputeType implementation detail', () => { + for (const name of AGENTCORE_ALARMS) { + const keys = byName(name).Properties.Dimensions.map((d: any) => d.Name); + expect(keys).not.toContain('ComputeType'); + } + }); + + // Milliseconds, unlike the ALB metric. Measured turns peak near 25s, so the + // previous 30000 threshold sat just above normal. + it('latency threshold is in milliseconds and clears a real long turn', () => { + const alarm = byName('agentcore-high-latency'); + expect(alarm.Properties.Threshold).toBe(120_000); + expect(alarm.Properties.ExtendedStatistic).toBe('p99'); + expect(alarm.Properties.Threshold).toBeGreaterThan(24_400); + }); + + it('separates system errors from user errors', () => { + expect(byName('agentcore-system-errors').Properties.MetricName) + .not.toBe(byName('agentcore-high-error-rate').Properties.MetricName); + }); + + it('throttle alarm fires on any throttle at all', () => { + expect(byName('agentcore-throttles').Properties.Threshold).toBe(0); + }); + + it('all four alarms are routed to the alarm topic', () => { + for (const name of AGENTCORE_ALARMS) { + expect(byName(name).Properties.AlarmActions).toHaveLength(1); + expect(byName(name).Properties.OKActions).toHaveLength(1); + } + }); + + describe('dashboard', () => { + it('graphs the verified namespace and metrics', () => { + const dashboards = template.findResources('AWS::CloudWatch::Dashboard'); + const agentcore = Object.values(dashboards).find((d: any) => + JSON.stringify(d.Properties.DashboardName).includes('agentcore-observability'), + ); + expect(agentcore).toBeDefined(); + const body = JSON.stringify((agentcore as any).Properties.DashboardBody); + + expect(body).toContain(NAMESPACE); + for (const metric of [ + 'Invocations', 'SystemErrors', 'UserErrors', 'Throttles', + 'Sessions', 'Latency', 'ActiveSessionCount', + ]) { + expect(body).toContain(metric); + } + }); + + // InputTokens/OutputTokens do not exist; the token metrics in this namespace + // are Memory-strategy counters, not model tokens. + it('does not graph non-existent token metrics, and points at the right dashboard', () => { + const dashboards = template.findResources('AWS::CloudWatch::Dashboard'); + const agentcore = Object.values(dashboards).find((d: any) => + JSON.stringify(d.Properties.DashboardName).includes('agentcore-observability'), + ); + const body = JSON.stringify((agentcore as any).Properties.DashboardBody); + expect(body).not.toContain('InputTokens"'); + expect(body).not.toContain('OutputTokens"'); + expect(body).toContain('prompt-cache-observability'); + }); + }); +}); diff --git a/infrastructure/test/observability-ai-path-alarms.test.ts b/infrastructure/test/observability-ai-path-alarms.test.ts new file mode 100644 index 000000000..8eb934486 --- /dev/null +++ b/infrastructure/test/observability-ai-path-alarms.test.ts @@ -0,0 +1,194 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; + +describe('AI-path alarms (Bedrock, Memory, Gateway, Code Interpreter)', () => { + const AGENTCORE_NS = 'AWS/Bedrock-AgentCore'; + let template: Template; + let alarms: Record; + + function byName(name: string): any { + const full = `${MOCK_PREFIX}-${name}`; + const found = Object.values(alarms).find((a) => a.Properties.AlarmName === full); + expect(found).toBeDefined(); + return found; + } + + function allNames(): string[] { + return Object.values(alarms) + .map((a) => a.Properties.AlarmName as string) + .filter((n) => typeof n === 'string'); + } + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + alarms = template.findResources('AWS::CloudWatch::Alarm'); + }); + + describe('Bedrock inference', () => { + it('alarms on throttles, server errors, and quota usage', () => { + for (const name of [ + 'bedrock-invocation-throttles', + 'bedrock-invocation-server-errors', + 'bedrock-tpm-quota-usage', + ]) { + expect(byName(name).Properties.Namespace).toBe('AWS/Bedrock'); + } + }); + + it('uses the account-wide roll-up rather than per-model alarms', () => { + for (const name of ['bedrock-invocation-throttles', 'bedrock-tpm-quota-usage']) { + expect(byName(name).Properties.Dimensions).toBeUndefined(); + } + }); + + it('throttle alarm fires on any throttle', () => { + const alarm = byName('bedrock-invocation-throttles'); + expect(alarm.Properties.MetricName).toBe('InvocationThrottles'); + expect(alarm.Properties.Threshold).toBe(0); + // Zero streams when verified — never fired, rather than absent. + expect(alarm.Properties.TreatMissingData).toBe('notBreaching'); + }); + + it('quota-usage alarm is a percentage gauge on a metric that has live data', () => { + const alarm = byName('bedrock-tpm-quota-usage'); + expect(alarm.Properties.MetricName).toBe('EstimatedTPMQuotaUsage'); + expect(alarm.Properties.Statistic).toBe('Maximum'); + expect(alarm.Properties.Threshold).toBe(80); + }); + }); + + describe('AgentCore Memory', () => { + it('alarms on hot-path system errors and throttles', () => { + for (const name of [ + 'agentcore-memory-system-errors', + 'agentcore-memory-throttles', + ]) { + const alarm = byName(name); + // Per-Operation streams mean these render as metric math. + expect(Array.isArray(alarm.Properties.Metrics)).toBe(true); + expect(JSON.stringify(alarm.Properties.Metrics)).toContain(AGENTCORE_NS); + } + }); + + // Extraction and Consolidation are async and do not break a live turn. + it('sums only the conversation hot-path operations', () => { + const json = JSON.stringify(byName('agentcore-memory-system-errors').Properties.Metrics); + for (const op of [ + 'CreateEvent', 'RetrieveMemoryRecords', 'GetMemoryRecord', 'ListEvents', 'GetMemory', + ]) { + expect(json).toContain(op); + } + expect(json).not.toContain('Extraction'); + expect(json).not.toContain('Consolidation'); + }); + + it('binds Resource to the Memory ARN', () => { + const metrics = byName('agentcore-memory-system-errors').Properties.Metrics; + const stat = metrics.find((m: any) => m.MetricStat); + const resource = stat.MetricStat.Metric.Dimensions.find((d: any) => d.Name === 'Resource'); + // Memory publishes a full ARN in this dimension. + expect(JSON.stringify(resource.Value)).toMatch(/Fn::GetAtt|Ref|arn:/); + }); + + it('stays inside the 10-metric math limit', () => { + const metrics = byName('agentcore-memory-system-errors').Properties.Metrics; + expect(metrics.filter((m: any) => m.MetricStat).length).toBe(5); + }); + }); + + describe('AgentCore Gateway', () => { + it('alarms on MCP system errors and throttles with the method roll-up', () => { + for (const name of [ + 'agentcore-gateway-system-errors', + 'agentcore-gateway-throttles', + ]) { + const alarm = byName(name); + expect(alarm.Properties.Namespace).toBe(AGENTCORE_NS); + const keys = alarm.Properties.Dimensions.map((d: any) => d.Name).sort(); + // Resource + Operation + Protocol, and deliberately NOT Method: a + // per-Method alarm set would multiply with every tool exposed. + expect(keys).toEqual(['Operation', 'Protocol', 'Resource']); + const protocol = alarm.Properties.Dimensions.find((d: any) => d.Name === 'Protocol'); + expect(protocol.Value).toBe('MCP'); + } + }); + }); + + describe('AgentCore Code Interpreter', () => { + it('alarms on session system errors across all three operations', () => { + const json = JSON.stringify( + byName('agentcore-code-interpreter-system-errors').Properties.Metrics, + ); + for (const op of [ + 'StartCodeInterpreterSession', 'InvokeCodeInterpreter', 'StopCodeInterpreterSession', + ]) { + expect(json).toContain(op); + } + }); + + // Memory and Gateway use full ARNs for this same key; an ARN here matches + // no stream. + it('binds Resource to the bare Code Interpreter ID, not an ARN', () => { + const metrics = byName('agentcore-code-interpreter-system-errors').Properties.Metrics; + const stat = metrics.find((m: any) => m.MetricStat); + const resource = stat.MetricStat.Metric.Dimensions.find((d: any) => d.Name === 'Resource'); + expect(JSON.stringify(resource.Value)).not.toContain('arn:aws:bedrock-agentcore'); + }); + + it('alarms on concurrent session count as an account-level gauge', () => { + const alarm = byName('agentcore-code-interpreter-active-sessions'); + expect(alarm.Properties.MetricName).toBe('ActiveSessionCount'); + const service = alarm.Properties.Dimensions.find((d: any) => d.Name === 'Service'); + expect(service.Value).toBe('AgentCore.CodeInterpreter'); + }); + }); + + describe('deliberate omissions', () => { + // AWS/Cognito publishes only success metrics on the ESSENTIALS plan; failure + // metrics need Plus. The auth signal is the token-enrichment Lambda. + it('creates no Cognito alarm, because no failure metric exists to watch', () => { + for (const alarm of Object.values(alarms)) { + expect((alarm as any).Properties.Namespace).not.toBe('AWS/Cognito'); + } + expect(allNames().filter((n) => /cognito|sign-in/i.test(n))).toEqual([]); + }); + + it('creates no AgentCore Browser alarm', () => { + expect(allNames().filter((n) => /browser/i.test(n))).toEqual([]); + }); + }); + + it('all AI-path alarms are routed to the alarm topic', () => { + for (const name of [ + 'bedrock-invocation-throttles', 'bedrock-invocation-server-errors', + 'bedrock-tpm-quota-usage', 'agentcore-memory-system-errors', + 'agentcore-memory-throttles', 'agentcore-gateway-system-errors', + 'agentcore-gateway-throttles', 'agentcore-code-interpreter-system-errors', + 'agentcore-code-interpreter-active-sessions', + ]) { + const alarm = byName(name); + expect(alarm.Properties.AlarmActions).toHaveLength(1); + expect(alarm.Properties.OKActions).toHaveLength(1); + } + }); +}); diff --git a/infrastructure/test/observability-alarm-routing.test.ts b/infrastructure/test/observability-alarm-routing.test.ts new file mode 100644 index 000000000..b5f3423f0 --- /dev/null +++ b/infrastructure/test/observability-alarm-routing.test.ts @@ -0,0 +1,169 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; +import * as fs from 'fs'; +import * as path from 'path'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_REGION } from './helpers/mock-config'; + +/** + * Fails if any alarm in the template lacks an action. An unrouted alarm still + * turns red in the console, so the gap is invisible from the one place an + * operator would look — a convention cannot protect against that. + */ +describe('Alarm routing — every alarm reaches a human', () => { + let template: Template; + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + // Every optional alarm-bearing subsystem ON, so the guard sees the + // widest possible set of alarms rather than only the always-on ones. + kbSync: { enabled: true }, + scheduledRuns: { enabled: true }, + managedKb: { + newDefault: true, + migrationEnabled: true, + reconcilerArmed: true, + perOwnerDefaultBytes: 100 * 1024 * 1024, + perOwnerElevatedBytes: 1024 * 1024 * 1024, + perKnowledgeBaseCeilingBytes: 500 * 1024 * 1024, + retentionWindowDays: 30, + storageAlarmGb: 500, + dailyCostAlarmUsd: 100, + }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + }); + + // Floor, so the guard below cannot pass trivially on an empty set. + it('synthesizes a substantial number of alarms', () => { + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + expect(Object.keys(alarms).length).toBeGreaterThanOrEqual(13); + }); + + /** THE guard. */ + it('every alarm has a non-empty AlarmActions', () => { + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + const unrouted: string[] = []; + + for (const [logicalId, alarm] of Object.entries(alarms)) { + const actions = (alarm as any).Properties?.AlarmActions; + if (!Array.isArray(actions) || actions.length === 0) { + const name = (alarm as any).Properties?.AlarmName; + unrouted.push(`${logicalId} (${JSON.stringify(name)})`); + } + } + + expect(unrouted).toEqual([]); + }); + + it('every alarm also notifies on recovery (OKActions)', () => { + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + const noOk: string[] = []; + + for (const [logicalId, alarm] of Object.entries(alarms)) { + const actions = (alarm as any).Properties?.OKActions; + if (!Array.isArray(actions) || actions.length === 0) { + noOk.push(logicalId); + } + } + + expect(noOk).toEqual([]); + }); + + it('all alarm actions point at the single platform alarm topic', () => { + const topics = template.findResources('AWS::SNS::Topic'); + expect(Object.keys(topics)).toHaveLength(1); + const topicLogicalId = Object.keys(topics)[0]; + + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + for (const alarm of Object.values(alarms)) { + for (const action of (alarm as any).Properties.AlarmActions) { + // Each action is { Ref: }. + expect(JSON.stringify(action)).toContain(topicLogicalId); + } + } + }); + + it('every alarm has a name carrying the project prefix', () => { + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + for (const [logicalId, alarm] of Object.entries(alarms)) { + const name = (alarm as any).Properties?.AlarmName; + expect(typeof name === 'string' ? name : JSON.stringify(name)).toContain( + 'test-project', + ); + expect(logicalId).toBeTruthy(); + } + }); +}); + +/** + * Source-level guards, which also cover flag-gated code paths a synth never + * reaches. + */ +describe('Alarm routing — source-level guard', () => { + const libDir = path.join(__dirname, '..', 'lib'); + + function walk(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return walk(full); + return entry.isFile() && entry.name.endsWith('.ts') ? [full] : []; + }); + } + + it('no construct calls new cloudwatch.Alarm() directly — use AlarmFactory', () => { + const offenders: string[] = []; + + for (const file of walk(libDir)) { + if (file.endsWith(path.join('observability', 'alarm-factory.ts'))) continue; + + const source = fs.readFileSync(file, 'utf-8'); + if (/new\s+cloudwatch\.Alarm\s*\(/.test(source)) { + offenders.push(path.relative(libDir, file)); + } + } + + expect(offenders).toEqual([]); + }); + + it('no config.production branching in the observability constructs', () => { + const obsDir = path.join(libDir, 'constructs', 'observability'); + const offenders: string[] = []; + + for (const file of walk(obsDir)) { + const source = fs.readFileSync(file, 'utf-8'); + if (/config\.production/.test(source)) { + offenders.push(path.relative(obsDir, file)); + } + } + + expect(offenders).toEqual([]); + }); + + it('the stale "no SNS wiring yet" comments are gone', () => { + const offenders: string[] = []; + for (const file of walk(libDir)) { + const source = fs.readFileSync(file, 'utf-8'); + if (/no SNS (topics|wiring)/i.test(source)) { + offenders.push(path.relative(libDir, file)); + } + } + expect(offenders).toEqual([]); + }); +}); diff --git a/infrastructure/test/observability-alarm-topic.test.ts b/infrastructure/test/observability-alarm-topic.test.ts new file mode 100644 index 000000000..de3bf72f4 --- /dev/null +++ b/infrastructure/test/observability-alarm-topic.test.ts @@ -0,0 +1,158 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { AlarmTopicConstruct } from '../lib/constructs/observability/alarm-topic-construct'; +import { PlatformStack } from '../lib/platform-stack'; +import { + createMockConfig, + mockSsmContext, + MOCK_ACCOUNT, + MOCK_PREFIX, + MOCK_REGION, +} from './helpers/mock-config'; + +function synth(): Template { + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'Test', { + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + new AlarmTopicConstruct(stack, 'AlarmTopic', { config: createMockConfig() }); + return Template.fromStack(stack); +} + +describe('AlarmTopicConstruct', () => { + let t: Template; + beforeAll(() => { + t = synth(); + }); + + it('creates one topic with the conventional name', () => { + t.resourceCountIs('AWS::SNS::Topic', 1); + t.hasResourceProperties('AWS::SNS::Topic', { + TopicName: `${MOCK_PREFIX}-alarms`, + }); + }); + + it('encrypts the topic with a customer-managed key, not the AWS-managed one', () => { + t.resourceCountIs('AWS::KMS::Key', 1); + const topic = Object.values(t.findResources('AWS::SNS::Topic'))[0]; + // A Ref/Fn::GetAtt to our own key resource, never the string + // 'alias/aws/sns' — see the next test for why that distinction matters. + expect(topic.Properties.KmsMasterKeyId).toBeDefined(); + expect(JSON.stringify(topic.Properties.KmsMasterKeyId)).not.toContain('alias/aws/sns'); + }); + + // Without both actions the publish is denied and the message dropped + // silently. Decrypt alone is insufficient: SNS envelope encryption has the + // publisher generate the data key. + it('key policy lets CloudWatch generate a data key AND decrypt', () => { + const key = Object.values(t.findResources('AWS::KMS::Key'))[0]; + const statements = key.Properties.KeyPolicy.Statement; + + const cwStatement = statements.find( + (s: any) => s.Principal?.Service === 'cloudwatch.amazonaws.com', + ); + expect(cwStatement).toBeDefined(); + expect(cwStatement.Effect).toBe('Allow'); + + const actions: string[] = Array.isArray(cwStatement.Action) + ? cwStatement.Action + : [cwStatement.Action]; + expect(actions).toContain('kms:GenerateDataKey*'); + expect(actions).toContain('kms:Decrypt'); + }); + + it('scopes the CloudWatch key grant to this account', () => { + const key = Object.values(t.findResources('AWS::KMS::Key'))[0]; + const cwStatement = key.Properties.KeyPolicy.Statement.find( + (s: any) => s.Principal?.Service === 'cloudwatch.amazonaws.com', + ); + expect(cwStatement.Condition.StringEquals['aws:SourceAccount']).toBe(MOCK_ACCOUNT); + }); + + it('enables key rotation', () => { + t.hasResourceProperties('AWS::KMS::Key', { EnableKeyRotation: true }); + }); + + it('allows CloudWatch to publish and denies non-TLS traffic', () => { + const policies = t.findResources('AWS::SNS::TopicPolicy'); + const doc = JSON.stringify(Object.values(policies)[0].Properties.PolicyDocument); + expect(doc).toContain('cloudwatch.amazonaws.com'); + expect(doc).toContain('sns:Publish'); + // enforceSSL renders as a Deny on aws:SecureTransport false. + expect(doc).toContain('SecureTransport'); + }); + + // Subscriptions are managed out-of-band so adding a recipient needs no deploy. + it('creates NO subscriptions (managed out-of-band on purpose)', () => { + t.resourceCountIs('AWS::SNS::Subscription', 0); + }); + + it('publishes the topic ARN to SSM and as a CfnOutput for discovery', () => { + t.hasResourceProperties('AWS::SSM::Parameter', { + Name: `/${MOCK_PREFIX}/observability/alarm-topic-arn`, + }); + const outputs = t.findOutputs('*'); + const outputJson = JSON.stringify(outputs); + expect(outputJson).toContain('AlarmTopicArn'); + expect(outputJson).toContain(`${MOCK_PREFIX}-AlarmTopicArn`); + }); + + it('destroys the CMK on stack delete rather than stranding a billable key', () => { + const key = Object.values(t.findResources('AWS::KMS::Key'))[0]; + expect(key.DeletionPolicy).toBe('Delete'); + }); +}); + +/** The gate lives in PlatformStack, so it needs a real stack synth. */ +describe('PlatformStack alarm topic gating', () => { + function synthStack(alarmTopicEnabled: boolean): { stack: PlatformStack; template: Template } { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const base = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + }); + const config = { + ...base, + observability: { ...base.observability, alarmTopicEnabled }, + }; + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + return { stack, template: Template.fromStack(stack) }; + } + + it('exposes the topic and names it {prefix}-alarms when enabled', () => { + const { stack, template } = synthStack(true); + expect(stack.alarmTopic).toBeDefined(); + template.hasResourceProperties('AWS::SNS::Topic', { + TopicName: `${MOCK_PREFIX}-alarms`, + }); + }); + + it('creates no topic, no CMK, and no alarm actions when disabled', () => { + const { stack, template } = synthStack(false); + expect(stack.alarmTopic).toBeUndefined(); + + const topics = template.findResources('AWS::SNS::Topic'); + expect(Object.keys(topics)).toHaveLength(0); + + // On the alias, not a bare count: the stack has other CMKs. + const aliases = template.findResources('AWS::KMS::Alias'); + const aliasNames = Object.values(aliases).map((a: any) => a.Properties.AliasName); + expect(aliasNames).not.toContain(`alias/${MOCK_PREFIX}-alarm-topic-key`); + + for (const alarm of Object.values(template.findResources('AWS::CloudWatch::Alarm'))) { + expect((alarm as any).Properties.AlarmActions).toBeUndefined(); + } + }); +}); diff --git a/infrastructure/test/observability-alb-ecs-alarms.test.ts b/infrastructure/test/observability-alb-ecs-alarms.test.ts new file mode 100644 index 000000000..8c09f815d --- /dev/null +++ b/infrastructure/test/observability-alb-ecs-alarms.test.ts @@ -0,0 +1,174 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; + +/** + * Synthesized from the real PlatformStack, because what matters here is that the + * alarms bound to the actual load balancer, target group, cluster and service. + */ +describe('ALB and ECS service alarms', () => { + let template: Template; + let alarms: Record; + + /** Find an alarm by its unprefixed name. */ + function byName(name: string): any { + const full = `${MOCK_PREFIX}-${name}`; + const found = Object.values(alarms).find((a) => a.Properties.AlarmName === full); + expect(found).toBeDefined(); + return found; + } + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + alarms = template.findResources('AWS::CloudWatch::Alarm'); + }); + + describe('ALB alarms', () => { + it('creates the six front-door alarms', () => { + for (const name of [ + 'alb-elb-5xx', + 'alb-target-5xx', + 'alb-unhealthy-hosts', + 'alb-target-connection-errors', + 'alb-rejected-connections', + 'alb-target-p99-latency', + ]) { + byName(name); + } + }); + + // An undimensioned ALB alarm silently watches every load balancer in the + // account: it deploys, evaluates, and never means what was intended. + it('target alarms carry BOTH LoadBalancer and TargetGroup dimensions', () => { + for (const name of ['alb-target-5xx', 'alb-unhealthy-hosts', 'alb-target-p99-latency']) { + const dims = byName(name).Properties.Dimensions; + const keys = dims.map((d: any) => d.Name).sort(); + expect(keys).toEqual(['LoadBalancer', 'TargetGroup']); + // Values are CFN references to the real resources, not literals. + for (const d of dims) { + expect(JSON.stringify(d.Value)).toMatch(/Fn::GetAtt|Ref/); + } + } + }); + + it('load-balancer-scoped alarms carry the LoadBalancer dimension', () => { + for (const name of ['alb-elb-5xx', 'alb-rejected-connections', 'alb-target-connection-errors']) { + const dims = byName(name).Properties.Dimensions; + expect(dims.map((d: any) => d.Name)).toContain('LoadBalancer'); + } + }); + + it('alarms on the right ALB metrics in the AWS/ApplicationELB namespace', () => { + const expected: Record = { + 'alb-elb-5xx': 'HTTPCode_ELB_5XX_Count', + 'alb-target-5xx': 'HTTPCode_Target_5XX_Count', + 'alb-unhealthy-hosts': 'UnHealthyHostCount', + 'alb-target-connection-errors': 'TargetConnectionErrorCount', + 'alb-rejected-connections': 'RejectedConnectionCount', + 'alb-target-p99-latency': 'TargetResponseTime', + }; + for (const [name, metricName] of Object.entries(expected)) { + const alarm = byName(name); + expect(alarm.Properties.MetricName).toBe(metricName); + expect(alarm.Properties.Namespace).toBe('AWS/ApplicationELB'); + } + }); + + // The metric stops arriving when no targets are registered, so absent data + // is the outage. NOT_BREACHING would leave this silent during one. + it('unhealthy-host alarm treats missing data as BREACHING', () => { + expect(byName('alb-unhealthy-hosts').Properties.TreatMissingData).toBe('breaching'); + }); + + it('error-count alarms treat missing data as NOT_BREACHING (no traffic is fine)', () => { + for (const name of ['alb-elb-5xx', 'alb-target-5xx', 'alb-rejected-connections']) { + expect(byName(name).Properties.TreatMissingData).toBe('notBreaching'); + } + }); + + // CloudWatch reports this metric in seconds, config is in ms. + it('latency threshold is converted from config ms to CloudWatch seconds', () => { + const alarm = byName('alb-target-p99-latency'); + // Default albP99LatencyMs is 120000 ms -> 120 s. + expect(alarm.Properties.Threshold).toBe(120); + expect(alarm.Properties.ExtendedStatistic).toBe('p99'); + // Comfortably above a normal streaming turn. + expect(alarm.Properties.Threshold).toBeGreaterThan(30); + }); + }); + + describe('ECS service alarms', () => { + it('creates the three service alarms', () => { + for (const name of ['app-api-cpu-high', 'app-api-memory-high', 'app-api-running-tasks-low']) { + byName(name); + } + }); + + it('CPU and memory alarms carry BOTH ClusterName and ServiceName', () => { + for (const name of ['app-api-cpu-high', 'app-api-memory-high']) { + const dims = byName(name).Properties.Dimensions; + const keys = dims.map((d: any) => d.Name).sort(); + expect(keys).toEqual(['ClusterName', 'ServiceName']); + } + }); + + it('CPU and memory alarms use the AWS/ECS namespace and configured thresholds', () => { + const cpu = byName('app-api-cpu-high'); + expect(cpu.Properties.Namespace).toBe('AWS/ECS'); + expect(cpu.Properties.MetricName).toBe('CPUUtilization'); + expect(cpu.Properties.Threshold).toBe(80); + + const mem = byName('app-api-memory-high'); + expect(mem.Properties.Namespace).toBe('AWS/ECS'); + expect(mem.Properties.MetricName).toBe('MemoryUtilization'); + expect(mem.Properties.Threshold).toBe(85); + }); + + it('running-task alarm reads Container Insights with both dimensions', () => { + const alarm = byName('app-api-running-tasks-low'); + expect(alarm.Properties.Namespace).toBe('ECS/ContainerInsights'); + expect(alarm.Properties.MetricName).toBe('RunningTaskCount'); + const keys = alarm.Properties.Dimensions.map((d: any) => d.Name).sort(); + expect(keys).toEqual(['ClusterName', 'ServiceName']); + }); + + it('running-task alarm fires below desired count and treats missing data as BREACHING', () => { + const alarm = byName('app-api-running-tasks-low'); + expect(alarm.Properties.ComparisonOperator).toBe('LessThanThreshold'); + expect(alarm.Properties.Threshold).toBe(1); // mock config desiredCount + expect(alarm.Properties.TreatMissingData).toBe('breaching'); + }); + }); + + it('all new alarms are routed to the alarm topic', () => { + for (const name of [ + 'alb-elb-5xx', 'alb-target-5xx', 'alb-unhealthy-hosts', + 'alb-target-connection-errors', 'alb-rejected-connections', + 'alb-target-p99-latency', 'app-api-cpu-high', 'app-api-memory-high', + 'app-api-running-tasks-low', + ]) { + const alarm = byName(name); + expect(alarm.Properties.AlarmActions).toHaveLength(1); + expect(alarm.Properties.OKActions).toHaveLength(1); + } + }); +}); diff --git a/infrastructure/test/observability-dynamodb-alarms.test.ts b/infrastructure/test/observability-dynamodb-alarms.test.ts new file mode 100644 index 000000000..4e53936b0 --- /dev/null +++ b/infrastructure/test/observability-dynamodb-alarms.test.ts @@ -0,0 +1,133 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; + +describe('DynamoDB per-table alarms', () => { + let template: Template; + let alarms: Record; + let tableCount: number; + + function ddbAlarmNames(): string[] { + return Object.values(alarms) + .map((a) => a.Properties.AlarmName as string) + .filter((n) => typeof n === 'string' && n.startsWith(`${MOCK_PREFIX}-ddb-`)); + } + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + alarms = template.findResources('AWS::CloudWatch::Alarm'); + tableCount = Object.keys(template.findResources('AWS::DynamoDB::Table')).length; + }); + + // Tied to the real table count, so a new table without an alarm fails here. + it('covers every table in the stack — one throttle alarm each', () => { + expect(tableCount).toBe(26); + // 26 per-table throttle alarms + 1 account-level UserErrors alarm. + expect(ddbAlarmNames()).toHaveLength(tableCount + 1); + }); + + it('every table has a throttle alarm naming that table', () => { + const names = new Set(ddbAlarmNames()); + // Derive expected table short-names from the template's OWN TableName + // properties, so this does not restate the list the construct is given. + const tableShortNames = Object.values(template.findResources('AWS::DynamoDB::Table')) + .map((t: any) => (t.Properties.TableName as string).replace(`${MOCK_PREFIX}-`, '')); + + const missing = tableShortNames + .map((short) => `${MOCK_PREFIX}-ddb-${short}-throttle`) + .filter((expected) => !names.has(expected)); + expect(missing).toEqual([]); + }); + + it('throttle alarm sums read and write events in one expression', () => { + const alarm = Object.values(alarms).find( + (a) => a.Properties.AlarmName === `${MOCK_PREFIX}-ddb-users-throttle`, + ); + expect(alarm).toBeDefined(); + // Metric math renders as Metrics[], not a flat MetricName. + expect(alarm.Properties.MetricName).toBeUndefined(); + const json = JSON.stringify(alarm.Properties.Metrics); + expect(json).toContain('ReadThrottleEvents'); + expect(json).toContain('WriteThrottleEvents'); + expect(json).toContain('reads + writes'); + expect(alarm.Properties.Threshold).toBe(10); // configured default + expect(alarm.Properties.TreatMissingData).toBe('notBreaching'); + }); + + // CloudWatch caps math-expression alarms at 10 metrics. + it('throttle expression stays well inside the 10-metric math limit', () => { + const alarm = Object.values(alarms).find( + (a) => a.Properties.AlarmName === `${MOCK_PREFIX}-ddb-users-throttle`, + ); + const metricCount = alarm.Properties.Metrics.filter((m: any) => m.MetricStat).length; + expect(metricCount).toBe(2); + }); + + it('throttle alarms bind TableName to the real table resource', () => { + const alarm = Object.values(alarms).find( + (a) => a.Properties.AlarmName === `${MOCK_PREFIX}-ddb-sessions-metadata-throttle`, + ); + const stats = alarm.Properties.Metrics.filter((m: any) => m.MetricStat); + expect(stats.length).toBeGreaterThan(0); + for (const m of stats) { + const dims = m.MetricStat.Metric.Dimensions; + expect(dims).toHaveLength(1); + expect(dims[0].Name).toBe('TableName'); + expect(JSON.stringify(dims[0].Value)).toMatch(/Ref|Fn::GetAtt/); + } + }); + + // Replaces 26 per-table SystemErrors alarms: that metric had zero streams in + // the live account, while account-level UserErrors had real data. + it('has one account-level UserErrors alarm instead of per-table system errors', () => { + const alarm = Object.values(alarms).find( + (a) => a.Properties.AlarmName === `${MOCK_PREFIX}-ddb-user-errors`, + ); + expect(alarm).toBeDefined(); + expect(alarm.Properties.Namespace).toBe('AWS/DynamoDB'); + expect(alarm.Properties.MetricName).toBe('UserErrors'); + // Published account-wide only; no dimension set exists for it. + expect(alarm.Properties.Dimensions).toBeUndefined(); + }); + + it('creates no per-table system-error alarms', () => { + const systemErrorAlarms = ddbAlarmNames().filter((n) => n.includes('system-error')); + expect(systemErrorAlarms).toEqual([]); + }); + + it('every DynamoDB alarm is routed to the alarm topic', () => { + for (const alarm of Object.values(alarms)) { + const name = alarm.Properties.AlarmName; + if (typeof name === 'string' && name.startsWith(`${MOCK_PREFIX}-ddb-`)) { + expect(alarm.Properties.AlarmActions).toHaveLength(1); + expect(alarm.Properties.OKActions).toHaveLength(1); + } + } + }); + + // Single-stack architecture against a hard 500-resource CFN limit, so the + // ceiling is asserted while there is still room to react. + it('stack stays clear of the 500-resource CloudFormation limit', () => { + const total = Object.keys(template.toJSON().Resources).length; + expect(total).toBeLessThan(460); + }); +}); diff --git a/infrastructure/test/observability-lambda-alarms.test.ts b/infrastructure/test/observability-lambda-alarms.test.ts new file mode 100644 index 000000000..8634fe43a --- /dev/null +++ b/infrastructure/test/observability-lambda-alarms.test.ts @@ -0,0 +1,160 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; + +describe('Lambda and DLQ alarms', () => { + let template: Template; + let alarms: Record; + + function names(): string[] { + return Object.values(alarms) + .map((a) => a.Properties.AlarmName as string) + .filter((n) => typeof n === 'string'); + } + + function byName(name: string): any { + const full = `${MOCK_PREFIX}-${name}`; + const found = Object.values(alarms).find((a) => a.Properties.AlarmName === full); + expect(found).toBeDefined(); + return found; + } + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + kbSync: { enabled: true }, + scheduledRuns: { enabled: true }, + managedKb: { + newDefault: true, + migrationEnabled: true, + reconcilerArmed: true, + perOwnerDefaultBytes: 100 * 1024 * 1024, + perOwnerElevatedBytes: 1024 * 1024 * 1024, + perKnowledgeBaseCeilingBytes: 500 * 1024 * 1024, + retentionWindowDays: 30, + storageAlarmGb: 500, + dailyCostAlarmUsd: 100, + }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + alarms = template.findResources('AWS::CloudWatch::Alarm'); + }); + + /** Functions that get BOTH an error and a throttle alarm here. */ + const FULLY_ALARMED = [ + 'artifact-render', + 'rag-ingestion', + 'kb-migration-dispatcher', + 'kb-migration-worker', + 'kb-migration-reconciler', + 'kb-ingestion-consumer', + ]; + + /** + * Functions whose own construct already defines an error alarm with a tuned + * threshold, so only a throttle alarm is added here. + */ + const THROTTLE_ONLY = [ + 'kb-sync-dispatcher', + 'kb-sync-worker', + 'scheduled-runs-dispatcher', + 'scheduled-runs-worker', + ]; + + it('creates error and throttle alarms for the previously unmonitored functions', () => { + for (const fn of FULLY_ALARMED) { + byName(`lambda-${fn}-errors`); + byName(`lambda-${fn}-throttles`); + } + }); + + // Those constructs keep their own error alarms with role-tuned thresholds + // (dispatcher 1, worker 3); a second at one shared threshold would conflict. + it('does not duplicate error alarms that already exist elsewhere', () => { + for (const fn of THROTTLE_ONLY) { + byName(`lambda-${fn}-throttles`); + expect(names()).not.toContain(`${MOCK_PREFIX}-lambda-${fn}-errors`); + } + // The originals are still present, with their tuned thresholds intact. + expect(byName('kb-sync-dispatcher-errors').Properties.Threshold).toBe(1); + expect(byName('kb-sync-worker-errors').Properties.Threshold).toBe(3); + expect(byName('scheduled-runs-dispatcher-errors').Properties.Threshold).toBe(1); + expect(byName('scheduled-runs-worker-errors').Properties.Threshold).toBe(3); + }); + + // Deploy-time machinery is excluded: its failure fails the deploy directly. + it('every runtime Lambda has an error alarm', () => { + const alarmNames = names(); + const functions = template.findResources('AWS::Lambda::Function'); + + const deployTimeOnly = /RagCors|AutoDelete|CustomResource|Provider|framework|LogRetention/i; + const runtimeFunctions = Object.keys(functions).filter((id) => !deployTimeOnly.test(id)); + + // Every runtime function should be represented by at least one error alarm. + // Cross-check on count rather than name-matching logical ids, since alarm + // names use short names and logical ids are CDK-generated. + const errorAlarms = alarmNames.filter((n) => /-errors$/.test(n)); + expect(errorAlarms.length).toBeGreaterThanOrEqual(FULLY_ALARMED.length); + expect(runtimeFunctions.length).toBeGreaterThan(0); + }); + + it('throttle alarms fire on any throttle at all', () => { + for (const fn of [...FULLY_ALARMED, ...THROTTLE_ONLY]) { + const alarm = byName(`lambda-${fn}-throttles`); + expect(alarm.Properties.MetricName).toBe('Throttles'); + expect(alarm.Properties.Namespace).toBe('AWS/Lambda'); + expect(alarm.Properties.Threshold).toBe(0); + } + }); + + it('alarms bind the FunctionName dimension to the real function', () => { + const alarm = byName('lambda-artifact-render-errors'); + const dims = alarm.Properties.Dimensions; + expect(dims).toHaveLength(1); + expect(dims[0].Name).toBe('FunctionName'); + expect(JSON.stringify(dims[0].Value)).toMatch(/Ref|Fn::GetAtt/); + }); + + // A timed-out invocation already records an Errors datapoint. + it('creates no per-function duration alarms', () => { + expect(names().filter((n) => /duration/i.test(n))).toEqual([]); + }); + + describe('dead-letter queue', () => { + // DLQ messages persist until drained or replayed, so this must not + // self-clear. + it('alarms when the kb-ingestion DLQ is not empty', () => { + const alarm = byName('dlq-kb-ingestion-not-empty'); + expect(alarm.Properties.Namespace).toBe('AWS/SQS'); + expect(alarm.Properties.MetricName).toBe('ApproximateNumberOfMessagesVisible'); + expect(alarm.Properties.Threshold).toBe(0); + expect(alarm.Properties.EvaluationPeriods).toBe(1); + const dims = alarm.Properties.Dimensions; + expect(dims[0].Name).toBe('QueueName'); + }); + }); + + it('every Lambda and DLQ alarm is routed to the alarm topic', () => { + for (const name of names().filter((n) => /-lambda-|^.*-dlq-/.test(n))) { + const alarm = Object.values(alarms).find((a) => a.Properties.AlarmName === name); + expect(alarm.Properties.AlarmActions).toHaveLength(1); + expect(alarm.Properties.OKActions).toHaveLength(1); + } + }); +}); diff --git a/infrastructure/test/observability-log-retention.test.ts b/infrastructure/test/observability-log-retention.test.ts new file mode 100644 index 000000000..23be60b2b --- /dev/null +++ b/infrastructure/test/observability-log-retention.test.ts @@ -0,0 +1,186 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; +import * as logs from 'aws-cdk-lib/aws-logs'; +import * as fs from 'fs'; +import * as path from 'path'; + +import { AppConfig } from '../lib/config'; +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_REGION } from './helpers/mock-config'; + +function synth(logRetentionDays: number): Template { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const base = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + kbSync: { enabled: true }, + scheduledRuns: { enabled: true }, + }); + const config: AppConfig = { + ...base, + observability: { ...base.observability, logRetentionDays }, + }; + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + return Template.fromStack(stack); +} + +describe('Log retention — one configured value everywhere', () => { + it('every log group uses the configured retention', () => { + const template = synth(30); + const groups = template.findResources('AWS::Logs::LogGroup'); + expect(Object.keys(groups).length).toBeGreaterThan(10); + + for (const [logicalId, group] of Object.entries(groups)) { + expect((group as any).Properties.RetentionInDays).toBe(30); + expect(logicalId).toBeTruthy(); + } + }); + + it('changing the configured value moves every log group together', () => { + const groups = synth(90).findResources('AWS::Logs::LogGroup'); + const values = new Set( + Object.values(groups).map((g: any) => g.Properties.RetentionInDays), + ); + expect(values).toEqual(new Set([90])); + }); + + it('accepts the full range of CloudWatch retention values', () => { + for (const days of [1, 7, 30, 365, 3653]) { + const groups = synth(days).findResources('AWS::Logs::LogGroup'); + const values = new Set( + Object.values(groups).map((g: any) => g.Properties.RetentionInDays), + ); + expect(values).toEqual(new Set([days])); + } + }); + + // CDK gives its own provider Lambdas a 731-day default, and those groups are + // declared nowhere here. A bare cdk.App lacks the cdk.json feature flags that + // materialise them, so this stands one in instead. + it('overrides CDK-generated log groups that default to 731 days', () => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const base = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + }); + const config: AppConfig = { + ...base, + observability: { ...base.observability, logRetentionDays: 30 }, + }; + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + + // Stand in for a CDK-managed group created with its own default retention. + new logs.CfnLogGroup(stack, 'PretendCdkManagedGroup', { + retentionInDays: 731, + }); + + const groups = Template.fromStack(stack).findResources('AWS::Logs::LogGroup'); + const values = new Set( + Object.values(groups).map((g: any) => g.Properties.RetentionInDays), + ); + // The Aspect rewrote it: no 731 survives anywhere. + expect(values).toEqual(new Set([30])); + }); + + // Service-created, so a CDK LogGroup construct cannot set its retention. + describe('service-created AgentCore Runtime log group', () => { + it('applies retention via a custom resource', () => { + const template = synth(30); + const customResources = template.findResources('Custom::AWS'); + const retentionResource = Object.values(customResources).find((r: any) => + JSON.stringify(r.Properties).includes('putRetentionPolicy'), + ); + expect(retentionResource).toBeDefined(); + + const props = JSON.stringify((retentionResource as any).Properties); + expect(props).toContain('CloudWatchLogs'); + expect(props).toContain('/aws/bedrock-agentcore/runtimes/'); + expect(props).toContain('-DEFAULT'); + // The call payload is assembled with Fn::Join, so the inner JSON arrives + // backslash-escaped. Match on the key/value pair rather than an exact + // literal so this does not break on escaping depth. + expect(props).toMatch(/retentionInDays\\*":30/); + }); + + it('re-applies on update, not just on create', () => { + const template = synth(30); + const customResources = template.findResources('Custom::AWS'); + const retentionResource = Object.values(customResources).find((r: any) => + JSON.stringify(r.Properties).includes('putRetentionPolicy'), + ) as any; + expect(retentionResource.Properties.Create).toBeDefined(); + expect(retentionResource.Properties.Update).toBeDefined(); + }); + + it('varies its physical id with the retention value', () => { + const idFor = (days: number) => { + const customResources = synth(days).findResources('Custom::AWS'); + const r = Object.values(customResources).find((x: any) => + JSON.stringify(x.Properties).includes('putRetentionPolicy'), + ) as any; + return JSON.stringify(r.Properties.Create); + }; + expect(idFor(30)).not.toBe(idFor(90)); + }); + + it('scopes its IAM policy to the runtime log group only', () => { + const template = synth(30); + const policies = template.findResources('AWS::IAM::Policy'); + const retentionPolicy = Object.values(policies).find((p: any) => + JSON.stringify(p.Properties.PolicyDocument).includes('logs:PutRetentionPolicy'), + ) as any; + expect(retentionPolicy).toBeDefined(); + const doc = JSON.stringify(retentionPolicy.Properties.PolicyDocument); + // Not a wildcard across every log group in the account. + expect(doc).toContain('/aws/bedrock-agentcore/runtimes/'); + expect(doc).not.toContain('"Resource":"*"'); + }); + }); +}); + +/** Source guard, covering flag-gated paths a synth never reaches. */ +describe('Log retention — source guard', () => { + const libDir = path.join(__dirname, '..', 'lib'); + + function walk(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return walk(full); + return entry.isFile() && entry.name.endsWith('.ts') ? [full] : []; + }); + } + + it('no construct hardcodes a RetentionDays value', () => { + const offenders: string[] = []; + for (const file of walk(libDir)) { + if (file.endsWith(path.join('observability', 'log-retention.ts'))) continue; + const source = fs.readFileSync(file, 'utf-8'); + if (/retention:\s*logs\.RetentionDays\./.test(source)) { + offenders.push(path.relative(libDir, file)); + } + } + expect(offenders).toEqual([]); + }); +}); diff --git a/infrastructure/test/observability-platform-dashboard.test.ts b/infrastructure/test/observability-platform-dashboard.test.ts new file mode 100644 index 000000000..95307c125 --- /dev/null +++ b/infrastructure/test/observability-platform-dashboard.test.ts @@ -0,0 +1,131 @@ +import * as cdk from 'aws-cdk-lib'; +import { Template } from 'aws-cdk-lib/assertions'; + +import { PlatformStack } from '../lib/platform-stack'; +import { createMockConfig, mockSsmContext, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; + +describe('Unified platform dashboard', () => { + let template: Template; + let dashboardBody: string; + let alarmCount: number; + + beforeAll(() => { + const cert = 'arn:aws:acm:us-east-1:123456789012:certificate/test'; + const config = createMockConfig({ + domainName: 'example.com', + infrastructureHostedZoneDomain: 'example.com', + certificateArn: cert, + frontend: { cloudFrontPriceClass: 'PriceClass_100', certificateArn: cert }, + artifacts: { retentionDays: 90, extraFrameAncestors: [], certificateArn: cert }, + mcpSandbox: { extraFrameAncestors: [], certificateArn: cert }, + fineTuning: { enabled: true, defaultQuotaHours: 0 }, + kbSync: { enabled: true }, + scheduledRuns: { enabled: true }, + }); + const app = new cdk.App(); + mockSsmContext(app, config); + const stack = new PlatformStack(app, 'TestPlatformStack', { + config, + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, + }); + stack.wireCompute(); + template = Template.fromStack(stack); + + const dashboards = template.findResources('AWS::CloudWatch::Dashboard'); + const platform = Object.values(dashboards).find((d: any) => + JSON.stringify(d.Properties.DashboardName).includes('platform-health'), + ); + expect(platform).toBeDefined(); + dashboardBody = JSON.stringify((platform as any).Properties.DashboardBody); + alarmCount = Object.keys(template.findResources('AWS::CloudWatch::Alarm')).length; + }); + + it('creates the dashboard with the conventional name', () => { + template.hasResourceProperties('AWS::CloudWatch::Dashboard', { + DashboardName: `${MOCK_PREFIX}-platform-health`, + }); + }); + + // CloudWatch charges $3/month beyond three. + it('keeps the stack at exactly three dashboards (the CloudWatch free ceiling)', () => { + template.resourceCountIs('AWS::CloudWatch::Dashboard', 3); + }); + + it('links to the two drill-down dashboards instead of duplicating them', () => { + expect(dashboardBody).toContain(`${MOCK_PREFIX}-agentcore-observability`); + expect(dashboardBody).toContain(`${MOCK_PREFIX}-prompt-cache-observability`); + }); + + it('names the SNS topic alarms route to, so an operator can find it', () => { + expect(dashboardBody).toContain(`${MOCK_PREFIX}-alarms`); + }); + + // On the dashboard, not just in code: whoever reads it is not reading CDK. + it('warns on-dashboard that SSE makes long response times normal', () => { + expect(dashboardBody).toMatch(/SSE/); + }); + + describe('row 1 — is traffic being served', () => { + it('graphs front-door request count and both flavours of 5xx', () => { + expect(dashboardBody).toContain('RequestCount'); + expect(dashboardBody).toContain('HTTPCode_ELB_5XX_Count'); + expect(dashboardBody).toContain('HTTPCode_Target_5XX_Count'); + }); + + it('graphs agent invocations against errors and throttles', () => { + expect(dashboardBody).toContain('AWS/Bedrock-AgentCore'); + expect(dashboardBody).toContain('Invocations'); + expect(dashboardBody).toContain('SystemErrors'); + expect(dashboardBody).toContain('UserErrors'); + expect(dashboardBody).toContain('Throttles'); + }); + + it('graphs running tasks against unhealthy targets', () => { + expect(dashboardBody).toContain('RunningTaskCount'); + expect(dashboardBody).toContain('UnHealthyHostCount'); + }); + }); + + describe('row 2 — saturation', () => { + it('graphs app-api CPU and memory', () => { + expect(dashboardBody).toContain('CPUUtilization'); + expect(dashboardBody).toContain('MemoryUtilization'); + }); + + it('graphs Bedrock quota headroom, the leading indicator', () => { + expect(dashboardBody).toContain('EstimatedTPMQuotaUsage'); + expect(dashboardBody).toContain('InvocationThrottles'); + }); + + it('graphs DynamoDB request errors', () => { + expect(dashboardBody).toContain('AWS/DynamoDB'); + }); + }); + + describe('row 3 — alarm status', () => { + // The list is discovered by walking the tree, so this checks it found all. + it('includes every alarm in the stack', () => { + expect(alarmCount).toBeGreaterThan(60); + // The widget references each alarm by ARN, which renders as + // {"Fn::GetAtt": [, "Arn"]} rather than a literal string — + // hence counting GetAtt Arn references rather than grepping for ':alarm:'. + const arnRefs = (dashboardBody.match(/Fn::GetAtt/g) || []).length; + expect(arnRefs).toBeGreaterThanOrEqual(alarmCount); + }); + + it('renders an alarm-status widget', () => { + expect(dashboardBody).toContain('alarm'); + expect(dashboardBody).toContain('All platform alarms'); + }); + }); + + it('exports the dashboard name', () => { + template.hasOutput('*', { + Export: { Name: `${MOCK_PREFIX}-PlatformDashboard` }, + }); + }); + + it('binds the runtime Name dimension identically to the alarms', () => { + expect(dashboardBody).toContain('::DEFAULT'); + }); +}); diff --git a/infrastructure/test/platform-stack.test.ts b/infrastructure/test/platform-stack.test.ts index adb5caa5d..6b36ebabd 100644 --- a/infrastructure/test/platform-stack.test.ts +++ b/infrastructure/test/platform-stack.test.ts @@ -138,8 +138,14 @@ describe('PlatformStack', () => { }); it('creates KMS keys', () => { - // OAuth token encryption + BFF cookie signing - template.resourceCountIs('AWS::KMS::Key', 2); + // OAuth token encryption + BFF cookie signing + alarm topic encryption. + // + // The third is the alarm topic's CMK. It is customer-managed rather than + // alias/aws/sns out of necessity, not preference: CloudWatch cannot be + // granted kms:GenerateDataKey* on an AWS-managed key, so an + // alias/aws/sns-encrypted topic accepts the alarm and silently drops the + // notification. See constructs/observability/alarm-topic-construct.ts. + template.resourceCountIs('AWS::KMS::Key', 3); }); }); diff --git a/infrastructure/test/prompt-cache-observability.test.ts b/infrastructure/test/prompt-cache-observability.test.ts index f6d9b4e0e..74c82b998 100644 --- a/infrastructure/test/prompt-cache-observability.test.ts +++ b/infrastructure/test/prompt-cache-observability.test.ts @@ -1,5 +1,12 @@ import * as cdk from 'aws-cdk-lib'; import { Template } from 'aws-cdk-lib/assertions'; +import * as sns from 'aws-cdk-lib/aws-sns'; + +import { + AppConfig, + OBSERVABILITY_DEFAULT_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD, + OBSERVABILITY_DEFAULT_PROMPT_CACHE_WASTED_USD_THRESHOLD, +} from '../lib/config'; import { PromptCacheObservabilityConstruct } from '../lib/constructs/observability/prompt-cache-observability-construct'; import { createMockConfig, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './helpers/mock-config'; @@ -10,15 +17,30 @@ import { createMockConfig, MOCK_ACCOUNT, MOCK_PREFIX, MOCK_REGION } from './help const MOCK_RUNTIME_LOG_GROUP = `/aws/bedrock-agentcore/runtimes/${MOCK_PREFIX}_agentcore_runtime-AbC123XyZ0-DEFAULT`; -function synth(production: boolean): Template { +/** + * Synthesize the construct with a real SNS topic attached. + * + * There is no `production` parameter any more. Thresholds are single configured + * values: this repo is forked by many institutions, so a fork with one + * environment should not have to reason about a `production` boolean, and a fork + * with three should not be limited to two. Per-environment differences live in + * the forker's deployment config and arrive as one value. + */ +function synth(observabilityOverrides: Partial = {}): Template { const app = new cdk.App(); const stack = new cdk.Stack(app, 'Test', { env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, }); - const config = createMockConfig({ production }); + const base = createMockConfig(); + const config: AppConfig = { + ...base, + observability: { ...base.observability, ...observabilityOverrides }, + }; + const topic = new sns.Topic(stack, 'TestAlarmTopic'); new PromptCacheObservabilityConstruct(stack, 'PromptCacheObservability', { config, runtimeLogGroupName: MOCK_RUNTIME_LOG_GROUP, + alarmTopic: topic, }); return Template.fromStack(stack); } @@ -26,7 +48,7 @@ function synth(production: boolean): Template { describe('PromptCacheObservabilityConstruct', () => { let t: Template; beforeAll(() => { - t = synth(false); + t = synth(); }); it('creates the dashboard with the conventional name', () => { @@ -64,8 +86,15 @@ describe('PromptCacheObservabilityConstruct', () => { for (const alarm of alarms) { expect(alarm.Properties.TreatMissingData).toBe('notBreaching'); expect(alarm.Properties.Namespace).toBe('AgentCoreStack/PromptCache'); - // Console-only: no SNS wiring yet anywhere in the stack. - expect(alarm.Properties.AlarmActions).toBeUndefined(); + // Routed to the platform alarm topic. This assertion previously read + // `toBeUndefined()` with the note "Console-only: no SNS wiring yet + // anywhere in the stack" — which was true of the whole stack, and is the + // gap the alarm topic + AlarmFactory closed. A cost alarm nobody is told + // about is the one kind that matters least in the console and most in an + // inbox: the motivating incident leaked $27 over five days precisely + // because no one was watching a screen. + expect(alarm.Properties.AlarmActions).toHaveLength(1); + expect(alarm.Properties.OKActions).toHaveLength(1); } }); @@ -74,7 +103,10 @@ describe('PromptCacheObservabilityConstruct', () => { AlarmName: `${MOCK_PREFIX}-prompt-cache-avoidable-miss`, MetricName: 'AvoidableMiss', Statistic: 'Sum', - Threshold: 50, + // The single default. Was `config.production ? 10 : 50`; the tighter + // value became the one default because a prefix-stability regression is + // a cost leak, and catching it earlier is cheaper for every fork. + Threshold: OBSERVABILITY_DEFAULT_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD, EvaluationPeriods: 3, ComparisonOperator: 'GreaterThanThreshold', }); @@ -82,11 +114,11 @@ describe('PromptCacheObservabilityConstruct', () => { AlarmName: `${MOCK_PREFIX}-prompt-cache-wasted-usd`, MetricName: 'WastedUsd', Statistic: 'Sum', - Threshold: 5, + Threshold: OBSERVABILITY_DEFAULT_PROMPT_CACHE_WASTED_USD_THRESHOLD, }); }); - it('alarms on one session accumulating $5 of partial-miss waste', () => { + it('alarms on one session accumulating the configured partial-miss waste', () => { // The fleet sums above cannot see a single conversation spending $0.43 a // turn for five days — this is the alarm that would have caught the // motivating incident, on its second day. @@ -103,25 +135,53 @@ describe('PromptCacheObservabilityConstruct', () => { }); }); - it('holds the session threshold at $5 in production too', () => { - // Unlike the fleet alarms, this one is not traffic-scaled: $5 of waste in - // one conversation is the same problem in dev and in prod. - synth(true).hasResourceProperties('AWS::CloudWatch::Alarm', { + /** + * Every threshold is a single configured value. An institution that wants a + * looser dev environment sets a different value there — it does not get one + * implicitly from a `production` flag it may never have set. That is the + * whole point of the single-value rule for an OSS repo: the defaults have to + * be right for a fork that configures nothing. + */ + it('thresholds come from config, not from a production branch', () => { + const custom = synth({ + promptCacheAvoidableMissThreshold: 77, + promptCacheWastedUsdThreshold: 8.5, + promptCacheSessionWastedUsdThreshold: 42, + }); + custom.hasResourceProperties('AWS::CloudWatch::Alarm', { + MetricName: 'AvoidableMiss', + Threshold: 77, + }); + custom.hasResourceProperties('AWS::CloudWatch::Alarm', { + MetricName: 'WastedUsd', + // Fractional dollars survive: parseFloatEnv, not parseIntEnv. + Threshold: 8.5, + }); + custom.hasResourceProperties('AWS::CloudWatch::Alarm', { MetricName: 'SessionPartialMissUsd', - Threshold: 5, + Threshold: 42, }); }); - it('uses stricter thresholds in production', () => { - const prod = synth(true); - prod.hasResourceProperties('AWS::CloudWatch::Alarm', { - MetricName: 'AvoidableMiss', - Threshold: 10, + /** + * The opt-out path: no topic means alarms are still created, just + * console-only. That was the stack's behaviour before the alarm topic + * existed, kept reachable for a fork that routes alerts another way. + */ + it('stays console-only when no alarm topic is supplied', () => { + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'NoTopic', { + env: { account: MOCK_ACCOUNT, region: MOCK_REGION }, }); - prod.hasResourceProperties('AWS::CloudWatch::Alarm', { - MetricName: 'WastedUsd', - Threshold: 1, + new PromptCacheObservabilityConstruct(stack, 'PromptCacheObservability', { + config: createMockConfig(), + runtimeLogGroupName: MOCK_RUNTIME_LOG_GROUP, }); + const noTopic = Template.fromStack(stack); + noTopic.resourceCountIs('AWS::CloudWatch::Alarm', 3); + for (const alarm of Object.values(noTopic.findResources('AWS::CloudWatch::Alarm'))) { + expect((alarm as any).Properties.AlarmActions).toBeUndefined(); + } }); it('exports the dashboard name', () => { diff --git a/infrastructure/test/repo-shape.test.ts b/infrastructure/test/repo-shape.test.ts index 6d970d3e6..6fdbd7046 100644 --- a/infrastructure/test/repo-shape.test.ts +++ b/infrastructure/test/repo-shape.test.ts @@ -108,6 +108,41 @@ describe('Repo shape — new architecture files exist', () => { it('scripts/stack-bootstrap/ is preserved', () => { expect(fs.existsSync(path.join(SCRIPTS, 'stack-bootstrap'))).toBe(true); }); + + /** + * The observability area. Named individually rather than by a directory + * existence check because each file carries a guarantee the others depend on: + * the factory is what makes every alarm routed, and log-retention.ts is the + * only place a RetentionDays constant may appear. + */ + const observabilityFiles = [ + 'alarm-topic-construct.ts', + 'alarm-factory.ts', + 'alb-alarms-construct.ts', + 'ecs-service-alarms-construct.ts', + 'dynamodb-alarms-construct.ts', + 'lambda-alarms-construct.ts', + 'ai-path-alarms-construct.ts', + 'platform-dashboard-construct.ts', + 'log-retention.ts', + 'prompt-cache-observability-construct.ts', + ]; + for (const file of observabilityFiles) { + it(`constructs/observability/${file} exists`, () => { + expect( + fs.existsSync(path.join(INFRA_LIB, 'constructs', 'observability', file)), + ).toBe(true); + }); + } + + it('the observability steering doc exists', () => { + // Carries the SNS+CMK gotcha, the no-config.production rule, the + // streaming-latency caveat, and the alarm runbook. + const steering = path.join( + __dirname, '..', '..', '.kiro', 'steering', 'observability.md', + ); + expect(fs.existsSync(steering)).toBe(true); + }); }); describe('Workflow YAML shape', () => { diff --git a/scripts/common/load-env.sh b/scripts/common/load-env.sh index 95f64b50c..f9e1487f6 100644 --- a/scripts/common/load-env.sh +++ b/scripts/common/load-env.sh @@ -253,6 +253,63 @@ build_cdk_context_params() { context_params="${context_params} --context managedKb.dailyCostAlarmUsd=\"${CDK_MANAGED_KB_DAILY_COST_ALARM_USD}\"" fi + # Observability. Forwarded only when non-empty, same as the managed-KB flags + # above, and read by config.ts as the FLAT dotted key. + if [ -n "${CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED:-}" ]; then + context_params="${context_params} --context observability.alarmTopicEnabled=\"${CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED}\"" + fi + if [ -n "${CDK_OBSERVABILITY_LOG_RETENTION_DAYS:-}" ]; then + context_params="${context_params} --context observability.logRetentionDays=\"${CDK_OBSERVABILITY_LOG_RETENTION_DAYS}\"" + fi + if [ -n "${CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.albTarget5xxThreshold=\"${CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD}\"" + fi + if [ -n "${CDK_OBSERVABILITY_ALB_P99_LATENCY_MS:-}" ]; then + context_params="${context_params} --context observability.albP99LatencyMs=\"${CDK_OBSERVABILITY_ALB_P99_LATENCY_MS}\"" + fi + if [ -n "${CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS:-}" ]; then + context_params="${context_params} --context observability.agentCoreLatencyMs=\"${CDK_OBSERVABILITY_AGENTCORE_LATENCY_MS}\"" + fi + if [ -n "${CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.agentCoreErrorThreshold=\"${CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD}\"" + fi + if [ -n "${CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.lambdaErrorThreshold=\"${CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD}\"" + fi + if [ -n "${CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT:-}" ]; then + context_params="${context_params} --context observability.lambdaDurationPercentOfTimeout=\"${CDK_OBSERVABILITY_LAMBDA_DURATION_PERCENT_OF_TIMEOUT}\"" + fi + if [ -n "${CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.dynamoThrottleThreshold=\"${CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD}\"" + fi + if [ -n "${CDK_OBSERVABILITY_ECS_CPU_PERCENT:-}" ]; then + context_params="${context_params} --context observability.ecsCpuPercent=\"${CDK_OBSERVABILITY_ECS_CPU_PERCENT}\"" + fi + if [ -n "${CDK_OBSERVABILITY_ECS_MEMORY_PERCENT:-}" ]; then + context_params="${context_params} --context observability.ecsMemoryPercent=\"${CDK_OBSERVABILITY_ECS_MEMORY_PERCENT}\"" + fi + if [ -n "${CDK_OBSERVABILITY_XRAY_SAMPLING_RATE:-}" ]; then + context_params="${context_params} --context observability.xraySamplingRate=\"${CDK_OBSERVABILITY_XRAY_SAMPLING_RATE}\"" + fi + if [ -n "${CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR:-}" ]; then + context_params="${context_params} --context observability.xraySamplingReservoir=\"${CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR}\"" + fi + if [ -n "${CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS:-}" ]; then + context_params="${context_params} --context observability.xrayInsightsNotifications=\"${CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS}\"" + fi + if [ -n "${CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED:-}" ]; then + context_params="${context_params} --context observability.agentCoreApplicationLogsEnabled=\"${CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED}\"" + fi + if [ -n "${CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.promptCacheAvoidableMissThreshold=\"${CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD}\"" + fi + if [ -n "${CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.promptCacheWastedUsdThreshold=\"${CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD}\"" + fi + if [ -n "${CDK_OBSERVABILITY_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD:-}" ]; then + context_params="${context_params} --context observability.promptCacheSessionWastedUsdThreshold=\"${CDK_OBSERVABILITY_PROMPT_CACHE_SESSION_WASTED_USD_THRESHOLD}\"" + fi + echo "${context_params}" } @@ -469,6 +526,20 @@ if [ "${LOAD_ENV_QUIET:-false}" != "true" ]; then log_config " HTTPS Enabled: Yes" fi + # Only overrides that are actually set; the synth prints resolved values. + if [ -n "${CDK_OBSERVABILITY_LOG_RETENTION_DAYS:-}" ]; then + log_config " Log Retention: ${CDK_OBSERVABILITY_LOG_RETENTION_DAYS} days (override)" + fi + if [ -n "${CDK_OBSERVABILITY_XRAY_SAMPLING_RATE:-}" ]; then + log_config " X-Ray Sampling: ${CDK_OBSERVABILITY_XRAY_SAMPLING_RATE} (override)" + fi + if [ -n "${CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED:-}" ]; then + log_config " Alarm Topic: ${CDK_OBSERVABILITY_ALARM_TOPIC_ENABLED} (override)" + fi + if [ -n "${CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED:-}" ]; then + log_config " AC App Logs: ${CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED} (override)" + fi + # Check AWS credentials if ! aws sts get-caller-identity &> /dev/null; then log_warn "AWS credentials not configured or invalid" diff --git a/scripts/observability/set-bsu-overrides.sh b/scripts/observability/set-bsu-overrides.sh new file mode 100755 index 000000000..fd086e47e --- /dev/null +++ b/scripts/observability/set-bsu-overrides.sh @@ -0,0 +1,193 @@ +#!/usr/bin/env bash +# +# Boise State's observability override profile. +# +# The committed defaults in infrastructure/lib/config.ts are deliberately +# COST-CONSCIOUS: they are what a fork inherits when it configures nothing, so +# they buy useful alerting at the cheapest setting and leave diagnostic depth +# opt-in. +# +# Boise State is not a typical fork. We author this platform, so we do far more +# diagnostic work than any deployer of it — we need deeper traces, longer log +# retention, and tighter thresholds. Those values belong HERE, in GitHub +# Variables scoped to a GitHub Environment, and NOT in committed code. That +# separation is the whole point: every institution's defaults stay right for +# them, and ours stay right for us, with no `config.production` ternary in +# between. +# +# ───────────────────────────────────────────────────────────────────────────── +# THIS SCRIPT IS NOT RUN BY CI AND MAKES NO AWS CHANGES. +# +# It mutates shared repository configuration (GitHub Variables), which is an +# operator action. Run it deliberately, from a machine authenticated to `gh` +# with repo admin rights. It prints what it will do and requires confirmation. +# ───────────────────────────────────────────────────────────────────────────── +# +# Usage: +# scripts/observability/set-bsu-overrides.sh --env development [--dry-run] +# scripts/observability/set-bsu-overrides.sh --env production [--dry-run] +# +# After running, the values flow: +# GitHub Variable +# -> .github/workflows/platform.yml (job-level env:) +# -> scripts/common/load-env.sh build_cdk_context_params() +# -> --context observability.=... +# -> infrastructure/lib/config.ts loadConfig() +# -> config.observability. +# +# The next platform.yml deploy logs the resolved values ("Observability: ..."), +# which is how you confirm a variable actually took effect rather than being +# accepted and ignored. + +set -euo pipefail + +ENVIRONMENT="" +DRY_RUN=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --env) ENVIRONMENT="$2"; shift 2 ;; + --dry-run) DRY_RUN=true; shift ;; + -h|--help) sed -n '2,45p' "$0"; exit 0 ;; + *) echo "Unknown argument: $1" >&2; exit 1 ;; + esac +done + +if [[ -z "${ENVIRONMENT}" ]]; then + echo "ERROR: --env is required (development or production)" >&2 + exit 1 +fi + +# ───────────────────────────────────────────────────────────────────────────── +# The profiles +# ───────────────────────────────────────────────────────────────────────────── +# +# Only values that DIFFER from the committed default are set. Anything omitted +# intentionally inherits the default, so this file stays a diff rather than a +# duplicate of config.ts that can drift from it. + +case "${ENVIRONMENT}" in +development) + # Dev is where we debug, so it is the most diagnostic-heavy environment. + # Traces matter more here than cost: dev traffic is a fraction of prod, so + # even 50% sampling is a small absolute number of traces. + declare -A OVERRIDES=( + # 50% of invocations traced (default 1%). Dev volume is low enough that + # this is affordable and high enough to catch an intermittent fault. + [CDK_OBSERVABILITY_XRAY_SAMPLING_RATE]="0.5" + [CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR]="5" + [CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS]="true" + # Full request/response payloads. High volume and a PII surface, which + # is exactly why it is off by default — acceptable in dev, where the + # data is ours and the debugging value is highest. + [CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED]="true" + # 14 days is enough to debug something from last sprint without paying + # to keep dev noise for a month. + [CDK_OBSERVABILITY_LOG_RETENTION_DAYS]="14" + # Tighter than default so we see problems in dev before prod does. + [CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD]="5" + [CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD]="1" + [CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD]="5" + [CDK_OBSERVABILITY_DYNAMO_THROTTLE_THRESHOLD]="1" + # Catch prompt-cache regressions at the first sign in dev. + [CDK_OBSERVABILITY_PROMPT_CACHE_AVOIDABLE_MISS_THRESHOLD]="5" + [CDK_OBSERVABILITY_PROMPT_CACHE_WASTED_USD_THRESHOLD]="0.5" + ) + ;; +production) + # Prod trades trace volume for retention and signal quality. Sampling is + # far lower than dev because prod traffic is orders of magnitude larger and + # X-Ray bills per trace recorded; retention is far longer because a prod + # incident review can reach back weeks. + declare -A OVERRIDES=( + # 10% — ten times the default, a fifth of dev. Enough to characterise + # latency and catch a recurring fault without paying for every turn. + [CDK_OBSERVABILITY_XRAY_SAMPLING_RATE]="0.1" + [CDK_OBSERVABILITY_XRAY_SAMPLING_RESERVOIR]="2" + [CDK_OBSERVABILITY_XRAY_INSIGHTS_NOTIFICATIONS]="true" + # Deliberately LEFT OFF in production. These records carry every user's + # prompt and the model's response verbatim: the highest-volume log + # source available and a real PII surface. Turn on temporarily, for a + # specific investigation, then turn off again. + # [CDK_OBSERVABILITY_AGENTCORE_APPLICATION_LOGS_ENABLED]="true" + # 90 days supports month-over-month incident review and quarterly + # reporting. + [CDK_OBSERVABILITY_LOG_RETENTION_DAYS]="90" + # Prod thresholds sit between the defaults and dev's: tight enough to + # catch a real regression, loose enough not to page on single-request + # noise at production volume. + [CDK_OBSERVABILITY_AGENTCORE_ERROR_THRESHOLD]="5" + [CDK_OBSERVABILITY_LAMBDA_ERROR_THRESHOLD]="3" + [CDK_OBSERVABILITY_ALB_TARGET_5XX_THRESHOLD]="5" + [CDK_OBSERVABILITY_ECS_CPU_PERCENT]="75" + [CDK_OBSERVABILITY_ECS_MEMORY_PERCENT]="80" + ) + ;; +*) + echo "ERROR: --env must be 'development' or 'production', got '${ENVIRONMENT}'" >&2 + exit 1 + ;; +esac + +# ───────────────────────────────────────────────────────────────────────────── +# Preflight +# ───────────────────────────────────────────────────────────────────────────── + +if ! command -v gh &> /dev/null && [[ "${DRY_RUN}" != "true" ]]; then + echo "ERROR: the GitHub CLI (gh) is required. https://cli.github.com/" >&2 + exit 1 +fi + +# Auth is only needed to actually write. A dry run must work without it, so the +# profile can be reviewed on any machine — including in a devcontainer that has +# no gh session. +if [[ "${DRY_RUN}" != "true" ]] && ! gh auth status &> /dev/null; then + echo "ERROR: not authenticated. Run 'gh auth login' first." >&2 + exit 1 +fi + +REPO_NAME="" +if [[ "${DRY_RUN}" != "true" ]]; then + REPO_NAME=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null || echo '') +fi + +echo "Boise State observability overrides" +echo " environment : ${ENVIRONMENT}" +echo " repository : ${REPO_NAME}" +echo " variables : ${#OVERRIDES[@]}" +echo +echo "Values to set (anything not listed inherits the cost-conscious default):" +for key in $(printf '%s\n' "${!OVERRIDES[@]}" | sort); do + printf ' %-58s = %s\n' "${key}" "${OVERRIDES[$key]}" +done +echo + +if [[ "${DRY_RUN}" == "true" ]]; then + echo "DRY RUN — nothing was changed." + echo + echo "Equivalent commands:" + for key in $(printf '%s\n' "${!OVERRIDES[@]}" | sort); do + echo " gh variable set ${key} --env ${ENVIRONMENT} --body '${OVERRIDES[$key]}'" + done + exit 0 +fi + +# Mutating shared repository configuration — confirm explicitly. +read -r -p "Set these ${#OVERRIDES[@]} variables on the '${ENVIRONMENT}' environment? [y/N] " reply +if [[ ! "${reply}" =~ ^[Yy]$ ]]; then + echo "Aborted; nothing was changed." + exit 0 +fi + +for key in $(printf '%s\n' "${!OVERRIDES[@]}" | sort); do + echo " setting ${key}" + gh variable set "${key}" --env "${ENVIRONMENT}" --body "${OVERRIDES[$key]}" +done + +echo +echo "Done. Verify on the next platform.yml deploy: the synth log prints a line" +echo "beginning 'Observability:' with the RESOLVED values. If a value there does" +echo "not match what you just set, the variable is not reaching --context —" +echo "check that it is listed in the deploy job's job-level env: block in" +echo ".github/workflows/platform.yml (workflow-level env: resolves vars.* to" +echo "empty strings)."