feat(observability): validator health metrics + readiness probe by luishsr · Pull Request #102 · raylsnetwork/axyl

luishsr · 2026-08-06T12:27:11Z

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

P2 of the Network Observability Initiative (axyl-private#428). Adds the validator/ network-health signals the audit found missing — the node already exports Narwhal/ Mysten's Prometheus suite, but the highest-value per-validator and readiness signals were tracked in-memory and never exposed. Readiness probe: the healthcheck now serves /readyz — 200 only when the node is voting (CvvActive) or an operational observer, 503 while a validator is still catching up (CvvInactive). Liveness (any other path) stays 200. Reads the live NodeMode via a watch::Receiver from the consensus bus, threaded at the spawn site. Lets a load balancer / on-call distinguish "process up" from "actually voting in the current epoch". New metrics (registered into the consensus default_registry, served on --metrics): - validator_participation{authority} — committed certificates per validator, counted at the bullshark commit site. A direct per-validator liveness signal, consistent across nodes (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. - committee_size — committee size for the current epoch, set per epoch. Dashboard: etc/monitoring/.../rayls-consensus.json — a starter Grafana dashboard (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. Composes with the etc/monitoring stack from axyl#99. Refs: raylsnetwork/axyl-private#428 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-06T12:27:33Z

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.

…ion metric Follow-up to the P2 review on #102: - Propagate a healthcheck bind failure (core.rs) instead of `let _ =`-swallowing it. A port already in use would previously start the node with no health endpoint and no signal — defeating the purpose of an explicitly-requested probe. Now returns the error. - Handle each healthcheck connection in its own task (health.rs). The accept loop bounded each request read with a 200ms timeout but still ran it inline, so a client that connects and sends nothing held the loop for the full timeout and delayed every other probe. A per-connection task keeps the accept loop free; the watch::Receiver is cloned per conn. - Reset the accept-loop backoff on a healthy accept (health.rs) so a past transient error doesn't keep the retry delay elevated. - Count cert-covered-skipped too-old rejections under a distinct `too_old_skipped` reason (certifier.rs) rather than dropping them from vote_request_rejections. Otherwise the metric reads zero while a node continuously trips the skip path, masking real trouble. Doc comment on the metric updated to list the three reason values. - Document the `/ready` alias alongside `/readyz` in the health module docs. Reviewer's other two points verified as non-issues, no change: the `authority` label is a ~44-char bs58 of a 32-byte AuthorityIdentifier with cardinality bounded by committee size (not a raw BLS key); and `current_round` (dashboard panel 4) is a real registered PrimaryMetrics gauge on the same default_registry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-06T12:43:42Z

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

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

Tasks:

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.

…ric with too_old_skipped) The re-review noted the epoch-mismatch stale-peer early-return dropped the rejection from vote_request_rejections — the same gap just fixed for too_old. Count it under a distinct `epoch_mismatch_stale` reason so the raw rejection rate stays visible across all four paths. Metric doc updated to list all reason values. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-06T12:48:44Z

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.

…lidator-metrics # Conflicts: # crates/middleware/orchestrator/src/epoch_manager/core.rs

github-actions · 2026-08-06T12:57:03Z

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 · 2026-08-06T12:57:12Z

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)

…omment_width CI's Format job runs `cargo +nightly fmt --all --check`, whose rustfmt.toml sets comment_width=100 + wrap_comments (nightly-only). One doc-comment line in the readiness module wrapped one word early; rewrap it so the nightly check passes. No code change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-06T13:15:58Z

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.

procdump · 2026-08-18T13:35:27Z

+/// handled in the accept loop and closed immediately after the response.
 ///
-/// To enable on node startup, use `rayls-network node --enable-healthcheck`.
+/// To enable on node startup, use `rayls-network node --healthcheck <PORT>`.


As you're changing parameter names - flagging again that this might break the test env as well as deployments?

Dug into this — the CLI flag itself is unchanged by this PR: --healthcheck <PORT> (pub healthcheck: Option<u16> in node.rs) already exists on main today, and nothing in this PR renames or retypes it. What changed here is only the doc comment on health.rs, which said --enable-healthcheck — a name that doesn't match the actual flag. I traced that mismatch back to c08dd6e ("Porting codebase from old repository", the original import) — it's been stale since before any of us touched this file; this PR is the first to fix the doc text to match the real flag. I also grepped axyl-automation-testing and etc/ for --enable-healthcheck/enable_healthcheck — zero hits, so nothing in the test env references the old (never-actually-real) name either. No Prometheus metric names were renamed in this PR either (the new ones — vote_request_rejections, committee_size, validator_participation — are net-new, nothing existing renamed). So I don't think there's a live break here, but let me know if you had a specific test-env or deployment reference in mind that I should check directly — happy to look at anything specific.

procdump · 2026-08-21T14:34:42Z

+//! - **liveness** (any path, e.g. `GET /`): always `200 OK` while the process is accepting
+//!   connections — the pre-existing behaviour.
+//! - **readiness** (`GET /readyz`, alias `GET /ready`): `200 OK` only when the node is actually
+//!   participating — voting (`CvvActive`) or an operational observer — and `503 Service


It seems pretty odd to me to use 503 when the validator is CvvInactive. What's the reason behind this decision?

procdump · 2026-08-21T14:45:07Z

+                        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 {
+                                // Any other path (incl. `/`) is a liveness probe: process is up.
+                                LIVE_200
+                            };


could you have used some framework for exposing this data instead of doing it the raw way string-matching the request that comes from the socket - i'd also argue that this makes the API very fragile later to extend.

luishsr requested review from bronxyz, kstoykov and procdump as code owners August 6, 2026 12:27

Merge remote-tracking branch 'origin/main' into feat/observability-va…

1fe3133

…lidator-metrics # Conflicts: # crates/middleware/orchestrator/src/epoch_manager/core.rs

procdump reviewed Aug 18, 2026

View reviewed changes

luishsr mentioned this pull request Aug 21, 2026

feat(observability): enable metrics via config + operator monitoring setup #99

Open

procdump reviewed Aug 21, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(observability): validator health metrics + readiness probe - #102

feat(observability): validator health metrics + readiness probe#102
luishsr wants to merge 5 commits into
mainfrom
feat/observability-validator-metrics

luishsr commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

procdump Aug 18, 2026

Uh oh!

luishsr Aug 21, 2026

Uh oh!

procdump Aug 21, 2026

Uh oh!

procdump Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

luishsr commented Aug 6, 2026

What

Readiness probe

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

Dashboard

Tests

Composition / scope

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Issues

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

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

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

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

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

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

7. /ready path is undocumented (minor)

What's good

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Follow-up Review

Issue 1 — Silent bind error ✅ Fixed

Issue 2 — Accept loop blocking for 200ms ✅ Fixed

Issue 3 — Backoff not reset on success ✅ Fixed

Issue 4 — Authority label cardinality ✅ Verified non-issue

Issue 5 — too_old_skipped rejections invisible in metric ✅ Fixed

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

Issue 7 — /ready alias undocumented ✅ Fixed

Summary

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Re-Review (commit 087c499)

Fix Verification: epoch_mismatch_stale counter ✅

Summary

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Merge Verification

Summary

Uh oh!

github-actions Bot commented Aug 6, 2026

Security Scan - Code

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Re-Review (post-sync 6b55dd5)

Style Fix Verification ✅

Summary

Uh oh!

procdump Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

luishsr Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

procdump Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

procdump Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

github-actions Bot commented Aug 6, 2026 •

edited

Loading

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

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

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

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

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

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

7. `/ready` path is undocumented (minor)

github-actions Bot commented Aug 6, 2026 •

edited

Loading

Issue 5 — `too_old_skipped` rejections invisible in metric ✅ Fixed

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

Issue 7 — `/ready` alias undocumented ✅ Fixed

github-actions Bot commented Aug 6, 2026 •

edited

Loading

PR Re-Review (commit `087c499`)

Fix Verification: `epoch_mismatch_stale` counter ✅

github-actions Bot commented Aug 6, 2026 •

edited

Loading

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

github-actions Bot commented Aug 6, 2026 •

edited

Loading

PR Re-Review (post-sync `6b55dd5`)