mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-29 02:00:56 +00:00
fix(telemetry): stop gating ledger.validate p95 and p99, which vary too much
The regression gate has been red on runs with no code change. Only two of the 25 gated keys ever tripped, both on the same span and never together: run 32862589645 failed p99 at 25.8750 ms against a 1.0600 ms baseline (+2341%), run 32867433073 failed p95 at 0.7500 ms against 0.2404 ms (+212%), and in each run the other quantile sat well inside its own bound. A real slowdown would move both. This is variance, not a defect. Measured across four CI runs: span.ledger.validate.p50 0.0484 to 0.0778 ms 1.6x spread kept span.ledger.validate.p95 0.1281 to 0.7500 ms 5.9x spread excluded span.ledger.validate.p99 0.3875 to 25.8750 ms 66.8x spread excluded Both excluded quantiles reach past their trip point on a healthy run. The mechanism is arrival timing, not slow code: the span opens only once a quorum-completing validation arrives (LedgerMaster.cpp:987, inside checkAccept, past the early return) and wraps the promotion work that follows, so one slow consensus round dominates the tail of a 3m rate window and which round that is differs every run. Widening is not available and must not be attempted later: tolerating 25.8750 ms against a 1.0600 ms baseline needs a bound of about 24.8 ms, which gates nothing. A bound admitting every healthy run's worst case admits every regression too. p50 stays gated; it is stable. THE GENERAL RULE, recorded so this does not recur: an absolute bound derived as hi_next minus baseline comes from the histogram ladder, so it budgets for quantization noise and for nothing else. It knows nothing about how far a metric moves between runs on identical code. Before gating any key, check its observed maximum across several runs against its trip point and gate it only with margin. Spread alone proves nothing: tx.apply.p50 swings 364x and never fires, because its 5 ms trip point absorbs the range. Of the 23 keys still gated the worst reaches 0.67 of its trip point. Mechanism: spans.names lists span names while _quantiles is shared, so dropping two quantiles of one span cannot be expressed by deleting a name. regression-metrics.json gains an excluded_keys map from a flat key to the reason it is not gated, subtracted by both prom_queries.py (so the key is never queried) and check_regression_bounds.py rule A. A per-name quantile override was rejected: a typo there leaves the key gating, whereas a typo in an exclusion subtracts nothing and new rule F rejects it, along with an empty reason, a leftover threshold override and a leftover baseline value. Derived figures recomputed from the committed baseline: 25 gated keys to 23, detection floor 2.02x-9.43x to 2.02x-9.42x, weakly guarded keys ten to nine, bound over baseline 102%-843% to 102%-842%. The baseline edit is a deletion of two entries only, with no value rewritten. Verified: both previously failing runs replay to zero regressions and exit 0; a tenfold increase injected into each of the 23 remaining keys in turn is still caught in all 23 cases; rule F was confirmed load-bearing by stubbing it out, which lets a stale exclusion pass.
This commit is contained in:
Pratik Mankawde
@@ -26,7 +26,7 @@ bucket ``(lo, hi]`` of its ladder, with ``hi_next`` the next edge above ``hi``,
|
||||
max_abs_increase_* == hi_next - baseline
|
||||
|
||||
so the gate trips only when the reading clears the bucket *above* the
|
||||
baseline's own. Five rules are checked:
|
||||
baseline's own. Six rules are checked:
|
||||
|
||||
A the baseline's key set equals the surface ``regression-metrics.json``
|
||||
declares (a stale key left behind reads as covered but never gates);
|
||||
@@ -38,7 +38,15 @@ baseline's own. Five rules are checked:
|
||||
stops being true;
|
||||
E no baseline carries the ladder-floor signature ``quantile x first_edge``,
|
||||
which means every sample landed in the first bucket and the number is
|
||||
interpolation arithmetic rather than a latency.
|
||||
interpolation arithmetic rather than a latency;
|
||||
F every entry in ``excluded_keys`` names a key the surface would otherwise
|
||||
declare, carries a reason, and has neither a threshold override nor a
|
||||
baseline value left behind.
|
||||
|
||||
Rule A subtracts ``excluded_keys`` before comparing, so a quantile removed from
|
||||
the gated set does not read as a missing baseline. Rule F is what keeps that
|
||||
subtraction honest: an exclusion is the one edit here that makes the gate cover
|
||||
LESS, so a stale or misspelt entry must fail rather than silently widen itself.
|
||||
|
||||
A PLACEHOLDER baseline -- ``"placeholder": true`` or an empty ``metrics``
|
||||
object -- exits 0, because that is the documented bootstrap state and CI has to
|
||||
@@ -119,6 +127,10 @@ def declared_keys(metrics_cfg):
|
||||
Deliberately reimplemented rather than imported from ``prom_queries.py``,
|
||||
which pulls in aiohttp; CI telemetry checks stay dependency-free. The key
|
||||
format is fixed by that file's own ``_key_format`` field.
|
||||
|
||||
``excluded_keys`` is NOT subtracted here: rule F needs the full product to
|
||||
tell a real exclusion from a misspelt one. Callers that want the gated
|
||||
surface subtract it themselves.
|
||||
"""
|
||||
keys = set()
|
||||
spans = metrics_cfg.get("spans", {})
|
||||
@@ -154,6 +166,57 @@ def resolve_override(key, thresholds):
|
||||
return thresholds.get("overrides", {}).get(group, {}).get(quantile)
|
||||
|
||||
|
||||
def check_exclusions(metrics_cfg, thresholds, baseline_metrics, declared):
|
||||
"""Apply rule F to every entry in ``excluded_keys``.
|
||||
|
||||
An exclusion is the only edit to this config that makes the gate cover
|
||||
LESS, so each entry has to prove it is deliberate and complete:
|
||||
|
||||
* it names a key the names x quantiles product would otherwise declare,
|
||||
so a typo or a stale entry surviving a surface change is caught rather
|
||||
than silently subtracting nothing;
|
||||
* it carries a non-empty reason, because "why is this not gated" is the
|
||||
question a future maintainer will ask and prose is the only answer;
|
||||
* no threshold override and no baseline value are left behind, since
|
||||
either would read as gated to anyone grepping for the key.
|
||||
|
||||
Args:
|
||||
metrics_cfg: Parsed regression-metrics.json.
|
||||
thresholds: Parsed regression-thresholds.json.
|
||||
baseline_metrics: The baseline's ``metrics`` map.
|
||||
declared: Output of declared_keys(), before exclusions come off.
|
||||
|
||||
Returns:
|
||||
A list of failure strings, empty when every entry is well formed.
|
||||
"""
|
||||
failures = []
|
||||
for key, reason in sorted(metrics_cfg.get("excluded_keys", {}).items()):
|
||||
if key not in declared:
|
||||
failures.append(
|
||||
f"{key}: listed in excluded_keys but not produced by the "
|
||||
f"names x quantiles product, so it subtracts nothing -- fix the "
|
||||
f"spelling or drop the entry (rule F)"
|
||||
)
|
||||
continue
|
||||
if not isinstance(reason, str) or not reason.strip():
|
||||
failures.append(
|
||||
f"{key}: excluded with no reason. Record why it is not gated, "
|
||||
f"with the measurement behind it (rule F)"
|
||||
)
|
||||
if resolve_override(key, thresholds) is not None:
|
||||
failures.append(
|
||||
f"{key}: excluded but still has a threshold override, which "
|
||||
f"reads as gated -- remove it from {THRESHOLDS} (rule F)"
|
||||
)
|
||||
if key in baseline_metrics:
|
||||
failures.append(
|
||||
f"{key}: excluded but still has a baseline value, so rule A "
|
||||
f"would pass while nothing gates it -- remove it from "
|
||||
f"{BASELINE} (rule F)"
|
||||
)
|
||||
return failures
|
||||
|
||||
|
||||
def check_key(key, entry, thresholds, ladders):
|
||||
"""Apply rules B, C, D and E to one gated key. Returns a list of failures."""
|
||||
value, unit = entry.get("value"), entry.get("unit", "")
|
||||
@@ -242,6 +305,10 @@ def main():
|
||||
failures = []
|
||||
|
||||
declared = declared_keys(metrics_cfg)
|
||||
failures.extend(check_exclusions(metrics_cfg, thresholds, gated, declared))
|
||||
# Rule A compares against the GATED surface, so a deliberately excluded
|
||||
# quantile is not reported as a baseline that was never captured.
|
||||
declared -= set(metrics_cfg.get("excluded_keys", {}))
|
||||
for key in sorted(set(gated) - declared):
|
||||
failures.append(
|
||||
f"{key}: in the baseline but not declared by {METRICS}, so it is "
|
||||
@@ -258,11 +325,19 @@ def main():
|
||||
failures.extend(check_key(key, gated[key], thresholds, ladders))
|
||||
|
||||
if not failures:
|
||||
excluded = metrics_cfg.get("excluded_keys", {})
|
||||
print(
|
||||
f"OK: {len(gated)} gated key(s); every absolute bound equals "
|
||||
f"hi_next - baseline, every key has an override, and the absolute "
|
||||
f"bound is the operative half of the AND for all of them"
|
||||
)
|
||||
# Printed, not silent: an exclusion narrows the gate, so the count
|
||||
# belongs in the CI log where a reviewer sees it without opening a file.
|
||||
if excluded:
|
||||
print(
|
||||
f" {len(excluded)} declared key(s) deliberately not gated: "
|
||||
f"{', '.join(sorted(excluded))} (see excluded_keys in {METRICS})"
|
||||
)
|
||||
return 0
|
||||
|
||||
print(
|
||||
|
||||
@@ -13,7 +13,7 @@ Two groups:
|
||||
is the documented bootstrap state, while a missing, unreadable or malformed
|
||||
input must FAIL. A checker that returns success without having checked
|
||||
anything is the failure this whole gate exists to prevent;
|
||||
* one case per rule (A to E), so a rule that stops flagging is caught.
|
||||
* one case per rule (A to F), so a rule that stops flagging is caught.
|
||||
|
||||
stdlib unittest only; the repo installs no third-party runner for CI.
|
||||
"""
|
||||
@@ -193,6 +193,66 @@ class TestRules(CheckerCase):
|
||||
self.assertEqual(code, 1, out)
|
||||
self.assertIn("(rule D)", out)
|
||||
|
||||
def test_rule_a_ignores_an_excluded_key(self):
|
||||
"""An excluded key must not read as a baseline that was never captured.
|
||||
|
||||
The unmodified tree already exercises this — span.ledger.validate p95
|
||||
and p99 are declared by the names x quantiles product, excluded, and
|
||||
absent from the baseline — so this asserts the subtraction is what makes
|
||||
it pass, by naming the keys in the reported exclusion line.
|
||||
"""
|
||||
code, out = self.run_checker()
|
||||
self.assertEqual(code, 0, out)
|
||||
self.assertIn("span.ledger.validate.p95", out)
|
||||
self.assertIn("deliberately not gated", out)
|
||||
|
||||
def test_rule_f_flags_exclusion_that_subtracts_nothing(self):
|
||||
"""A misspelt or stale exclusion silently narrows nothing — catch it."""
|
||||
self.edit_json(
|
||||
METRICS,
|
||||
lambda d: d["excluded_keys"].update({"span.ledger.validate.p97": "typo"}),
|
||||
)
|
||||
code, out = self.run_checker()
|
||||
self.assertEqual(code, 1, out)
|
||||
self.assertIn("(rule F)", out)
|
||||
self.assertIn("subtracts nothing", out)
|
||||
|
||||
def test_rule_f_flags_exclusion_without_a_reason(self):
|
||||
self.edit_json(
|
||||
METRICS,
|
||||
lambda d: d["excluded_keys"].update({"span.ledger.validate.p95": " "}),
|
||||
)
|
||||
code, out = self.run_checker()
|
||||
self.assertEqual(code, 1, out)
|
||||
self.assertIn("(rule F)", out)
|
||||
self.assertIn("no reason", out)
|
||||
|
||||
def test_rule_f_flags_override_left_behind(self):
|
||||
"""An excluded key still carrying a bound reads as gated."""
|
||||
self.edit_json(
|
||||
THRESHOLDS,
|
||||
lambda d: d["overrides"]["span.ledger.validate"].update(
|
||||
{"p95": {"max_pct_increase": 50.0, "max_abs_increase_ms": 0.25}}
|
||||
),
|
||||
)
|
||||
code, out = self.run_checker()
|
||||
self.assertEqual(code, 1, out)
|
||||
self.assertIn("(rule F)", out)
|
||||
self.assertIn("still has a threshold override", out)
|
||||
|
||||
def test_rule_f_flags_baseline_value_left_behind(self):
|
||||
"""Excluded but still in the baseline: rule A passes, nothing gates."""
|
||||
self.edit_json(
|
||||
BASELINE,
|
||||
lambda d: d["metrics"].update(
|
||||
{"span.ledger.validate.p95": {"unit": "ms", "value": 0.24}}
|
||||
),
|
||||
)
|
||||
code, out = self.run_checker()
|
||||
self.assertEqual(code, 1, out)
|
||||
self.assertIn("(rule F)", out)
|
||||
self.assertIn("still has a baseline value", out)
|
||||
|
||||
def test_rule_e_flags_ladder_floor_signature(self):
|
||||
"""ledger.store's quantiles were the ladder floor times the quantile."""
|
||||
store = {"p50": 0.005, "p95": 0.0095, "p99": 0.0099}
|
||||
|
||||
@@ -239,7 +239,8 @@ How it runs inside the validation pipeline:
|
||||
|
||||
1. `run-full-validation.sh` executes the normal workload and validation suite.
|
||||
2. After validation, `capture_timings.py` queries Prometheus for every
|
||||
metric in `regression-metrics.json` and writes `reports/timings.json`.
|
||||
metric `regression-metrics.json` declares and does not list in
|
||||
`excluded_keys`, then writes `reports/timings.json`.
|
||||
3. `compare_to_baseline.py` reads `timings.json`,
|
||||
`baselines/baseline-timings.json`, and `regression-thresholds.json`,
|
||||
then either:
|
||||
@@ -269,6 +270,12 @@ Per-run tuning:
|
||||
top of the next bucket up — so refreshing the baseline obliges you to
|
||||
re-derive the bounds. See `_absolute_bound_derivation` in that file;
|
||||
`.github/scripts/telemetry/check_regression_bounds.py` enforces it in CI.
|
||||
- That bound budgets for **quantization** noise only, so a key whose run-to-run
|
||||
variance is larger than it cannot be gated at all — `span.ledger.validate.p95`
|
||||
and `.p99` are excluded for that reason and are listed, with the measurements,
|
||||
in `excluded_keys` in `regression-metrics.json`. Check a key's observed maximum
|
||||
across runs against `baseline + bound` before gating it; widening the bound is
|
||||
not the fix. See `baselines/README.md`.
|
||||
|
||||
See [`baselines/README.md`](./baselines/README.md) for the baseline
|
||||
lifecycle and refresh process.
|
||||
|
||||
@@ -25,9 +25,9 @@ was invoked with. Capture and comparison are profile-agnostic — they only read
|
||||
Prometheus — so all existing profiles (`full-validation`, `quick-smoke`, `stress`)
|
||||
continue to work unchanged.
|
||||
|
||||
## Current state: 25 metrics gate, on a baseline captured 2026-08-24
|
||||
## Current state: 23 metrics gate, on a baseline captured 2026-08-24
|
||||
|
||||
`baseline-timings.json` holds real captured values for the 25 keys the harness gates. The
|
||||
`baseline-timings.json` holds real captured values for the 23 keys the harness gates. The
|
||||
previous entries, captured on 2026-06-05, were voided into a placeholder first: they predated the
|
||||
spanmetrics ladder re-cut of 2026-08-04 (`3860c93db2`), which made every sub-millisecond quantile
|
||||
in that capture bucket-edge arithmetic rather than a latency (a p95 of `0.95` ms is `0.95 × 1 ms`).
|
||||
@@ -64,7 +64,15 @@ Two earlier generations of this bound were wrong, in opposite directions:
|
||||
| ------------------------------ | -------------------------------------------- | --------------------- | ---------------------------------------- |
|
||||
| flat | 10 ms `p50`/`p95`, 15 ms `p99`, 20000 us job | 5 / 28 keys | 2 / 25 keys |
|
||||
| 2 × enclosing bucket width | per metric | 28 / 28 keys | **21 / 25 keys** |
|
||||
| `hi_next − baseline` (current) | per metric | 25 / 25 keys | **0 / 25 keys** |
|
||||
| `hi_next − baseline` (current) | per metric | 23 / 23 keys | **0 / 23 keys** |
|
||||
|
||||
The first two rows were measured when 28 and 25 keys were gated; the current row was re-measured
|
||||
over today's 23 by injecting a 10x regression into each gated key in turn against a real CI
|
||||
`timings.json`, which the gate flagged in all 23 cases. The zero in the last column is by
|
||||
construction rather than by sampling: rule C in
|
||||
[`check_regression_bounds.py`](../../../../.github/scripts/telemetry/check_regression_bounds.py)
|
||||
fails the build unless every trip point is exactly `hi_next`, and a trip point at a bucket edge
|
||||
cannot be crossed by interpolation inside that bucket.
|
||||
|
||||
The flat bound was calibrated for a 5-25 ms band the spans do not occupy: 18 of the 28
|
||||
quantiles gated at the time sat below 1 ms, so it sat 1.15x to 2000x above the metric it guarded,
|
||||
@@ -86,11 +94,10 @@ signature described below.
|
||||
|
||||
The guarantee costs sensitivity where the ladder is coarse: the detection floor is
|
||||
`hi_next / baseline`, so a baseline sitting just above an edge is guarded loosely. Measured over
|
||||
the current baseline the floor ranges 2.02x to 9.43x. Do **not** read these as guarded:
|
||||
the current baseline the floor ranges 2.02x to 9.42x. Do **not** read these as guarded:
|
||||
|
||||
| key | baseline | fires at | floor |
|
||||
| --------------------------------- | ---------- | --------- | ----- |
|
||||
| `span.ledger.validate.p99` | 1.0600 ms | 10 ms | 9.43x |
|
||||
| `span.ledger.build.p50` | 1.0612 ms | 10 ms | 9.42x |
|
||||
| `span.tx.process.p95` | 0.7240 ms | 5 ms | 6.91x |
|
||||
| `span.tx.apply.p50` | 0.7917 ms | 5 ms | 6.32x |
|
||||
@@ -103,7 +110,7 @@ the current baseline the floor ranges 2.02x to 9.43x. Do **not** read these as g
|
||||
|
||||
`span.ledger.build.p50` is the one that matters most: ledger construction is the hot path this
|
||||
gate exists to guard, and at a 9.42x floor it could get almost ten times slower and still pass.
|
||||
All ten are limited by two 5x-wide
|
||||
All nine are limited by two 5x-wide
|
||||
ladder steps, 1 ms → 5 ms and 5000 us → 25000 us. The fix is a 2 ms edge (ideally 3 ms as well) in
|
||||
the collector's spanmetrics `buckets` list plus the matching entries in `kMillisecondBuckets`, and
|
||||
a 10000 us edge in `kMicrosecondBuckets`. That work belongs to the branch that owns the ladders.
|
||||
@@ -129,6 +136,58 @@ presence is still asserted by `../expected_spans.json` and `docker/telemetry/int
|
||||
and its rate is still on the ledger-operations dashboard; only the latency gate drops it.
|
||||
`check_regression_bounds.py` rule E fails the build if a key with this signature is gated again.
|
||||
|
||||
## Known exclusion: `ledger.validate` p95 and p99 vary more than any bound can absorb
|
||||
|
||||
`span.ledger.validate.p95` and `.p99` are **not** gated. `p50` still is. They are the first
|
||||
exclusion at _quantile_ rather than _span_ granularity, which is why
|
||||
[`../regression-metrics.json`](../regression-metrics.json) grew an `excluded_keys` map — `spans.names`
|
||||
lists span names and `_quantiles` is shared across all of them, so removing two quantiles of one
|
||||
span cannot be expressed by deleting a name.
|
||||
|
||||
Measured across four CI runs:
|
||||
|
||||
| key | baseline | trip point | observed min | observed max | spread |
|
||||
| --------------------------------- | --------- | ---------- | ------------ | ------------ | ------ |
|
||||
| `span.ledger.validate.p50` (kept) | 0.0779 ms | 0.25 ms | 0.0484 ms | 0.0778 ms | 1.6x |
|
||||
| `span.ledger.validate.p95` | 0.2404 ms | 0.5 ms | 0.1281 ms | 0.7500 ms | 5.9x |
|
||||
| `span.ledger.validate.p99` | 1.0600 ms | 10 ms | 0.3875 ms | 25.8750 ms | 66.8x |
|
||||
|
||||
Both excluded quantiles reach past their trip point on an ordinary run, so CI reddened twice with
|
||||
no code change: run `32867433073` read `p95` = 0.7500 ms (+212%) and run `32862589645` read
|
||||
`p99` = 25.8750 ms (+2341%). The two failures landed on **different** quantiles in different runs
|
||||
while the other quantile stayed well inside its bound in the same run — the signature of variance,
|
||||
not of a regression.
|
||||
|
||||
The mechanism is arrival timing, not slow code. The span opens only once a quorum-completing
|
||||
validation arrives ([`LedgerMaster.cpp:987`](../../../../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L987),
|
||||
inside `checkAccept`, past the `tvc < minVal` early return) and wraps the promotion work that
|
||||
follows — `setValidated`, `setFull`, `setValidLedger`, `pendSaveValidated`. Its duration therefore
|
||||
tracks when peer validations arrive in a 5-node cluster and what promotion then schedules, so a
|
||||
single slow consensus round dominates the tail of a 3 m rate window, and which round that is
|
||||
differs every run.
|
||||
|
||||
**Widening the bound is not an option and must not be attempted.** Tolerating 25.8750 ms against a
|
||||
1.0600 ms baseline needs a bound of ~24.8 ms, i.e. a gate that fires at nothing a regression could
|
||||
plausibly reach. A bound that admits every healthy run's worst case admits every regression too.
|
||||
`check_regression_bounds.py` rule F fails the build if either key is re-gated with a bound while
|
||||
still listed in `excluded_keys`, and the per-key reasons in that map record this in full.
|
||||
|
||||
### The general rule this exposed
|
||||
|
||||
`hi_next − baseline` is derived from the **ladder**, so it budgets for **quantization** noise — one
|
||||
bucket of interpolation headroom — and for nothing else. It knows nothing about how far the metric
|
||||
itself moves between runs on identical code. Where run-to-run workload variance is the larger term,
|
||||
the bound is simply the wrong size and the gate reddens on a healthy run.
|
||||
|
||||
**Before gating any key, check its observed maximum across several runs against its trip point
|
||||
(`baseline + bound`), and gate it only if the maximum stays below that with margin.** Spread alone
|
||||
proves nothing: `span.tx.apply.p50` swings 364x across runs (0.0064 → 2.3378 ms) and never fires,
|
||||
because its 5 ms trip point absorbs the whole range. It is spread **relative to the trip point**
|
||||
that decides. Measured over the runs behind this baseline, the worst surviving key reaches 0.67 of
|
||||
its trip point (`span.consensus.ledger_close.p95`) and the other 22 sit at or below 0.60 — the two
|
||||
excluded keys, at 1.50 and 2.59, were the only ones above 1.0. A key that fails this test is not
|
||||
fixed by widening its bound; exclude it and say why.
|
||||
|
||||
## Bootstrapping the baseline
|
||||
|
||||
1. Merge a CI run with a `"placeholder": true` baseline. The telemetry-validation
|
||||
@@ -158,6 +217,11 @@ Refreshing the baseline also obliges you to re-derive the absolute bounds in
|
||||
A value that moves into a different bucket needs a different bound, and a bound left behind
|
||||
either stops catching regressions or starts firing on quantization noise.
|
||||
|
||||
It also obliges you to re-check each key's run-to-run spread against its new trip point, per
|
||||
[The general rule this exposed](#the-general-rule-this-exposed). A refreshed baseline can land in a
|
||||
bucket whose `hi_next` no longer clears the metric's own variance, which turns the key into a
|
||||
recurring false positive — the failure that excluded `ledger.validate` p95 and p99.
|
||||
|
||||
## The baseline is only valid at the log level it was captured at
|
||||
|
||||
Every timing here is coupled to the `log_level` that `run-full-validation.sh` writes into
|
||||
|
||||
@@ -58,14 +58,6 @@
|
||||
"unit": "ms",
|
||||
"value": 0.07787769784172663
|
||||
},
|
||||
"span.ledger.validate.p95": {
|
||||
"unit": "ms",
|
||||
"value": 0.2404310344827586
|
||||
},
|
||||
"span.ledger.validate.p99": {
|
||||
"unit": "ms",
|
||||
"value": 1.060000000000033
|
||||
},
|
||||
"span.rpc.ws_message.p50": {
|
||||
"unit": "ms",
|
||||
"value": 0.15990942879094971
|
||||
|
||||
@@ -68,13 +68,24 @@ def _build_simple_entries(
|
||||
cfg: dict,
|
||||
prefix: str,
|
||||
window: str,
|
||||
excluded: frozenset[str] = frozenset(),
|
||||
) -> list[QueryEntry]:
|
||||
"""Build QueryEntry list for a single-template category (spans, rpc)."""
|
||||
"""Build QueryEntry list for a single-template category (spans, rpc).
|
||||
|
||||
``excluded`` holds flat keys the surface declares but does not gate (see
|
||||
``excluded_keys`` in regression-metrics.json). They are dropped here rather
|
||||
than filtered later, so a key that no longer gates is never queried and
|
||||
never appears in ``timings.json`` — a captured key with no baseline reports
|
||||
as "new metric (not in baseline)", which reads as coverage while being
|
||||
unable to gate.
|
||||
"""
|
||||
tmpl = cfg.get("_query_template", "")
|
||||
unit = cfg.get("_unit", "ms")
|
||||
entries: list[QueryEntry] = []
|
||||
for name in cfg.get("names", []):
|
||||
for q in cfg.get("_quantiles", []):
|
||||
if f"{prefix}.{name}.p{_quantile_label(q)}" in excluded:
|
||||
continue
|
||||
expr = (
|
||||
tmpl.replace("{quantile}", _format_quantile(q))
|
||||
.replace("{name}", name)
|
||||
@@ -90,8 +101,14 @@ def _build_simple_entries(
|
||||
return entries
|
||||
|
||||
|
||||
def _build_job_entries(cfg: dict, window: str) -> list[QueryEntry]:
|
||||
"""Build QueryEntry list for the job_queue category (multi-phase)."""
|
||||
def _build_job_entries(
|
||||
cfg: dict, window: str, excluded: frozenset[str] = frozenset()
|
||||
) -> list[QueryEntry]:
|
||||
"""Build QueryEntry list for the job_queue category (multi-phase).
|
||||
|
||||
``excluded`` is applied for the same reason as in _build_simple_entries; the
|
||||
key format is flat, so one exclusion list covers both categories.
|
||||
"""
|
||||
unit = cfg.get("_unit", "us")
|
||||
phases = cfg.get("_phases", ["queued", "running"])
|
||||
tmpl_map = {
|
||||
@@ -105,6 +122,8 @@ def _build_job_entries(cfg: dict, window: str) -> list[QueryEntry]:
|
||||
if not tmpl:
|
||||
continue
|
||||
for q in cfg.get("_quantiles", []):
|
||||
if f"job.{name}.{phase}.p{_quantile_label(q)}" in excluded:
|
||||
continue
|
||||
expr = (
|
||||
tmpl.replace("{quantile}", _format_quantile(q))
|
||||
.replace("{name}", name)
|
||||
@@ -131,15 +150,19 @@ def build_query_plan(metrics_path: str | Path, window: str = "3m") -> list[Query
|
||||
``regression`` workload profile.
|
||||
|
||||
Returns:
|
||||
A list of ``QueryEntry`` values, one per (metric × quantile).
|
||||
A list of ``QueryEntry`` values, one per (metric × quantile), less any
|
||||
key listed in the config's ``excluded_keys`` map.
|
||||
"""
|
||||
with open(metrics_path) as f:
|
||||
cfg = json.load(f)
|
||||
|
||||
excluded = frozenset(cfg.get("excluded_keys", {}))
|
||||
plan: list[QueryEntry] = []
|
||||
plan.extend(_build_simple_entries(cfg.get("spans", {}), "span", window))
|
||||
plan.extend(_build_simple_entries(cfg.get("rpc_methods", {}), "rpc", window))
|
||||
plan.extend(_build_job_entries(cfg.get("job_queue", {}), window))
|
||||
plan.extend(_build_simple_entries(cfg.get("spans", {}), "span", window, excluded))
|
||||
plan.extend(
|
||||
_build_simple_entries(cfg.get("rpc_methods", {}), "rpc", window, excluded)
|
||||
)
|
||||
plan.extend(_build_job_entries(cfg.get("job_queue", {}), window, excluded))
|
||||
return plan
|
||||
|
||||
|
||||
|
||||
@@ -3,6 +3,11 @@
|
||||
"_key_format": "{category}.{name}.p{quantile} (e.g. span.tx.process.p99, job.transaction.queued.p95). Only the categories defined below are captured; there is no rpc_methods group, so no rpc.* key is produced or gated (FU-4).",
|
||||
"_excluded_spans": "rpc.process is deliberately absent from spans.names. It is created only in ServerHandler::processRequest() on the HTTP/JSON-RPC path, which the workload load generators, being WebSocket-only, never reach, so its quantiles were captured as null every run and could never gate. (The harness shell scripts do issue a few HTTP JSON-RPC health polls, far too few to produce a meaningful quantile.) See baselines/README.md.",
|
||||
"_excluded_ledger_store": "ledger.store is deliberately absent from spans.names too, for a different reason: it is below the ladder's resolution. The 2026-08-24 capture returned p50/p95/p99 of exactly 0.005/0.0095/0.0099 ms, which is 0.5/0.95/0.99 x the ladder's first edge of 0.01 ms — the signature of every sample landing in the first bucket, so the numbers are interpolation arithmetic on the bucket floor rather than latencies. That is physically plausible: LedgerMaster.cpp:463 wraps an in-memory ledgerHistory_.insert, which completes in single-digit microseconds. While all mass stays under 10 us the reported quantile cannot move materially, so NO absolute bound can gate it — every ledger.store slowing from 2 us to 9 us, 4.5x, leaves the reported value unchanged. Three keys that read as covered but cannot fire are worse than no keys (the same argument that excluded rpc.process), so they were removed rather than left in with a bound that looks derived. Restoring the key needs sub-10us edges on the collector's spanmetrics ladder (for example 0.001ms and 0.005ms) plus the matching entries in HistogramBuckets.h — that is the ladder's branch, not this file. ledger.store presence is still asserted by expected_spans.json and docker/telemetry/integration-test.sh, and its rate is still on the ledger-operations dashboard; only the latency gate drops it.",
|
||||
"_excluded_quantiles": "A THIRD KIND OF EXCLUSION, and the only one that deleting a name cannot express. spans.names lists span NAMES while _quantiles is shared across all of them, so the declared surface is the names x quantiles product and dropping ONE quantile of ONE span needs a subtraction. excluded_keys below is that subtraction: a flat {category}.{name}.p{quantile} key, exactly as _key_format defines it, mapped to the reason it is not gated. It can only ever remove a key, never add one, so a typo cannot silently start gating something new -- and check_regression_bounds.py rule F rejects an entry that would not otherwise be declared, an entry with an empty reason, and an entry that still carries a threshold override or a baseline value, so the exclusion cannot rot into dead config. Both prom_queries.py (which builds the capture plan) and check_regression_bounds.py (rule A) subtract it, so an excluded key is not queried, never reaches timings.json, and is not expected in the baseline. NOTHING ELSE CHANGES: the quantile is still computable from Prometheus with the _query_template above, the span is still asserted by expected_spans.json, and its rate is still on the ledger-operations dashboard. Only the latency gate drops it.",
|
||||
"excluded_keys": {
|
||||
"span.ledger.validate.p95": "Run-to-run variance is larger than the bound this ladder can derive. Measured across four CI runs the value spans 0.1281 to 0.7500 ms, a 5.9x spread, against a baseline of 0.2404 ms whose trip point is the next ladder edge at 0.5 ms -- so an ordinary run clears the trip point with nothing having regressed. Run 32867433073 read 0.7500 ms, +212%, and turned CI red. The derived bound models QUANTIZATION noise only (hi_next - baseline is one bucket of headroom); the dominant noise term for this span is peer-validation arrival timing in a 5-node cluster, and that term was never measured before the key was gated. Widening is not available: a bound that tolerated 0.7500 ms would reach past the 1 ms edge and leave the key gating nothing. This is a variance limit, not a defect and not a missing bound -- do NOT re-gate it by widening.",
|
||||
"span.ledger.validate.p99": "The same mechanism as p95, two orders of magnitude worse. Across the same four runs the value spans 0.3875 to 25.8750 ms, a 66.8x spread, against a baseline of 1.0600 ms and a 10 ms trip point; run 32862589645 read 25.8750 ms, +2341%. The span opens only once a quorum-completing validation arrives (LedgerMaster.cpp:987, inside checkAccept, past the tvc < minVal early return) and wraps the promotion work that follows -- setValidated, setFull, setValidLedger, pendSaveValidated -- so its duration tracks peer-validation arrival timing and what promotion then triggers. One slow consensus round therefore dominates the tail of a 3m rate window, and which round that is differs every run. A bound tolerating 25.8750 ms would be ~24.8 ms against a 1.0600 ms baseline, which gates nothing at all. Note that the two CI failures landed on DIFFERENT quantiles in different runs while the other quantile stayed well inside its bound in the same run: that asymmetry is the signature of variance, not of a regression."
|
||||
},
|
||||
"spans": {
|
||||
"_query_template": "histogram_quantile({quantile}, sum by (le) (rate(span_duration_milliseconds_bucket{span_name=\"{name}\"}[{window}])))",
|
||||
"_unit": "ms",
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -3729,12 +3729,21 @@ Key properties:
|
||||
metric it guarded, and a 10x regression injected into each key in turn was
|
||||
caught on only 5 of 28.
|
||||
- **The detection floor is `hi_next / baseline`, so some keys are only weakly
|
||||
guarded.** It ranges 2.02x to 9.43x over the current baseline;
|
||||
`span.ledger.build.p50` and `span.ledger.validate.p99` are effectively
|
||||
not guarded at 9.4x. `baselines/README.md` lists all ten weak keys and the
|
||||
ladder edges that would fix them.
|
||||
guarded.** It ranges 2.02x to 9.42x over the current baseline;
|
||||
`span.ledger.build.p50` is effectively not guarded at 9.4x.
|
||||
`baselines/README.md` lists all nine weak keys and the ladder edges that would
|
||||
fix them.
|
||||
- **The bound covers quantization noise only, so a key whose run-to-run variance
|
||||
exceeds it cannot be gated.** `span.ledger.validate.p95` and `.p99` are
|
||||
excluded for exactly that reason — measured spreads of 5.9x and 66.8x across
|
||||
four CI runs, both reaching past their trip points on healthy runs, because
|
||||
the span's duration follows peer-validation arrival timing rather than code
|
||||
speed. Widening their bounds would gate nothing, so they are listed in
|
||||
`excluded_keys` in `regression-metrics.json` and `check_regression_bounds.py`
|
||||
rule F keeps that exclusion honest. Before gating any key, check its observed
|
||||
maximum across runs against `baseline + bound`; see `baselines/README.md`.
|
||||
- **For every currently gated metric the absolute bound decides; the percentage
|
||||
bound does not.** Measured, the bound is 102%-843% of its own baseline, above
|
||||
bound does not.** Measured, the bound is 102%-842% of its own baseline, above
|
||||
both configured percentage bounds. This is _not_ a general property: the span
|
||||
ladder's top steps are only 1.25x-1.5x apart, so a baseline between about
|
||||
2667-3000 ms or 3334-4000 ms gets an absolute bound worth under 50% of itself
|
||||
|
||||
Reference in New Issue
Block a user