From 3836078a78aafc55f87d6a2fbc15d20c7316824e Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 25 Aug 2026 18:21:35 +0100 Subject: [PATCH] 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. --- .../telemetry/check_regression_bounds.py | 79 ++++++++++++++++++- .../telemetry/test_check_regression_bounds.py | 62 ++++++++++++++- docker/telemetry/workload/README.md | 9 ++- docker/telemetry/workload/baselines/README.md | 76 ++++++++++++++++-- .../workload/baselines/baseline-timings.json | 8 -- docker/telemetry/workload/prom_queries.py | 37 +++++++-- .../workload/regression-metrics.json | 5 ++ .../workload/regression-thresholds.json | 14 +--- docs/telemetry-runbook.md | 19 +++-- 9 files changed, 268 insertions(+), 41 deletions(-) diff --git a/.github/scripts/telemetry/check_regression_bounds.py b/.github/scripts/telemetry/check_regression_bounds.py index 94189fc04d..03d0288946 100644 --- a/.github/scripts/telemetry/check_regression_bounds.py +++ b/.github/scripts/telemetry/check_regression_bounds.py @@ -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( diff --git a/.github/scripts/telemetry/test_check_regression_bounds.py b/.github/scripts/telemetry/test_check_regression_bounds.py index 4c7a4000cb..ba025b0036 100644 --- a/.github/scripts/telemetry/test_check_regression_bounds.py +++ b/.github/scripts/telemetry/test_check_regression_bounds.py @@ -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} diff --git a/docker/telemetry/workload/README.md b/docker/telemetry/workload/README.md index 5c10527978..eebd106e44 100644 --- a/docker/telemetry/workload/README.md +++ b/docker/telemetry/workload/README.md @@ -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. diff --git a/docker/telemetry/workload/baselines/README.md b/docker/telemetry/workload/baselines/README.md index 119bef5e5f..eaa3c8e9ca 100644 --- a/docker/telemetry/workload/baselines/README.md +++ b/docker/telemetry/workload/baselines/README.md @@ -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 diff --git a/docker/telemetry/workload/baselines/baseline-timings.json b/docker/telemetry/workload/baselines/baseline-timings.json index c02ca2b558..07b289c864 100644 --- a/docker/telemetry/workload/baselines/baseline-timings.json +++ b/docker/telemetry/workload/baselines/baseline-timings.json @@ -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 diff --git a/docker/telemetry/workload/prom_queries.py b/docker/telemetry/workload/prom_queries.py index a26a2b5459..08c1afc64c 100644 --- a/docker/telemetry/workload/prom_queries.py +++ b/docker/telemetry/workload/prom_queries.py @@ -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 diff --git a/docker/telemetry/workload/regression-metrics.json b/docker/telemetry/workload/regression-metrics.json index 911868b595..dbc0ba055b 100644 --- a/docker/telemetry/workload/regression-metrics.json +++ b/docker/telemetry/workload/regression-metrics.json @@ -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", diff --git a/docker/telemetry/workload/regression-thresholds.json b/docker/telemetry/workload/regression-thresholds.json index ada4245ff5..7ea8672996 100644 --- a/docker/telemetry/workload/regression-thresholds.json +++ b/docker/telemetry/workload/regression-thresholds.json @@ -1,8 +1,8 @@ { "_description": "Per-metric regression thresholds. A metric regresses when current - baseline exceeds BOTH the percentage and absolute bounds (AND, not OR \u2014 this tolerates small-value noise). Defaults apply unless a per-metric override exists.", "_bucket_note": "SpanMetrics latency histograms use explicit buckets [0.01,0.05,0.1,0.25,0.5,1,5,10,25,50,100,250,500]ms then [1,2,3,4,5,10,30]s (20 edges; docker/telemetry/otel-collector-config.yaml is the authoritative list). Second-scale consensus spans have 2s/3s/4s boundaries, so their quantiles quantize to ~1s widths there \u2014 the ladder is NOT uniformly 2x-or-coarser, which matters for _percentage_bound_note. The native job_queue histograms are microsecond-valued on the ladder [1,2,5,10,25,50,100,250,500,1000,5000,25000,100000,500000]us then [1,5,10,30,60]s (19 edges; include/xrpl/telemetry/HistogramBuckets.h is authoritative). NOTE: BOTH ladders were re-cut, and a baseline captured before its own ladder changed is an interpolation artefact, not a latency. The job_queue floor moved 100us \u2192 1us. The span ladder was re-cut on 2026-08-04 in 3860c93db2, moving the floor 1ms \u2192 0.01ms; so any sub-millisecond span quantile captured before that date is equally void \u2014 a p95 reading 0.95ms is 0.95 \u00d7 the old 1ms first edge, not a measurement. An earlier note asserted that the surviving span baselines were unaffected by the ladder work; that is wrong for every span quantile below 1ms. Only the band from 1ms to 1s is safe: those edges are byte-identical across the two ladders. The re-cut also ADDED edges above 1s (2s/3s/4s/10s/30s), so a span whose quantiles land in the second-scale range \u2014 consensus.round ~3.9s, consensus.establish ~1.9s, the ledger.acquire tail \u2014 is distorted just as much, and any pre-2026-08-04 baseline for it is equally void. Do not read this note as licensing a stale second-scale baseline.", - "_absolute_bound_derivation": "HOW EVERY max_abs_increase_* NUMBER BELOW WAS OBTAINED. Rule: locate the baseline value in the half-open bucket (lo, hi] of its own ladder, take hi_next = the next edge above hi, and set the bound to (hi_next - baseline). The trip point is therefore exactly hi_next: the gate fires only when the reported value EXCEEDS the top of the bucket above the baseline's own bucket. WHY THAT AND NOT A MULTIPLE OF THE BUCKET WIDTH: histogram_quantile returns a value interpolated inside whichever bucket the true quantile falls in, so a reading taken while the true quantile sits anywhere in the baseline's bucket OR anywhere in the one immediately above is at most hi_next and cannot fire. Firing requires the true quantile to have moved at least two buckets up. A multiple of the ENCLOSING width cannot deliver that, because once the quantile crosses hi the interpolation happens across the NEXT bucket, which on this ladder is up to 8x wider \u2014 (0.5,1] has width 0.5 and (1,5] has width 4 \u2014 so the reading's excursion is not bounded by any multiple of the enclosing width. Worked example: span.tx.process.p99 has baseline 0.9945ms in bucket (0.5, 1], hi_next = 5, so its bound is 4.0055ms and the gate fires only above 5ms. Bounds are stored as exact doubles rather than rounded figures so that rounding cannot break the guarantee and so check_regression_bounds.py can assert each one against the ladder to within a 1e-12 relative tolerance -- tight enough that a bound rounded for readability, such as 4.0055 for 4.005485184848892, is rejected; _derivation_table below shows the arithmetic for each one. Measured over the committed baseline this rule yields a detection floor of 2.02x to 9.43x of baseline, per key. WHAT THIS REPLACED, IN TWO GENERATIONS: (1) a single flat pair of bounds (10ms for span p50/p95, 15ms for span p99, 20000us for job_queue p95) justified as 'roughly two bucket widths in the 5-25ms band where most span quantiles actually sit'. The 2026-08-24 capture falsifies that premise \u2014 18 of the 28 quantiles gated at that time sat below 1ms \u2014 so the absolute bound sat 1.15x to 2000x above the metric it guarded and, because the rule is an AND, the percentage bound could never carry a regression on its own; a 10x regression injected into each key in turn was caught on only 5 of 28, and a 100x regression injected into span.ledger.store.p95 produced 0 regressions and exit 0. (2) a first correction to 2 \u00d7 the ENCLOSING bucket width, which caught 10x on 28 of 28 but placed the trip point INSIDE the adjacent bucket -- and so left a single-crossing false positive reachable -- on 21 of the 25 keys gated today, 4 of them tripping on a tail-mass shift under 1.5% of samples. That is the assumption this rule removes. RE-DERIVE THESE NUMBERS whenever baseline-timings.json is refreshed or either ladder changes: a refreshed baseline can land in a different bucket, which changes hi_next. .github/scripts/telemetry/check_regression_bounds.py enforces the rule in CI so a stale bound cannot survive a baseline refresh. LIMITATION \u2014 WHICH KEYS ARE ONLY WEAKLY GUARDED: the guarantee costs sensitivity wherever the ladder is coarse, and the detection floor is hi_next/baseline, so a baseline sitting just above an edge is guarded loosely. span.ledger.build.p50 (baseline 1.0612ms, fires at 10ms, 9.42x) and span.ledger.validate.p99 (1.0600ms, 9.43x) are NOT meaningfully guarded: ledger.build p50 fires only at 10ms, so it could get up to 9.4x slower -- 1.06ms to just under 10ms -- and still pass. span.tx.process.p95 (6.91x), span.tx.apply.p50 (6.32x), span.rpc.ws_message.p95 (5.92x), job.acceptLedger.running.p95 (5.74x), span.consensus.accept.p50 (5.74x), span.consensus.ledger_close.p99 (5.37x), span.rpc.ws_message.p99 (5.06x) and span.tx.process.p99 (5.03x) are weak. All ten are limited by two 5x-wide ladder steps, 1ms\u21925ms and 5000us\u219225000us. The fix is a 2ms edge (and ideally 3ms) in the collector's spanmetrics ladder plus the matching edges in kMillisecondBuckets, and a 10000us edge in kMicrosecondBuckets \u2014 that work belongs to the branch that owns the ladders, not here. Until then do not read these keys as guarded. span.ledger.store is absent from the overrides below because it was removed from the gated surface entirely: its quantiles were the ladder floor times the quantile, so no bound could gate it. See _excluded_ledger_store in regression-metrics.json.", - "_percentage_bound_note": "For every key gated today the absolute bound is the binding half of the AND and the percentage bound never decides the outcome: measured, (bound / baseline) ranges from 102% (span.tx.apply.p99) to 843% (span.ledger.validate.p99), all above the 50% and 5% percentage bounds configured here, and the minimum trip multiple of all 25 keys is set by the absolute bound. THIS IS NOT A GENERAL GUARANTEE, and an earlier version of this note wrongly claimed it was, on the false premise that 'every step of both ladders is at least a factor of 2'. The span ladder breaks that three times at the top: 2s->3s is 1.5x, 3s->4s is 1.33x, 4s->5s is 1.25x, so second-scale consensus quantiles quantize to ~1s widths there. Because the bound is (hi_next - baseline), a baseline between about 2667ms and 3000ms, or between about 3334ms and 4000ms, gets an absolute bound worth less than 50% of itself and the PERCENTAGE bound becomes the operative one -- at which point the metric fires on a 50% move that is smaller than one bucket width, and the single-crossing guarantee in _absolute_bound_derivation is lost. That band is not hypothetical: the collector config names consensus.round (~3.9s) as a reason those edges exist, and 3900ms sits in the second sub-band with an absolute bound of 5000 - 3900 = 1100, only 28.2% of baseline. Whoever gates a key whose baseline lands in either sub-band MUST lower its max_pct_increase below (bound / baseline) for that key, or state explicitly that the metric is percentage-gated and the bucket guarantee does not hold for it. check_regression_bounds.py enforces this as rule D so the trap cannot be walked into silently. The percentage entries are required and still meaningful regardless: compare_to_baseline.py treats a missing max_pct_increase as 'no threshold configured' and would stop gating the metric entirely; they record the intended relative tolerance (consensus spans 5%, everything else 50%); and they are the operative bound on the defaults path (see _defaults_note).", + "_absolute_bound_derivation": "HOW EVERY max_abs_increase_* NUMBER BELOW WAS OBTAINED. Rule: locate the baseline value in the half-open bucket (lo, hi] of its own ladder, take hi_next = the next edge above hi, and set the bound to (hi_next - baseline). The trip point is therefore exactly hi_next: the gate fires only when the reported value EXCEEDS the top of the bucket above the baseline's own bucket. WHY THAT AND NOT A MULTIPLE OF THE BUCKET WIDTH: histogram_quantile returns a value interpolated inside whichever bucket the true quantile falls in, so a reading taken while the true quantile sits anywhere in the baseline's bucket OR anywhere in the one immediately above is at most hi_next and cannot fire. Firing requires the true quantile to have moved at least two buckets up. A multiple of the ENCLOSING width cannot deliver that, because once the quantile crosses hi the interpolation happens across the NEXT bucket, which on this ladder is up to 8x wider \u2014 (0.5,1] has width 0.5 and (1,5] has width 4 \u2014 so the reading's excursion is not bounded by any multiple of the enclosing width. Worked example: span.tx.process.p99 has baseline 0.9945ms in bucket (0.5, 1], hi_next = 5, so its bound is 4.0055ms and the gate fires only above 5ms. Bounds are stored as exact doubles rather than rounded figures so that rounding cannot break the guarantee and so check_regression_bounds.py can assert each one against the ladder to within a 1e-12 relative tolerance -- tight enough that a bound rounded for readability, such as 4.0055 for 4.005485184848892, is rejected; _derivation_table below shows the arithmetic for each one. Measured over the committed baseline this rule yields a detection floor of 2.02x to 9.42x of baseline, per key. WHAT THIS RULE DOES NOT COVER, AND THE ONE CHECK TO RUN BEFORE GATING ANY KEY: 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 much 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. So before adding a key here, capture it over several runs and check its OBSERVED MAXIMUM against its trip point (baseline + bound); gate it only if the observed maximum stays below that trip point with margin. Spread on its own proves nothing -- span.tx.apply.p50 swings 364x across runs and never fires, because its 5ms trip point absorbs the whole range -- it is spread RELATIVE TO THE TRIP POINT that decides. Measured across the runs behind this baseline, the worst surviving key reaches 0.67 of its trip point (span.consensus.ledger_close.p95), and the only two that exceeded it were span.ledger.validate p95 and p99, now excluded. A key that fails this test is not fixed by widening its bound: see excluded_keys in regression-metrics.json. WHAT THIS REPLACED, IN TWO GENERATIONS: (1) a single flat pair of bounds (10ms for span p50/p95, 15ms for span p99, 20000us for job_queue p95) justified as 'roughly two bucket widths in the 5-25ms band where most span quantiles actually sit'. The 2026-08-24 capture falsifies that premise \u2014 18 of the 28 quantiles gated at that time sat below 1ms \u2014 so the absolute bound sat 1.15x to 2000x above the metric it guarded and, because the rule is an AND, the percentage bound could never carry a regression on its own; a 10x regression injected into each key in turn was caught on only 5 of 28, and a 100x regression injected into span.ledger.store.p95 produced 0 regressions and exit 0. (2) a first correction to 2 \u00d7 the ENCLOSING bucket width, which caught 10x on 28 of 28 but placed the trip point INSIDE the adjacent bucket -- and so left a single-crossing false positive reachable -- on 21 of the 25 keys gated at the time, 4 of them tripping on a tail-mass shift under 1.5% of samples. That is the assumption this rule removes. RE-DERIVE THESE NUMBERS whenever baseline-timings.json is refreshed or either ladder changes: a refreshed baseline can land in a different bucket, which changes hi_next. .github/scripts/telemetry/check_regression_bounds.py enforces the rule in CI so a stale bound cannot survive a baseline refresh. LIMITATION \u2014 WHICH KEYS ARE ONLY WEAKLY GUARDED: the guarantee costs sensitivity wherever the ladder is coarse, and the detection floor is hi_next/baseline, so a baseline sitting just above an edge is guarded loosely. span.ledger.build.p50 (baseline 1.0612ms, fires at 10ms, 9.42x) is NOT meaningfully guarded: it fires only at 10ms, so it could get up to 9.4x slower -- 1.06ms to just under 10ms -- and still pass. span.tx.process.p95 (6.91x), span.tx.apply.p50 (6.32x), span.rpc.ws_message.p95 (5.92x), job.acceptLedger.running.p95 (5.74x), span.consensus.accept.p50 (5.74x), span.consensus.ledger_close.p99 (5.37x), span.rpc.ws_message.p99 (5.06x) and span.tx.process.p99 (5.03x) are weak. All nine are limited by two 5x-wide ladder steps, 1ms\u21925ms and 5000us\u219225000us. The fix is a 2ms edge (and ideally 3ms) in the collector's spanmetrics ladder plus the matching edges in kMillisecondBuckets, and a 10000us edge in kMicrosecondBuckets \u2014 that work belongs to the branch that owns the ladders, not here. Until then do not read these keys as guarded. span.ledger.store is absent from the overrides below because it was removed from the gated surface entirely: its quantiles were the ladder floor times the quantile, so no bound could gate it. See _excluded_ledger_store in regression-metrics.json.", + "_percentage_bound_note": "For every key gated today the absolute bound is the binding half of the AND and the percentage bound never decides the outcome: measured, (bound / baseline) ranges from 102% (span.tx.apply.p99) to 842% (span.ledger.build.p50), all above the 50% and 5% percentage bounds configured here, and the minimum trip multiple of all 23 keys is set by the absolute bound. THIS IS NOT A GENERAL GUARANTEE, and an earlier version of this note wrongly claimed it was, on the false premise that 'every step of both ladders is at least a factor of 2'. The span ladder breaks that three times at the top: 2s->3s is 1.5x, 3s->4s is 1.33x, 4s->5s is 1.25x, so second-scale consensus quantiles quantize to ~1s widths there. Because the bound is (hi_next - baseline), a baseline between about 2667ms and 3000ms, or between about 3334ms and 4000ms, gets an absolute bound worth less than 50% of itself and the PERCENTAGE bound becomes the operative one -- at which point the metric fires on a 50% move that is smaller than one bucket width, and the single-crossing guarantee in _absolute_bound_derivation is lost. That band is not hypothetical: the collector config names consensus.round (~3.9s) as a reason those edges exist, and 3900ms sits in the second sub-band with an absolute bound of 5000 - 3900 = 1100, only 28.2% of baseline. Whoever gates a key whose baseline lands in either sub-band MUST lower its max_pct_increase below (bound / baseline) for that key, or state explicitly that the metric is percentage-gated and the bucket guarantee does not hold for it. check_regression_bounds.py enforces this as rule D so the trap cannot be walked into silently. The percentage entries are required and still meaningful regardless: compare_to_baseline.py treats a missing max_pct_increase as 'no threshold configured' and would stop gating the metric entirely; they record the intended relative tolerance (consensus spans 5%, everything else 50%); and they are the operative bound on the defaults path (see _defaults_note).", "_defaults_note": "A MISSING OVERRIDE IS DETECTED BY CI, NOT BY THESE DEFAULTS. .github/scripts/telemetry/check_regression_bounds.py fails the build at lint time, naming the key and the exact value its bound should have, before the workload ever runs. That is the mechanism; the defaults below are only a runtime backstop for the case where that check is bypassed. The defaults carry the FLOOR of each ladder as their absolute bound \u2014 0.01ms for spans, 1us for job_queue \u2014 deliberately too small to bind for any real metric, which leaves max_pct_increase (50%) as the operative bound on this path. Measured: a metric with no override and a baseline of 3900ms passes at +49% and fires at +51%; a job metric with a baseline of 5000us behaves the same. The backstop is honestly imperfect and the earlier version of this note oversold it. At 50% relative it CAN false-fire: a metric whose baseline is 1.06ms inside the 4ms-wide (1,5] bucket fires on a single-bucket-width move (measured: 1.06 \u2192 5.06ms, +377%, regressed). An earlier note called that 'the intended signal that the override is missing', which was wrong \u2014 CI prints REGRESSION and a reader cannot tell it from a real one, and rejecting a tighter alternative for exactly that cries-wolf risk while shipping it here would be inconsistent. The check is what makes the signal legible. The backstop is kept only because a metric silently not gated at all is the worse of the two failures.", "_derivation_table": { "_format": "override key: in -> hi_next - baseline = ", @@ -13,7 +13,7 @@ "span.consensus.accept": "p50 1.7435897435897438 in (1,5] -> hi_next 10 - baseline = 8.256410256410255 | p95 8.9296875 in (5,10] -> hi_next 25 - baseline = 16.0703125 | p99 15.150000000000082 in (10,25] -> hi_next 50 - baseline = 34.849999999999916", "span.consensus.ledger_close": "p50 0.15142857142857144 in (0.1,0.25] -> hi_next 0.5 - baseline = 0.34857142857142853 | p95 0.49328358208955236 in (0.25,0.5] -> hi_next 1 - baseline = 0.5067164179104476 | p99 0.9314285714285726 in (0.5,1] -> hi_next 5 - baseline = 4.068571428571428", "span.ledger.build": "p50 1.0612244897959187 in (1,5] -> hi_next 10 - baseline = 8.938775510204081 | p95 4.679591836734694 in (1,5] -> hi_next 10 - baseline = 5.320408163265306 | p99 5.075000000000041 in (5,10] -> hi_next 25 - baseline = 19.924999999999958", - "span.ledger.validate": "p50 0.07787769784172663 in (0.05,0.1] -> hi_next 0.25 - baseline = 0.17212230215827337 | p95 0.2404310344827586 in (0.1,0.25] -> hi_next 0.5 - baseline = 0.2595689655172414 | p99 1.060000000000033 in (1,5] -> hi_next 10 - baseline = 8.939999999999968", + "span.ledger.validate": "p50 0.07787769784172663 in (0.05,0.1] -> hi_next 0.25 - baseline = 0.17212230215827337 (p95 and p99 are NOT gated -- see excluded_keys in regression-metrics.json: their run-to-run spread, 5.9x and 66.8x over four CI runs, exceeds any bound this rule can derive)", "span.rpc.ws_message": "p50 0.15990942879094971 in (0.1,0.25] -> hi_next 0.5 - baseline = 0.3400905712090503 | p95 0.8443074003795064 in (0.5,1] -> hi_next 5 - baseline = 4.155692599620494 | p99 0.9878368121442125 in (0.5,1] -> hi_next 5 - baseline = 4.012163187855787", "span.tx.apply": "p50 0.791666666666667 in (0.5,1] -> hi_next 5 - baseline = 4.208333333333333 | p95 4.627368421052632 in (1,5] -> hi_next 10 - baseline = 5.372631578947368 | p99 4.959157894736842 in (1,5] -> hi_next 10 - baseline = 5.040842105263158", "span.tx.process": "p50 0.34281067382135394 in (0.25,0.5] -> hi_next 1 - baseline = 0.6571893261786461 | p95 0.7239865112994349 in (0.5,1] -> hi_next 5 - baseline = 4.276013488700565 | p99 0.9945148151511076 in (0.5,1] -> hi_next 5 - baseline = 4.005485184848892" @@ -111,14 +111,6 @@ "p50": { "max_pct_increase": 50.0, "max_abs_increase_ms": 0.17212230215827337 - }, - "p95": { - "max_pct_increase": 50.0, - "max_abs_increase_ms": 0.2595689655172414 - }, - "p99": { - "max_pct_increase": 50.0, - "max_abs_increase_ms": 8.939999999999968 } }, "span.rpc.ws_message": { diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index de70a911d3..17303ac9d3 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -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