mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 22:30:57 +00:00
fix(telemetry): resolve microsecond latencies below 100us
The microsecond ladder's first edge was 100us, which sat ABOVE the mass of every instrument using it. Measured on devnet: 99.3% of job_queued_us samples, 92.5% of job_running_us and 90.4% of getobject_lookup_us fell in that first bucket. histogram_quantile then interpolated inside bucket 0 and returned `quantile / fraction_in_bucket_0 x first_edge` -- p75/p95/p99 of job_queued_us read 75.52/95.66/99.69us against a prediction of 75.53/95.67/99.70. Three-decimal agreement: those panels were reporting arithmetic on the bucket edge, not latency. The fix was already half-written. kSubMillisecondBoundaries had been parked in MetricsRegistry.cpp as [[maybe_unused]] with a comment noting exactly this problem for nodestore reads. Its edges are now folded into kMicrosecondBuckets rather than deleted, so the parked intent is carried forward: 1..1000us resolution where the mass is, upper edges unchanged so multi-second stalls stay measurable. Also moves the GetObject count and charge ladders into HistogramBuckets.h, so all five ladders have one owner and one set of invariant tests (29 now). Adds check_bucket_parity.py, wired into the existing OTel naming workflow. The C++ millisecond ladder and the collector's spanmetrics ladder are specified to agree over their shared range; they were identical when shipped, then the collector side alone was extended and nothing noticed for eleven phases. The check asserts containment rather than equality, because jobs outlive spans -- jobq_updatepaths averages ~60s, which no span approaches, so demanding equality would force a ceiling that censors it. Verified it rejects a missing collector edge, a bogus in-range edge, and a return to the 5s ceiling. ledger-data-sync's "Job Queue Wait p95 By Type" moves off the beast jobq_*_q_milliseconds pair onto job_queued_us filtered by job_type. Those beast metrics are ms-quantised at the source (Event rounds up to a whole millisecond), so 94-100% of their samples sat in the first bucket and no ladder change could fix them. Note the label values are camelCase (job_type="ledgerData"), not the lowercase metric-name fragments. Both histogram-fed alert thresholds re-validated and left unchanged, with the measured basis recorded so neither gets tuned against the old artefact: only 0.0022% of job_queued_us samples exceed the 1s threshold, and every edge bracketing the 1000ms ios_latency threshold survived the ladder change. Docs: the rpc_size "known issue -- tracked separately" notes in the runbook and 09-data-collection-reference are now resolved notes, the stale 10-edge span_duration bucket list is corrected to the collector's real 20, and the runbook gains a "Reading A Histogram Percentile" section covering both saturation traps and the expected discontinuity after a ladder change.
This commit is contained in:
128
.github/scripts/telemetry/check_bucket_parity.py
vendored
Executable file
128
.github/scripts/telemetry/check_bucket_parity.py
vendored
Executable file
@@ -0,0 +1,128 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Assert the C++ millisecond ladder agrees with the collector's spanmetrics ladder.
|
||||
|
||||
The two are specified to match so a span-derived latency panel and a native
|
||||
histogram panel can be read on the same scale. They *were* identical when first
|
||||
shipped. Then the collector ladder alone was extended -- sub-millisecond edges
|
||||
below 1ms and second-scale edges up to 30s -- and nothing checked the other
|
||||
side, so the C++ ladder stayed capped at 5s. Every quantile above 5s then read
|
||||
back as a flat 5000, because Prometheus returns the second-highest edge for a
|
||||
quantile landing in the `+Inf` bucket. That looks like a measurement rather
|
||||
than an error, which is why it survived for eleven phases.
|
||||
|
||||
The rule is containment, not equality:
|
||||
|
||||
* every representable collector edge MUST appear in the C++ ladder, so the
|
||||
shared range reads identically;
|
||||
* the C++ ladder MAY carry extra edges ABOVE the collector's highest edge,
|
||||
because jobs outlive spans -- the updatepaths job type was measured
|
||||
averaging ~60s, which no span approaches. Demanding equality would force a
|
||||
ceiling that censors it, reintroducing the bug this guards against;
|
||||
* collector edges below 1ms are expected to be ABSENT rather than missing:
|
||||
beast::insight::Event rounds every duration up to a whole millisecond
|
||||
before it reaches the histogram, so those edges could never collect a
|
||||
sample.
|
||||
|
||||
Exit 0 when the ladders agree, 1 with a diff when they do not.
|
||||
"""
|
||||
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
HEADER = Path("include/xrpl/telemetry/HistogramBuckets.h")
|
||||
COLLECTOR = Path("docker/telemetry/otel-collector-config.yaml")
|
||||
|
||||
# beast::insight::Event applies ceil<milliseconds>, so anything below 1ms
|
||||
# collapses onto the 1ms edge.
|
||||
REPRESENTABLE_FLOOR_MS = 1.0
|
||||
|
||||
UNIT_TO_MS = {"ms": 1.0, "s": 1000.0}
|
||||
|
||||
|
||||
def collector_edges_ms():
|
||||
"""Parse the spanmetrics bucket list, normalising each edge to milliseconds."""
|
||||
text = COLLECTOR.read_text()
|
||||
match = re.search(r"buckets:\s*\[(.*?)\]", text, re.S)
|
||||
if not match:
|
||||
sys.exit(f"{COLLECTOR}: no 'buckets:' list found")
|
||||
|
||||
edges = []
|
||||
for raw in match.group(1).split(","):
|
||||
token = raw.strip()
|
||||
if not token:
|
||||
continue
|
||||
parsed = re.fullmatch(r"([0-9.]+)(ms|s)", token)
|
||||
if not parsed:
|
||||
sys.exit(f"{COLLECTOR}: cannot parse bucket edge {token!r}")
|
||||
edges.append(float(parsed.group(1)) * UNIT_TO_MS[parsed.group(2)])
|
||||
return edges
|
||||
|
||||
|
||||
def cpp_edges_ms():
|
||||
"""Parse kMillisecondBuckets out of the header that owns every ladder."""
|
||||
text = HEADER.read_text()
|
||||
match = re.search(r"kMillisecondBuckets\{(.*?)\};", text, re.S)
|
||||
if not match:
|
||||
sys.exit(f"{HEADER}: kMillisecondBuckets not found")
|
||||
return [
|
||||
float(token.strip().replace("'", ""))
|
||||
for token in match.group(1).split(",")
|
||||
if token.strip()
|
||||
]
|
||||
|
||||
|
||||
def main():
|
||||
collector = collector_edges_ms()
|
||||
cpp = cpp_edges_ms()
|
||||
required = [edge for edge in collector if edge >= REPRESENTABLE_FLOOR_MS]
|
||||
if not required:
|
||||
sys.exit(f"{COLLECTOR}: no edges at or above {REPRESENTABLE_FLOOR_MS} ms")
|
||||
collector_top = max(required)
|
||||
|
||||
missing = [edge for edge in required if edge not in cpp]
|
||||
# An extra C++ edge inside the collector's range means the two scales
|
||||
# disagree where they overlap. Above the collector's top it is a deliberate
|
||||
# extension.
|
||||
inside_range = [e for e in cpp if e not in required and e < collector_top]
|
||||
|
||||
if not missing and not inside_range:
|
||||
extensions = [e for e in cpp if e > collector_top]
|
||||
summary = f"OK: all {len(required)} representable collector edges present"
|
||||
if extensions:
|
||||
pretty = ", ".join(f"{e:g}" for e in extensions)
|
||||
summary += (
|
||||
f"; {len(extensions)} extension edge(s) above "
|
||||
f"{collector_top:g} ms: [{pretty}]"
|
||||
)
|
||||
print(summary)
|
||||
return 0
|
||||
|
||||
print("Bucket ladder parity violated.", file=sys.stderr)
|
||||
print(
|
||||
f" collector (>= {REPRESENTABLE_FLOOR_MS:g} ms): "
|
||||
f"{[f'{e:g}' for e in required]}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
f" HistogramBuckets.h : {[f'{e:g}' for e in cpp]}", file=sys.stderr
|
||||
)
|
||||
for edge in missing:
|
||||
print(f" MISSING from the C++ ladder: {edge:g} ms", file=sys.stderr)
|
||||
for edge in inside_range:
|
||||
print(
|
||||
f" C++ edge {edge:g} ms lies inside the collector's range but is not "
|
||||
"a collector edge -- add it to the collector or drop it here",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
"\nThe two ladders must agree over their shared range. Extra C++ edges are\n"
|
||||
"permitted only ABOVE the collector's highest edge. Change both sides, or\n"
|
||||
"change the spec in OpenTelemetryPlan/Phase7_taskList.md.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
1
.github/workflows/on-pr.yml
vendored
1
.github/workflows/on-pr.yml
vendored
@@ -72,6 +72,7 @@ jobs:
|
||||
.github/scripts/levelization/**
|
||||
.github/scripts/otel-naming/**
|
||||
.github/scripts/rename/**
|
||||
.github/scripts/telemetry/**
|
||||
.github/workflows/reusable-check-levelization.yml
|
||||
.github/workflows/reusable-check-otel-naming.yml
|
||||
.github/workflows/reusable-check-rename.yml
|
||||
|
||||
@@ -33,3 +33,11 @@ jobs:
|
||||
# it enforces each rule only when the layer it needs is present, so it
|
||||
# works whether telemetry changes land in one PR or several.
|
||||
run: python .github/scripts/otel-naming/check_otel_naming.py
|
||||
- name: Check histogram bucket parity
|
||||
# The C++ millisecond ladder and the collector's spanmetrics ladder are
|
||||
# specified to agree over their shared range. They were identical when
|
||||
# first shipped, then the collector side alone was extended and nothing
|
||||
# noticed for eleven phases: native histograms stayed capped at 5s while
|
||||
# spans reached 30s, so every quantile above 5s reported a flat 5000.
|
||||
# Nothing but a check keeps two lists in step.
|
||||
run: python .github/scripts/telemetry/check_bucket_parity.py
|
||||
|
||||
Reference in New Issue
Block a user