diff --git a/.github/scripts/telemetry/check_regression_bounds.py b/.github/scripts/telemetry/check_regression_bounds.py new file mode 100644 index 0000000000..94189fc04d --- /dev/null +++ b/.github/scripts/telemetry/check_regression_bounds.py @@ -0,0 +1,284 @@ +#!/usr/bin/env python3 +"""Assert every workload-gate absolute bound is the one its own baseline implies. + +The regression gate in ``docker/telemetry/workload`` fails CI when a span or +job-queue quantile grows. Whether it *can* fail is decided by +``regression-thresholds.json``, and that file's numbers are derived from +``baselines/baseline-timings.json`` plus the two histogram ladders. Nothing +tied the three together, and the gate has now been broken three times by the +same class of drift: + + 1. the microsecond ladder's floor moved 100us -> 1us, voiding every + job_queue baseline captured before it; + 2. the spanmetrics ladder's floor moved 1ms -> 0.01ms, voiding every + sub-millisecond span baseline captured before it; + 3. the absolute bounds stayed calibrated for a 5-25ms band the spans had + left, so a 100x regression on ``span.ledger.store.p95`` reported zero + regressions and exit 0. + +Each time the gate stayed green, which is indistinguishable from a passing +build. Documentation did not prevent recurrence, so this is a check. + +The rule it enforces is the one recorded in ``regression-thresholds.json`` +under ``_absolute_bound_derivation``: for a baseline sitting in the half-open +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: + + A the baseline's key set equals the surface ``regression-metrics.json`` + declares (a stale key left behind reads as covered but never gates); + B every gated key has a per-metric override, not a fallback default; + C each absolute bound equals ``hi_next - baseline``; + D each percentage bound stays below ``100 * bound / baseline``, so the + absolute bound remains the operative half of the ``AND`` -- the span + ladder's 2s/3s/4s edges are only 1.25x-1.5x apart, where this silently + 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. + +A PLACEHOLDER baseline -- ``"placeholder": true`` or an empty ``metrics`` +object -- exits 0, because that is the documented bootstrap state and CI has to +stay green while a baseline is being recaptured. A missing, unreadable or +malformed input is a different thing and exits 1: a check that reports success +without having checked anything is the same green-build-that-is-not failure this +script exists to prevent, so renaming or deleting one of its inputs must not +silence it. + +Exit 0 when every rule holds, 1 with per-key detail otherwise. +""" + +import json +import re +import sys +from pathlib import Path + +WORKLOAD = Path("docker/telemetry/workload") +BASELINE = WORKLOAD / "baselines/baseline-timings.json" +THRESHOLDS = WORKLOAD / "regression-thresholds.json" +METRICS = WORKLOAD / "regression-metrics.json" +COLLECTOR = Path("docker/telemetry/otel-collector-config.yaml") +HEADER = Path("include/xrpl/telemetry/HistogramBuckets.h") + +UNIT_TO_MS = {"ms": 1.0, "s": 1000.0} +# A bound may differ from the derived value only by double round-tripping. +REL_TOLERANCE = 1e-12 + + +def read_text_or_exit(path): + """Read a required text input, or exit 1 naming the input that failed.""" + try: + return path.read_text() + except OSError as exc: + sys.exit(f"{path}: required input could not be read -- {exc}") + + +def read_json_or_exit(path): + """Read and parse a required JSON input, or exit 1 naming what failed.""" + try: + return json.loads(read_text_or_exit(path)) + except json.JSONDecodeError as exc: + sys.exit(f"{path}: required input is not valid JSON -- {exc}") + + +def span_edges_ms(): + """Parse the spanmetrics bucket list, normalising each edge to milliseconds.""" + match = re.search(r"buckets:\s*\[(.*?)\]", read_text_or_exit(COLLECTOR), 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 microsecond_edges(): + """Parse kMicrosecondBuckets out of the header that owns every ladder.""" + match = re.search(r"kMicrosecondBuckets\{(.*?)\};", read_text_or_exit(HEADER), re.S) + if not match: + sys.exit(f"{HEADER}: kMicrosecondBuckets not found") + return [ + float(token.strip().replace("'", "")) + for token in match.group(1).split(",") + if token.strip() + ] + + +def declared_keys(metrics_cfg): + """Rebuild the flat key set regression-metrics.json declares. + + 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. + """ + keys = set() + spans = metrics_cfg.get("spans", {}) + for name in spans.get("names", []): + for quantile in spans.get("_quantiles", []): + keys.add(f"span.{name}.p{_quantile_label(quantile)}") + jobs = metrics_cfg.get("job_queue", {}) + for name in jobs.get("names", []): + for phase in jobs.get("_phases", []): + for quantile in jobs.get("_quantiles", []): + keys.add(f"job.{name}.{phase}.p{_quantile_label(quantile)}") + return keys + + +def _quantile_label(quantile): + """0.95 -> '95', 0.5 -> '50', matching capture_timings.py's key format.""" + return f"{quantile * 100:g}".replace(".", "") + + +def brackets(value, edges): + """Return ``(lo, hi, hi_next)`` for the bucket ``(lo, hi]`` holding value.""" + padded = [0.0] + list(edges) + for i in range(1, len(padded)): + if value <= padded[i]: + hi_next = padded[i + 1] if i + 1 < len(padded) else None + return padded[i - 1], padded[i], hi_next + return None, None, None + + +def resolve_override(key, thresholds): + """Return the override rule for a key, or None if it falls back to defaults.""" + group, quantile = key.rsplit(".", 1) + return thresholds.get("overrides", {}).get(group, {}).get(quantile) + + +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", "") + edges = ladders.get(unit) + if value is None or edges is None: + return [f"{key}: baseline has no value, or unknown unit {unit!r}"] + + failures = [] + first_edge = edges[0] + quantile = int(key.rsplit(".p", 1)[1]) / 100.0 + if abs(value - quantile * first_edge) <= 1e-9 * first_edge: + failures.append( + f"{key}: baseline {value!r} equals quantile {quantile:g} x the ladder " + f"floor {first_edge:g}{unit}, so every sample landed in the first " + f"bucket and this is bucket arithmetic, not a latency. No absolute " + f"bound can gate it -- add a finer ladder edge or drop the metric " + f"from {METRICS} (rule E)" + ) + return failures + + _, _, hi_next = brackets(value, edges) + if hi_next is None: + return [ + f"{key}: baseline {value!r}{unit} sits in or above the ladder's top " + f"bucket, so there is no hi_next to derive a bound from -- extend the " + f"ladder (rule C)" + ] + + rule = resolve_override(key, thresholds) + if rule is None: + failures.append( + f"{key}: no per-metric override, so it falls back to the defaults and " + f"gates on the percentage bound alone. Add an override with " + f"max_abs_increase = {hi_next - value!r} (rule B)" + ) + return failures + + bound = rule.get("max_abs_increase_ms", rule.get("max_abs_increase_us")) + expected = hi_next - value + if bound is None or abs(bound - expected) > REL_TOLERANCE * expected: + failures.append( + f"{key}: absolute bound is {bound!r}, expected {expected!r} " + f"(hi_next {hi_next:g} - baseline {value!r}) (rule C)" + ) + + pct = rule.get("max_pct_increase") + if pct is None: + failures.append(f"{key}: no max_pct_increase, so the metric never gates") + elif bound is not None and pct >= 100.0 * bound / value: + failures.append( + f"{key}: max_pct_increase {pct:g}% is at or above the absolute bound's " + f"{100.0 * bound / value:.1f}% of baseline, so the percentage bound " + f"becomes the operative one and the bucket guarantee is lost. Lower it " + f"or document the metric as percentage-gated (rule D)" + ) + return failures + + +def main(): + missing = [ + p for p in (BASELINE, THRESHOLDS, METRICS, COLLECTOR, HEADER) if not p.exists() + ] + if missing: + print("Cannot check workload regression bounds.", file=sys.stderr) + for path in missing: + print(f" {path}: required input is absent", file=sys.stderr) + print( + "\nA missing input is not a reason to pass. Deleting or renaming one of\n" + "these would otherwise leave the gate reporting success without having\n" + "checked a single bound -- the failure this script exists to prevent. If\n" + "the workload harness has genuinely moved, update the paths here.", + file=sys.stderr, + ) + return 1 + + baseline = read_json_or_exit(BASELINE) + thresholds = read_json_or_exit(THRESHOLDS) + metrics_cfg = read_json_or_exit(METRICS) + + if baseline.get("placeholder") is True or not baseline.get("metrics"): + print("OK: baseline is a placeholder, bounds cannot be derived yet") + return 0 + + ladders = {"ms": span_edges_ms(), "us": microsecond_edges()} + gated = baseline["metrics"] + failures = [] + + declared = declared_keys(metrics_cfg) + for key in sorted(set(gated) - declared): + failures.append( + f"{key}: in the baseline but not declared by {METRICS}, so it is " + f"reported every run and can never gate -- remove it (rule A)" + ) + for key in sorted(declared - set(gated)): + failures.append( + f"{key}: declared by {METRICS} but absent from the baseline, so it " + f"never gates -- capture a baseline for it (rule A)" + ) + + for key in sorted(gated): + if key in declared: + failures.extend(check_key(key, gated[key], thresholds, ladders)) + + if not failures: + 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" + ) + return 0 + + print( + "Workload regression bounds are not derived from the baseline.", file=sys.stderr + ) + for failure in failures: + print(f" {failure}", file=sys.stderr) + print( + f"\nThe rule is recorded in {THRESHOLDS} under _absolute_bound_derivation:\n" + "a bound is hi_next - baseline, where hi_next is the edge above the top of\n" + "the bucket holding the baseline. Refreshing a baseline therefore obliges\n" + "you to re-derive its bound; see baselines/README.md.", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/telemetry/test_check_regression_bounds.py b/.github/scripts/telemetry/test_check_regression_bounds.py new file mode 100644 index 0000000000..4c7a4000cb --- /dev/null +++ b/.github/scripts/telemetry/test_check_regression_bounds.py @@ -0,0 +1,226 @@ +#!/usr/bin/env python3 +"""Tests for check_regression_bounds.py. + +The checker reads five files by path relative to the working directory, so each +test assembles a scratch tree holding copies of the real inputs, mutates one +thing, and runs the checker as a subprocess there. Testing the real entry point +is deliberate: the contract under test is the exit code CI reads, and an +in-process call would not exercise it. + +Two groups: + +* the input-handling contract -- a placeholder baseline must PASS because that + 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. + +stdlib unittest only; the repo installs no third-party runner for CI. +""" + +import json +import os +import shutil +import stat +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +SCRIPT_DIR = Path(__file__).resolve().parent +CHECKER = SCRIPT_DIR / "check_regression_bounds.py" +REPO = SCRIPT_DIR.parents[2] + +WORKLOAD = "docker/telemetry/workload" +BASELINE = f"{WORKLOAD}/baselines/baseline-timings.json" +THRESHOLDS = f"{WORKLOAD}/regression-thresholds.json" +METRICS = f"{WORKLOAD}/regression-metrics.json" +COLLECTOR = "docker/telemetry/otel-collector-config.yaml" +HEADER = "include/xrpl/telemetry/HistogramBuckets.h" +INPUTS = (BASELINE, THRESHOLDS, METRICS, COLLECTOR, HEADER) + + +class CheckerCase(unittest.TestCase): + """Base class giving each test an isolated copy of the checker's inputs.""" + + def setUp(self): + self.tree = Path(tempfile.mkdtemp()) + self.addCleanup(self._cleanup) + for rel in INPUTS: + dest = self.tree / rel + dest.parent.mkdir(parents=True, exist_ok=True) + shutil.copy(REPO / rel, dest) + script = self.tree / ".github/scripts/telemetry/check_regression_bounds.py" + script.parent.mkdir(parents=True, exist_ok=True) + shutil.copy(CHECKER, script) + + def _cleanup(self): + for path in self.tree.rglob("*"): + if path.is_file(): + path.chmod(stat.S_IRUSR | stat.S_IWUSR) + shutil.rmtree(self.tree, ignore_errors=True) + + def run_checker(self): + """Run the checker in the scratch tree, returning (code, stdout+stderr).""" + proc = subprocess.run( + [sys.executable, ".github/scripts/telemetry/check_regression_bounds.py"], + cwd=self.tree, + capture_output=True, + text=True, + ) + return proc.returncode, proc.stdout + proc.stderr + + def edit_json(self, rel, mutate): + """Load a scratch input, hand it to mutate(), write it back.""" + path = self.tree / rel + data = json.loads(path.read_text()) + mutate(data) + path.write_text(json.dumps(data, indent=2)) + + +class TestInputHandling(CheckerCase): + """A placeholder passes; a missing or broken input must not.""" + + def test_unmodified_tree_passes(self): + code, out = self.run_checker() + self.assertEqual(code, 0, out) + self.assertIn("gated key(s)", out) + + def test_placeholder_flag_passes(self): + self.edit_json(BASELINE, lambda d: d.update(placeholder=True)) + code, out = self.run_checker() + self.assertEqual(code, 0, out) + self.assertIn("placeholder", out) + + def test_empty_metrics_baseline_passes(self): + self.edit_json(BASELINE, lambda d: d.update(metrics={})) + code, out = self.run_checker() + self.assertEqual(code, 0, out) + self.assertIn("placeholder", out) + + def test_missing_baseline_fails_naming_the_input(self): + (self.tree / BASELINE).unlink() + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("baseline-timings.json", out) + + def test_missing_collector_config_fails_naming_the_input(self): + (self.tree / COLLECTOR).unlink() + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("otel-collector-config.yaml", out) + + @unittest.skipIf(os.geteuid() == 0, "root ignores the read permission bit") + def test_unreadable_baseline_fails(self): + (self.tree / BASELINE).chmod(0) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("baseline-timings.json", out) + self.assertIn("could not be read", out) + self.assertNotIn("Traceback", out) + + def test_malformed_baseline_json_fails(self): + (self.tree / BASELINE).write_text("{ not json") + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("valid JSON", out) + + def test_malformed_thresholds_json_fails(self): + (self.tree / THRESHOLDS).write_text("]") + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("valid JSON", out) + + +class TestRules(CheckerCase): + """One case per rule, so a rule that stops flagging is caught.""" + + def test_rule_a_flags_baseline_key_not_declared(self): + self.edit_json( + BASELINE, + lambda d: d["metrics"].update( + {"span.rpc.process.p99": {"unit": "ms", "value": 9.0}} + ), + ) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("(rule A)", out) + + def test_rule_a_flags_declared_key_without_baseline(self): + self.edit_json(METRICS, lambda d: d["spans"]["names"].append("consensus.round")) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("(rule A)", out) + + def test_rule_b_flags_missing_override(self): + self.edit_json(THRESHOLDS, lambda d: d["overrides"].pop("span.ledger.build")) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("(rule B)", out) + + def test_rule_c_flags_rounded_bound(self): + self.edit_json( + THRESHOLDS, + lambda d: d["overrides"]["span.tx.process"]["p99"].update( + max_abs_increase_ms=4.0055 + ), + ) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("(rule C)", out) + + def test_rule_c_accepts_bound_within_relative_tolerance(self): + """The tolerance is 1e-12 relative, not exact equality.""" + exact = 4.005485184848892 + self.edit_json( + THRESHOLDS, + lambda d: d["overrides"]["span.tx.process"]["p99"].update( + max_abs_increase_ms=exact * (1 + 5e-13) + ), + ) + code, out = self.run_checker() + self.assertEqual(code, 0, out) + + def test_rule_d_flags_percentage_bound_becoming_operative(self): + self.edit_json( + THRESHOLDS, + lambda d: d["overrides"]["span.tx.apply"]["p99"].update( + max_pct_increase=150.0 + ), + ) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("(rule D)", 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} + self.edit_json(METRICS, lambda d: d["spans"]["names"].append("ledger.store")) + self.edit_json( + BASELINE, + lambda d: d["metrics"].update( + { + f"span.ledger.store.{q}": {"unit": "ms", "value": v} + for q, v in store.items() + } + ), + ) + self.edit_json( + THRESHOLDS, + lambda d: d["overrides"].update( + { + "span.ledger.store": { + q: {"max_pct_increase": 50.0, "max_abs_increase_ms": 0.05 - v} + for q, v in store.items() + } + } + ), + ) + code, out = self.run_checker() + self.assertEqual(code, 1, out) + self.assertIn("(rule E)", out) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/reusable-check-otel-naming.yml b/.github/workflows/reusable-check-otel-naming.yml index a37e7e1632..a92164572b 100644 --- a/.github/workflows/reusable-check-otel-naming.yml +++ b/.github/workflows/reusable-check-otel-naming.yml @@ -41,3 +41,20 @@ jobs: # 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 + - name: Test the workload regression-bounds checker + # Its own tests, run before the check itself so a broken rule is + # reported as a broken rule rather than as a threshold violation. They + # also pin the input-handling contract: a placeholder baseline passes + # (that is the documented bootstrap state) while a missing, unreadable + # or malformed input fails, so deleting an input cannot silence the + # check. stdlib unittest only, as with the naming checker. + run: python -m unittest discover -s .github/scripts/telemetry -p 'test_*.py' --verbose + - name: Check workload regression bounds + # The workload gate's absolute bounds are derived from the committed + # baseline plus the two ladders, and nothing tied the three together. + # That let the gate break three times the same way -- the microsecond + # floor moved, the span floor moved, then the bounds stayed calibrated + # for a band the spans had left, so a 100x regression reported zero + # regressions and exit 0. Every failure looked like a green build. + # This asserts each bound is still the one its own baseline implies. + run: python .github/scripts/telemetry/check_regression_bounds.py diff --git a/docker/telemetry/workload/README.md b/docker/telemetry/workload/README.md index adba3a7864..4e5a1b9b4c 100644 --- a/docker/telemetry/workload/README.md +++ b/docker/telemetry/workload/README.md @@ -264,7 +264,11 @@ Per-run tuning: - `REGRESSION_WINDOW` env var overrides the default Prometheus `rate()` window (`3m`). Keep close to the workload duration. - Metric surface lives in `regression-metrics.json`; thresholds in - `regression-thresholds.json`; both are reviewed changes. + `regression-thresholds.json`; both are reviewed changes. Each gated key's + absolute bound is `hi_next - baseline` — the distance from its baseline to the + 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. 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 c80e1bf9dc..119bef5e5f 100644 --- a/docker/telemetry/workload/baselines/README.md +++ b/docker/telemetry/workload/baselines/README.md @@ -25,27 +25,109 @@ 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: the baseline is a placeholder +## Current state: 25 metrics gate, on a baseline captured 2026-08-24 -`baseline-timings.json` currently carries `"placeholder": true` and an empty `metrics` object, -so **no metric gates right now**. Its entries were captured on 2026-06-05 against a spanmetrics -ladder that was re-cut on 2026-08-04 in `3860c93db2`, which makes every sub-millisecond quantile +`baseline-timings.json` holds real captured values for the 25 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`). Because the comparator only flags a metric when the current value _exceeds_ the baseline, a -stale-high baseline passes everything silently — so the entries were voided instead of left in -place. The file's `_note` records why, and which numbers were dropped. +stale-high baseline passes everything silently, so the entries had to be dropped rather than left +in place. They stay retrievable from this file's git history. -To restore gating, follow [Bootstrapping the baseline](#bootstrapping-the-baseline) below — a -placeholder is exactly the state that loop expects. Pasting the CI block **replaces the whole -file**, `_note` included; that is intended, and the voided numbers stay retrievable from this -file's git history. +**A placeholder must not outlive one run.** CI stays green the whole time one stands, so an +un-copied block is not a failure anyone will notice — it is a silent loss of regression coverage +that looks identical to a passing gate. Voiding a baseline is the one hand edit this file allows; +_setting_ one always comes from a printed CI block, per the "Refreshing the baseline" rule below. -**Do not let the placeholder outlive one run.** CI stays green the whole time the placeholder -stands, so an un-copied block is not a failure anyone will notice — it is a silent loss of -regression coverage that looks identical to a passing gate. +## Absolute bounds are derived per metric, from the ladder -Voiding a baseline is the one hand edit this file allows; _setting_ one always comes from a -printed CI block, per the "Refreshing the baseline" rule below. +`../regression-thresholds.json` gives every gated key its own `max_abs_increase_*`, equal to +**`hi_next − baseline`**: locate the baseline in the half-open bucket `(lo, hi]` of its ladder, +take `hi_next` as the next edge above `hi`, and the bound is the distance from the baseline to +`hi_next`. The trip point is therefore exactly `hi_next` — the gate fires only once the reading +clears the bucket **above** the baseline's own. + +That is what buys the guarantee. `histogram_quantile` returns a value interpolated inside +whichever bucket the true quantile falls in, so any reading taken while the quantile is still in +the baseline's bucket, or anywhere in the one immediately above, is at most `hi_next` and cannot +fire. Firing needs the quantile to have moved at least two buckets up. A multiple of the +_enclosing_ bucket's width cannot deliver this, because once the quantile crosses `hi` the +interpolation happens across the **next** bucket, which on this ladder is up to 8x wider — +`(0.5, 1]` is 0.5 ms wide and `(1, 5]` is 4 ms wide. The full derivation, both ladders, and a +per-key table of the arithmetic are in that file's `_absolute_bound_derivation` and +`_derivation_table`. + +Two earlier generations of this bound were wrong, in opposite directions: + +| generation | bound | 10x regression caught | single-crossing false positive reachable | +| ------------------------------ | -------------------------------------------- | --------------------- | ---------------------------------------- | +| 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** | + +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, +and because the rule is an `AND` the percentage bound could never carry a regression alone. A 100x +regression injected into `span.ledger.store.p95` reported **0 regressions, exit 0**. The second +generation fixed the magnitude but kept an assumption that does not hold — that the reading's +excursion is bounded by the enclosing bucket's width — which put 21 of 25 trip points inside the +adjacent bucket, so a single legitimate bucket crossing could turn CI red. + +**Refreshing the baseline means re-deriving the bounds**, because a refreshed value can land in a +different bucket and so get a different `hi_next`. This is no longer a documentation-only rule: +[`.github/scripts/telemetry/check_regression_bounds.py`](../../../../.github/scripts/telemetry/check_regression_bounds.py) +fails CI when a bound is not the one its own baseline implies, when a gated key has no override, +when a baseline key is not declared by `../regression-metrics.json` (or the reverse), when the +percentage bound would become the operative one, and when a baseline carries the ladder-floor +signature described below. + +### Which keys are only weakly guarded + +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: + +| 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 | +| `span.rpc.ws_message.p95` | 0.8443 ms | 5 ms | 5.92x | +| `job.acceptLedger.running.p95` | 17428.6 us | 100000 us | 5.74x | +| `span.consensus.accept.p50` | 1.7436 ms | 10 ms | 5.74x | +| `span.consensus.ledger_close.p99` | 0.9314 ms | 5 ms | 5.37x | +| `span.rpc.ws_message.p99` | 0.9878 ms | 5 ms | 5.06x | +| `span.tx.process.p99` | 0.9945 ms | 5 ms | 5.03x | + +`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 +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. + +## Known exclusion: `ledger.store` is below the ladder's resolution + +`span.ledger.store` is **not** gated. 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 × 0.01` ms — the ladder's first +edge times the quantile, the signature of every sample landing in the first bucket. Those numbers +are interpolation arithmetic on the bucket floor, not latencies. It is physically plausible: +[`LedgerMaster.cpp:463`](../../../../src/xrpld/app/ledger/detail/LedgerMaster.cpp#L463) wraps an +in-memory `ledgerHistory_.insert`, which completes in single-digit microseconds. + +While all the mass stays under 10 us the reported quantile cannot move materially, so **no +absolute bound can gate this key** — every `ledger.store` slowing from 2 us to 9 us, a 4.5x +regression, 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 from +`../regression-metrics.json` rather than left in with a bound that looks derived. + +Restoring the key needs sub-10 us edges on the collector's spanmetrics ladder (for example +`0.001ms` and `0.005ms`) plus the matching entries in `HistogramBuckets.h`. `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. +`check_regression_bounds.py` rule E fails the build if a key with this signature is gated again. ## Bootstrapping the baseline @@ -70,6 +152,12 @@ new numbers should become the norm, open a PR pasting the fresh timings into Do **not** edit `baseline-timings.json` by hand outside of this process — every entry should trace back to a real CI run so variance characteristics are preserved. +Refreshing the baseline also obliges you to re-derive the absolute bounds in +`../regression-thresholds.json`, per +[Absolute bounds are derived per metric](#absolute-bounds-are-derived-per-metric-from-the-ladder). +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. + ## 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 9a1b452478..c02ca2b558 100644 --- a/docker/telemetry/workload/baselines/baseline-timings.json +++ b/docker/telemetry/workload/baselines/baseline-timings.json @@ -54,18 +54,6 @@ "unit": "ms", "value": 5.075000000000041 }, - "span.ledger.store.p50": { - "unit": "ms", - "value": 0.005 - }, - "span.ledger.store.p95": { - "unit": "ms", - "value": 0.0095 - }, - "span.ledger.store.p99": { - "unit": "ms", - "value": 0.0099 - }, "span.ledger.validate.p50": { "unit": "ms", "value": 0.07787769784172663 diff --git a/docker/telemetry/workload/regression-metrics.json b/docker/telemetry/workload/regression-metrics.json index 050ce10a4f..911868b595 100644 --- a/docker/telemetry/workload/regression-metrics.json +++ b/docker/telemetry/workload/regression-metrics.json @@ -2,6 +2,7 @@ "_description": "Metric surface for the OTel-driven regression gate. Each entry names a metric, the quantiles to capture, and how to query Prometheus. The comparator compares current run against baseline-timings.json under these exact keys.", "_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.", "spans": { "_query_template": "histogram_quantile({quantile}, sum by (le) (rate(span_duration_milliseconds_bucket{span_name=\"{name}\"}[{window}])))", "_unit": "ms", @@ -12,7 +13,6 @@ "tx.apply", "ledger.build", "ledger.validate", - "ledger.store", "consensus.ledger_close", "consensus.accept" ] diff --git a/docker/telemetry/workload/regression-thresholds.json b/docker/telemetry/workload/regression-thresholds.json index 7e474569a9..ada4245ff5 100644 --- a/docker/telemetry/workload/regression-thresholds.json +++ b/docker/telemetry/workload/regression-thresholds.json @@ -1,26 +1,167 @@ { - "_description": "Per-metric regression thresholds. A metric regresses when current - baseline exceeds BOTH the percentage and absolute bounds (AND, not OR — this tolerates small-value noise). Defaults apply unless a per-metric override exists.", - "_bucket_note": "SpanMetrics latency histograms use explicit buckets [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). An earlier version of this note claimed 15 edges starting at 1ms and justified the 10ms absolute span bound as \"~2 low-end bucket widths\" — that derivation is void, because the sub-millisecond edges make the low-end bucket width 0.01ms, not 5ms. The 10ms bound is retained on its own merit: it is roughly two bucket widths in the 5-25ms band where most span quantiles actually sit, so it still absorbs single-bucket quantization jitter while catching multi-bucket regressions. Second-scale consensus spans have 2s/3s/4s boundaries, so their quantiles quantize to ~1s widths there. The job_queue running bound is widened similarly — per-ledger apply work scales with TxQ burst load. NOTE: BOTH ladders were re-cut, and a baseline captured before its own ladder changed is an interpolation artefact, not a latency. The native job_queue histograms are microsecond-valued and their floor moved 100us → 1us. The span ladder was re-cut too, on 2026-08-04 in 3860c93db2, moving the floor 1ms → 0.01ms; so any sub-millisecond span quantile captured before that date is equally void — a p95 reading 0.95ms is 0.95 × 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 — consensus.round ~3.9s, consensus.establish ~1.9s, the ledger.acquire tail — 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.", + "_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).", + "_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 = ", + "job.acceptLedger.queued": "p95 91.10576923076925 in (50,100] -> hi_next 250 - baseline = 158.89423076923075", + "job.acceptLedger.running": "p95 17428.571428571428 in (5000,25000] -> hi_next 100000 - baseline = 82571.42857142858", + "job.transaction.queued": "p95 476.6129032258061 in (250,500] -> hi_next 1000 - baseline = 523.3870967741939", + "job.transaction.running": "p95 427.1084337349388 in (250,500] -> hi_next 1000 - baseline = 572.8915662650612", + "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.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" + }, "defaults": { "span": { - "p50": { "max_pct_increase": 50.0, "max_abs_increase_ms": 10.0 }, - "p95": { "max_pct_increase": 50.0, "max_abs_increase_ms": 10.0 }, - "p99": { "max_pct_increase": 50.0, "max_abs_increase_ms": 15.0 } + "p50": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 0.01 + }, + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 0.01 + }, + "p99": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 0.01 + } }, "job_queue": { - "p95": { "max_pct_increase": 50.0, "max_abs_increase_us": 20000.0 } + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_us": 1.0 + } } }, "overrides": { - "span.consensus.ledger_close": { - "p50": { "max_pct_increase": 5.0, "max_abs_increase_ms": 200.0 }, - "p95": { "max_pct_increase": 5.0, "max_abs_increase_ms": 500.0 }, - "p99": { "max_pct_increase": 5.0, "max_abs_increase_ms": 1000.0 } + "job.acceptLedger.queued": { + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_us": 158.89423076923075 + } + }, + "job.acceptLedger.running": { + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_us": 82571.42857142858 + } + }, + "job.transaction.queued": { + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_us": 523.3870967741939 + } + }, + "job.transaction.running": { + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_us": 572.8915662650612 + } }, "span.consensus.accept": { - "p50": { "max_pct_increase": 5.0, "max_abs_increase_ms": 200.0 }, - "p95": { "max_pct_increase": 5.0, "max_abs_increase_ms": 500.0 }, - "p99": { "max_pct_increase": 5.0, "max_abs_increase_ms": 1000.0 } + "p50": { + "max_pct_increase": 5.0, + "max_abs_increase_ms": 8.256410256410255 + }, + "p95": { + "max_pct_increase": 5.0, + "max_abs_increase_ms": 16.0703125 + }, + "p99": { + "max_pct_increase": 5.0, + "max_abs_increase_ms": 34.849999999999916 + } + }, + "span.consensus.ledger_close": { + "p50": { + "max_pct_increase": 5.0, + "max_abs_increase_ms": 0.34857142857142853 + }, + "p95": { + "max_pct_increase": 5.0, + "max_abs_increase_ms": 0.5067164179104476 + }, + "p99": { + "max_pct_increase": 5.0, + "max_abs_increase_ms": 4.068571428571428 + } + }, + "span.ledger.build": { + "p50": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 8.938775510204081 + }, + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 5.320408163265306 + }, + "p99": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 19.924999999999958 + } + }, + "span.ledger.validate": { + "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": { + "p50": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 0.3400905712090503 + }, + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 4.155692599620494 + }, + "p99": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 4.012163187855787 + } + }, + "span.tx.apply": { + "p50": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 4.208333333333333 + }, + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 5.372631578947368 + }, + "p99": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 5.040842105263158 + } + }, + "span.tx.process": { + "p50": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 0.6571893261786461 + }, + "p95": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 4.276013488700565 + }, + "p99": { + "max_pct_increase": 50.0, + "max_abs_increase_ms": 4.005485184848892 + } } } } diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index 95a1369593..e51afb528a 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -3703,10 +3703,48 @@ Key properties: - **A metric regresses only when it exceeds BOTH the percentage and the absolute bound.** The `AND` is deliberate: SpanMetrics latency histograms use explicit - buckets, so a quantile sitting near a low bucket boundary can jump a whole - bucket (1 ms to 5 ms) with no real change. Bounds live in - `regression-thresholds.json` — `defaults` per category and quantile, with - per-metric `overrides` (e.g. `span.consensus.ledger_close` is held to 5%). + buckets, so a quantile sitting near a bucket boundary can jump a whole bucket + with no real change. Bounds live in `regression-thresholds.json` — `defaults` + per category and quantile, plus a per-metric `override` for every gated key. +- **The absolute bound is derived per metric, as `hi_next − baseline`.** Locate + the baseline in the half-open bucket `(lo, hi]` of its ladder and take + `hi_next` as the next edge above `hi`; the trip point is then exactly + `hi_next`, so the gate fires only once the reading clears the bucket _above_ + the baseline's own. That is what makes a single bucket crossing unable to turn + CI red: `histogram_quantile` interpolates inside whichever bucket the quantile + falls in, so any reading produced while the quantile is at most one bucket + above the baseline's is at most `hi_next`. A multiple of the _enclosing_ + bucket width cannot deliver that, because after crossing `hi` the + interpolation happens across the next bucket, which here is up to 8x wider + (`(0.5, 1]` is 0.5 ms, `(1, 5]` is 4 ms). Derivation, both ladders and a + per-key table live in `regression-thresholds.json` under + `_absolute_bound_derivation` and `_derivation_table`. **Refreshing + `baseline-timings.json` obliges you to re-derive these bounds** — a value that + moves into a different bucket gets a different `hi_next` — and + `.github/scripts/telemetry/check_regression_bounds.py` fails CI if you do not. +- **A single flat bound cannot work here.** The gated quantiles span 0.078 ms to + 17 ms, so one figure is inert at the bottom of that range and trigger-happy at + the top. The flat 10/15 ms span bound it replaced sat 1.15x to 2000x above the + 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. +- **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 + 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 + and the percentage bound takes over — `consensus.round` at ~3.9 s lands + exactly there. `check_regression_bounds.py` rule D fails the build rather than + letting that happen silently. The percentage entries are still required (a + missing one turns the metric into "no threshold configured" and stops it + gating) and they are the operative bound on the `defaults` path. +- **`span.ledger.store` is not gated**, because its quantiles are the ladder's + first edge times the quantile — every sample lands under 10 us, so no bound + can move. See `baselines/README.md`. - **A metric with no configured threshold is captured but never gates.** It is reported with a note instead. Today only `span.*` and `job.*` keys have thresholds; `rpc.*` is not produced and would not gate if it were (see