feat(aso-overlay): CloudWatch EMF instrumentation for READ path (SITES-48140)#2822
Merged
Merged
Conversation
…S-48140)
Instrument GET /config/:service/redirects.txt and the ASO overlay auth handler
with CloudWatch Embedded Metric Format (EMF) via stdout envelopes under the new
Mysticat/AsoOverlay namespace. No IAM change; no PutMetricData calls; reuses
the existing metrics-emf.js pattern already proven on Mysticat/GitHubService
and Mysticat/Brands.
Metrics emitted (frozen catalog in src/support/aso-overlay-metrics.js):
- AsoOverlayRequestTotal (Count, dims: Environment, Outcome, Tier)
- AsoOverlayRequestDurationMs (Milliseconds, dims: Environment, Outcome)
- AsoOverlayEtagPresent (Count, dims: Environment)
- AsoOverlayConditionalGet304 (Count, dims: Environment)
- AsoOverlayIfNoneMatchInvalid (Count, dims: Environment, Reason=unquoted)
- AsoOverlayS3ReadDurationMs (Milliseconds, dims: Environment, Outcome)
- AsoOverlayAuthKeyUsed (Count, dims: Environment, Slot=current|previous)
- AsoOverlayAuthFailed (Count, dims: Environment, Reason=missing|invalid|malformed)
Cardinality: no per-tenant dimensions in CloudWatch (10k+ services would blow
the budget). Tenant-level views come from Fastly access logs in Splunk.
Key rotation observability: AsoOverlayAuthKeyUsed{Slot=previous} non-zero
signals rotation in-flight; sustained non-zero across > 24h signals the
sidecar fleet hasn't picked up the new key.
Tests: 14 controller-level + 7 auth-handler metric tests. 100% coverage on
new files (redirects.js, aso-overlay-key-handler.js, aso-overlay-metrics.js,
metrics-emf.js). Full suite: 14968 passing.
Complementary follow-ups (tracked on SITES-48140):
- Fastly VCL: emit key-slot-matched + 401-sub-reason as log fields for the
edge-decided requests that never reach origin (owner: platform team,
requires Fastly rights the ASO team doesn't have today).
- mystique #3381: aso_overlay_fastly_purge_* Prometheus metrics on WRITE path.
- Grafana dashboard (aso-overlay) reading Mysticat/AsoOverlay from
cloudwatch-main datasource.
- Splunk on-call dashboard from sourcetype=dx_aem_sites_spacecat_cdn_prod
for edge-side gross traffic + status class by URL.
- Grafana alert rules for rotation-slot=previous > 24h, 401 spike, 304
rate collapse, S3 AccessDenied spike.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
cc @alinarublea. Also FYI Maksym Yakobchuk (Slack: @iakobchu) — this closes the observability gap Cornel flagged and is the origin-side half of the ASO overlay dashboards (paired with adobe/spacecat-infrastructure#675). AsoOverlayAuthKeyUsed{Slot=previous} in the metric catalog is the rotation-in-flight signal you'll want to watch when key rotation testing kicks off. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Self-review pass by 10-agent code review team (senior-staff-engineer,
distinguished-engineer, senior-authn-authz-specialist, senior-security-researcher,
qa-engineer x 2 PRs) surfaced two critical bugs and several important
metric-shape issues. Fixes below.
Critical fixes:
1. Drop the `Tier` dimension from AsoOverlayRequestTotal. The origin route is
`/config/:service/redirects.txt` (Fastly strips the public tier prefix
before proxying), so `context.pathInfo.suffix.split('/')[1]` was returning
the CM service identifier (cm-p<N>-e<N>) — every customer became a distinct
dimension. At 10k+ services this would produce ~300k unique CloudWatch
metric combinations = ~$90k/month, violating the "no per-tenant dimensions"
contract the frozen catalog docblock declares. Environment (auto-added by
emitMetric) already carries the dev/prod split.
2. Rename `Outcome` -> `S3Result` on AsoOverlayS3ReadDurationMs. Same dimension
key had two semantic domains (request-level 200/304/etc vs S3-level
NoSuchKey/AccessDenied/Success); a dashboard slice "filter by Outcome=200"
would ambiguously match both. New enum S3_RESULT.{SUCCESS,NO_SUCH_KEY,
ACCESS_DENIED,UNEXPECTED} keeps the semantic clean. Extracted `emitS3Duration`
helper to remove the 4x-duplicated emit block.
Important fixes:
3. Rename AUTH_FAIL_REASON.MALFORMED -> CONFIG_MISSING. "Malformed" implied
client-side malformed data; the actual condition is a server-side deploy
misconfiguration (ASO_OVERLAY_API_KEY env var unset). On-call reading
"AuthFailed{Reason=malformed}" would waste time inspecting client headers.
4. Cache the `ifNoneMatchMatches()` result once. It was called twice per
S3-success path (once for the invalid-INM guard, once for the 304 branch),
which duplicates the trim/split/normalize work on the hot path.
5. Fix the missing S3ReadDurationMs assertion on the 500-s3 test path.
6. Fix comment drift on AUTH_FAIL_REASON.MISSING (was "Header absent OR not
path-scoped"; only fires when path-scoped AND header absent).
Test additions:
- New drift-guard test asserts every metric name emitted from getRedirects
appears in the frozen ASO_OVERLAY_METRICS catalog. Previously the "drift
guard" comment was aspirational — nothing enforced it. Now an added metric
that isn't cataloged fails CI.
- Added explicit S3ReadDurationMs{S3Result=success} assertion on the 200 path
and S3Result-value assertions on all 3 error branches.
- Removed the two Tier-specific tests (Tier dimension no longer emitted).
Verification: 76 tests pass; 100% lines/branches/functions coverage on all 4
touched files (redirects.js, aso-overlay-key-handler.js, aso-overlay-metrics.js,
metrics-emf.js). ESLint clean; type-check passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
abhishekgarg18
temporarily deployed
to
dev-branches
GitHub Actions
Inactive
…nvalidates (SITES-48140)
Alina's live-repro on mystique#3381 caught that our Fastly purge is currently
a silent no-op:
- Fastly's `POST /service/{sid}/purge/<path>` endpoint is a SURROGATE-KEY
purge (the path after `/purge/` is the key name), not a URL purge.
- Our 200 response emitted no Surrogate-Key header.
- So mystique was purging a key nothing was tagged with -> HTTP 200 + zero
effect (verified by `x-cache=HIT` age incrementing instead of resetting).
The URL-purge alternative (`POST /purge/{host}{path}`) is blocked by the VCL
`recv 40` aso-overlay-route auth gate (purge requests carry no X-ASO-API-Key).
Fix: emit `Surrogate-Key: aso-overlay-<service>` on both 200 and 304
responses. Mystique#3381 then purges that exact key via the same endpoint
it's already calling, which now does something real.
Key format:
- `aso-overlay-<service>` where <service> is the CM identifier (cm-p<N>-e<N>)
- Origin route is `/config/:service/redirects.txt` (Fastly strips tier
prefix), so per-service uniqueness is sufficient — no tier segment needed.
- Namespace prefix `aso-overlay-` guards against key collisions with future
overlays under other routes.
Coordination with mystique#3381: Maksym updates `fastly_purge.py` to call
`POST /service/{sid}/purge/aso-overlay-<service>` (drop the full URL path;
use the same key format). Unit tests there still pass because they mock the
HTTP call — end-to-end verification requires a live Fastly service.
Also included on the 304 response so any stragglers still cached at that
status get purged in the same call (RFC 7232 requires cache-control + etag
on 304; Surrogate-Key is an operationally-linked companion).
Tests: 54 passing (added Surrogate-Key assertions to the 200 and 304 tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tually invalidates (SITES-48140)" This reverts commit 863c0dd.
5 tasks
abhishekgarg18
temporarily deployed
to
dev-branches
GitHub Actions
Inactive
miakobchuk
pushed a commit
that referenced
this pull request
Jul 15, 2026
…(SITES-48140) (#2826) ## Summary Adds a `Surrogate-Key: aso-overlay-<service>` header to the 200 and 304 responses of `GET /config/:service/redirects.txt`, so that Mystique can targeted-purge Fastly's edge cache for a specific tenant's overlay via the existing `POST /service/{sid}/purge/{key}` endpoint after a successful S3 write. Pairs with: - experience-platform/mystique#3381 - Mystique's `fastly_purge.py` calls the key-purge endpoint - experience-platform/mystique-deploy#568 - wires FASTLY_ASO_PURGE_TOKEN env var ## Why this is needed Alina live-verified mystique#3381 against prod (`DlxppS2VoAizEqJ9bPasq6`, dev tenant `cm-p117653-e365853`) and found the purge is a silent no-op as currently wired: Fastly's `POST /service/{sid}/purge/<path>` endpoint is a **surrogate-key** purge (the path after `/purge/` is the key name), not a URL purge. Because our 200 responses had no `Surrogate-Key` header, the purge invalidated a key nothing was tagged with -> HTTP 200 + zero effect (`x-cache=HIT` age incrementing across polls, never resetting to 0). Two fix options were considered: 1. **Surrogate keys** (this PR) - origin tags responses with a key, Mystique purges that key. No VCL change needed, works with the existing purge endpoint. 2. **URL purge + VCL exemption** - would require adding a FASTLYPURGE method exemption in `fastly/vcl/aso-prod/recv/40-aso-overlay-route.vcl` (that VCL auth-gates all requests, including purge). Touches prod VCL. Alina recommended option 1; taking that path. ## Key format `aso-overlay-<service>` where `<service>` is the CM identifier (e.g., `aso-overlay-cm-p117653-e365853`). - Namespaced with `aso-overlay-` prefix so future overlays under other routes cannot collide with this key space - Service-scoped only (no tier segment): Fastly VCL strips the public `/config/<tier>/...` prefix before proxying to origin, so per-service uniqueness is sufficient - Emitted on both 200 and 304 responses so any 304 stragglers still cached at that status get purged in the same call (RFC 7232 requires we carry cache-control + etag on 304; Surrogate-Key is an operationally-linked companion) ## Coordination Once this PR lands: - Maksym updates mystique#3381's `fastly_purge.py` endpoint from `POST /service/{sid}/purge{url_path}` to `POST /service/{sid}/purge/aso-overlay-<tenant_id>` - Drop `_URL_TIER_BY_BUCKET_SEGMENT` from `handler.py` since the key is now tier-agnostic (also cleans up the stage-returns-None edge case) ## Test plan - [x] `test/controllers/redirects.test.js`: added `surrogate-key` header assertions on both the 200 happy-path test and the 304 conditional-GET test (38 passing) - [x] ESLint + type-check clean - [ ] Post-deploy on dev: hit the endpoint via curl, verify response includes `Surrogate-Key: aso-overlay-<service>` - [ ] After mystique#3381 lands + deploys, trigger a Deploy in ESS UI, verify `x-cache` flips from HIT to MISS and `age` resets to 0 - [ ] Once verified end-to-end, spacecat-infrastructure Fastly VCL `beresp.ttl` can bump from 10s -> 24h for the cost win Separately: this change is deliberately scoped narrow (17 lines) and split out from the paired observability PR (#2822) so it can be reviewed and shipped on its own timeline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Abhishek Garg <abhigarg+adobe@adobe.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
miakobchuk
approved these changes
Jul 15, 2026
solaris007
pushed a commit
that referenced
this pull request
Jul 15, 2026
## [1.654.3](v1.654.2...v1.654.3) (2026-07-15) ### Bug Fixes * **aso-overlay:** add Surrogate-Key header for Fastly targeted purge (SITES-48140) ([#2826](#2826)) ([9c1fa79](9c1fa79)), closes [experience-platform/mystique#3381](https://github.com/experience-platform/mystique/issues/3381) [experience-platform/mystique-deploy#568](https://github.com/experience-platform/mystique-deploy/issues/568) [mystique#3381](https://github.com/mystique/issues/3381) [mystique#3381](https://github.com/mystique/issues/3381) [#2822](#2822)
solaris007
pushed a commit
that referenced
this pull request
Jul 16, 2026
# [1.655.0](v1.654.4...v1.655.0) (2026-07-16) ### Features * **aso-overlay:** CloudWatch EMF instrumentation for READ path (SITES-48140) ([#2822](#2822)) ([4606c73](4606c73)), closes [adobe/spacecat-infrastructure#675](https://github.com/adobe/spacecat-infrastructure/issues/675)
Member
|
🎉 This PR is included in version 1.655.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Member
Author
|
@miakobchuk here's the complete picture on Grafana access + how dashboards get deployed. Nothing hidden — putting it all inline so you have a durable reference. Do you need any Grafana-specific groups to edit dashboards in the UI later?Yes — you need to be in the Okta group that maps to the SAML assertion attribute The mapping is defined in Each Grafana workspace is a separate Amazon Managed Grafana instance with its own SSO app:
Note: dev workspace is bound to adobe-stage Okta (not adobe prod) — quirky but documented in the repo CLAUDE.md. To get admin access, request membership in the group that provides the
If your role instead sends If you're not sure which group, DM How dashboards get deployed — full flowTwo directions exist, and dashboards use only the first: Direction 1: JSON-first (what we do for
cd spacecat-infrastructure
AWS_PROFILE=spacecat-dev ./scripts/deploy-dashboards.sh dev mysticat-aso-overlay.json
AWS_PROFILE=spacecat-stage ./scripts/deploy-dashboards.sh stage mysticat-aso-overlay.json
AWS_PROFILE=spacecat-prod ./scripts/deploy-dashboards.sh prod mysticat-aso-overlay.jsonThe script (
The AWS auth is what gates who can run the deploy. You need To get KLAM access:
Direction 2: UI-edit + sync-back (for alerts, per repo MUST rule) Alerts follow the opposite flow — you're not allowed to push alert JSON as your first artifact:
The MUST rule from
Dev/stage sync is disallowed to prevent an experimental state from overwriting the prod-verified copy. What's live right now for the ASO overlay work
Grafana prod dashboard deploy is the last step — needs the KLAM prod flow above. Once deployed, the same 8 panels populate with real prod data. Docs
Nothing more to explain — this is the full picture. Ping if you hit a specific step and I'll walk you through the exact command. |
This was referenced Jul 16, 2026
Closed
miakobchuk
pushed a commit
that referenced
this pull request
Jul 16, 2026
…500) (#2839) ## The symptom Every `GET /config/dev/*/redirects.txt` on dev is returning **HTTP 500** with: \`\`\` x-error: response.headers.raw is not a function \`\`\` Reproduced with a valid \`ASO_OVERLAY_API_KEY\` on multiple tenants (\`cm-p117653-e365853\`, \`cm-p128729-e2179647\`). Consistent on both the 200 and 304 conditional-GET code paths. Endpoint was working at 2026-07-16T00:03Z (Maksym's end-to-end verification); broke after v1.655.0 deployed (SITES-48140 EMF instrumentation, #2822). ## Root cause The error is thrown at [\`@adobe/helix-universal/aws-adapter.js:254\`](https://github.com/adobe/helix-universal/blob/main/src/aws-adapter.js#L254): \`\`\`js ...splitHeaders(response.headers.raw(), con.log), \`\`\` That helper expects \`@adobe/fetch\`-style Headers (which has \`.raw()\`), but the Response reaching the adapter has native Web-Fetch-API Headers (from \`undici\`) whose Headers has no \`.raw()\`. **Bundle inspection** (\`dist/spacecat-services/api-service@1.657.0.zip\`, extracted \`index.js\`) shows esbuild inlines **two Response classes** side by side: - \`Response2\` (line 2552) — \`@adobe/fetch\` shim, has \`.raw()\` on Headers ✅ - \`Response6\` (line 57669, plus 3 duplicated copies) — \`undici\` (native fetch spec), no \`.raw()\` ❌ At source level, \`createResponse\` (from \`@adobe/spacecat-shared-http-utils\`) correctly imports \`Response\` from \`@adobe/fetch\` and returns \`Response2\` — verified by a standalone local test. But after #2822 added \`metrics-emf.js\` imports to both \`RedirectsController\` and \`AsoOverlayKeyHandler\`, the extended module graph now resolves to \`Response6\` at bundle time along the overlay code path. Rather than chase the exact esbuild resolution behavior (opaque, fragile, and could regress again on any future import graph change), we **normalize at the outermost seam** — the wrapper that runs LAST on the response path before the AWS Lambda adapter sees the response. ## The fix New \`ensureFetchResponseWrapper\` installed as the OUTERMOST wrapper: - **Fast path** (every response that's already an \`@adobe/fetch\` Response): the wrapper does nothing. Two property checks (\`response.headers && typeof response.headers.raw === 'function'\`) and a return of the original response object. **Zero-cost passthrough.** - **Slow path** (the specific failure mode: response reaching us is a native Response): the wrapper rebuilds it as an \`@adobe/fetch\` Response with the same status, headers, and body. All existing behavior preserved. This is defense-in-depth: it fixes the specific interaction from #2822 AND protects against any future PR that could introduce the same shift via a different code path. ## Zero-regression argument (this is the important part) The wrapper is intentionally the smallest change that fixes the symptom: 1. **Every current endpoint uses \`createResponse\` from \`@adobe/spacecat-shared-http-utils\`**, which constructs \`@adobe/fetch\` Response. On the deployed Lambda, most of those already return \`Response2\` correctly today — those hit the wrapper's fast path (two property checks + return the exact same object reference). **Byte-identical outputs to what those endpoints produce today.** 2. **The two files that use bare \`new Response(...)\` today** (\`src/controllers/proxy.js\` and \`src/support/multipart-form-data.js\` error paths) hit the slow path — but they'd hit the exact same aws-adapter failure the overlay endpoint hits today, so if they were working before this PR they must be low-traffic paths that don't exercise the aws-adapter Response serialization (e.g. only used in local dev). If either is on a hot path in prod, this PR **fixes** them; it can't regress them. 3. **The slow path preserves status, headers, and body content bit-for-bit** — it buffers the body via \`response.arrayBuffer()\` and reconstructs with the same \`status\` and \`headers\`. The only theoretical difference is streaming behavior: our service does not stream any response bodies (all are small text/JSON payloads capped by the maximum-response-size guard in aws-adapter itself). 4. **Non-object results** (\`undefined\`, \`null\`, primitives — e.g. what helix-status can return for raw payloads) are passed through untouched via an early-return guard. 5. **7 unit tests** cover all these cases explicitly — fast path identity, slow path rewrap on 200/404/304, empty body, non-object returns, error propagation, single-value header preservation (the shape the overlay endpoint actually uses). 6. **Full existing test suite still green** (4908 passing across \`test/support/\` + \`test/controllers/redirects.test.js\` locally). **Known limitation, out of scope for this PR:** multi-value headers like \`Set-Cookie\` on the slow path would collapse to their last value. api-service is a token-authenticated API and does not set cookies; if that ever changes, the wrapper's \`forEach\` loop will need to switch to \`getSetCookie()\` for that one header per WHATWG fetch spec. Documented inline. ## Why we didn't catch this in dev Two independent gaps: 1. **Unit tests import from source** where module resolution goes to \`node_modules/@adobe/fetch\` directly — \`createResponse\` returns \`Response2\`, \`.raw()\` works, tests pass. 2. **CI's \`bundle-build\` step** invokes the bundled Lambda but only against \`/health\` (JSON, different code path from our text/plain overlay). Follow-up ticket to add a bundle-level integration test that exercises \`/config/dev/*/redirects.txt\` on the bundled artifact. Tracked as a separate item so this PR stays scoped to the fix. ## Test plan - [x] 7 new unit tests on the wrapper (fast path, slow path 200/304/404, empty body, non-object results, error propagation, header preservation) - [x] Existing suites green (\`test/support/\` + \`test/controllers/redirects.test.js\` — 4908 passing locally) - [x] Lint + type-check clean - [ ] **Deploy to dev via CI** — user asked for this to run through the standard deploy loop so we can live-verify - [ ] Post-deploy: curl \`/config/dev/cm-p117653-e365853/redirects.txt\` with valid key → confirm 200/304/404 shape (no more 500) - [ ] Unblocks Alina's TTL bump (task 37) and the fleet-wide 18-env verification (task 11) Supersedes revert PR #2838 — closing that in favor of this forward-fix. Jira: [SITES-48140](https://jira.corp.adobe.com/browse/SITES-48140) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Abhishek Garg <abhigarg+adobe@adobe.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
solaris007
pushed a commit
that referenced
this pull request
Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Instruments
GET /config/:service/redirects.txtand the ASO overlay auth handler with CloudWatch Embedded Metric Format (EMF) via stdout envelopes under the newMysticat/AsoOverlaynamespace. No IAM change; no PutMetricData calls; reuses the existingmetrics-emf.jspattern already proven onMysticat/GitHubService(webhooks) andMysticat/Brands.Closes the observability gap Cornel called out: "aso needs to have its own monitoring/observability in place — each service needs to have its own dashboards, dispatcher will not monitor for broken API keys".
Tracked in SITES-48140 (linked to SITES-44966).
Paired PR on
spacecat-infrastructure: adobe/spacecat-infrastructure#675 — Grafana + Splunk dashboards visualizing these metrics.Metrics emitted
Frozen catalog in
src/support/aso-overlay-metrics.js:AsoOverlayRequestTotalAsoOverlayRequestDurationMsAsoOverlayEtagPresentAsoOverlayConditionalGet304AsoOverlayIfNoneMatchInvalidunquotedAsoOverlayS3ReadDurationMsAsoOverlayAuthKeyUsedcurrent|previousAsoOverlayAuthFailedDesign notes
Cardinality: No per-tenant dimensions (10k+ services would blow the budget). Tenant-level views come from Fastly access logs in Splunk.
Rotation observability (crown jewel):
AsoOverlayAuthKeyUsed{Slot=previous}non-zero signals rotation in-flight; sustained non-zero across > 24h signals the sidecar fleet hasn't picked up the new key.INM validity:
AsoOverlayIfNoneMatchInvalid{Reason=unquoted}spikes indicate specific sidecar rollouts parsing ETags wrong (RFC 7232 §2.3 requires quoted opaque-tag). Empty/whitespace-only INM is normalized tonullbygetHeader()so is not tracked separately.Auth handler scope:
AsoOverlayKeyHandlerruns on every request. Metrics are emitted only for the overlay route to avoid flooding the namespace.Fault tolerance: All metric emission is wrapped in try/catch inside
emitMetric()— a stdout write failure never breaks the request path.Complementary follow-ups (tracked on SITES-48140)
mystique#3381 —aso_overlay_fastly_purge_*Prometheus metrics on WRITE path.Test plan
Mysticat/AsoOverlaywith correct dimensions🤖 Generated with Claude Code