fix(0056): freshness live-only false-fire + ledger-processor alarms#97
Merged
Merged
Conversation
Post-deploy findings A + B from the 0070 go-live / 0082 verification. A: the SDEX push-freshness alarm sat permanently in ALARM under live-only operation. The seeded backfill_progress rows are status='running' with a NULL last_push_at, and the coalesce(last_push_at, started_at) fallback aged them from seed time even with no backfill pushing. Gate AGE_QUERY on `status='running' AND last_push_at IS NOT NULL` so seed rows and the paused/completed seams publish no metric (NOT_BREACHING -> OK); a running backfill that has pushed then stalls still fires (AC #5). Supersedes the finding-#4 started_at fallback, which could not distinguish "backfill overdue" from "no backfill at all". B: the core ledger-processor Lambda shipped unmonitored (lag_seconds was only a comment; it emits no custom metric). Add three alarms to ObservabilityStack from AWS-native metrics, built by deterministic name so no cross-stack ref / processor redeploy is needed: -lag (ingest-queue ApproximateAgeOfOldestMessage > opsAlarms.ledgerProcessorLagSeconds, default 60s, sustained 3x1m), -errors (AWS/Lambda Errors >= 1), -dlq (ingest-DLQ depth >= 1, the poison-pill gap the Errors alarm misses under reportBatchItemFailures). All wired to prices-{env}-ops-alarms. Rust: 4 unit tests green, fmt/clippy/lambda-build clean. Infra: tsc/eslint/prettier clean, cdk synth verifies all three alarms.
… prewhere Self-review (code-review PR #97) caught a regression the unit tests missed: dropping coalesce made `toInt64(... - toUnixTimestamp(last_push_at))` a Nullable(Int64) column (the WHERE ... IS NOT NULL removes rows but not the column's static type), which the clickhouse crate cannot deserialize into the plain i64 `age_seconds` field. Every probe run would have errored -> the -errors alarm false-pages AND no PushAgeSeconds is published, blinding the freshness alarm. - Wrap in assumeNotNull(last_push_at) (safe: WHERE guarantees non-NULL) so the column stays non-Nullable, matching the repo's toUnixTimestamp(Nullable) -> Option / strip convention (cf. sink.rs ExistingProgress). - Pin SETTINGS optimize_move_to_prewhere_if_final = 0 so the non-sort-key status/last_push_at predicates cannot be moved to PREWHERE and surface a stale pre-FINAL row version. - Fix the now-inaccurate SOROBAN_AMM_STREAM forensic-value doc comment. - Assert both query invariants in the guard unit test. The true deser path still needs the Docker-gated CH integration test to run; the unit test only asserts the query shape.
Follow-up to the PR #97 self-review (findings 2/3/4): - Halt blind spot (finding 2): the lag/errors/DLQ alarms all key on the PRESENCE of enqueued/failed messages, so a producer-side stop (S3->SNS->SQS delivery halts, subscription removed) drains the queue and invokes nothing, leaving all three OK while ingestion is dead. Add prices-{env}-ledger-processor-no-invocations: AWS/Lambda Invocations < 1 over 15 min with treatMissingData BREACHING (missing = zero invocations = the halt). Pubnet closes a ledger every ~5-6s, so 15-min silence is a real outage. Soften the task-doc "covers the live-ingestion plane" claim. - Altitude (finding 3): extract ledgerProcessorFunctionName / ingestQueueName / ingestDlqName helpers exported from ComputeStack (which now uses them for its own resources) and import them into ObservabilityStack, so the alarm metric dimensions share one source of truth by name (no cross-stack CFN ref). A rename can no longer silently orphan an alarm on a non-existent metric. - Flapping (finding 4): raise default ledgerProcessorLagSeconds 60 -> 120 and the lag sustain 3 -> 5x1min so routine deploys / cold starts (oldest message ages a few min while briefly paused, then drains) don't false-page; a real stall keeps climbing well past it. Synth-verified (4 ledger-processor alarms, helper-derived names on both stacks); tsc/eslint/prettier clean.
Closes the test-coverage gap that let the PR #97 Nullable(Int64) regression land: the unit tests only cover age_metrics() + the query string shape, never the execute/deserialize path. freshness_it.rs runs the exact AGE_QUERY against a local Docker ClickHouse and asserts: - fetch_all::<StreamAge> executes + deserializes (the regression guard: fails on a Nullable(Int64) age_seconds column against the plain-i64 field), - the status='running' AND last_push_at IS NOT NULL gate (seed / paused / completed rows all excluded — finding A, no live-only false-fire), - FINAL returns the latest version, not a stale pre-merge 'running' row (a flipped running->completed stream is excluded), - the live-only end state publishes zero rows (alarm stays OK). Passes on ClickHouse 26.3.10.60 (prod-pinned). Docker-gated (#[ignore]).
This was referenced Jul 7, 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
1 participant
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.
Resolves the two post-deploy findings on task 0056 surfaced by the 0070 go-live + 0082 verification. Both were code gaps (not just fire-tests); the fire-tests + ops-topic subscribe remain operational.
Finding A — SDEX push-freshness false-fires under live-only
Root cause: seeded
backfill_progressrows arestatus='running'withlast_push_at=NULL, and thecoalesce(last_push_at, started_at)fallback aged them from seed time — so with no backfill actually pushing, the alarm sat permanently in ALARM even though live SDEX ingestion was healthy.Fix (
packages/backfill-freshness-probe/src/lib.rs):AGE_QUERYgates onWHERE status = 'running' AND last_push_at IS NOT NULL, age =now() - last_push_at. Live-only seed rows and the paused/completed seams publish no metric →NOT_BREACHING→ OK; a running backfill that has pushed then stalls still fires (AC #5). Supersedes review finding #4'sstarted_atfallback, which couldn't tell "backfill overdue" from "no backfill at all" — the source of the false-page.Finding B — the core ledger-processor was unmonitored
prices.ledger_processor.lag_secondsexisted only as a comment; the processor emits no custom metric. Rather than add Rust emission + a processor redeploy, three alarms ride AWS-native metrics, built by deterministic physical name soObservabilityStackneeds no cross-stack ref (mirrors theopsAlarmsTopicNameimport pattern):…-ledger-processor-lagAWS/SQS ApproximateAgeOfOldestMessage(ingest queue)opsAlarms.ledgerProcessorLagSeconds(new config, default 60s), sustained 3×1m…-ledger-processor-errorsAWS/Lambda Errors…-ledger-processor-dlqApproximateNumberOfMessagesVisiblereportBatchItemFailures)All three →
prices-{env}-ops-alarms,NOT_BREACHING.Verification
cargo test4/4,fmt --check,clippy,--features lambdabuild — clean.nx build/eslint/prettierclean;cdk synth Prices-production-Observabilityshows all three alarms with correct namespaces.Still operational (task 0056 stays active)
make deploy-production-observability) + EventBridge (freshness probe) stacks.prices-production-ops-alarms.notes/G-alarm-fire-test.md.