Commit b4a8f78
authored
fix(metrics): widen lean_block_building_time_seconds buckets past 1s (lambdaclass#574)
## 🗒️ Description / Motivation
`lean_block_building_time_seconds` used the leanMetrics bucket set,
whose top finite
bound is 1s. Block builds on our devnets routinely take 2-3s, so in
practice every
sample landed in the `+Inf` bucket.
When a quantile falls in `+Inf`, `histogram_quantile` has no finite
bucket left to
interpolate 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
metric
encloses. The sibling
`lean_block_building_payload_aggregation_time_seconds` already
reaches 4s, so a wider range on the enclosing timer is consistent with
what's already
there.
## What Changed
- `crates/blockchain/src/metrics.rs` — buckets `0.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 the
table 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.05`
boundaries.
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_seconds`
still 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.1` through `1`) is still present here. Divergent bucket sets only
corrupt a query
that does `sum by (le)` *across* clients; I checked the leanMetrics
dashboards, and the
panel for this metric queries per-instance
(`rate(..._bucket{network=~, job=~, instance=~})`) with no `sum by
(le)`, so each
histogram stays internally consistent. `rate(_sum)/rate(_count)` is
unaffected either
way.
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` — clean
- `make lint` — clean
- `make test` — 30/30 test binaries pass, 0 failures (fork-choice, STF,
signature, and
SSZ spec tests included)
## Related Issues / PRs
- Related to lambdaclass#566 (previous metrics work)
## ✅ Verification Checklist
- [x] Ran `make fmt` — clean
- [x] Ran `make lint` (clippy with `-D warnings`) — clean
- [x] Ran `make test` (`cargo test --workspace --profile release-fast`)
— all passing1 parent 4209de9 commit b4a8f78
2 files changed
Lines changed: 11 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
460 | 460 | | |
461 | 461 | | |
462 | 462 | | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
463 | 466 | | |
464 | 467 | | |
465 | 468 | | |
466 | 469 | | |
467 | 470 | | |
468 | | - | |
| 471 | + | |
469 | 472 | | |
470 | 473 | | |
471 | 474 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
39 | | - | |
| 39 | + | |
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
48 | 54 | | |
49 | 55 | | |
50 | 56 | | |
| |||
0 commit comments