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

[luishsr]

What

The "turn on & unify" P1 of the Network Observability Initiative (axyl-private#429, research task #381). The research audit found the node is already heavily instrumented — it inherited Narwhal/Mysten's full Prometheus suite (consensus/primary/worker/network/executor metrics) and has reth's execution metrics — but everything is off by default, split across two registries, with no config-file route to enable it. This PR closes the turn-on & unify gaps.

Config — enable metrics from parameters.yaml

Both endpoints are now enable-able from config, not just CLI flags:

metrics_address: "0.0.0.0:9184"        # consensus / Narwhal suite (--metrics)
reth_metrics_address: "0.0.0.0:9001"   # reth execution layer (--reth-metrics)

• Both Option<SocketAddr> with #[serde(default)] — every existing parameters.yaml still parses and stays off (covered by tests).
• The --metrics / --reth-metrics CLI flags override the config values when passed (mirrors the existing --network override); startup logs announce each active endpoint.
• Removes the dead PrometheusMetricsParameters struct — defined with a Default impl but wired nowhere, and typed Multiaddr where the live path is SocketAddr.
• Fixes a stale --enable-healthcheck doc reference (the real flag is --healthcheck).

Monitoring — etc/monitoring/

A ready-to-run operator stack:

• prometheus.yml — a two-target scrape (consensus + execution + node_exporter), labeled by layer, which resolves the split-registry issue (the two endpoints live in separate registries, so they're scraped as separate jobs).
• docker-compose.yml — one-command Prometheus + Grafana + node_exporter.
• Grafana datasource auto-provisioning + a README walkthrough pointing operators at reth's official Grafana dashboard #20638.

Tests

4 config::node::tests cover backward-compat (no field → off), parsing when present, and the defaults for both endpoints. Compiles clean.

Scope / next

This is turn-on & unify only. The validator-identity metrics layer (per-validator liveness/participation, committee/epoch/reputation), a real readiness probe, and a Rayls consensus/validator Grafana dashboard are the next deliverable, tracked in #428. The full research (current-state audit + reuse-vs-build) is attached to #381.

🤖 Generated with Claude Code

[github-actions]

Claude finished @luishsr's task in 3m 20s —— View job

---

PR Review

• Gather context
• Read changed files
• Analyze code quality, bugs, security, performance
• Post review findings

---

Summary

Clean, well-scoped PR. The config changes are backward-compatible and idiomatic, the dead PrometheusMetricsParameters removal is overdue, and the monitoring stack is a genuinely useful operator addition. A few things worth addressing before merge:

---

Medium — Startup log inconsistency

Parameters::tracing() doesn't see CLI-only overrides.

Parameters::tracing() is called in Primary::new() (crates/consensus/primary/src/primary.rs:38). It logs self.metrics_address — the config-file value — not the final merged address. The CLI override in network-cli/src/node.rs:272 applies the merge into a local variable (let metrics = metrics.or(...)) rather than writing it back into parameters.metrics_address. This creates two distinct behaviours:

• Config-file-only: Parameters::tracing() fires AND node.rs fires → two identical log lines for the same endpoint.
• CLI-only: Parameters::tracing() is silent even though metrics ARE active → consensus startup logs say nothing about the metrics endpoint.

The simplest fix is to write the merged value back into parameters before handing it off to the builder:

// in network-cli/src/node.rs, after the merge
rayls_infrastructure_config.parameters.metrics_address = metrics;
rayls_infrastructure_config.parameters.reth_metrics_address = reth.reth_metrics.prometheus;

That way Parameters::tracing() always reflects the final active state, and node.rs's info! logs can be removed (or kept as a startup banner — they're at a different target:). Fix this →

---

Medium — No validation that the two addresses don't collide

metrics_address and reth_metrics_address can be set to the same SocketAddr. There is no validation — the conflict is only discovered at bind time with an OS error that doesn't name the culprit. A quick check in Parameters::tracing() or the builder would make operator errors obvious:

if self.metrics_address == self.reth_metrics_address && self.metrics_address.is_some() {
    tracing::warn!("metrics_address and reth_metrics_address are the same — one will fail to bind");
}

Fix this →

---

Low — Log message inconsistency

Parameters::tracing() (node.rs:386) emits:

"Consensus Prometheus metrics endpoint" / "Reth execution-layer Prometheus metrics endpoint"

network-cli/src/node.rs:273 emits:

"consensus Prometheus metrics enabled" / "reth execution-layer Prometheus metrics enabled"

Different capitalisation, different wording ("endpoint" vs "enabled"). If both fire (config-file path), an operator reading the structured log will see two differently-worded lines for the same event. Standardise to one message and one call site (see Medium issue above).

---

Low — Docker image tags not pinned

etc/monitoring/docker-compose.yml:18,32,49 uses :latest for all three images. A docker compose pull in six months could silently pull a breaking release. Pin to minor versions at minimum (e.g. prom/prometheus:v2.53.3). Fix this →

---

Nit — 0.0.0.0 in docs / config without a firewall callout

The README and config example both suggest 0.0.0.0:9184 / 0.0.0.0:9001. On a cloud node, this binds to the public interface. The healthcheck server has an explicit firewall warning — it would be good to add an equivalent note to the metrics_address / reth_metrics_address field docs and the README Enable the endpoints section, steering operators toward 127.0.0.1 unless Prometheus is remote.

---

What's good

• Option<SocketAddr> + #[serde(default)] is exactly the right type — zero-cost when absent, fully backward-compatible, no parsing surprises.
• Removal of PrometheusMetricsParameters is clean: wrong type (Multiaddr instead of SocketAddr), wired nowhere, leaked a TCP port allocation in its Default impl even when metrics were off.
• Option::or() merge semantics (let metrics = metrics.or(config.metrics_address)) is idiomatic and correctly prioritises the CLI flag.
• 4 tests cover all the backward-compat cases that matter (no field → None, parse-when-present, default impl).
• extra_hosts: host.docker.internal:host-gateway in docker-compose correctly handles Linux Docker Desktop's lack of native host.docker.internal resolution.
• Stale --enable-healthcheck doc fix is a good catch.

[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)

[bronxyz]

Thanks for the research done!
The clean-up part for the PrometheusMetricsParameters and the docs tidying are good.
We already expose both consensus and reth metrics in all of our scripts via --metrics 0.0.0.0:9100 and --reth-metrics 0.0.0.0:9200 and currently the entire stack is configured around that.
Let's discuss how to proceed with this.

[luishsr]

Thanks @bronxyz — and good context on the scripts.

One clarification on how this interacts with the flag-based setup, since it changes the picture: the CLI flags take precedence over config. In network-cli/src/node.rs the resolution is metrics.or(parameters.metrics_address) (same for reth), so whenever --metrics / --reth-metrics are passed they win, and the config is only consulted when the flag is absent. Both fields default to None (off), and existing parameters.yaml files parse unchanged. So your scripts and the whole 9100/9200 stack are unaffected regardless of what we decide here — there's no competing source of truth.

Given that, the only open question is whether the config path is worth keeping as an additive option:

• Keep it (additive default): lets a deployment enable metrics declaratively in the same parameters.yaml it already uses for gc_depth/gas_limit/etc., without every launch path having to remember the flags. Directly addresses the initiative's chore(deps)(deps): bump thin-vec from 0.2.14 to 0.2.18 #1 finding ("metrics are off unless a flag is passed"). Costs a little config surface area.
• Flags-only (descope the two fields): keep the dead-code cleanup + docs + the etc/monitoring operator stack you're happy with, and drop metrics_address / reth_metrics_address since every launch path already sets the flags. Less surface; we can revisit if a non-script launch path ever needs it.

I'm happy either way — which do you prefer? If flags-only, I'll strip the two config fields and keep the rest.

[bronxyz]

I'd say we can keep the params as fallback, but let's align the ports with the script defaults, so we can avoid any potential ports collision.