feat(observability): validator health metrics + readiness probe

[luishsr]

What

P2 of the Network Observability Initiative (axyl-private#428). The research audit found the node already exports Narwhal/Mysten's full Prometheus suite — but the highest-value per-validator and readiness signals were tracked in-memory and never exposed. This PR surfaces them.

Readiness probe

The healthcheck now serves /readyz — 200 only when the node is voting (CvvActive) or an operational observer, and 503 while a validator is still catching up (CvvInactive). Liveness (any other path) stays 200. It reads the live NodeMode via a watch::Receiver from the consensus bus, threaded in at the spawn site. This lets a load balancer / on-call distinguish "process up" from "actually voting in the current epoch" — which the unconditional-200 healthcheck couldn't.

New metrics (consensus registry, served on --metrics / metrics_address)

• validator_participation{authority} — committed certificates per validator, counted at the bullshark commit site (where the sub-dag certificates are already iterated). A direct per-validator liveness signal, and consistent across nodes since every node commits the same sub-dags. This is the audit's biggest gap — "did validator X vote/certify?"
• vote_request_rejections{authority, reason} — this node's header vote requests rejected by a peer (from VoteFailureTracker), by rejecting validator and reason (too_old / epoch_mismatch). Node-local: rising totals mean this node is falling behind. Complements the network-consistent validator_participation.
• committee_size — committee size for the current epoch, set at the per-epoch metrics hook.

Dashboard

etc/monitoring/.../rayls-consensus.json — a starter Grafana dashboard (9 panels: epoch, committee size, bad nodes, round progress, commit-latency p95, leader election, the two new per-validator series, peers) with auto-provisioning, portable via a datasource variable.

Tests

3 tests on the readiness probe (path routing, mode→status mapping, and a live server test flipping CvvInactive→CvvActive and seeing 503→200). The metrics follow the established register_*_with_registry! idiom; compiles clean across consensus-primary, primary-metrics, and orchestrator.

Composition / scope

• The etc/monitoring/ dashboard composes with the monitoring stack in feat(observability): enable metrics via config + operator monitoring setup #99 (this branch is off main, so feat(observability): enable metrics via config + operator monitoring setup #99's prometheus.yml / docker-compose aren't on it yet; they merge cleanly as separate files).
• Deferred follow-ups: on-chain ValidatorStatus export (Active / PendingActivation / Exited — needs an EVM ConsensusRegistry read joined via Committee) and round-lag as its own metric (currently derived in the dashboard). Tracked under #428.

🤖 Generated with Claude Code

[github-actions]

Claude finished @luishsr's task in 2m 25s —— View job

---

PR Review: feat(observability): validator health metrics + readiness probe

Tasks:

• Gather context and read changed files
• Review health.rs — readiness probe logic
• Review consensus.rs — new metric definitions
• Review bullshark.rs — validator participation counting
• Review certifier.rs — vote rejection tracking
• Review core.rs — epoch manager changes
• Review Grafana dashboard files
• Synthesize findings and post review

---

Summary

This is well-scoped observability work. The readiness probe design is sound (watch-channel-based mode propagation, path routing, proper HTTP formatting), and the metrics follow the established register_*_with_registry! idiom. The Bullshark integration site is the correct place to count committed certificates. A few issues worth addressing before merge:

---

Issues

1. Startup bind error is silently swallowed — core.rs:275-280 (bug)

if let Some(port) = self.builder.healthcheck {
    let _ = HealthcheckServer::spawn(
        node_task_manager.get_spawner(),
        port,
        self.consensus_bus.node_mode().subscribe(),
    )
    .await;
}

let _ = ... silently drops any eyre::Result::Err returned from HealthcheckServer::spawn. If the port is already bound (e.g., operator misconfiguration), TcpListener::bind returns Err and the server never starts — but the node continues without the health endpoint and without alerting the operator. This should propagate:

if let Some(port) = self.builder.healthcheck {
    HealthcheckServer::spawn(
        node_task_manager.get_spawner(),
        port,
        self.consensus_bus.node_mode().subscribe(),
    )
    .await?;
}

Fix this →

---

2. Accept loop blocks for 200ms per slow client — health.rs:92-97

let wants_readiness = matches!(
    tokio::time::timeout(Duration::from_millis(200), socket.read(&mut buf))
        .await,
    Ok(Ok(n)) if n > 0 && request_targets_readiness(&buf[..n])
);

The health check runs in a single-threaded accept loop (no tokio::spawn per connection). A client that connects but sends nothing will hold the loop for the full 200ms timeout, delaying every subsequent health check during that window. A load balancer probing every second with multiple instances can create a meaningful queue.

Recommendation: spawn each connection into a short-lived task (or use socket.try_read after a ready poll):

let node_mode = node_mode.clone();
tokio::spawn(async move {
    let mut buf = [0u8; 256];
    let wants_readiness = matches!(
        tokio::time::timeout(Duration::from_millis(200), socket.read(&mut buf)).await,
        Ok(Ok(n)) if n > 0 && request_targets_readiness(&buf[..n])
    );
    let response = if wants_readiness {
        readiness_response(*node_mode.borrow())
    } else {
        LIVE_200
    };
    let _ = socket.write_all(response).await;
});

Fix this →

---

3. Backoff not reset on success — health.rs:84-85 (minor)

let mut backoff = Duration::from_millis(100);
let max_backoff = Duration::from_secs(5);

The backoff is never reset after a successful accept(). After any transient error, subsequent errors will use the already-elevated backoff even if the root cause was momentary. Reset it on success:

Ok((mut socket, _)) => {
    backoff = Duration::from_millis(100); // reset on success
    // ...
}

---

4. Label cardinality: verify authority identifier length — consensus.rs:139-155, certifier.rs:418-421, bullshark.rs:197-200

The authority label on both vote_request_rejections and validator_participation is derived from .to_string() on AuthorityIdentifier / peer_id. If this emits a full BLS public key (96 bytes hex = 192 chars), it creates a high-cardinality label set that can cause Prometheus performance issues and render dashboard panels unreadable.

Please confirm the Display impl for AuthorityIdentifier emits a short, stable identifier (e.g., an index or a truncated key) rather than the full raw key.

---

5. Skipped too_old rejections are not counted in the metric — certifier.rs:393-406 (design gap)

if self.vote_failures.skip_cert_covered(committed_round, Self::CERT_COVERED_WEDGE_WINDOW) {
    warn!(..., "ignoring too-old rejection: ...");
    return VoteErrorAction::Continue;  // metric NOT incremented
}

Cert-covered-skipped rejections are filtered from vote_request_rejections. An operator looking at the metric sees only "rejections that counted toward demotion" — they cannot distinguish between "no rejections" and "all rejections were skipped." This could mask a node that is continuously triggering the skip logic. Consider adding a separate counter (or a reason=too_old_skipped label) so the raw rejection rate is always visible.

---

6. Grafana panel 4 references current_round — verify this metric exists

Panel 4 queries current_round, which is not defined in this PR's consensus.rs and doesn't appear in the existing metric list. If it's a Narwhal upstream metric that isn't exported by this node, the panel will be empty. Verify this metric name against the actual Prometheus output of a running node.

---

7. /ready path is undocumented (minor)

request_targets_readiness also matches /ready (line 53), but only /readyz is mentioned in the module doc, PR description, and CLI docs. Either document the /ready alias or remove it to avoid confusion.

---

What's good

• Readiness probe design is clean: the watch::Receiver<NodeMode> approach gives zero-copy, lock-free mode reads per request. borrow() on a watch receiver is exactly the right primitive here.
• Bullshark integration site (committed sub-dag iteration) is the correct, network-consistent location for validator_participation — every node commits the same sub-dags.
• committee_size epoch reset is handled correctly — the gauge is set in run_epoch before tasks start, so it always reflects the current epoch's committee.
• Tests are well-structured: path routing, mode→status, and a live server state-transition test. The tokio::time::sleep(10ms) before the first probe is a reasonable guard for the server bind.
• Dashboard datasource variable ${DS} makes it portable across environments.

[github-actions]

Claude finished @luishsr's task in 1m 54s —— View job

---

PR Re-Review: feat(observability): validator health metrics + readiness probe

Tasks:

• Gather context and read CLAUDE.md
• Review diff since last review commit
• Check fix for silent bind error (core.rs)
• Check fix for blocking accept loop (health.rs)
• Check fix for backoff not reset (health.rs)
• Check fix for skipped rejections not counted (certifier.rs)
• Check authority label cardinality (consensus.rs, certifier.rs, bullshark.rs)
• Check current_round metric in Grafana dashboard
• Check /ready path documentation
• Synthesize findings and post review

---

Follow-up Review

The fix commit (65bef89) cleanly addresses all 7 issues from the prior review. Here's a precise account of each:

---

Issue 1 — Silent bind error ✅ Fixed

core.rs:277-282: The let _ = ...await was replaced with .await?;. A port-bind failure now propagates as an eyre::Error and the node fails fast with a clear message rather than silently starting without the health endpoint.

---

Issue 2 — Accept loop blocking for 200ms ✅ Fixed

health.rs:100-123: Each accepted connection is now moved into tokio::spawn(async move {...}). The node_mode watch receiver is cloned per connection (cheap). The accept loop itself returns immediately after spawning and is never held by the read timeout.

One minor observation (not blocking): the tokio::spawn pool is unbounded — a flood of concurrent connects creates as many tasks as connections. The existing firewall guidance in the doc covers this, and for a health endpoint serving probes this is fine in practice.

---

Issue 3 — Backoff not reset on success ✅ Fixed

health.rs:84-93: The initial value is now stored as initial_backoff and reset via backoff = initial_backoff; on every successful accept(). Clean and readable.

---

Issue 4 — Authority label cardinality ✅ Verified non-issue

Commit message confirms AuthorityIdentifier displays as a ~44-character bs58 encoding of a 32-byte key — not a 192-char hex BLS key. Cardinality is bounded by committee size (tens of validators), well within Prometheus limits.

---

Issue 5 — too_old_skipped rejections invisible in metric ✅ Fixed

certifier.rs:406-413: The cert-covered-skip path now increments vote_request_rejections under reason="too_old_skipped" before returning VoteErrorAction::Continue. The metric doc comment in consensus.rs was updated to list all three values (too_old | too_old_skipped | epoch_mismatch).

One small follow-on gap to be aware of (not blocking, analogous to the original issue): the epoch-mismatch stale-peer early-return at certifier.rs:456-464 (!should_count_epoch_rejection → return Continue) also doesn't increment the metric — no epoch_mismatch_stale reason is emitted. It's the same pattern as the original too_old_skipped omission. Worth tracking under #428 if epoch-from-lagging-peers visibility becomes important.

---

Issue 6 — current_round in Grafana panel ✅ Verified non-issue

Commit message confirms current_round is a real PrimaryMetrics gauge registered on the default registry, so the panel will populate.

---

Issue 7 — /ready alias undocumented ✅ Fixed

health.rs:6: Module docstring now explicitly lists GET /readyz with alias GET /ready. The readiness_path_is_matched unit test validates both paths (/readyz and /ready) and two negative cases.

---

Summary

All prior review issues are resolved. The code is clean and the fix commit is well-scoped. The one new observation (epoch-mismatch stale path has no metric) is a minor gap that mirrors the original too_old_skipped issue and is suitable for a follow-up rather than blocking this PR. Ready to merge.

[github-actions]

Claude finished @luishsr's task in 1m 14s —— View job

---

PR Re-Review (commit 087c499)

Tasks:

• Gather context and identify scope of new commit
• Read certifier.rs — verify epoch-mismatch stale path now increments metric
• Read consensus.rs — verify metric doc updated for new label value
• Synthesize findings and post review

---

Fix Verification: epoch_mismatch_stale counter ✅

Commit 087c499 addresses the residual gap called out in the previous re-review. The fix is minimal, correct, and symmetric with the too_old_skipped pattern from 65bef89.

What was added (certifier.rs:464-471)

// Count it under a distinct reason even though a stale peer can't demote us —
// mirrors `too_old_skipped`, keeping the raw rejection rate visible.
self.consensus_bus
    .consensus_metrics()
    .vote_request_rejections
    .with_label_values(&[&peer_id.to_string(), "epoch_mismatch_stale"])
    .inc();
return VoteErrorAction::Continue;

The increment fires before return — no code path through the stale-peer branch escapes the counter.

All four rejection paths now covered:

Path	reason label	Counts toward demotion
Too-old, cert store covers it	too_old_skipped	No
Too-old, counted	too_old	Yes
Epoch mismatch, stale peer	epoch_mismatch_stale	No
Epoch mismatch, counted	epoch_mismatch	Yes

Doc comment (consensus.rs) — updated to list all four reason values and explain the *_skipped / *_stale distinction. The explanation is clear and accurate.

No further gaps found. I re-read the DagError::TooOldRejectedByPeer and DagError::EpochRejectedByPeer match arms end-to-end — there are no remaining early-return paths that bypass the metric.

---

Summary

All issues from both prior reviews are now resolved. The PR is clean, well-tested, and the metric coverage is complete. Ready to merge.

[github-actions]

Claude finished @luishsr's task in 1m 34s —— View job

---

PR Re-Review (post-merge-sync 1fe3133)

Tasks:

• Gather context from prior reviews
• Check diff since last review commit (087c499)
• Verify merge conflict resolution in core.rs
• Confirm no other PR-specific files affected by merge
• Final status assessment

---

Merge Verification

The sync event was triggered by commit 1fe3133 — a merge from origin/main into this branch. The merge had one conflict in core.rs, which was resolved correctly.

Conflict: core.rs:97-116

The conflict was between main's liveness-only healthcheck spawn (let _ = HealthcheckServer::spawn(..., port)) and this PR's readiness-aware version (HealthcheckServer::spawn(..., port, node_mode().subscribe()).await?). The merge correctly kept the PR's version:

if let Some(port) = self.builder.healthcheck {
    HealthcheckServer::spawn(
        node_task_manager.get_spawner(),
        port,
        self.consensus_bus.node_mode().subscribe(),
    )
    .await?;
}

All previously reviewed fixes remain intact:

• .await? propagates bind errors (Issue 1 ✅)
• node_mode().subscribe() correctly threaded (readiness probe ✅)
• Comment updated to describe both liveness and readiness semantics ✅

No other PR-specific files were touched by the merge — consensus.rs, certifier.rs, bullshark.rs, health.rs, and the Grafana dashboard files are byte-for-byte identical to 087c499.

---

Summary

The merge was clean. All prior review findings remain resolved. Ready to merge.

[github-actions]

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results

Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

[github-actions]

Claude finished @luishsr's task in 34s —— View job

---

PR Re-Review (post-sync 6b55dd5)

Tasks:

• Gather context from prior reviews
• Inspect latest commit (6b55dd5) — style fix scope
• Verify no functional changes introduced
• Final status assessment

---

Style Fix Verification ✅

Commit 6b55dd5 is a pure doc-comment reflow — no code change.

What changed (health.rs:8):

-//!   Unavailable` while a validator is still catching up (`CvvInactive`). This lets a load
-//!   balancer / on-call distinguish "process up" from "actually voting in the current epoch".
+//!   Unavailable` while a validator is still catching up (`CvvInactive`). This lets a load balancer
+//!   / on-call distinguish "process up" from "actually voting in the current epoch".

One doc-comment line was rewrapped to comply with the comment_width = 100 + wrap_comments setting in rustfmt.toml (nightly-only). The CI Format job (cargo +nightly fmt --all --check) was failing on this line; the fix moves one word (balancer) to match the nightly formatter's output exactly.

No logic, no signatures, no metric definitions, no test code — nothing functional was touched.

---

Summary

All prior review findings remain resolved. This final commit is a formatting-only CI fix. Ready to merge.