From 8dd64d4dcd82af9aae54bfc5ce955ec8756c4509 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:27:06 +0100 Subject: [PATCH] fix(telemetry): add second-scale spanmetrics histogram buckets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P95 of second-scale spans was a meaningless interpolation. The spanmetrics histogram topped out at [.. 1s, 5s], so consensus.round (~3.9s) and consensus.establish (~1.9s) all fell into one 1s-5s bucket and histogram_quantile interpolated linearly across that 4s-wide gap — the "Build vs Close" / "Ledger Close Duration" panels' P95 read ~4800ms purely as an artifact (verified: sum/count avg = 3824ms). ledger.acquire was worse: ~17% of samples exceeded the 5s ceiling, so its p95/p99 were unmeasurable. Add 2s, 3s, 4s (resolve the 1-5s pile-up) and 10s, 30s (give the ledger.acquire catch-up tail a measurable home). All ten existing boundaries are preserved and the list stays strictly ascending (the connector binary-searches buckets and silently misbuckets otherwise). Pin unit=ms so a future collector default-unit flip can't rename the metric to _seconds. Buckets chosen from the live mainnet distribution, not guessed. Native beast::insight histograms (ms-scale RPC/IO timers in Telemetry.cpp) are 100% under 5s, so they keep the original buckets — this is collector-only. Applies on collector restart (cumulative series reset once, handled by rate()). Runbook and regression-threshold bucket notes updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../otel-collector-config.grafanacloud.yaml | 30 ++++++++++++++++++- docker/telemetry/otel-collector-config.yaml | 30 ++++++++++++++++++- .../workload/regression-thresholds.json | 2 +- docs/telemetry-runbook.md | 12 ++++++-- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/docker/telemetry/otel-collector-config.grafanacloud.yaml b/docker/telemetry/otel-collector-config.grafanacloud.yaml index a78bdf751f..20b93c7c61 100644 --- a/docker/telemetry/otel-collector-config.grafanacloud.yaml +++ b/docker/telemetry/otel-collector-config.grafanacloud.yaml @@ -144,8 +144,36 @@ connectors: - deployment.environment - xrpl.network.type histogram: + # Pin unit=ms so the metric stays span_duration_milliseconds_* and le + # labels stay in ms even if a future collector flips the default to + # seconds (connector.spanmetrics.useSecondAsDefaultMetricsUnit gate). + unit: ms explicit: - buckets: [1ms, 5ms, 10ms, 25ms, 50ms, 100ms, 250ms, 500ms, 1s, 5s] + # Buckets MUST stay strictly ascending (the connector binary-searches + # them and silently misbuckets otherwise). Sub-second boundaries cover + # RPC/tx/ledger spans; the 2s-4s boundaries resolve second-scale + # consensus spans (consensus.round ~3.9s, consensus.establish ~1.9s) + # that used to pile into a single 1s-5s bucket; 10s/30s give the + # ledger.acquire tail (~17% exceeds 5s during catch-up) a measurable + # home so its p95/p99 stop reading as +Inf. + buckets: + [ + 1ms, + 5ms, + 10ms, + 25ms, + 50ms, + 100ms, + 250ms, + 500ms, + 1s, + 2s, + 3s, + 4s, + 5s, + 10s, + 30s, + ] dimensions: - name: command - name: rpc_status diff --git a/docker/telemetry/otel-collector-config.yaml b/docker/telemetry/otel-collector-config.yaml index a2d1967f66..03ddeac31e 100644 --- a/docker/telemetry/otel-collector-config.yaml +++ b/docker/telemetry/otel-collector-config.yaml @@ -123,8 +123,36 @@ connectors: - deployment.environment - xrpl.network.type histogram: + # Pin unit=ms so the metric stays span_duration_milliseconds_* and le + # labels stay in ms even if a future collector flips the default to + # seconds (connector.spanmetrics.useSecondAsDefaultMetricsUnit gate). + unit: ms explicit: - buckets: [1ms, 5ms, 10ms, 25ms, 50ms, 100ms, 250ms, 500ms, 1s, 5s] + # Buckets MUST stay strictly ascending (the connector binary-searches + # them and silently misbuckets otherwise). Sub-second boundaries cover + # RPC/tx/ledger spans; the 2s-4s boundaries resolve second-scale + # consensus spans (consensus.round ~3.9s, consensus.establish ~1.9s) + # that used to pile into a single 1s-5s bucket; 10s/30s give the + # ledger.acquire tail (~17% exceeds 5s during catch-up) a measurable + # home so its p95/p99 stop reading as +Inf. + buckets: + [ + 1ms, + 5ms, + 10ms, + 25ms, + 50ms, + 100ms, + 250ms, + 500ms, + 1s, + 2s, + 3s, + 4s, + 5s, + 10s, + 30s, + ] dimensions: - name: command - name: rpc_status diff --git a/docker/telemetry/workload/regression-thresholds.json b/docker/telemetry/workload/regression-thresholds.json index e0eabfb95a..ae6789f1ba 100644 --- a/docker/telemetry/workload/regression-thresholds.json +++ b/docker/telemetry/workload/regression-thresholds.json @@ -1,6 +1,6 @@ { "_description": "Per-metric regression thresholds. A metric regresses when current - baseline exceeds BOTH the percentage and absolute bounds (AND, not OR — this tolerates small-value noise). Defaults apply unless a per-metric override exists.", - "_bucket_note": "SpanMetrics latency histograms use explicit buckets [1,5,10,25,50,100,250,500,1000,5000]ms. A quantile sitting near a low-end boundary can jump a full bucket (e.g. 1ms->5ms) between runs with no real change, so absolute span bounds are set to ~2 low-end bucket widths (10ms) to tolerate that quantization noise while still catching genuine multi-bucket regressions. The job_queue running bound is widened similarly — per-ledger apply work scales with TxQ burst load.", + "_bucket_note": "SpanMetrics latency histograms use explicit buckets [1,5,10,25,50,100,250,500,1000,2000,3000,4000,5000,10000,30000]ms. A quantile sitting near a low-end boundary can jump a full bucket (e.g. 1ms->5ms) between runs with no real change, so absolute span bounds are set to ~2 low-end bucket widths (10ms) to tolerate that quantization noise while still catching genuine multi-bucket regressions. Second-scale consensus spans now have 2s/3s/4s boundaries (previously all fell in one 1s-5s bucket); their quantiles quantize to ~1s widths there. The job_queue running bound is widened similarly — per-ledger apply work scales with TxQ burst load.", "defaults": { "span": { "p50": { "max_pct_increase": 50.0, "max_abs_increase_ms": 10.0 }, diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index a727ca5574..8f7c2891be 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -717,12 +717,20 @@ does not apply to these dimensions. ### Histogram Buckets -Configured in `otel-collector-config.yaml`: +Configured in `otel-collector-config.yaml` (spanmetrics connector, `unit: ms`): ``` -1ms, 5ms, 10ms, 25ms, 50ms, 100ms, 250ms, 500ms, 1s, 5s +1ms, 5ms, 10ms, 25ms, 50ms, 100ms, 250ms, 500ms, 1s, 2s, 3s, 4s, 5s, 10s, 30s ``` +Sub-second boundaries cover RPC/tx/ledger spans; 2s-4s resolve second-scale +consensus spans (`consensus.round`, `consensus.establish`) that would otherwise +pile into one 1s-5s bucket and make `histogram_quantile` a meaningless +interpolation; 10s/30s give the `ledger.acquire` catch-up tail a measurable home. +Boundaries must stay strictly ascending. The native beast::insight histograms +(ms-scale RPC/IO timers) keep the original 1ms-5s buckets in +`Telemetry.cpp` — they never exceed 5s, so they need no high-range buckets. + ## System Metrics (OTel native -- beast::insight) xrpld has a built-in metrics framework (`beast::insight`) that exports metrics natively via OTLP to the OTel Collector. These complement the span-derived RED metrics by providing system-level gauges, counters, and timers that don't map to individual trace spans.