gateway: optional durable lifecycle journal for external health monitors - #86363
gateway: optional durable lifecycle journal for external health monitors#86363vxcozy wants to merge 2 commits into
Conversation
Opt-in via HERMES_GATEWAY_CHURN_PATH: when set to an absolute path, the gateway appends one JSON record per lifecycle event (start, replace) to a bounded journal capped at 256 records. External monitors can read the file to detect crash loops and silent process replacement that log scraping and pid polling miss. Off by default; a write failure never affects startup.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
gateway: optional durable lifecycle journal for external health monitors
|
Review follow-up: gateway.churn_path in config.yaml now bridges to the internal env var (config is authoritative when present; a directly set env var still works), and _read_tail only drops its first window line when the window actually starts mid-record.
|
Thanks for taking a look so quickly @Enough1122! Both correctness points are fixed in 0a31f47. The path moved into config.yaml as gateway.churn_path, which bridges to the internal env var. That follows the AGENTS.md rule for behavioral settings. Config wins when both are set, and the env var alone still works. _read_tail now reads the byte just before its window and only drops the first line when the window starts mid-record. There are tests for the exact-boundary and mid-line cases. I'm leaving the rewrite-per-event point alone. Events are process starts, so the file stays around 30 KB with the 256-record cap. The full rewrite is what makes sure a reader never sees a partial window, and a test pins that. A plain append would save little and lose the guarantee. |
Related: merged #64536 provides a separate OTLP monitoring path; this PR's opt-in local lifecycle journal is complementary rather than a duplicate. |
|
Thanks, @alt-glitch. That matches how I'd frame it too. The one thing worth adding is where the two actually diverge. The health export samples state on an interval, 60 seconds for metrics by default, so a gateway that dies and gets replaced between two samples still reads as running at both ends. This journal writes at the moment of the handoff instead, and records which pid replaced which. That's the silent churn case it was built for. It also runs without a collector, which is what makes it useful on a single unattended box where the monitor is just a script reading a file. |
Summary
This adds an opt-in journal that records gateway lifecycle events (start, replace) to a bounded JSONL file. It is off by default and activates only when
HERMES_GATEWAY_CHURN_PATHnames an absolute path. Nothing else changes.Motivation
We run Hermes unattended on a dedicated machine, with an external watchdog that pages a human when something breaks. The hardest failure to catch from outside is silent gateway churn: the process dies and relaunches (or gets replaced during an update), everything looks healthy afterward, and whatever was in flight during the gap is just gone. Log scraping is fragile across restarts and rotation, and polling the process table misses fast replacements.
A small durable journal solves this: the gateway appends one record per lifecycle event, and any monitor can read the file and apply its own idea of how much churn is too much. We have carried this as a local patch since July and re-applied it across every update. It has caught real incidents for us, including a crash loop after an OS update and a replacement that dropped an in-flight change. Upstreaming it means any install can get the same visibility without carrying a diff.
What's in the PR
gateway/churn.py(new, ~220 lines): the writer. Bounded at 256 records with atomic rewrite at the cap, advisory file locking (fcntl on POSIX, msvcrt on Windows), and records limited to process identity: event type, timestamp, old pid, new pid. No thresholds and no policy; deciding what churn is acceptable belongs to whatever reads the file.gateway/run.py(20 inserted lines, nothing removed): after the gateway owns its PID and finishes adapter startup, it writes a start record, or a replace record when it took over a running instance. Both are no-ops when the env var is unset.tests/gateway/test_churn.py: activation gating, the record cap, racing writers across processes, readers never seeing a partial window, and separation from the existing takeover-marker contract.Design choices
The writer is deliberately dumb. It does not rotate by time, does not interpret events, and refuses relative paths instead of guessing a base directory. A write failure logs a warning and never affects gateway startup: visibility must not become a new way to go down. Records are one JSON object per line, so consumers need nothing beyond the standard library.
Compatibility and testing
Off by default, zero behavior change with the env var unset, no new dependencies. On current main:
tests/gatewaycollects clean with this change (5,637 tests, same as without it) and the new file passes (5 tests). The run.py diff is insertion-only.