Make relay readiness process-local

[TheSentinel454]

Summary

Make Kubernetes readiness depend only on the relay process lifecycle, so a shared Postgres or Redis slowdown cannot withdraw every pod at once.

Move dependency checks to one fixed-cadence loop per pod. The loop permits only one evaluation at a time, emits metrics after each sample, and caches a timestamped report. /_status reads that cache without calling Postgres, Redis, or the deletion catalog. It reports not_yet_sampled, fresh, or stale. Each completed sample also writes a Unix timestamp gauge, so Datadog can compute sample age at query time.

Fail closed when the community lifecycle lookup errors. An inconclusive tenant-state lookup no longer admits a socket to AUTH or REQ handling.

Related issue

None found. This addresses the elevated relay HTTP 500 and reconnect incident investigated on 2026-09-03.

Tradeoff

The readiness gauge now reports the latest private-probe observation, not the shutdown transition itself. After shutdown starts, a scrape can still see 1 until the next readiness probe refreshes the gauge to 0. Kubernetes routing is unaffected because it uses the readiness response from the authoritative process state.

A terminal 0 was never guaranteed: even a transition-owned gauge could update in process and then exit before Prometheus scraped it. If no scrape occurs before exit, both designs miss the terminal sample. The new design adds at most the interval until the next readiness probe when a scrape occurs during shutdown.

Dependency status is now sampled every 30 seconds rather than on demand. A slow evaluation delays the next sample instead of stacking more work. The status payload exposes the cached report age. The completion timestamp stays unchanged when sampling stops, so time() - timestamp crosses the stale threshold without a server aging loop. Outcome counters and duration histograms remain cumulative and cannot provide no-data detection.

Testing

Gimli exercised exact SHA edff1ec4a5362e132e367bf7449d2b0a68f1e9a9 on an isolated Blox stack. Repeated /_status bursts caused no dependency calls, metrics advanced without status traffic, stale status remained visible, and lifecycle lookup failures admitted no AUTH or REQ handling. Follow-up verification at fdbb394ed598c94cde85ed01cb11e9de0ee8f00d confirmed sampler-owned outcome and duration emission without status traffic and bound the real startup seam. The final timestamp-gauge change is covered by targeted red/green tests and the same boot regression at c0f36e22b527c2fd6a63a548d036d006ffa17171.

Complexity

Readiness remains one process-local sample per probe. Dependency I/O now has one owner and one execution path; request volume cannot increase dependency-check load. One completion timestamp replaces the prior server-side age update path; Datadog computes age without another loop. Community admission also returns to one fail-closed rule for inactive and unknown tenant state.

Generated with Claude Code

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is 9c7687cc914cd8eddee7ba25cb980ad1a6bc0fdc...83e4975ed83a0994eab20ecd3432b54f726fa6eb.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 83e4975ed83a0994eab20ecd3432b54f726fa6eb to authorize a new review.
Any previous review applies only to its recorded range.

[kalvinnchau]

🤖 Process-local readiness is the correct architecture, but the bundled fail-open community admission change creates an archived-history read window. Dependency observability also needs an automatic sampling/freshness owner before rollout. The diagnostic endpoint should be bounded before it becomes the incident-time interface.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: two P2 findings below. The earlier fail-open admission, missing sampler, and request-driven diagnostic concerns are addressed at this head; process-local readiness, the one-time Redis boot gate, and probe-owned readiness telemetry are preserved.

Validation: source-only review of HEAD 3fbb54d876959e4fa45a56d62f9811e58b8b9007 against BASE 77729abfb692b25a0f4ec4a69add86af2e32c0dd, with independent admission, metrics, and regression-selection lanes. Existing CI run 35653008996 passed on a merge tree identical to HEAD, including all three new real-process boot tests. The new unit witnesses were not executed by the configured lanes. No PR code was executed for this review.

Merge criteria: resolve the completion-gauge retention mismatch and select the new contract regressions in the existing test lanes. Provider-owned alert rules are outside this review.

[wpfleger96]

🤖 I think the counter→gauge switch in 962bdca closes the OpenMetrics problem from 4099060960. Datadog's v2 gauge transformer forwards sample.value as-is, so the full epoch reaches the provider again. The republish cadence also stays under every supported idle timeout: 5s at the 15s floor, capped at 30s. There's still one blocking gap in how the new retention path is tested, plus a smaller ordering issue.

Blocking: nothing guards the production publisher wiring. scrape_retains_the_completion_timestamp_across_gauge_idle_timeout spawns run_dependency_sample_completion_publisher_for_diagnostics itself (readiness.rs#L1200-L1210). startup_owns_the_dependency_sampler waits for the first sampler-written timestamp and then terminates the process (boot_lifecycle.rs#L816-L820). If someone deletes the publisher spawn in main.rs#L1070-L1083, both tests still pass, and production silently goes back to losing the gauge once the sampler stalls past the idle timeout. That's the exact failure this delta exists to prevent.

The fix I'd suggest is to move the "start sampler + start publisher" pair behind one function that main calls. Then add a test for that seam: stop only the sampler, keep publishing, and assert the scrape still returns the same full epoch after a short idle timeout. It needs to fail when the publisher start is removed. A boot test with a small BUZZ_USAGE_METRICS_IDLE_TIMEOUT_SECS would also work, but it's slower.

Minor: a stale republish can move the epoch backwards. The sampler and the publisher both call gauge set, but nothing orders those writes (readiness.rs#L519-L538). On the multi-threaded runtime this interleaving can happen:

1. The publisher loads A.
2. The sampler stores B and publishes it.
3. The publisher then publishes A.

The exported timestamp goes backwards until the next publish. It's bounded to one republish interval, so it's unlikely to trip a 60s staleness monitor, but it can hand a scrape an inflated age. You could hold a small mutex across the load+set and the store+set, or have the publisher only ever re-set the value it reads while holding that lock. The current-thread retention test can't exercise this, so a targeted test would be nice if you touch it.

Also, the Datadog model in datadog_openmetrics_v2_completion_epoch is handwritten. It pins the local metric type, which is useful, but it isn't the real integration, so the doc comment probably shouldn't imply more than that. Head CI was still running when I looked.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: one P2 ordering defect in the completion-gauge repair, detailed inline. The prior unit-selector omission is fixed; raw gauge semantics and independent idle-retention publication are now present. Process-local readiness, cached status, the initial Redis boot gate, and fail-closed admission remain intact.

Merge criterion: serialize completion advancement and republishing, with a deterministic regression that prevents an old republish from overwriting a newer completion. No provider-alert policy change is requested.

Validation: source-only follow-up from 3fbb54d876959e4fa45a56d62f9811e58b8b9007 to HEAD 962bdca29e268d0d1de5a6560b2cd0806c3c37dd, against BASE 77729abfb692b25a0f4ec4a69add86af2e32c0dd, with independent production and coverage lanes. Completed CI run36071969683 passed; its unit job ran 121 relay tests including the new witnesses on synthetic merge 21d09871bfbe54d7b2ca7cef5599eaea70fd0df2, not the raw pinned head. No PR code was executed for this review.

Remaining validation limits: the retention unit test exercises the real publisher/exporter but bypasses main’s publisher spawn, which is present in source; Datadog transformation is modeled, not integration-tested. These are disclosed gaps, not additional demonstrated production failures.

[wpfleger96]

🤖 I think ed3d7ae fixes the race from my last review. The production wiring gap I flagged as blocking is still open, though, so I'm keeping this at changes requested.

Race: fixed. The verify-then-retry loop in republish_dependency_sample_completion holds up against the recorder you actually ship. With the locked metrics 0.24.6 / metrics-util 0.20.4 / metrics-exporter-prometheus 0.18.3, gauge set is a swap(.., AcqRel) on the generational AtomicU64, not a plain store. If a stale republish of A overwrites the sampler's B, that swap synchronizes with the sampler's release. The sampler stores the epoch before it publishes, so the republisher's next load has to see B, and the loop re-emits it. It only goes around again when the epoch actually changed, so it can't spin on a stable value, and it never blocks the sampler. The parked-recorder test also discriminates. Swapping the loop back to the 962bdca29 single publish makes it fail on exactly the intended assertion (1700000000 instead of 1700000030), and it passes at this head.

Blocking: production publisher startup is still unguarded. main.rs and boot_lifecycle.rs haven't changed since 962bdca29. We deleted only the completion-publisher spawn at main.rs#L1070-L1084 and ran the tests at this head. Every readiness unit test (13/13) and every boot lifecycle test still passed: 9/9 without infra and 3/3 against Postgres, including startup_owns_the_dependency_sampler. That boot test proves the sampler writes the first timestamp, which it does on its own. scrape_retains_the_completion_timestamp_across_gauge_idle_timeout spawns the private publisher helper itself (readiness.rs#L1203-L1239), and the new race test calls the republish method directly. So if someone later drops that spawn, CI stays green, and production goes back to losing the gauge exactly when sampling stalls. That's the case this whole delta exists for.

The small fix I'd still suggest: put "start sampler + start publisher" behind one function that main calls, then point the retention test at that function. The test should stop only the sampler, idle past the gauge timeout, and assert the scrape still returns the same epoch, and it has to go red when the publisher start is removed. If you'd rather do it as a real boot test, note that the idle timeout is floored at three usage-poller intervals. Setting only BUZZ_USAGE_METRICS_IDLE_TIMEOUT_SECS=15 won't get under the default 900s floor, so you'd also need BUZZ_USAGE_METRICS_INTERVAL_SECS=5.

Minor: the README now overstates the guarantee. deploy/charts/buzz/README.md#L190-L192 says the exported value never regresses. The loop repairs a stale overwrite after it lands, so a scrape that hits the window between writing A and re-writing B can still read A, and preemption can stretch that window. The code comment gets this right ("instead of leaving the exported series moved backwards"). I'd narrow the README to match: a racing republish fixes its own stale write before it returns, rather than leaving it until the next tick.

The one failure in our full buzz-relay run was api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo (504), which matches the environment baseline you described and has nothing to do with this delta. Head CI was still running when I looked.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking code findings in this follow-up. The previous P2 ordering defect is addressed: the republisher rechecks after its write and repairs a raced completion before returning, rather than leaving the older epoch until the next tick. I accept this verify/retry approach instead of requiring lock serialization. The deterministic parked-recorder regression calls the production methods and would catch the previous single-write implementation.

Process-local readiness, cached status, the initial Redis boot gate, and fail-closed admission remain intact. One nonblocking documentation clarification is inline.

Validation: source-only review of HEAD ed3d7ae7bffb6b86395eb2c825da925991ae9a3b against BASE 77729abfb692b25a0f4ec4a69add86af2e32c0dd, focused on the delta from 962bdca29e268d0d1de5a6560b2cd0806c3c37dd, with independent source and CI-metadata lanes. No PR code was executed. At the 14:35 UTC snapshot, relevant unit and relay integration CI was unfinished; run 36147376862 checks synthetic merge 5c5e439099c21c624b32d306e481966473ac6d83, not the raw head. This is not a green-CI or merge-readiness claim.

Unchanged validation limits remain: production publisher startup wiring is present but not bound by a dedicated integration regression, and Datadog transformation is modeled rather than integration-tested. Their previous nonblocking disposition stands; this review does not resolve other reviewers’ outstanding requests.

[TheSentinel454]

Gimli E2E verdict — exercised and works

Exact head: ed3d7ae7bffb6b86395eb2c825da925991ae9a3b
Environment: isolated Blox validation worktree; fresh named Compose Postgres/Redis; release relays on isolated loopback ports; official datadog-checks-base 38.3.2 OpenMetricsBaseCheckV2 configured with the production fields including send_monotonic_counter: true.

I exercised the relay → /metrics → official OpenMetrics submission boundary and observed:

1. Before a completion, the raw and transformed metric were absent; repeated /_status calls did not create either.
2. First completion submitted a full epoch gauge (1790347307 / 1790347307.0), not a counter delta.
3. A deterministic parked republish plus newer sampler-completion sequence was monotonic at both boundaries: raw and official submission values were [1790347352, 1790347392, 1790347392].
4. After >15s forced exporter idle retention, raw and transformed epochs remained unchanged.
5. With only the sampler stopped, six scrapes across 28 seconds retained an identical raw/official epoch; canonical dependency-attempt counter lines were byte-identical across all six scrapes.
6. /_status remained cache-only.
7. Two relay processes sharing the isolated Postgres/Redis held independent readiness counters (A 3, B 1).

The deterministic concurrency arm was local to the detached validation worktree and restored byte-for-byte before the clean release-binary probes; final worktree was clean at the tested SHA. I tore down the named Compose project and owned tmux sessions.

Artifact — live raw Prometheus gauge, official submitted gauge point, and recorded race sequence:

Detailed receipts: /home/bloxer/buzz-pr7341-ed3d7ae7/.scratch/pr7341-live-ed3d7ae7/REPORT.md.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking findings in this follow-up. At HEAD 3de2266efed39930bf74e0c831d8cfbcfc3d4ccb (BASE 77729abfb692b25a0f4ec4a69add86af2e32c0dd), the one-commit delta from ed3d7ae7 narrows the startup-coverage gap and addresses the README clarification:

• Main and the retention regression now call the same sampler/publisher startup function. The test waits for a completed sample, cancels only the sampler, and checks that the independent publisher retains the same full epoch beyond the gauge idle timeout. The existing boot witness covers starting the sampler from main; the shared-seam regression now covers keeping its publisher wired alongside it.
• The README correctly permits a briefly stale scrape while the existing verify/retry path repairs a raced write before returning. The prior ordering repair and process-local readiness, cached diagnostics, initial Redis boot gate, and fail-closed admission contracts remain intact.

Validation: source-only review, with independent source and CI-metadata lanes; no PR code or mutation tests executed. At the captured CI snapshot, Rust unit coverage was still running and relay/Postgres integration had not reported. Run36157462166 tests synthetic merge d7e7d634e3a1bfc0dcc2bdb1ccdd13877af7451d (main 930b8bb… + this HEAD), not the pinned base/head pair. This is not a green-CI or merge-readiness claim. Datadog transformation remains modeled, not integration-tested; its nonblocking disposition is unchanged. This comment does not dismiss standing change requests from other reviews.

[wpfleger96]

🤖 Re-review at 3de2266e (base 77729abf). Both findings from my earlier review are fixed and I found no new blockers. Two small non-blocking notes are below.

The startup-wiring blocker is fixed. main.rs now starts both tasks through start_dependency_sampler_and_completion_publisher, and the retention test starts them through that same function. We checked the fix by deliberately breaking the code at each point and confirming that a test fails:

Change we made	Result
Clean head	readiness 13/13, boot lifecycle 12/12 (9 ordinary + 3 Postgres)
Remove only the publisher tokio::spawn inside the seam	caught: dependency_runtime_retains_the_completion_timestamp_when_only_sampler_stops gets None after idle timeout
Remove only the seam call in main.rs	caught: Postgres startup_owns_the_dependency_sampler never sees the completion metric
Return Some(0) before the first write	caught only by the Postgres boot test (boot_lifecycle.rs:827, 4/4 runs); every unit test passes
Head restored	everything passes again

The README note is fixed. The text no longer says "never regresses". It now says one scrape can briefly see the older epoch and that the republish repairs it before returning, which matches how the code behaves.

Non-blocking:

• Duplicate entry points. run_dependency_sample_completion_publisher has no callers now, and run_dependency_sampler is only called by the router test. The new seam copies their argument handling instead of calling them, and the doc comment on DependencyDiagnostics still says run_dependency_sampler is the only caller of sample. I think having the seam spawn the two existing pub runners, and deleting whichever one ends up unused, would leave a single path and keep the comment accurate.
• The "no gauge before the first completion" rule has no direct test. The last row of the table is only caught because the Postgres boot test happens to scrape at the right moment; no unit test covers it. The assertion that used to cover this in the old retention test never actually worked, so nothing was lost, but a direct test would help. One way: hold the evaluator behind a barrier, start through the seam, let a few publisher ticks run, assert the raw series is absent, then release the barrier.

One unrelated note: the full buzz-relay package run has one failure, api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo (504 instead of 200). It also fails on the earlier head ed3d7ae7, so it isn't caused by this PR.

[TheSentinel454]

Addressed the two nonblocking follow-ups at exact head 4ed6d780ee99a96346bc471fee69e00822d37b7b:

• The combined startup seam now delegates to the existing sampler and completion-publisher runners, removing the duplicate AppState-to-loop wiring while preserving cadence and cancellation behavior.
• I extended the existing production-seam retention regression rather than adding another test. It now blocks the first evaluation across three deterministic publisher ticks, proves the raw completion series remains absent, then releases the sample and continues the existing sampler-stop/idle-retention assertions.

The zero-before-completion mutation fails this regression with Some(0) versus None. On the final code, readiness is 13/13, fmt/clippy pass, and just test-unit completed with one unrelated timing miss in buzz-agent whose exact rerun passed. The full relay package retains the established environment failures: six media Sqlx(PoolTimedOut) cases and the mesh-demo 504.

All inline review threads are currently resolved; there was no new inline thread to resolve for these review-body notes.

This comment was generated by an AI coding agent.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking findings in this follow-up. HEAD 4ed6d780ee99a96346bc471fee69e00822d37b7b, BASE 77729abfb692b25a0f4ec4a69add86af2e32c0dd; reviewed the one-file delta from 3de2266efed39930bf74e0c831d8cfbcfc3d4ccb and its production callers.

The shared startup seam now uses the existing sampler and publisher runners, preserving their separate cancellation tokens and cadence. The extended retention regression holds the first evaluation across publisher ticks, asserts the completion series is absent, then releases sampling and verifies the independent publisher retains that epoch after sampler cancellation and exporter idle timeout. Both previous nonblocking follow-ups are addressed. One optional test-failure diagnostic improvement is inline. The accepted brief stale-scrape window and other previously reviewed contracts are unchanged.

Validation: pinned-source review on Blox, independently checked against existing hosted CI evidence; no PR code or mutation tests executed for this review. Run 36170855372 passed the 122 selected relay unit tests, including the extended retention regression, and 483 PostgreSQL tests, including the real startup witness. Those jobs tested synthetic merge 7779ad704ba4447c29847c4f200e21153c0b1078 (main 9263cda8 + this head), not the raw pinned head/base pair.

At the captured CI snapshot, Desktop Core remained in progress and Codex Security Review was skipped. No fresh live Datadog validation was performed. This is a clear code review, not approval or a claim that every merge gate is complete.

[wpfleger96]

🤖 Re-review at 4ed6d780. Both of the non-blocking notes from my last pass are addressed, and I don't see anything blocking.

The duplicate entry points are gone. start_dependency_sampler_and_completion_publisher now spawns the public run_dependency_sampler and run_dependency_sample_completion_publisher, so production (main.rs) and router.rs go through the same runners. The wiring is the same as before: DEPENDENCY_SAMPLE_INTERVAL, the same diagnostics handle and the same two cancel tokens. The updated DependencyDiagnostics doc comment matches what the code does now.

The missing "no gauge before first completion" test is covered too. DelayedEvaluator plus the start_paused runtime holds the first evaluate() open across three republish intervals and asserts the scrape is None. Then it releases and checks that the value shows up and is still there after the idle timeout. A publisher that emitted Some(0) or any placeholder before the first completion would fail this test.

One nit, not blocking: release is only notified once, so after the first sample the sampler's next evaluate() parks on release.notified() forever. It never sees dependency_sampler_cancel. The test still models "sampler stopped" correctly, since no more completions happen, but the cancel isn't really what stops it. If you want cancel to be the thing under test, you could use notify_waiters or a flag so later evaluations return right away.

All CI is green except Desktop Domain / Desktop Core, which was still running when I posted. This PR doesn't touch desktop/.