Repository navigation
feat(health): page when the database snapshot stops happening - #140
Merged
Merged
Conversation
The snapshot worker's report lives only on /api/v1/health, and the scheduled probe reads /healthz, so a backup could die or start failing and nothing would say so until a human opened the endpoint by hand. A green /healthz is now followed by a second question to /api/v1/health, whose address is derived from the /healthz URL the probe is already given - so the host wrapper on the Pi needs no edit and the check reaches the timer with the merge. Red (exit 6, not 5, which the wrapper already spends on "LAN green, public ingest not") on status "error", on a last_run_at older than SNAPSHOT_STALE_AFTER_SECONDS, and on a status "ok" whose timestamp is missing or unparseable. status "unknown" stays silent: it means both "the worker has not run yet" and "snapshots are disabled", and disabled is the shipped default everywhere but production, so reddening on it would lie on every local instance. SNAPSHOT_CHECK=require is the caller asserting snapshots are expected here and turns that - plus a missing snapshot field and an unreadable /api/v1/health - red too; off skips the question. /healthz is unchanged in fields and in the meaning of ok, with a test pinning both against a worker reporting a failed cycle: that endpoint is the container liveness contract, and a failed backup must not mark a working container unhealthy or fail a deploy. Co-Authored-By: Wayland <wayland@agents.flopbut.local> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (8)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
docs/operations/duckdb-recovery.mdnow says the recovery point comes from snapshots. The mechanism works; nothing watches it.snapshotHealthexists only on/api/v1/health, while the scheduled probe reads/healthz, whose body carriesok,spans,last_ingest_at,newest_span_age_secondsand nothing else. The snapshot worker could die or start failing and the first notice would be a human opening the endpoint by hand.What changed
scripts/probe-healthz.shfollows a green/healthzwith a second question to/api/v1/health:snapshotobjectstatus: "error"last_errorquoted into the linestatus: "ok",last_run_atolder thanSNAPSHOT_STALE_AFTER_SECONDS(default 43200 = 2 × the shipped6hinterval)status: "ok",last_run_atabsent or unparseablestatus: "unknown"snapshotfield, or/api/v1/healthunreadableunknownis the ambiguous half — it means both "the worker has not run yet" and "snapshots are disabled", and disabled is the shipped default everywhere but production. Reddening on it would lie on every local instance, so it is silent by default.SNAPSHOT_CHECK=requireis the caller asserting that snapshots are expected on this instance and turns it — plus a missing field and an unreadable endpoint — red.SNAPSHOT_CHECK=offskips the question entirely.~/opsedit needed. The/api/v1/healthaddress is derived from the/healthzURL the wrapper already passes (same entry point, other path), so the merge reaches the timer by itself.API_HEALTH_URLoverrides the derivation.~/ops/cotel-healthz.shalready spends 5 on "the LAN half is green and the public ingest half is not", and passes a probe's own code through otherwise./healthzis untouched, in fields and in the meaning ofok. It is the container liveness contract (cotel --healthcheck, the DockerHEALTHCHECK,scripts/wait-for-healthy.sh), and a failed backup must not mark a working container unhealthy or fail a deploy.probe-edge-ingest.shand never reads/healthz. Through Cloudflare Access the defaultautodegrades quietly; the docs say not to setrequireon a vantage point behind Access.Verification
Green on live production (LAN):
Classification tests —
bash scripts/probe-healthz_test.sh→passed=27 failed=0(16 new; snapshot error, overdue, missing/unparseable timestamp,unknownunder both modes, absent field, unreadable endpoint,off, config validation, and the URL derivation, which every/snap-*/healthzcase exercises because the probe is given only the/healthzURL)./healthzcontract —go test ./internal/dashboard/ -run TestHealthzgreen (ingolang:1.24, no Go toolchain on this host). NewTestHealthzIgnoresTheSnapshotWorkerwritessnapshot_last_status=errortosettingsand pins the status code,ok, and the exact field set.Docs —
npm run buildindocs/completes clean.The alert path, not just the check. Driven through the real host wrapper and the real pager on the drill marker, with a mock serving a green
/healthzand an/api/v1/healthreportingstatus: "error":A real alert issue was created and assigned, and the assignment wake was claimed by a live run — delivery, not just detection. The green tick that closes it is run after the assignment run finishes, with the recovery wake checked in
diagnostics/wakesfor a request id of its own.Docs
docs/operations/duckdb-snapshots.md— new Who is watching it section: what is red, why the threshold is two intervals, how to check by hand.docs/operations/health-probe.md— the snapshot question, the verdict table, the two vantage points it must not be enabled for, exit 6, and the probe env table.README.md/docs/index.md— the three new probe env vars in the env tables, marked as read byscripts/probe-healthz.shrather than the binary;/healthzsection states why worker health is deliberately absent from it.CHANGELOG.md— entry under Unreleased → Added.Known limitation (needs a
~/opsedit, not made here)The alert's title still reads
cotel prod /healthz is redon a snapshot failure:PC_ALERT_SUBJECTis chosen by the host wrapper per half, not per verdict, and the wrapper is out of scope for this repo. The probe's verdict line in the body names the snapshot failure, and--probe-onlyreprints it. Say the word and I will write it up for the board.Summary by CodeRabbit
/healthzreports container readiness, while snapshot and retention health are available through/api/v1/health. Snapshot issues affect alerting, not/healthzliveness.