fix(metrics): widen lean_block_building_time_seconds buckets past 1s - #574
Conversation
The leanMetrics bucket set for this histogram tops out at 1s, but block builds on our devnets routinely take 2-3s. Every sample therefore landed in `+Inf`, and `histogram_quantile` had no finite bucket left to interpolate in, so it returned the upper bound of the last finite bucket. The panel read as a flat 1s line regardless of how long builds actually took. Buckets now span 0.1s to 8s, the same range as `lean_block_proposal_attestation_build_phase_seconds`, whose phases this metric encloses. The tradeoff is sub-100ms resolution: attestation-free blocks skip the prover and build in single-digit milliseconds, and those now all collapse into the first bucket. The phase histogram still resolves that end, and it is not the range we need to watch. This deviates from the leanMetrics spec, so both the histogram and docs/metrics.md carry a note explaining why, to keep it from being "corrected" back to a set that cannot measure the thing. Divergent buckets do not affect other clients: Prometheus stores each bucket as its own series, and every boundary that was shared before (0.1 through 1) is still present. The leanMetrics dashboard panel for this metric queries per-instance rather than `sum by (le)` across clients, so each histogram stays internally consistent.
Greptile SummaryThe PR extends the
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified. The implementation and documentation consistently apply the widened histogram range, and no established consumer or contract is broken by the changed boundaries.
|
| Filename | Overview |
|---|---|
| crates/blockchain/src/metrics.rs | Widens the block-building histogram buckets and records why this metric intentionally differs from the specification. |
| docs/metrics.md | Updates the documented bucket set and clearly explains the intentional deviation. |
Reviews (1): Last reviewed commit: "fix(metrics): widen lean_block_building_..." | Re-trigger Greptile
🤖 Kimi Code ReviewThe change is correct and well-justified. Key observations:
No security vulnerabilities, memory safety issues, or performance regressions introduced. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
No code-correctness, security, memory-safety, or consensus-layer findings beyond that. This is a metrics-only change, and the rationale for widening the buckets is sound. Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview:
|
2 participants
🗒️ Description / Motivation
lean_block_building_time_secondsused the leanMetrics bucket set, whose top finitebound is 1s. Block builds on our devnets routinely take 2-3s, so in practice every
sample landed in the
+Infbucket.When a quantile falls in
+Inf,histogram_quantilehas no finite bucket left tointerpolate in and returns the upper bound of the last finite one. The p99 panel was
reporting a flat 1s line no matter how long builds actually took, so the metric could
not answer the question it exists to answer.
This widens the range to 0.1s - 8s, matching
lean_block_proposal_attestation_build_phase_seconds, whose phases this metricencloses. The sibling
lean_block_building_payload_aggregation_time_secondsalreadyreaches 4s, so a wider range on the enclosing timer is consistent with what's already
there.
What Changed
crates/blockchain/src/metrics.rs— buckets0.01…1→0.1, 0.25, 0.5, 0.75, 1, 2, 4, 8,plus a comment recording why this deviates from the spec set.
docs/metrics.md— updated the Block Production table row and added a note below thetable explaining the deviation.
Correctness / Behavior Guarantees
No behavior change: this is bucket boundaries on a histogram, nothing on the block
production path.
Tradeoff. Starting at 0.1 drops the
0.01 / 0.025 / 0.05boundaries.Attestation-free blocks skip the prover and build in single-digit milliseconds, and
those now all fall into the first bucket.
lean_block_proposal_attestation_build_phase_secondsstill resolves that end down to 0.001, and the sub-100ms region isn't the one we need
to watch, so this seemed like the right side to give up.
Cross-client impact: none. Prometheus stores each bucket as an independent series,
so other clients' exposition is untouched, and every boundary that was shared before
(
0.1through1) is still present here. Divergent bucket sets only corrupt a querythat does
sum by (le)across clients; I checked the leanMetrics dashboards, and thepanel for this metric queries per-instance
(
rate(..._bucket{network=~, job=~, instance=~})) with nosum by (le), so eachhistogram stays internally consistent.
rate(_sum)/rate(_count)is unaffected eitherway.
This does put us out of step with the leanMetrics spec, which is why both the code and
the docs carry a note: without one, this reads like drift and gets "corrected" back to
a set that can't measure the thing. Worth proposing the wider range upstream so the
other clients get a usable panel too.
Tests Added / Run
No new tests: bucket boundaries have no assertions to make against them.
make fmt— cleanmake lint— cleanmake test— 30/30 test binaries pass, 0 failures (fork-choice, STF, signature, andSSZ spec tests included)
Related Issues / PRs
✅ Verification Checklist
make fmt— cleanmake lint(clippy with-D warnings) — cleanmake test(cargo test --workspace --profile release-fast) — all passing