feat(tooling): add event-monitor arrival-time dashboard - #538
Conversation
Standalone Rust/axum collector + vanilla-JS dashboard that dials the /lean/v0/events SSE stream of several ethlambda nodes, timestamps each event on one collector clock, and visualizes arrival offset within the slot (rolling per-node beeswarm) and inter-node propagation delay per block/aggregate/head (a second delta beeswarm, delay behind the first node to see each id). Own Cargo workspace with no ethlambda-crate deps; speaks only the documented SSE/HTTP wire shape frozen in CONTRACT.md. Rolling window is adjustable live from the header, a fresh page load backfills recent history via GET /api/history so it is never blank, and ?demo=1 runs the frontend fully offline on synthetic data.
🤖 Kimi Code ReviewOverall: This is a well-architected standalone monitoring tool with clear separation of concerns, good documentation ( 1. Critical: Invalid dependency versionFile: toml = "1.1.3"Issue: The Fix: toml = "0.8"2. Code correctness & robustnessFile: let url = format!(
"{}/lean/v0/events?topics={}",
node.url.trim_end_matches('/'),
topics.join(",")
);Issue: Topics are not URL-encoded. If a topic name contains reserved characters (e.g., Fix: Use let url = format!("{}/lean/v0/events", node.url.trim_end_matches('/'));
let url = reqwest::Url::parse_with_params(&url, &[("topics", topics.join(","))])?;File: axum::serve(listener, app).await?;Issue: No graceful shutdown handling. The server will not respond to SIGINT/SIGTERM cleanly, potentially dropping active SSE connections. Fix: Add signal handling: let server = axum::serve(listener, app);
let shutdown = tokio::signal::ctrl_c();
tokio::select! {
result = server => result?,
_ = shutdown => tracing::info!("shutting down"),
}File: Issue: No validation that Fix: Add validation after loading: if config.nodes.is_empty() {
return Err(anyhow::anyhow!("config must specify at least one node"));
}File: let serve_dir = ServeDir::new(static_dir).fallback(ServeFile::new(index_html));Issue: Fix: Resolve to absolute path: let static_dir = std::fs::canonicalize(static_dir)?;3. Security considerationsFile: Fix: Add use tower_http::cors::CorsLayer;
// ...
Router::new()
.layer(CorsLayer::permissive()) // or restricted to specific origins4. Performance & correctness observationsFile: let canonical = serde_json::to_string(&input).expect("aggregate id input is always valid JSON");Observation: The File: i64::try_from(duration.as_millis()).unwrap_or(i64::MAX)Good: Correctly handles the theoretical overflow case. File: const HISTORY_MAX_EVENTS: usize = 50_000;Good: Hard cap prevents unbounded memory growth during attestation floods. File: rate.textContent = `${status.events_per_sec.toFixed(1)}/s`;Issue: If the backend sends Fix: Add a guard: if (Number.isFinite(status.events_per_sec)) {
rate.textContent = `${status.events_per_sec.toFixed(1)}/s`;
} else {
rate.textContent = "—";
}5. TestingFile: Good: Integration test with a fake SSE server is well-written and covers the normalization pipeline end-to-end. Suggestion: Add a test for the pruning logic in Summary of required changes
The rest are suggestions for hardening. The architecture is sound and the separation between the Rust collector and JS frontend via Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
No consensus-path code is touched here, so fork choice, attestation validation, STF, XMSS, and SSZ correctness are not directly affected by this PR. I couldn’t run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview: PR 538 —
|
Greptile SummaryThis PR adds a standalone dashboard for monitoring event arrival and propagation times. The main changes are:
Confidence Score: 5/5No additional blocking issue qualifies for a new follow-up comment.
|
| Filename | Overview |
|---|---|
| tooling/event-monitor/src/config.rs | Defines monitor settings, node endpoints, and default event topics. |
| tooling/event-monitor/src/hub.rs | Adds live event broadcasting, node status storage, and bounded history backfill. |
| tooling/event-monitor/src/collector.rs | Connects to node event streams and publishes normalized events and status updates. |
| tooling/event-monitor/web/app.js | Bootstraps metadata and history, merges live events, and updates the dashboard. |
Reviews (2): Last reviewed commit: "Merge branch 'main' into feat/event-moni..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn default_topics() -> Vec<String> { | ||
| vec![ | ||
| "block".to_string(), | ||
| "attestation".to_string(), | ||
| "aggregate".to_string(), | ||
| ] | ||
| } |
There was a problem hiding this comment.
The current event endpoint rejects the entire request when any topic is unknown. With these defaults, nodes on main return HTTP 400 for attestation and aggregate, so the collector also loses supported block events and remains reconnecting/down. The default must use currently supported topics or the collector must retry with unsupported topics removed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tooling/event-monitor/src/config.rs
Line: 57-63
Comment:
**Default Subscription Is Rejected**
The current event endpoint rejects the entire request when any topic is unknown. With these defaults, nodes on `main` return HTTP 400 for `attestation` and `aggregate`, so the collector also loses supported `block` events and remains reconnecting/down. The default must use currently supported topics or the collector must retry with unsupported topics removed.
How can I resolve this? If you propose a fix, please make it concise.Quality cleanups from a /simplify pass; no behavior change and the serialized wire shape is untouched: - serialize HistorySnapshot directly, dropping the field-identical HistoryResponse DTO and its hand copy - add NodeConfig::endpoint() as the single home for the node URL join, used by the collector and the timing bootstrap - fold the checkpoint-topic payload into SlotBlockPayload (serde ignores the extra state field) and trim ReorgPayload to the surfaced fields - extract node_status() for the four repeated status literals - fetch genesis + spec concurrently so a dead node costs one timeout - propagation panel: reject unknown nodes at ingest, drop a dead branch, and cache per-dot jitter + the id's min arrival at insert rather than recomputing both every animation frame
The propagation panel offered a `head` toggle, but head arrival spread is a fork-choice output derived from the same attestation set as `block`, so it told us nothing the block lane didn't already show. Dropping the toggle also lets the collector stop subscribing to a topic no panel renders. `safe_target` and `chain_reorg` were normalized backend-side and then discarded by both panels' topic filters, so they only ever cost a match arm and a payload struct. `chain_reorg` in particular is a point-in-time event rather than a per-node arrival race, so it does not belong on a beeswarm. Keeps CONTRACT.md authoritative: the §2 topic table and the propagation toggle spec now match what the code accepts and renders.
🤖 Codex Code Review
No consensus-layer logic is touched in this PR, so fork choice, attestation validation, STF, XMSS, and SSZ correctness are unaffected here. I couldn’t run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewI have findings to report, but let me give the full written review directly since the task calls for markdown output, not the structured tool. Review:
|
Reconnect backoff reset was keyed on a clean end-of-stream. The common case is the opposite: a node restart surfaces as a stream error after hours of healthy streaming, so every restart ratcheted the delay one step permanently and after ~7 of them a perfectly healthy node reported `down` and took MAX_BACKOFF to reconnect. Key the reset on how long the session lasted instead, via an `Attempt` that is healthy once it survives one heartbeat. That also closes the inverse hazard the old rule left open: a peer which accepts the request and immediately closes the stream ended "cleanly", hit the reset, and was retried at INITIAL_BACKOFF forever instead of backing off. Both the collector's history ring and the dashboard's rolling window key retention off the highest slot seen, and that watermark only ever moves up. One event from a node on a different genesis therefore aged out every real event and blanked both panels until a restart. Drop events whose slot is more than MAX_FUTURE_SLOTS ahead of the slot the collector's own clock is in, warning once per node per connection so the misconfiguration is visible rather than silent. Only the future side is bounded: old slots are legitimate and common (finalized_checkpoint trails head, a syncing node replays history) and cannot move the watermark. Placing the check in `normalize` covers the history ring and both canvas panels from one spot, since no bogus event reaches the frontend. The frontend applies `status` immediately but buffers `chain` during startup backfill, so the /api/history snapshot could overwrite a fresher live status with an older one. Live status now wins. Test fixtures paired slot-128 and slot-12 payloads with sub-second arrival times, i.e. events far in the future, which the new bound rejects. They now place arrival inside a plausible slot via `arrival_for`, keeping every original assertion; `block_topic_maps_id_to_block_root` asserts CONTRACT §2's worked offset of 123ms rather than a negative offset, which `offset_ms_can_be_negative_under_clock_skew` already covers. CONTRACT.md gains the slot bound in §2 and the live-status-wins rule in §4.
Each tool under tooling/ declares its own [workspace] table, which is what keeps it decoupled from the root workspace but also means every existing check misses it: `cargo fmt --all`, `cargo check --workspace` and `cargo clippy --workspace` all stop at the root workspace members, as do `make lint` and `make test`. tooling/event-monitor therefore landed with nothing verifying it, and would have stayed green only for as long as someone remembered to run cargo by hand in that directory. Add `tooling-lint` (fmt --check + clippy -D warnings) and `tooling-test`, both looping over `tooling/*/Cargo.toml` so a second tool is picked up without touching the Makefile. `set -e` inside the loop is load-bearing: without it a failing clippy would be masked by the exit status of the next command in the loop body. The `[ -e ]` guard keeps the recipes a clean no-op if the glob ever matches nothing. Wire them into `lint` and `test` rather than leaving them standalone, so the documented pre-commit workflow cannot pass locally while CI fails, and extend `fmt` to format tooling too, so `make fmt` does not leave work that `tooling-lint` then rejects. The CI Lint job calls the same two targets instead of restating the commands, and its rust-cache now lists tooling/event-monitor: separate workspaces have their own target dir and Cargo.lock, so without listing it the tooling build is recompiled from scratch every run and its lockfile does not feed the cache key. Tests run in Lint rather than in the Test job because clippy --all-targets has already compiled them there, and because they need none of the Test job's leanSpec fixtures.
Keeps the Makefile out of it: the tooling checks live entirely in the workflow, so `make fmt` / `lint` / `test` go back to meaning exactly what they meant before and nothing in the root build description knows about tooling/. The two steps now run cargo directly under `working-directory: tooling/event-monitor`. Failure propagation still holds without the explicit `set -e` the Makefile recipes needed, because the runner's default shell for a multi-line `run` is `bash -e`, so a failing `cargo fmt --check` aborts the step before clippy runs (verified: fmt violation exits 1, clippy violation exits 101). The rust-cache `workspaces` entry stays, since caching a separate workspace's target dir and folding its Cargo.lock into the cache key is a workflow concern either way. Trade-off this accepts: `make lint` and `make test` no longer cover tooling, so they can pass locally while CI fails on it. Running cargo inside tooling/event-monitor is the local equivalent.
Slot geometry was resolved once at startup and never revisited. A regenerated
genesis is routine on these devnets and silently invalidates every `offset_ms`
computed afterwards: nothing errors, every dot just piles up against one edge and
the panel quietly lies. Re-resolve every REFRESH_INTERVAL and republish through a
watch channel, so collectors pick it up per frame and `/api/meta` per request
without a restart.
On a change the retained history is dropped. Those events' offsets belong to the
previous epoch and their slot numbers to a different chain, so serving them in
the same backfill would mix two incomparable series. Dropping them also clears
the slot watermark, which matters more than the events: the ring only ever moves
that mark upward, so a chain restarted at low slots would otherwise have every
one of its events pruned as "older than the retain window". Verified against a
stub node: after the regen the ring holds the new chain's low slots with in-slot
offsets, which does not happen without the reset.
Already-loaded tabs keep the geometry and client-side watermark they fetched, so
they need a reload; that is now stated in CONTRACT.md §2 and the README rather
than left to be discovered.
Widen the arrival axis to two slots at two scales. It clamped to
[0, ms_per_slot], which made a block arriving a full slot late look exactly like
one landing on the boundary — and spilling past the boundary is the failure mode
worth seeing. The first slot keeps full resolution (90.9% of the width at 5
intervals) and the second is compressed into a tinted band half a first-slot
interval wide, saturating beyond that. Solving
`total = main + main * FRACTION / intervals` for the split keeps the band exactly
that fraction of an interval regardless of canvas width; checked numerically for
monotonicity and endpoints.
Document that `offset_ms` includes the collector<->node round trip. One clock is
deliberate and is what makes propagation deltas skew-free, since the events carry
no node-side timestamp, but it means nodes reached over different links carry a
systematic offset that reads as lag. Better stated than silently over-read.
Share retained events as Arc<NormalizedEvent>. Each event was deep-copied once
into the ring and once per broadcast receiver, and `/api/history` cloned up to
HISTORY_MAX_EVENTS of them while holding the mutex every collector needs to
publish. Serde's `rc` feature keeps the wire shape identical, as the contract
requires.
Reject `topics = []` and `nodes = []` at load. An empty topic list produces
`?topics=`, which every node answers with 400, so the collector retried forever
without ever saying why. Also drop tokio's `signal` feature, which was enabled
but unused, and fix the DEFAULT_INTERVALS_PER_SLOT comment, which described a
condition ("config didn't need a network fetch at all") that `bootstrap` never
has, since it always fetches.
Every finding here is the same defect class: the tool reporting a healthy-looking state for something that is actually broken, which for a monitor is worse than reporting nothing. The shared HTTP client had no connect or read timeout, so a socket that completes the handshake and then never answers parked the collector in `send()` indefinitely. It reported `reconnecting` forever and `down` was unreachable for that entire failure class, making a blackholed node indistinguishable from one that is merely slow to come up; no retry was ever even logged. A half-open connection where the node stops sending was worse still: the heartbeat kept publishing `connected` at 0.0/s. Both timeouts belong on the client, since `RequestBuilder::timeout` is a total deadline that would tear down a healthy long-lived SSE stream on schedule. The read timeout sits at three of upstream's 15s keep-alives, trading slow convergence to `down` on a blackhole (a few minutes) for tolerance of a loaded node. A geometry change left every open tab permanently blank. The server resets its slot watermark; the frontend's only ever moves up, so a restarted chain's low slots computed an age past the window and were filtered out on arrival. That reads as "the chain died", the one conclusion this tool must not fabricate. The change is now broadcast on `/stream` as `event: geometry` and the tab raises a reload banner instead of emptying quietly. `?demo=1` also grows a clock-skewed node, since it was the one path that could not exercise the negative-offset rendering below. A negative `offset_ms` is documented as legitimate but was clamped to x=0, where an event 30ms before its boundary drew identically to one landing on it, inside the range this panel resolves. There is no room left of the lane labels to plot it, so it still clamps, but keeps its topic fill and gains a ring in the overflow accent: the same treatment the right edge already gives the mirror-image case. A missing `static_dir` started cleanly, logged that the dashboard was ready, and answered `GET /` with 404. It defaults to the relative `web`, so this fired on any invocation from outside `tooling/event-monitor/`; `validate` exists for exactly this class and now checks for `index.html`. The static fallback answered every unmatched path with `200 text/html`, so a typo'd asset surfaced as a JS parse error and a wrong API path lost a frozen contract's only wrong-path signal. This is one page, not a client-routed SPA: `/` is routed explicitly and the rest 404s. Bootstrap passed a malformed spec response straight through. Nothing crashed (`slot_at` floors the divisor, the frontend defaults it), but every dot piled against one edge unexplained, and a `genesis_time` off the wire could overflow the `offset_ms` multiply, a debug-build panic. One range check at the single point geometry enters the process covers both, and covers the config overrides too. A future `genesis_time` stays legal: a devnet is routinely configured before its genesis fires. Three smaller things. The reconnect loop announced `Reconnecting` on its way into every attempt, so a node that stayed down flipped its chip red/amber/red once per cycle; an unchanged state is no longer re-announced, while heartbeats still always publish their refreshed rate. `/api/history` shipped up to HISTORY_MAX_EVENTS that the panels discard against their own 2000-per-node cap, now capped per `(node, topic)` so an attestation flood cannot crowd blocks out of the propagation backfill either. The tooling CI steps pass `--locked`, so the committed Cargo.lock is actually enforced. CONTRACT.md gains `event: geometry` and the 404 rule in §4, the timeout and geometry-validation rationale in §2, and the clamped-negative dot plus the banner in §6.
1 participant
Summary
Adds
tooling/event-monitor/, a standalone live arrival-time dashboard forlean-consensus (ethlambda) nodes. It dials the
GET /lean/v0/eventsSSE streamof several nodes, timestamps each event on a single collector clock, and serves
a browser dashboard visualizing:
The rolling window is adjustable live from the header, and a fresh page load
backfills recent history from the collector so it is never blank.
Design
tooling/event-monitor/(empty[workspace]table); it is not a member of the parentethlambdaworkspace and does not affect the main build.CONTRACT.mdis the frozen interface between the Rust/axum backend (src/) and the vanilla-JS frontend (web/, no build step).?demo=1runs the frontend fully offline with synthetic data.This PR contains the tooling plus the CI wiring needed to actually check it. The
RPC / event-bus changes that make a node emit these events shipped separately in
the chain-events series (below).
Required ethlambda API functionality
All dependencies are satisfied on
main— the default dashboard view worksagainst a current node with no extra PRs.
GET /lean/v0/events?topics=<csv>GET /lean/v0/genesis,GET /lean/v0/config/specblock,head,justified_checkpoint,finalized_checkpointattestation,aggregateblock_gossipThe collector accepts
block,block_gossip,head,justified_checkpoint,finalized_checkpoint,attestationandaggregate; any other topic is loggedand skipped.
safe_target/chain_reorg(#533, closed) were removed in4eeafff — both panels' topic filters discarded them, and
chain_reorgis apoint-in-time event rather than a per-node arrival race, so it does not belong on
a beeswarm.
Both panels default to
block/attestation/aggregate.Review fixes
Three defects found in review, each with a regression test:
downwith 10s reconnects. Now keyed on session duration (survived ≥1 heartbeat). This also closes the inverse case: a peer that accepts the request and instantly closes the stream used to be retried atINITIAL_BACKOFFforever.MAX_FUTURE_SLOTSahead of the collector's own slot are now dropped, warning once per node per connection. Only the future side is bounded; old slots are legitimate (finalized_checkpointtrails head) and cannot move the watermark.statusimmediately but bufferschainduring backfill, so the/api/historysnapshot could overwrite a newer live status. Live status now wins.Also from that review:
offset_ms. Collectors re-read per frame and/api/metaper request; an already-open tab needs a reload to pick up new geometry.offset_msincludes the collector↔node round trip. One clock is what makes propagation deltas skew-free, but nodes reached over different links carry a systematic offset that reads as lag. Now documented inREADME.mdandCONTRACT.md §2.Arc-shared, sopublish_chainno longer deep-copies per event and/api/historyno longer clones up toHISTORY_MAX_EVENTSevents while holding the mutex.topics = []/nodes = []are rejected at load rather than becoming an opaque retry loop against a 400.Known and deliberate, left as follow-ups: the collector cannot see upstream
: error - dropped N messagescomments (eventsource-streamswallows them), soa lagging subscription silently under-samples; both canvases still repaint
unconditionally at 60fps; and
bootstraphas no retry, so the monitor exits ifstarted before its nodes are listening.
CI
tooling/event-monitordeclares its own[workspace], socargo fmt --all,cargo check --workspace,cargo clippy --workspace,make lintandmake testall stop at the root workspace members and never reached it — it landed with
nothing verifying it. The
Lintjob now also runscargo fmt --check,cargo clippy --all-targets -- -D warningsandcargo testunderworking-directory: tooling/event-monitor, andrust-cachelists it as a secondworkspace so its target dir is cached and its
Cargo.lockfeeds the cache key.Testing
cargo testintooling/event-monitor/: 41 passed (39 lib + 2 integration), 0 failed.cargo fmt --checkandcargo clippy --all-targets -- -D warnings: clean.?demo=1offline synthetic mode renders both panels with no live nodes.genesis_timemid-run is picked up within one refresh interval (WARN slot geometry changed),/api/metathen serves the new epoch, and the history ring holds the restarted chain's low slots with in-slot offsets — which only works because the watermark is reset, otherwise they would prune as older than the previous high-water mark.Run