Export overlay broadcast fan-out drops and throttle the backpressure WARN

[tomerweller]

Closes #3792

Summary

OverlayManager::broadcast drops an outbound message per-peer when that peer's channel is Full, but the drop only bumped the aggregate messages_dropped — the one messages_* field never bridged to Prometheus. This hid 1.73M dropped SCP votes over 29 days on the deployed node (15% of broadcasts reaching zero peers), and the per-call WARN amplified to ~24k lines/second during an event-loop park.

This PR adds two dedicated, henyey-prefixed series and throttles the WARN — all purely additive instrumentation; the drop/send control flow is unchanged, so no observable-surface behavior changes.

• henyey_overlay_broadcast_fanout_drop_total{type} — per-msg_type fan-out drops (mirrors the send_by_type pattern), so the SCP-dominated broadcast loss is not conflated with the 5 other sites feeding messages_dropped.
• henyey_overlay_broadcast_blackout_total — calls that reached ZERO peers (dropped>0 && sent==0), the case worth alerting on.

Both are pre-registered at zero via the existing catalog machinery and bridged in update_overlay_metrics. The aggregate messages_dropped is still incremented alongside for cross-site continuity (#3623). The per-call warn! is gated by a single AtomicU64 + a pure should_emit_now CAS helper to at most one line/second; the counters remain the source of truth so no drop volume is lost.

Plan reference

Converged Plan comment

Test plan

• cargo fmt --check
• cargo clippy -p henyey-overlay -p henyey-app --all-targets -- -D warnings (clean)
• cargo test -p henyey-overlay (498 unit + integration/doctests pass)
• cargo test -p henyey-app (1257 unit + integration pass)

Regression test (kind: bug-fix)

• Tests:
  • crates/app/src/metrics.rs::test_overlay_broadcast_drop_metrics_bridged — direct regression for "invisible to /metrics"; asserts both series render HELP and are pre-registered at 0 ({type="scp_message"} 0).
  • crates/overlay/src/manager/mod.rs::test_broadcast_fanout_drop_by_type_counter — per-type drop counter increments; aggregate messages_dropped continuity asserted.
  • crates/overlay/src/manager/mod.rs::test_broadcast_blackout_on_zero_sent — blackout increments on zero-sent; negative case (≥1 peer accepts) leaves it 0.
  • crates/overlay/src/manager/mod.rs::test_should_emit_now_rate_limit — pure WARN-gate helper.
• Pre-fix: committed as 0dad5be — app test FAILED (series absent from catalog); overlay tests failed to compile (fields/helper absent).
• Post-fix: all four PASS after 90835d8.

Deviations from plan

None.

🤖 Generated with Claude Code

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Cycle 1 complete review. The fix bridges the previously-unexported broadcast fan-out drops to /metrics via two dedicated series and throttles the log-amplification WARN. Logic is correct, well-commented, and thoroughly unit-tested with a genuine main-failing regression test.

Full review

Change under review (metrics + logging only):

• New dedicated per-type series henyey_overlay_broadcast_fanout_drop_total{type} and unlabeled henyey_overlay_broadcast_blackout_total, bridged in crates/app/src/metrics.rs::refresh_gauges alongside the existing sibling absolute() bridges (consistent pattern).
• New OverlayMetrics fields broadcast_fanout_drop_by_type: [Counter; COUNT] and broadcast_blackout: Counter, wired into snapshot() and reset().
• OverlayManager::broadcast now classifies the message kind before the fan-out move, increments the per-type counter on every Full drop, increments blackout iff dropped>0 && sent==0, and gates the WARN via a pure should_emit_now CAS interval gate.

Correctness — verified:

• broadcast_blackout increments exactly on dropped>0 && sent==0 (inside the dropped>0 block, guarded by sent==0) — matches the issue's blackout definition. Both positive and negative paths are unit-tested (test_broadcast_blackout_on_zero_sent).
• Reset completeness: broadcast_blackout is in the reset_counters list; the broadcast_fanout_drop_by_type array is reset in the dedicated trailing loop. Both covered.
• snapshot() copies both new fields; OverlayMetrics::new() = Self::default(), so the added fields auto-initialize to zero.
• should_emit_now: CAS loop is correct; saturating_sub makes clock regressions fail-closed (throttled); SystemTime error maps to now_ms=0 (also throttles). Unit-tested (test_should_emit_now_rate_limit).
• Aggregate messages_dropped is still fed for cross-site continuity (URGENT: validator OOM restart-loop — stalled event loop buffers SCP unbounded during post-catchup transition (escalation of #3582; NOT a b0231cc4 regression) #3623) — the issue explicitly asked for a dedicated series rather than bridging the conflated aggregate; this does exactly that.

Test-verification gate (kind: bug-fix): satisfied. test_overlay_broadcast_drop_metrics_bridged asserts both HELP lines and the pre-registered type="scp_message" 0 series render; both series are absent from the catalog on origin/main, so it provably fails pre-fix. Plus test_broadcast_fanout_drop_by_type_counter and test_broadcast_blackout_on_zero_sent exercise the runtime increment paths.

Concern class check: cycle 1, no prior verdicts. Complete change-list produced. No blocking concerns.

[tomerweller]

🔍 Reviewer: Parity

Verdict: APPROVE

Summary: Parity-critical crate (crates/overlay/) but the change touches only the internal metrics and logging surface, which docs/PARITY.md explicitly permits deviating on. No observable/interop surface is affected.

Full review

Observable/interop surface (per docs/PARITY.md) — unchanged:

• No change to SCP/overlay wire bytes: the set of messages placed on outbound_tx and the Full-drop behavior itself (try_send → dropped += 1) are byte-for-byte identical to before. The diff only observes the drop (counter increment) and logs less often.
• No change to hashes, tx result/meta XDR, history archive format, HTTP/RPC contracts, or crypto outputs.
• The two new metric names are henyey_-namespaced (henyey_overlay_broadcast_fanout_drop_total, henyey_overlay_broadcast_blackout_total) — henyey-internal additions, not claiming a stellar_-prefixed core-parity series, so they cannot regress interop parity.

Allowed-deviation surface (metrics, logging) — the entirety of this change:

• New Prometheus series and a WARN throttle. Both are in the "MAY deviate freely" category. The throttle preserves full measurement volume in the counters (source of truth), only reducing log line count — no observable semantics change.

PARITY_STATUS.md: no update required — this adds observability, it does not implement/remove parity-affecting functionality. (The issue's §4 open question about outbound-SCP recoverability is a separate herder-emit question, explicitly out of scope for this metrics PR.)

Concern class check: cycle 1, no prior verdicts. No parity concerns.

[tomerweller]

✅ Merged

Commit: 9f534c2

Follow-up issues filed for unaddressed inline review comments: none (no inline comments; both reviewers APPROVE, CI 41/41 green)