feat(relay): add detailed readiness metrics - #7149
Merged
Merged
Conversation
🔐 Codex Security Review
|
ravarora2
marked this pull request as ready for review
September 1, 2026 00:19
Preserve per-dependency readiness outcomes and durations so operators can distinguish pool waits, query failures, and catalog validation failures. Signed-off-by: Ravneet Arora <rarora@squareup.com>
Co-authored-by: Ravneet Arora <rarora@squareup.com> Signed-off-by: Ravneet Arora <rarora@squareup.com>
ravarora2
force-pushed
the
codex/readiness-metrics
branch
from
September 1, 2026 21:33
a91d3ff to
86a7a19
Compare
wpfleger96
approved these changes
Sep 1, 2026
wpfleger96
left a comment
Member
There was a problem hiding this comment.
🤖 Combined review verdict from the coordinated three-lane review (two independent source reviews + live E2E verification), at head 86a7a194a.
Approved. No CRITICAL or IMPORTANT findings in any lane.
- Both source lanes independently verified the generation fence, shutdown dominance, the public/private telemetry boundary, the fail-closed shared deadline, the typed bounded label sets, and the 99-series ceiling proof through the production router and recorder. The
AppState::begin_shutdown()consolidation leaves exactly one writer of theshutting_downatomic, keeping the readiness fence, WebSocket-upgrade refusal, and mesh drain consistent. - The three ignored Postgres readiness tests were confirmed discovered and passing in the isolated PostgreSQL CI lane at this head (384/384), covering pool exhaustion/recovery, closed-pool, query timeout/error classification, and cancellation balance.
- Live E2E against the built relay binary with dedicated Postgres/Redis confirmed the ready/not-ready/shutdown response contracts, zero public-listener telemetry (including under forged request-controlled values), correct
reasonlabeling across dependency failures, SIGTERM dominance over an in-flight probe without gauge resurrection, and the documented histogram buckets.
Two non-blocking notes, no action required for merge:
PublicationState::shutdown_generationstores au64that is only ever checked withis_some(); the generation value computed inbegin_shutdown()is never compared. A plainboolwould say what it means.- The public
/_readinesscompatibility route still runs the full dependency evaluation for any unauthenticated caller — unchanged from main, noted only as pre-existing.
johnmatthewtennant
added a commit
that referenced
this pull request
Sep 2, 2026
…e-read-model * origin/main: fix(desktop): retain automatic mentions only in threads (#7144) feat: add databricks fable 5.1 model capabilities (#7213) docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) feat(relay): add detailed readiness metrics (#7149) feat(desktop): add Pi agent preset (#7208) Signed-off-by: John Tennant <jtennant@squareup.com>
johnmatthewtennant
added a commit
that referenced
this pull request
Sep 2, 2026
…l' into jtennant/project-state-mutations * origin/jtennant/project-state-read-model: Fix historical migration catalog tests fix(desktop): retain automatic mentions only in threads (#7144) feat: add databricks fable 5.1 model capabilities (#7213) docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) feat(relay): add detailed readiness metrics (#7149) feat(desktop): add Pi agent preset (#7208) Signed-off-by: John Tennant <jtennant@squareup.com>
johnmatthewtennant
added a commit
that referenced
this pull request
Sep 2, 2026
…' into jtennant/project-related-channels-desktop * origin/jtennant/project-state-mutations: Fix Project channel preservation test Fix historical migration catalog tests fix(desktop): retain automatic mentions only in threads (#7144) feat: add databricks fable 5.1 model capabilities (#7213) docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) feat(relay): add detailed readiness metrics (#7149) feat(desktop): add Pi agent preset (#7208) Signed-off-by: John Tennant <jtennant@squareup.com>
wpfleger96
pushed a commit
that referenced
this pull request
Sep 2, 2026
…-history * origin/main: fix(acp): replace real user name in base prompt mention example (#7250) ci: split CI into reusable workflows (#7168) fix(desktop): retain automatic mentions only in threads (#7144) feat: add databricks fable 5.1 model capabilities (#7213) docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) feat(relay): add detailed readiness metrics (#7149) feat(desktop): add Pi agent preset (#7208) feat(buzz-agent): add DatabricksAuthCoordinator single-flight OAuth (#5545) fix(desktop): preserve keyring identity during recovery (#7203) feat(mobile): prepare `buzz-push-gateway` for deployment (#7158) ci: relax file-size ceilings by surface (#6485) fix(mobile): isolate extension linker flags; complete iOS build in CI (#7187) chore(ci): lower Codex security review effort (#7179) fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185) fix(dev): keep the canonical profile when launching from desktop/ (#7143) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96
pushed a commit
that referenced
this pull request
Sep 2, 2026
* origin/main: feat(agents): harness-agnostic effort write path and spawn bridge (#4625) chore(db): drop Phase-A NIP-FI relay-side authority ledger (#7221) fix(acp): replace real user name in base prompt mention example (#7250) ci: split CI into reusable workflows (#7168) fix(desktop): retain automatic mentions only in threads (#7144) feat: add databricks fable 5.1 model capabilities (#7213) docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) feat(relay): add detailed readiness metrics (#7149) feat(desktop): add Pi agent preset (#7208) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> # Conflicts: # desktop/src-tauri/src/commands/agent_models_update.rs # desktop/src-tauri/src/commands/agents_deploy.rs # desktop/src-tauri/src/managed_agents/types/requests.rs # desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx # desktop/src/shared/api/types.ts
ravarora2
added a commit
that referenced
this pull request
Sep 2, 2026
## Why
Buzz already reports coarse writer/reader database checkout waits, but
those
signals cannot explain which startup or serving operation is blocked by
pool
pressure. That makes rollout diagnosis and postmortems ambiguous: a
readiness
probe, NIP-42 authentication, authorization check, reconnect history
repair,
event write, and background maintenance can all wait on the same pool
while
appearing identical.
This PR implements Package 2A of the pod-handoff plan: operation-aware
pool
borrow causality. It is observability-only; it does not change
configured pool
sizes, SQL semantics, transaction ordering, or timeout policy. Physical
DNS/TCP/TLS/Postgres authentication and session initialization remain
the
separate Package 2B boundary.
## Metric contract
The final contract separates three questions:
| Question | Metric |
|---|---|
| How long did checkout wait? |
`buzz_db_pool_acquire_duration_seconds{pool_role,operation}` |
| How did the attempt end? |
`buzz_db_pool_acquire_attempts_total{pool_role,operation,outcome}` |
| Who is waiting now for a tracked operation? |
`buzz_db_pool_waiters{pool_role,operation}` |
Outcomes are `success`, `timeout`, `error`, and `cancelled`. Operations
are
`bootstrap`, `readiness`, `tenant_resolution`, `authentication`,
`authorization`, `subscription_history`, `event_write`, and
`maintenance`.
Only these eleven pool/operation pairs are constructible:
```text
writer/bootstrap reader/bootstrap
writer/readiness
writer/tenant_resolution
writer/authentication
writer/authorization reader/authorization
writer/subscription_history reader/subscription_history
writer/event_write
writer/maintenance
```
The duration histogram intentionally does not carry `outcome`. Result
data
remains available on the terminal counter for historical counts and
rates,
without multiplying the expensive histogram family. Nine finite buckets
plus
`+Inf`, sum, and count produce 12 duration series per valid pair.
Together with
four outcome counters and one waiter gauge, the new-family ceiling is
exactly
187 raw Prometheus series per pod, asserted from the production
exporter.
The existing coarse acquisition families remain temporarily for
dashboard
compatibility while the new series are validated in staging.
The operation-specific waiter family covers the explicitly routed,
deployment-critical operations above; it is not a census of every
possible
SQLx checkout in the process. The dashboard pairs it with SQLx pool
active/idle/max gauges for whole-pool capacity context, and treats a
missing
operation series as unknown rather than healthy zero.
## What changed
### Cancellation-safe acquisition ownership
- Add writer- and reader-specific typed operation APIs so invalid label
pairs
cannot be constructed and store modules cannot emit reader labels.
- Own every polled acquisition with one RAII terminal guard.
- Record exactly one duration and terminal outcome for success, timeout,
error,
or cancellation.
- Emit nothing for a future that is created but never polled.
- Balance the operation-specific waiter count exactly once on every
terminal or
dropped future.
- Periodically refresh every expected waiter pair, including healthy
zero, so
missing telemetry is not presented as zero. Reader pairs are emitted
only
when a distinct read pool is configured; a writer-only pod cannot
fabricate
healthy reader-zero state.
### Production attribution
Route the deployment-critical acquisition paths through caller-owned
semantic
entry points, including:
- writer and reader bootstrap;
- the real post-#7149 readiness acquisition and deletion-catalog
validation;
- tenant resolution and community lifecycle checks;
- NIP-42 allowlist authentication;
- membership, moderation, invite, operator, Git, agent-owner, and policy
authorization;
- operator community create/list/archive/unarchive, reverse host/channel
tenant
resolution, and REQ row-community conformance lookups;
- writer/reader subscription history, feed, thread, and routed fallback
paths;
- primary and command event writes, replaceable events, mention
indexing,
reaction/channel/member/archive side effects, and thread metadata;
- push matching, usage rollups/leadership, replica-fence startup and
recurring
probes, periodic reconciliation, channel/deletion reapers, partitions,
and
other bounded maintenance/bootstrap paths.
Shared helpers now accept caller-owned intent or expose named semantic
variants
instead of assigning one misleading operation to every caller. No known
P0 path
uses `other`.
### Readiness and size-one-pool correctness
- Rebase on the post-#7149 readiness implementation and instrument the
actual
`Db::readiness_check` acquisition rather than the superseded ping-only
seam.
- Acquire once for deletion-catalog validation and run its queries on
that
connection, preserving the shared readiness deadline.
- Scope the channel-roster catalog checkout before the behavior probe so
a
size-one writer pool cannot self-deadlock during startup verification.
### Exporter, documentation, and CI
- Register metric HELP/type/unit metadata through the production
Prometheus
builder.
- Configure dedicated checkout buckets at 1ms, 5ms, 10ms, 25ms, 50ms,
150ms,
500ms, 1s, and 3s.
- Add a production scrape-contract test for exact names, labels,
buckets,
valid pairs, sensitive-label exclusion, and the 187-series ceiling.
- Add source mutation guards for the P0 semantic entry points and
raw-checkout
bypasses.
- Add an exact backend-integration CI selector for the production
attribution,
cancellation, readiness, and size-one-pool PostgreSQL tests.
- Document the frozen label vocabulary, valid combinations, semantics,
and
cardinality budget in the Helm chart README.
## Dashboard intent
The new Stage 2 row in **Buzz Startup & Rollout Safety** is
deployment-first:
- baseline-versus-candidate attempts, failure rates, cancellation rates,
and
maximum wait by operation;
- outcome counts and percentages over time by SHA/ReplicaSet;
- acquisition wait heatmap, average, and maximum through the rollout;
- historical waiter pressure beside writer active/idle/max context;
- per-pod postmortem drilldown, including terminated pods;
- a smaller current-waiter table with explicit stale/missing semantics.
Percentile widgets remain disabled until Datadog metadata confirms
percentile
support for the new distribution. Current gauges use no fill,
interpolation, or
`default_zero`; missing means unknown.
## Risk assessment
Moderate. The patch touches many database acquisition call sites, but
preserves
the selected physical pool and executes the same SQL on the acquired
connection. The main risks are incorrect semantic attribution,
cancellation
double-counting, and a helper accidentally acquiring twice. Typed APIs,
production-method PostgreSQL tests, source guards, the raw scrape
contract, and
the size-one-pool regression cover those risks.
No tenant, community, user, pubkey, event, channel, SQL, URL, pod,
version,
ReplicaSet, or request-controlled value is emitted as an application
metric
label. Deployment identity is supplied by infrastructure enrichment.
## Verification
- `cargo fmt --all -- --check` — passed.
- `cargo clippy -p buzz-db -p buzz-relay --all-targets --all-features --
-D warnings`
— passed.
- `cargo test -p buzz-db` — 122 passed, 0 failed, 263 ignored;
source-contract integration test: 3 passed, 0 failed.
- Focused relay compatibility, metric-contract, and readiness tests —
passed.
- `scripts/test-postgres-test-discovery.sh` — passed.
- Full `buzz-relay` package run from the identical tree reached 1,015
passes;
the six media-test failures all stopped in their shared local PostgreSQL
setup with `Sqlx(PoolTimedOut)` because Docker/PostgreSQL was
unavailable.
The same six failed in isolation, while every changed exact test passed.
- Exact implementation head:
`f92910b353086e9edf85918ca5f72190edbbe22f`.
- Exact multi-architecture staging image:
`dev-sha-f92910b353086e9edf85918ca5f72190edbbe22f-run-33607968668-1`
(`sha256:161712c8ed2e265a15df9b63e02248d5973481f875ff129d7d2ae78a09d487a2`).
- Focused staging GitOps PR:
<squareup/builderbot-platform-core-infrastructure#299>
— merged after renderer, inventory, infrastructure test, Kargo, Semgrep,
and
Intersect gates passed; the source/generated-artifact diff was exactly
two
image lines.
- Exact GitHub head reports 47 terminal checks: 35 successful and 12
intentionally skipped. PostgreSQL, unit, lint, security, both server
cross-compiles, backend integration, relay E2E, desktop, mobile, image,
Helm, Semgrep, zizmor, and DCO gates are green.
- Datadog readback identifies two exact-image pods,
`buzz-d79c8d8f7-ckv2l` and `buzz-d79c8d8f7-qzqdp`, in ReplicaSet
`buzz-d79c8d8f7`; both report the full source SHA above.
- Both pods report all eleven allowed pool/operation waiter pairs at
current
zero, with no invalid pair. The acceptance window observed nonzero
success
receipts for readiness, tenant resolution, authorization, subscription
history, event write, and maintenance, and no timeout, error, or
cancelled
outcome. Maximum observed wait was about 101 ms for maintenance and 50
ms
for reader subscription history.
The main **Buzz Startup & Rollout Safety** dashboard now has a live
Stage 2
database row with eight widgets and nineteen fully scoped queries. Final
readback preserved all seven top-level groups, found zero under-scoped
Row 6
queries, and confirmed the tracked-operation waiter boundary in the
panel
descriptions.
Generated with Codex.
---------
Signed-off-by: Ravneet Arora <rarora@squareup.com>
wpfleger96
pushed a commit
that referenced
this pull request
Sep 2, 2026
…agent-edit * origin/main: feat(desktop): add persistent Bestie experience (#7223) fix(desktop): harden profile batch and thread-reply fetches against relay slowness (#7188) docs(nip-fi): adopt deny-until-TTL and extend enforcement to HTTP ingress (#7254) fix(composer): align wrapped inline chip fragments (#7242) Add operation-aware database pool acquisition metrics (#7195) fix(desktop): keep explicit agent profiles bound to their exact key (#7131) fix(desktop): discover authenticated owned relay agents (#7122) feat(agents): harness-agnostic effort write path and spawn bridge (#4625) chore(db): drop Phase-A NIP-FI relay-side authority ledger (#7221) fix(acp): replace real user name in base prompt mention example (#7250) ci: split CI into reusable workflows (#7168) fix(desktop): retain automatic mentions only in threads (#7144) feat: add databricks fable 5.1 model capabilities (#7213) docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) feat(relay): add detailed readiness metrics (#7149) feat(desktop): add Pi agent preset (#7208) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> # Conflicts: # desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx # desktop/src/features/agents/ui/agentInstanceEditPinning.test.mjs
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.
Summary
This PR makes relay readiness failures diagnosable without weakening the existing fail-closed readiness contract. It distinguishes Postgres pool acquisition from query execution, Redis pool acquisition, deletion-catalog validation, and the overall two-second deadline; exports a bounded Prometheus contract for rollout dashboards; and fixes the concurrency, shutdown, and listener-boundary semantics needed for those signals to be trustworthy.
Why
The previous
/_readinessimplementation exposed only an aggregate ready/not-ready result. During a rollout, operators could not tell whether a pod was blocked on:Adding metrics to the existing handler also exposed three correctness hazards that this PR resolves:
200 ready, and resurrect ready gauges while the process is draining.Behavior
Readiness evaluation
multiple_dependencies_failed; exhaustion of the shared deadline is reported asoverall_timeoutwhen no more specific completed outcome wins.200 {"status":"ready"}.Ordered publication and shutdown
ReadinessCoordinatoris process-owned and uses one mutex as the linearization point for probe generations, current-state publication, and terminal shutdown.begin_shutdown()and probe commit serialize through the same coordinator.Shutdown without dependency evaluation records only:
buzz_readiness_checks_total{reason="shutting_down"}; andbuzz_readiness_state{check="overall"} = 0.It does not fabricate dependency failures, dependency state changes, or zero-duration latency samples. If shutdown wins after an in-flight evaluation actually ran, those completed dependency attempts may remain as attempt telemetry, but they cannot overwrite shutdown-dominant current state.
Listener and response contract
/_readinessroute is the sole authority for rollout readiness telemetry./_readinessfor compatibility, evaluates the same dependencies, and preserves the existing response shape, but it emits nobuzz_readiness_*metrics.200 {"status":"ready"}.503 {"status":"shutting_down"}.reasonpluspostgres,redis, anddeletion_catalogbooleans.Prometheus contract
The final schema is intentionally capped at 99 raw Prometheus series per pod.
buzz_readiness_checks_totalreasonbuzz_readiness_dependency_checks_totaldependency, typedoutcomebuzz_readiness_check_duration_secondscheckbuzz_readiness_statecheckClosed label sets
reason:Valid
dependency/outcomepairs are enforced by typed enums:postgres:success | pool_timeout | pool_error | operation_timeout | operation_errorredis:success | pool_timeout | pool_errordeletion_catalog:success | operation_timeout | operation_errorcheckisoverall | postgres | redis | deletion_catalog.The readiness histogram has 15 configured buckets concentrated around the two-second deadline:
The contract deliberately removes the redundant overall
resultlabel and the histogramoutcomelabel. Pod, ReplicaSet, version, rollout, raw error, SQL, URL, tenant, user, community, pubkey, and request-controlled values are prohibited application labels; infrastructure enrichment can supply deployment identity outside the application metric.Postgres production seam and CI
The real
Db::readiness_checkpath now supports deterministic production-seam testing while preserving the same acquisition/query implementation used by the relay. The isolated PostgreSQL lane automatically discovers and executes three ignored integration tests covering:This prevents the central SQLx/Postgres behavior from being merely compiled but never executed in CI.
Tests and verification
cargo fmt --all -- --check./scripts/test-postgres-test-discovery.shcargo test -p buzz-dbcargo test -p buzz-relay readinesscargo test -p buzz-relay real_health_route_GET /_readiness-> Prometheus render assertionsThe route-to-scrape regression test uses the production Prometheus builder and verifies the
2,2.5, and+Infreadiness buckets. It fails if the health recording call, health route, public/health boundary, generation fence, shutdown fence, or bucket override is removed.Risk assessment
Medium. This changes the live readiness publication path and adds synchronization around probe commit/shutdown. The risk is bounded by:
Operational notes
buzz_readiness_stateas the latest sampled current state, not as an event stream.shutting_downshould be excluded from dependency-failure alerts because no dependency failure is implied.default_zero()for missing current-state data; missing/stale is unknown, not healthy.Non-goals
This PR does not add the broader process-startup lifecycle, worker/listener supervision, shutdown coordinator, WebSocket/huddle handoff, client recovery telemetry, dashboard mutations, Datadog configuration changes, or deployment changes. Those remain separate follow-up work.
References
deploy/charts/buzz/README.md