feat(0125): cloudwatch dashboard widgets, write-latency metric, viewer user - #280
feat(0125): cloudwatch dashboard widgets, write-latency metric, viewer user#280adamkoot wants to merge 13 commits into
Conversation
…sor write path The Tranche 3 dashboard needs a ClickHouse write-latency series and the cluster cannot supply one: it sits on Hetzner behind mTLS with no metric stream, and the runtime identities hold no grant on system.*. Ratified Decision B2 measures the real writer instead of a canary — it observes the actual batch INSERT that merge pressure and disk stalls hit first. Timed at the two write_candles call sites in reconcile.rs rather than inside the sink, so the CandleSink trait and both impls stay untouched and no feature cfg leaks into the write path. Samples are recorded only after the '?', so failed writes are not measured and no error path changes. The carrier is an Option on RunStats: a run that persisted nothing must publish NO datapoint, not a 0 ms minimum that would poison the p50 forever. 1+N INSERTs per invocation fold into one StatisticSet. The publish sits in the handler's Ok arm, which is reachable only after the cursor commit succeeded — the rows are already durable and the Err arm that pushes a BatchItemFailure is unreachable from there. A CloudWatch failure logs a warning and the invocation still returns success. The role's PutMetricData grant was conditioned on PricesApi/LedgerProcessor, a namespace nothing has ever published to; its value now matches METRIC_NAMESPACE in metrics.rs. A drift there fails every publish with AccessDenied and leaves the widget empty with nothing failing loudly.
…-only viewer prices-production-overview has been an 811-byte scaffold with one TextWidget since 2026-07-06. It now carries the full widget set: an acceptance strip that answers the M2 criteria (p95 latency, 5xx rate, cache hit ratio) and Tranche 3 AC 8 on one screen, then API, ingestion, ClickHouse/backfill, workers, and enrichment/oracle rows. Every widget is built against a metric+dimension pair verified to carry live data in the account on 2026-09-03. The two pairs that have never published are deliberately absent: Stream=sdex_archive (only soroban_amm has ever emitted) and the cleanup worker (its schedule is disabled, so it has no Lambda metrics at all). Both would render as No data and fail AC 1. The alarm strip is DERIVED by walking the construct tree, never a list of names, so it needs no maintenance when an alarm is added. The nine per-worker -errors alarms owned by EventBridgeStack are imported by ARN rather than by a cross-stack reference, which would have coupled the two stacks' deploys. The DashboardAlarmCount output makes the coverage — 49 — mechanically checkable. Two name helpers, restApiName and workerErrorAlarmName, because a drifted physical name in a metric or alarm reference fails SILENTLY: the panel just goes empty. Both are now the single source of truth for the resource and for the dashboard. periodOverride is set to inherit. Its default is auto, which silently overrides every per-widget period and would make the 14-day and 7-day trend rows inert with nothing failing at synth. defaultInterval carries the global range instead of start, since setting both throws. The viewer is an IAM user with exactly CloudWatchReadOnlyAccess — not the account-wide ReadOnlyAccess, which would expose Secrets Manager metadata, S3 listings and Lambda config — and carries NO password: the console login is an out-of-band operator step. verify-dashboard-synth.mjs asserts all of the above against the synthesized template, deriving the expected alarm count from the template's own alarm resources so it stays honest as the alarm set changes.
…operator runbook Acceptance criterion 4 asks for the ClickHouse-metric mechanism and its cost in writing, so this is part of the deliverable rather than a write-up of it. Records what shipped and why each placement is the only safe one: the timer at the call site rather than in the sink, the Option carrier that keeps an idle run from publishing a 0 ms minimum, and the publish in the Ok arm after the cursor commit where it cannot reach the BatchItemFailure path. Both named substitutions are written down verbatim for the evidence file: the frozen submission's DB CPU served by the ClickHouse host and write-path metrics because ADR 0007 replaced RDS, and the read-only IAM role served by an IAM user because no external principal is known to trust. The B2 gate figures are recorded with the branch taken — 106 165 invocations, avg 238.05 ms, p95 354.65 ms, against a ~5 s cadence — because B2 is a deviation from this task's own recommendation to extend the probes, and the measurement is what justifies it. Cost is ~4.50 USD a month. The alarm count is corrected from seven to 49 and the criterion reworded off the stale number. Two findings the work turned up are named so they are not lost: the SDEX push-freshness alarm reads a stream dimension that has never published, so a stalled backfill would not fire it; and the cleanup worker is deployed with a disabled schedule and no metrics at all. Both need their own tasks — this one changes no alarms. Five criteria ticked; 1, 7 and 8 stay open with what they wait on, all of it operator work in the runbook.
…solve CloudWatch cannot compute a percentile from a StatisticSet: SampleCount / Sum / Minimum / Maximum discard the distribution, and nothing behind them can answer p50 or p95. The dashboard asks ClickHouseWriteLatencyMs for p95 in the row-0 acceptance strip and for p50/p95 in the ingestion trend row, so as published the headline tile — the one a Stellar reviewer opens first — would have read "No data" for ever, with nothing failing at synth, at deploy or at render. So the accumulator keeps the samples themselves (`samples_ms: Vec<f64>`) and the datum carries `set_values(Some(values))` with no `Counts` (defaults to 1 per value). Builder name verified against the installed crate at ~/.cargo/registry/src/index.crates.io-*/aws-sdk-cloudwatch-1.116.0/src/types/ _metric_datum.rs:196. `Values` accepts at most 150 entries per datum, so a long run chunks into further datums of the same metric inside the same PutMetricData call — the "one publish per invocation" batching rationale survives; only the encoding changes. An idle run still publishes nothing. The reconcile timers now record only when the candle slice is non-empty: write_candles short-circuits on an empty slice with no round-trip, so timing it folded a ~0 ms in-process no-op into the same samples as real network writes. A run with AMM trades and no classic SDEX trades (or the reverse) is routine, not an edge case. Control flow and every error path are unchanged — the writes and their `?` stay exactly where they were. The synth script now fails when any Prices/Ingest widget asks for a pNN stat while metrics.rs still publishes via statistic_values(), so this class of silent empty panel cannot ship again.
.claude/* except settings.json and the three shared project skills, .planning/ and .gsd/ are local planning state, not shipped code; lore/ remains the task system of record.
… runbook step Five captures of the live prices-production-overview (2026-09-03 12:56 UTC): acceptance strip with all 49 alarms OK, API, ingestion with the new write latency, ClickHouse/backfill/workers, enrichment/oracle. Evidence for Tranche 3 AC 8 that milestone 1 could not give. Runbook step 0 added: cdk synth packages the pre-built ledger-processor bootstrap, so a deploy without cargo lambda build ships the previous binary — which is what the first deploy of this task did.
The header said the write-latency metric "is empty until ComputeStack is deployed". Written before the first deploy; on the live dashboard it reads as a defect. Say what the metric is instead: one raw value per INSERT, so the p50/p95 widgets are real percentiles.
…es, plain section text The three API tiles read their own 24-hour window so a quiet night no longer renders them as --. 5xx and DLQ depth are FILL(..., 0): for an error count and a dead-letter queue, no datapoint means zero. The ingest queue age and free-disk graphs draw the alarm threshold from the same opsAlarms key the alarm reads; the mTLS tile states its threshold in the title. Section headers now say what each row is and what the charts show. verify-dashboard-synth.mjs asserts the 24h windows, the two FILLs and that annotation values equal envs/<env>.json opsAlarms. Also unblocks CI format:check: prettier-ignore the k6 output under docs/loadtest-results (generated per run, 0121), and format the verify script.
…publish, tighter guard CR-01: the Stellar viewer no longer carries CloudWatchReadOnlyAccess, which also reads every log group and trace in the account shared with the block explorer. It gets an inline policy of exactly the CloudWatch Get/List/ Describe calls the dashboard needs, and the synth guard rejects any managed *ReadOnlyAccess on the viewer or any non-read action in its policy. WR-04: the CloudWatch client is bounded (connect 1s, attempt 3s, operation 6s, 2 attempts) so a stalled endpoint cannot hold a reserved-concurrency-1 invocation to the 60s limit. WR-05/06, IN-03/04: the percentile check cannot pass vacuously; threshold lines are matched to their own widget; --env is validated; AccessKey is forbidden like LoginProfile. WR-02/03: one worker list. SCHEDULED_WORKERS in lambda-baseline.ts feeds the alarm-strip import and the workers row; createWorkerLambda refuses a name not on it; the guard derives the imported count from the body. WR-07, IN-01/02: loud log on a missing ENV_NAME; the fourth top-row tile reads 24h like the other three; chunking documented as a limit defence. Left out on purpose: the two CI steps (WR-01, IN-05) — the push token has no workflow scope; recorded in the task as a follow-up.
Code review — dashboard widgetsReviewed The PR is in good shape. Two findings are verified against the branch; three are plausible but were not independently reproduced. Verified1. The script's header reads "This script is that human, run by CI." CI does not run it. So the three defects it exists to catch — an alarm constructed after the widget block (the strip silently under-covers), a renamed REST API, a reintroduced 2.
Diffed on this branch: So the gap is not hypothetical for a future tenth worker — three existing workers already have no duration or no-invocations alarms, and the docstring would tell the next maintainer not to look. Either derive Not independently verified3. It computes Worth prioritising: that tile is an M2 acceptance exhibit. 4.
5. The block explaining that a count is read off the body "rather than kept as a constant here — a constant was the second hand-maintained copy of the worker list" now sits directly above Checked and correct — recording so they are not re-litigated
|
…licit health-alarm coverage Review finding 3: the 5xx-rate tile had no FILL on its 5xx input, so a quiet, healthy day rendered it as -- while the graph beside it drew zero. It is an M2 exhibit; now 100 * (FILL(e5xx, 0) / requests). Finding 4: main.rs loaded aws_config twice per container (S3Fetcher and the CloudWatch client). One load, shared; the publish timeouts sit on the CloudWatch client's own config so they never apply to ledger downloads or credential resolution. Finding 2: SCHEDULED_WORKERS claimed the health alarms derive from it — they do not, and three workers (cleanup, asset-discovery, supply) have none. The stack now names those three and asserts every scheduled worker is in exactly one of workerHealth or that list; the docstring says what the list actually guarantees. asset-discovery/supply alarms are a decision for 0256 and a follow-up, recorded in the task. Finding 5: the imported-count JSDoc sat above MIN_ALARMS; moved to the count it describes, and MIN_ALARMS says why it is hand-maintained. Finding 1 (CI step) still waits on a workflow-scoped push token.
…ocessor crate verify-dashboard-synth.mjs asserted on the synthesized dashboard but nothing ran it (deep review WR-01, PR review finding 1). It now runs after the production synth in the Rust job, where the nine bootstraps already exist. cargo clippy gains prices-ledger-processor with --no-deps: the crate that owns the ClickHouseWriteLatencyMs publish path was outside the -D warnings gate (IN-05); --no-deps keeps prices-ingest-core's pre-existing findings out.
main.rs now loads one SdkConfig shared by the S3 and CloudWatch clients (PR #280 review, finding 4), so the constructor that loaded its own has no callers. Removing it also removes the last aws_config use outside main.rs, so a second load cannot creep back in through it.
This reverts commit 61eb1db.
2 participants
Summary
prices-production-overviewgets a real widget set (API p50/p95/p99, 4xx/5xx and cache-hit ratio, ingestion lag and DLQ, ClickHouse write latency, backfill, mTLS NotAfter, worker health) plus an alarm strip covering all 49 alarms — derived from the stack's own alarm constructs, not a name list. One dashboard,periodOverride: INHERIT, trend widgets with their own window; the four row-0 acceptance tiles read the last 24 h.Prices/IngestClickHouseWriteLatencyMs: the ledger-processor times its real candle INSERTs and publishes raw values (≤150 per datum, so p50/p95 resolve) in theOkarm after the cursor commit, best-effort, behind thelambdafeature. Error paths untouched. IAM condition narrowed to the new namespace. OneSdkConfigload shared by the S3 and CloudWatch clients; the CloudWatch client carries its own timeouts (connect 1 s / attempt 3 s / operation 6 s, 2 attempts) so a stalled endpoint cannot hold a reserved-concurrency-1 invocation.prices-${env}-stellar-viewerwith an inline policy of exactly the CloudWatchGet*/List*/Describe*calls the dashboard needs — notCloudWatchReadOnlyAccess, which would also grantlogs:*Get*/FilterLogEventsandxray:Get*across the account shared with the block explorer. No login profile in code (operator creates it out of band). Tranche 3 AC 8 "read-only IAM role" is satisfied by a user + policy and named as such.SCHEDULED_WORKERS/SCHEDULE_DISABLED_WORKERSinlambda-baseline.tsare the single source for the worker list:createWorkerLambdathrows on an unknown name, the alarm-strip import and the workers row derive from it, and ObservabilityStack asserts every worker is in exactly one ofworkerHealthor the named exemption list (cleanup,asset-discovery,supplyhave no duration/no-invocations alarms today — the-errorsalarm is their coverage; a decision for 0256 and a supply task, not this PR).tools/scripts/verify-dashboard-synth.mjs(npm run infra:verify-dashboard) runs in CI after "Synth production app", and CI now also clippiesprices-ledger-processor --features lambdaunder-D warnings. The guard asserts on the synthesized template: metric fragments present, alarm strip = own alarms + imported (count read off the body), no known empty-panel dimension pairs, percentile stats backed by raw-value publication, threshold lines matched to their widget, viewer has no managed*ReadOnlyAccess/ no non-read action / noLoginProfile/ noAccessKey.5 s cadence) and cost ($4.55 + $0.30/month); two out-of-scope findings for follow-up (SDEX push-freshness alarm reads a series that never published;cleanupschedule disabled). ACs 1, 7, 8 stay open until deploy.Review follow-ups
All five findings from the review of
379d81aare in (6f7ec69,ac5d5a5): 5xx-rate tileFILLs its input like the graph beside it; oneaws_configload in the ledger-processor (S3Fetcher::from_envstays —asset-discoverybuilds its fetcher through it);SCHEDULED_WORKERSdocstring narrowed to what it guarantees, with the exemption list asserted;MIN_ALARMSJSDoc moved to the count it describes; synth guard wired into CI. A cross-file deep review on top of that produced the viewer inline policy and the CloudWatch client timeouts above.Deploy
Both stacks need a redeploy: Compute first (rebuild the ledger-processor
bootstrapwith--features lambdabefore synth —cdk synthdoes not compile it), then Observability. Runbook in the task file.