You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Three observability gaps found on edge-nl-01 (2026-07-27): a fired SLO alert that cannot be attributed to anything, a dependency that silently changes gate outcomes with no probe, and a pressure signal blind to the resource that actually kills this box.
1. The latency histogram has no route label, so its SLO alert is unactionable
LoopoverRequestLatencySLOBreach fired ~17:23–20:15 UTC reporting p95 = 9.75 s against a 1 s SLO. But sum by (le, route) (rate(loopover_http_request_duration_seconds_bucket[5m])) returns a single unlabelled series — there is no route label at all — so the alert names no route, and at ~0.6 req/min a 5-minute p95 is computed from roughly three requests.
The reason it has no label is the constraint the fix must respect: a raw path (or any per-PR/per-repo id inside one) is unbounded cardinality, which takes a Prometheus down rather than merely misinforming it.
Add a bounded route-group label. An allowlist, not a path sanitizer — a "replace ids with :param" normalizer still trusts the path's shape, and one unmatched pattern reintroduces unbounded cardinality silently. An allowlist can only be wrong in the safe direction.
Every unknown path, including every 404, must collapse to a single other bucket.
(The companion min-request-rate gate on the alert expression is alertmanager config on the host, tracked in the parent — not repo code.)
2. No readiness probe for BROWSER_WS_ENDPOINT
src/selfhost/health.ts probes Redis, Qdrant, the GitHub App and codex — every dependency the review path needs except the browser. A browserless outage is therefore invisible in /ready while it silently changes gate outcomes: the screenshot-table gate reads absent visual evidence as a close signal (#9464 stopped that from closing PRs; this makes the outage itself visible rather than inferred afterwards).
Add a probe, registered only when the endpoint is configured — an instance with visual review off is not degraded for lacking a browser.
Keep it cheap enough for a poll loop: probe the HTTP sibling, do not launch Chromium. Cache/single-flight like the codex probe so /ready polling cannot itself load a struggling backend.
A configured-but-unparseable endpoint must fail readiness closed, not be skipped.
Never put a ?token= into a probe URL that could be logged.
3. Host pressure watches CPU only
src/selfhost/host-pressure.ts:12-22 uses loadavg / cpus().length, consumed at src/selfhost/maintenance-admission.ts:269-282. On this GPU box the realistic killer is memory — Ollama at 9.86 GiB and browserless at 1.45 GiB — and nothing sheds or even observes available memory; the OOM killer decides, which takes the whole container and every in-flight job rather than deferring one maintenance job.
Add a memory dimension to the pressure signal, with the same fail-open null means "skip this check" contract as the load signal — an unavailable reading must never itself defer work, and must never be clamped into a misleading 0.
Gate the maintenance-drain escape on it too: draining onto a near-OOM box is what the signal exists to prevent.
Keep the existing host_load_high reason stable — operators alert and dashboard on those strings.
Tests
Known routes group correctly; unknown/attacker-shaped/very-long paths always yield other; a fuzz sweep produces no value outside the fixed set; a per-PR path leaks no id.
The browser probe is absent when unconfigured, reports ready/not-ready correctly, drops the query string, maps wss:→https:, fails closed on an unparseable endpoint, caches within its TTL, single-flights concurrent checks, and re-probes once the TTL lapses.
Memory pressure defers with its own distinct reason, is strictly-greater (not >=), skips on null, gates the drain escape, and leaves CPU outranking it.
Expected outcome
A fired latency alert names a route; a browserless outage is visible where every other dependency's is; and memory pressure defers a maintenance job instead of letting the OOM killer take the container.
Parent: #9487
Summary
Three observability gaps found on
edge-nl-01(2026-07-27): a fired SLO alert that cannot be attributed to anything, a dependency that silently changes gate outcomes with no probe, and a pressure signal blind to the resource that actually kills this box.1. The latency histogram has no route label, so its SLO alert is unactionable
LoopoverRequestLatencySLOBreachfired ~17:23–20:15 UTC reporting p95 = 9.75 s against a 1 s SLO. Butsum by (le, route) (rate(loopover_http_request_duration_seconds_bucket[5m]))returns a single unlabelled series — there is noroutelabel at all — so the alert names no route, and at ~0.6 req/min a 5-minute p95 is computed from roughly three requests.The reason it has no label is the constraint the fix must respect: a raw path (or any per-PR/per-repo id inside one) is unbounded cardinality, which takes a Prometheus down rather than merely misinforming it.
otherbucket.(The companion min-request-rate gate on the alert expression is alertmanager config on the host, tracked in the parent — not repo code.)
2. No readiness probe for
BROWSER_WS_ENDPOINTsrc/selfhost/health.tsprobes Redis, Qdrant, the GitHub App and codex — every dependency the review path needs except the browser. A browserless outage is therefore invisible in/readywhile it silently changes gate outcomes: the screenshot-table gate reads absent visual evidence as a close signal (#9464 stopped that from closing PRs; this makes the outage itself visible rather than inferred afterwards)./readypolling cannot itself load a struggling backend.?token=into a probe URL that could be logged.3. Host pressure watches CPU only
src/selfhost/host-pressure.ts:12-22usesloadavg / cpus().length, consumed atsrc/selfhost/maintenance-admission.ts:269-282. On this GPU box the realistic killer is memory — Ollama at 9.86 GiB and browserless at 1.45 GiB — and nothing sheds or even observes available memory; the OOM killer decides, which takes the whole container and every in-flight job rather than deferring one maintenance job.nullmeans "skip this check" contract as the load signal — an unavailable reading must never itself defer work, and must never be clamped into a misleading 0.host_load_highreason stable — operators alert and dashboard on those strings.Tests
other; a fuzz sweep produces no value outside the fixed set; a per-PR path leaks no id.wss:→https:, fails closed on an unparseable endpoint, caches within its TTL, single-flights concurrent checks, and re-probes once the TTL lapses.>=), skips onnull, gates the drain escape, and leaves CPU outranking it.Expected outcome
A fired latency alert names a route; a browserless outage is visible where every other dependency's is; and memory pressure defers a maintenance job instead of letting the OOM killer take the container.