feat(moq-relay): make /health a plain liveness probe, drop sysinfo#1746
Conversation
The `/health` endpoint grew in-process host overload checks (CPU, RAM, network, load average) backed by the sysinfo crate, plus an external `--web-health-api` deferral. Host health monitoring belongs in a separate process that watches the host, not in the relay, and the thresholds went unused. Reduce `/health` to a pure liveness probe that always returns 200 ok, and remove all `--web-health-*` flags, env vars, and TOML keys along with the `HealthConfig`/`Health`/`MemLimit`/`LoadLimit` types. Dropping sysinfo 0.39 also drops the crate's 1.95 MSRV bump (it needed std's `cfg_select!`); moq-relay returns to the workspace MSRV of 1.85. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe pull request removes the load-shedding health monitoring system from 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/bin/relay/http.md`:
- Around line 82-89: The documentation for the `/health` endpoint describes the
response body as `ok`, but the actual implementation in rs/moq-relay/src/web.rs
(Line 385) and the smoke test in rs/moq-relay/tests/smoke.rs (Line 269) validate
that the response body is `ok\n` with a trailing newline. Update the
documentation example and description in the `/health` section to explicitly
show the newline character in the response body (change `ok` to `ok\n`) to
accurately reflect the actual API behavior and prevent confusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 558a609e-b1d6-4ca8-a0f9-cacb8630cc17
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
doc/bin/relay/config.mddoc/bin/relay/http.mdrs/moq-relay/Cargo.tomlrs/moq-relay/src/config.rsrs/moq-relay/src/health.rsrs/moq-relay/src/lib.rsrs/moq-relay/src/main.rsrs/moq-relay/src/web.rsrs/moq-relay/tests/smoke.rs
💤 Files with no reviewable changes (5)
- rs/moq-relay/src/main.rs
- doc/bin/relay/config.md
- rs/moq-relay/src/health.rs
- rs/moq-relay/src/lib.rs
- rs/moq-relay/src/config.rs
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Patch bump for the /health simplification. Per the relay's semver policy (no external consumers yet) breaking relay changes are released as patch bumps rather than a minor/major upgrade. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
The relay's
/healthendpoint had grown into a host overload monitor: in-process CPU/RAM/network/load-average thresholds (sampled via thesysinfocrate) plus an external--web-health-apideferral, all returning503when a threshold was breached.Host health monitoring belongs in a separate process that watches the host, not in the relay, and the thresholds were going unused. This reverts that surface:
/healthis now a pure liveness probe that always returns200 ok(unauthenticated, so load-balancer probes don't need a JWT).--web-health-*CLI flags,MOQ_WEB_HEALTH_*env vars, and[web.health]TOML keys.HealthConfig,Health,MemLimit, andLoadLimittypes (src/health.rs, ~684 lines).MSRV
sysinfo0.39 relies on std'scfg_select!, stabilized in Rust 1.95, which had forcedmoq-relayto override the workspace MSRV withrust-version = "1.95". Droppingsysinfolets the crate return torust-version.workspace = true(1.85), matching every other crate in the workspace.Files
src/health.rsdeleted; wiring removed fromweb.rs,main.rs,lib.rs, and the TOML-clobber regression test inconfig.rs.Cargo.toml: droppedsysinfo, reverted the MSRV override.reqwest/url/wiremock/serde_withare retained (used elsewhere).doc/bin/relay/config.mddrops the[web.health]section;doc/bin/relay/http.mddocuments the liveness probe and points host monitoring at a separate process.Public API note
This removes public items (
HealthConfig,Health,MemLimit,LoadLimit) and CLI flags frommoq-relay. Targetingmainrather thandev: this isn't a wire-protocol change nor one of the consumed-library APIs the branch-targeting rules route todev, and the relay has no external consumers yet (breaking relay changes are treated as patch bumps).Version
0.12.9 is already released, so this lands in
[Unreleased]. The version bump + changelog are left to release-plz rather than hand-edited.Test plan
cargo build -p moq-relaycargo clippy -p moq-relay --all-targetscargo test -p moq-relay(119 unit + 3 smoke, includinghealth_endpoint_reports_ok)cargo fmt -p moq-relay -- --check(Written by Claude)