diff --git a/.codecov.yml b/.codecov.yml index 5c22f7f8cb..19123aa38b 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -58,13 +58,17 @@ ignore: - "src/tests/" - "include/xrpl/beast/test/" - "include/xrpl/beast/unit_test/" - # Telemetry modules — conditionally compiled behind XRPL_ENABLE_TELEMETRY, - # which is not enabled in coverage builds. + # Telemetry modules. Telemetry compiles in by default (conanfile.py, + # CMakeLists.txt), but the unit-test suite never starts an exporter, so these + # files record no coverage. This block belongs on the earliest branch that + # adds telemetry code — codecov config only flows child-ward, so an ignore + # added on a later branch can never cover the branches before it. - "src/xrpld/telemetry/" - "src/libxrpl/telemetry/" - "include/xrpl/telemetry/" - "src/libxrpl/beast/insight/OTelCollector.cpp" - "include/xrpl/beast/insight/OTelCollector.h" - # Per-module span-name constant headers (compile-time constants only, - # colocated with their subsystem rather than under telemetry/). + # Per-module span-name and span-label constant headers: compile-time + # constants only, colocated with their subsystem rather than under telemetry/. - "**/*SpanNames.h" + - "**/*SpanLabels.h" diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index e45216ecf9..5d8aaf104b 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -1840,8 +1840,24 @@ def metric_prefixes(names: Set[str]) -> Set[str]: # statsd_gauges / statsd_counters -- beast::insight metrics, whose wire names # come from formatName() lowercasing an insight metric path; # spanmetrics -- synthesised by the collector's spanmetrics connector from -# span names, not declared in C++ at all. -NON_OTEL_METRIC_GROUPS = frozenset({"statsd_gauges", "statsd_counters", "spanmetrics"}) +# span names, not declared in C++ at all; +# job_queue_per_type_gauges -- beast::insight gauges in the "jobq" group, +# created per job type by JobTypeData's constructor, so the wire name embeds +# a job-type name and there is no declared instrument to point at. This +# group needs the exemption only because the jobq_ FAMILY became owned when +# jobq_saturation was declared in MetricNames.h: Rule K checks a name only +# when its family is owned, so before that these entries were skipped for +# the accidental reason that nothing in the family was declared. Declaring +# constants for them is not an option -- there is one triple per job type, +# minted at runtime. +NON_OTEL_METRIC_GROUPS = frozenset( + { + "statsd_gauges", + "statsd_counters", + "spanmetrics", + "job_queue_per_type_gauges", + } +) def expected_metric_names( diff --git a/.github/scripts/telemetry/check_regression_bounds.py b/.github/scripts/telemetry/check_regression_bounds.py new file mode 100644 index 0000000000..03d0288946 --- /dev/null +++ b/.github/scripts/telemetry/check_regression_bounds.py @@ -0,0 +1,359 @@ +#!/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. 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); + 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; + 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 +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. + + ``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", {}) + 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_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", "") + 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) + 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 " + 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: + 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( + "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..ba025b0036 --- /dev/null +++ b/.github/scripts/telemetry/test_check_regression_bounds.py @@ -0,0 +1,286 @@ +#!/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 F), 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_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} + 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/.github/workflows/telemetry-validation.yml b/.github/workflows/telemetry-validation.yml index 55c81d4465..8cb1305c32 100644 --- a/.github/workflows/telemetry-validation.yml +++ b/.github/workflows/telemetry-validation.yml @@ -234,7 +234,7 @@ jobs: # and never reads them. Load shape comes from the default # --profile full-validation. They are still passed so the flags stay # exercised if they are ever wired up. - ARGS="--xrpld ${{ env.BUILD_DIR }}/xrpld --skip-loki" + ARGS="--xrpld ${{ env.BUILD_DIR }}/xrpld" ARGS="$ARGS --rpc-rate $RPC_RATE" ARGS="$ARGS --rpc-duration $RPC_DURATION" ARGS="$ARGS --tx-tps $TX_TPS" diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index 2210a9a268..2f91b7aa46 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -78,11 +78,11 @@ The authoritative `[telemetry]` example lives in `cfg/xrpld-example.cfg`. Teleme | `batch_size` | uint | `512` | Spans per export batch | | `batch_delay_ms` | uint | `5000` | Max delay before sending batch (ms) | | `max_queue_size` | uint | `2048` | Maximum queued spans | -| `trace_transactions` | bool | `true` | Enable transaction tracing | -| `trace_consensus` | bool | `true` | Enable consensus tracing | -| `trace_rpc` | bool | `true` | Enable RPC tracing | -| `trace_peer` | bool | `true` | Enable peer message tracing (high volume) | -| `trace_ledger` | bool | `true` | Enable ledger tracing | +| `trace_transactions` | 0 or 1 | `1` | Enable transaction tracing | +| `trace_consensus` | 0 or 1 | `1` | Enable consensus tracing | +| `trace_rpc` | 0 or 1 | `1` | Enable RPC tracing | +| `trace_peer` | 0 or 1 | `1` | Enable peer message tracing (high volume) | +| `trace_ledger` | 0 or 1 | `1` | Enable ledger tracing | | `consensus_trace_strategy` | string | `"deterministic"` | Consensus trace ID strategy: `"deterministic"` (trace_id = prevLedgerHash[0:16]) or `"attribute"` (random). Parsed at `TelemetryConfig.cpp:155-156`, consumed at `RCLConsensus.cpp:1291,1296`. **Not validated** — see the note below | | `service_name` | string | `"xrpld"` | Service name (`service.name`) for traces and metrics | | `service_instance_id` | string | node public key (base58) | Instance identifier (`service.instance.id`). Traces, span metrics and native `XRPL_METRIC_*` metrics all fall back to the node key; **`beast::insight` metrics do not** — see the note in §5.1.1 | diff --git a/OpenTelemetryPlan/06-implementation-phases.md b/OpenTelemetryPlan/06-implementation-phases.md index 4e925d16fb..9be3127c15 100644 --- a/OpenTelemetryPlan/06-implementation-phases.md +++ b/OpenTelemetryPlan/06-implementation-phases.md @@ -1044,7 +1044,8 @@ later metric families. The categories are: only asks Grafana for the dashboard and its panel count; it does not run the panel queries. - **Log-trace correlation** — `trace_id` present in Loki plus a Tempo reverse - lookup (skipped in CI via `--skip-loki`, not absent from the suite) + lookup (gated in CI; `run-full-validation.sh` prints a node/mount/collector/Loki + diagnostic alongside them so a failure names the leg that broke) See [Phase10_taskList.md](./Phase10_taskList.md) for the per-task breakdown. @@ -1053,8 +1054,9 @@ See [Phase10_taskList.md](./Phase10_taskList.md) for the per-task breakdown. 1. `rpc.process` -> `rpc.command.*` hierarchy — not assertable under the harness's WebSocket-only load, because `rpc.process` is created only on the HTTP path. This is a load-shape limitation, not a context-propagation bug. -2. Log-trace correlation — implemented and passing locally; CI passes - `--skip-loki`. +2. Log-trace correlation — the two `validate_telemetry.py` checks are now gated + in CI, but `integration-test.sh`'s own `check_log_correlation()` is still run + by no workflow. 3. Legacy `beast::insight` coverage — `expected_metrics.json` asserts a representative subset, not all ~270 families. 4. Sustained load / backpressure — the `stress` profile exists in diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index 31ce23a267..68f2056ed8 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -391,55 +391,63 @@ Join a transaction's work to its ledger with `{span.current_ledger_seq=}`. #### Consensus Attributes -| Attribute | Type | Set On | Description | -| --------------------------- | ------- | -------------------------------------------------------------------------------------------------- | -------------------------------------------------------- | -| `consensus_ledger_id` | string | `consensus.round` | Previous-ledger id anchoring the round | -| `ledger_seq` | int64 | `consensus.round`, `consensus.ledger_close`, `consensus.accept.apply`, `consensus.validation.send` | Ledger sequence number | -| `consensus_mode` | string | `consensus.round`, `consensus.ledger_close` | Node mode: `"Proposing"`, `"Observing"`, `"Wrong"`, etc. | -| `consensus_round_id` | int64 | `consensus.round` | Round identifier | -| `consensus_phase` | string | `consensus.round` | Current phase name (updated on each transition) | -| `trace_strategy` | string | `consensus.round` | Trace-id strategy (`deterministic` / `attribute`) | -| `previous_ledger_seq` | int64 | `consensus.round` | Sequence of the previous ledger | -| `previous_proposers` | int64 | `consensus.round` | Proposer count in the previous round | -| `previous_round_time_ms` | int64 | `consensus.round` | Duration of the previous round | -| `consensus_round` | int64 | `consensus.proposal.send` | Proposal sequence number for the broadcast proposal | -| `is_bow_out` | boolean | `consensus.proposal.send` | Whether the proposal is a bow-out (resigning the round) | -| `tx_count_open` | int64 | `consensus.ledger_close` | Transactions in the open ledger at close | -| `close_time_resolution_ms` | int64 | `consensus.ledger_close` | Close-time rounding granularity | -| `converge_percent` | int64 | `consensus.establish`, `consensus.update_positions`, `consensus.check` | Convergence percentage | -| `establish_count` | int64 | `consensus.establish`, `consensus.check` | Establish-phase iteration count | -| `proposers` | int64 | `consensus.establish`, `consensus.update_positions`, `consensus.accept` | Number of proposers | -| `disputes_count` | int64 | `consensus.establish`, `consensus.update_positions` | Number of disputed transactions | -| `tx_id` | string | `consensus.update_positions` | Disputed transaction id (per-dispute event) | -| `dispute_our_vote` | boolean | `consensus.update_positions` | Our vote on the disputed tx | -| `dispute_yays` | int64 | `consensus.update_positions` | Yes votes on the disputed tx | -| `dispute_nays` | int64 | `consensus.update_positions` | No votes on the disputed tx | -| `avalanche_threshold` | int64 | `consensus.update_positions` | Escalated weight needed to change our vote | -| `close_time_threshold` | int64 | `consensus.update_positions` | Close-time agreement threshold percentage | -| `agree_count` | int64 | `consensus.check` | Agreeing proposer count | -| `disagree_count` | int64 | `consensus.check` | Disagreeing proposer count | -| `threshold_percent` | int64 | `consensus.check` | Agreement threshold percentage | -| `have_close_time_consensus` | boolean | `consensus.update_positions`, `consensus.check` | Whether the close time reached consensus | -| `proposers_finished` | int64 | `consensus.check` | Proposers that have already validated the next ledger | -| `consensus_stalled` | boolean | `consensus.check` | Whether `checkConsensus` reported a stall | -| `consensus_result` | string | `consensus.check` | Check outcome | -| `quorum` | int64 | `consensus.accept` | Quorum required | -| `round_time_ms` | int64 | `consensus.accept`, `consensus.accept.apply` | Total consensus round duration in milliseconds | -| `consensus_state` | string | `consensus.accept.apply` | Consensus outcome: `"finished"` or `"moved_on"` | -| `close_time` | int64 | `consensus.accept.apply` | Agreed-upon ledger close time (epoch seconds) | -| `close_time_correct` | boolean | `consensus.accept.apply` | Whether validators agreed on close time | -| `close_resolution_ms` | int64 | `consensus.accept.apply` | Close-time rounding granularity in milliseconds | -| `proposing` | boolean | `consensus.accept.apply`, `consensus.validation.send` | Whether this node was a proposer | -| `parent_close_time` | int64 | `consensus.accept.apply` | Parent ledger close time | -| `close_time_self` | int64 | `consensus.accept.apply` | This node's close-time vote | -| `close_time_vote_bins` | string | `consensus.accept.apply` | Distribution of close-time votes | -| `resolution_direction` | string | `consensus.accept.apply` | Whether close resolution increased/decreased/unchanged | -| `tx_count` | int64 | `consensus.accept.apply` | Transactions in the accepted set | -| `ledger_hash` | string | `consensus.validation.send` | Full hash of the validated ledger (shared with peer) | -| `full_validation` | boolean | `consensus.validation.send` | Whether this is a full validation | -| `validation_sign_time` | int64 | `consensus.validation.send` | Validation signing time | -| `mode_old` | string | `consensus.mode_change` | Operating mode before the transition | -| `mode_new` | string | `consensus.mode_change` | Operating mode after the transition | +| Attribute | Type | Set On | Description | +| ---------------------------- | ------- | -------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | +| `consensus_ledger_id` | string | `consensus.round` | Previous-ledger id anchoring the round | +| `ledger_seq` | int64 | `consensus.round`, `consensus.ledger_close`, `consensus.accept.apply`, `consensus.validation.send` | Ledger sequence number | +| `consensus_mode` | string | `consensus.round`, `consensus.ledger_close` | Node mode: `"Proposing"`, `"Observing"`, `"Wrong"`, etc. | +| `consensus_round_id` | int64 | `consensus.round` | Round identifier | +| `consensus_phase` | string | `consensus.round` | Current phase name (updated on each transition) | +| `trace_strategy` | string | `consensus.round` | Trace-id strategy (`deterministic` / `attribute`) | +| `previous_ledger_seq` | int64 | `consensus.round` | Sequence of the previous ledger | +| `previous_proposers` | int64 | `consensus.round` | Proposer count in the previous round | +| `previous_round_time_ms` | int64 | `consensus.round` | Duration of the previous round | +| `consensus_round` | int64 | `consensus.proposal.send` | Proposal sequence number for the broadcast proposal | +| `is_bow_out` | boolean | `consensus.proposal.send` | Whether the proposal is a bow-out (resigning the round) | +| `tx_count_open` | int64 | `consensus.ledger_close` | Transactions in the open ledger at close | +| `close_time_resolution_ms` | int64 | `consensus.ledger_close` | Close-time rounding granularity | +| `start_reason` | string | `consensus.phase.open` | Entry path: `"initial"` or `"recovered"` | +| `previous_close_agree` | boolean | `consensus.phase.open` | Whether the prior ledger's close time was agreed | +| `peer_positions_at_open` | int64 | `consensus.phase.open` | Positions held after buffered proposals are replayed | +| `early_close_triggered` | boolean | `consensus.phase.open` | Round skipped the timer because peers had already closed | +| `tx_sets_acquired` | int64 | `consensus.phase.open` | Peer transaction sets held at close, excluding our own | +| `close_reason` | string | `consensus.phase.open` | `"anomaly"`, `"others_closed"`, `"idle"`, or `"normal"` | +| `proposers_validated` | int64 | `consensus.phase.open` | Trusted validators of the previous ledger, at close | +| `converge_percent` | int64 | `consensus.establish`, `consensus.update_positions`, `consensus.check` | Convergence percentage | +| `establish_count` | int64 | `consensus.establish`, `consensus.check` | Establish-phase iteration count | +| `close_time_avalanche_state` | string | `consensus.establish` | Terminal regime: `"init"`, `"mid"`, `"late"`, or `"stuck"` | +| `proposers` | int64 | `consensus.establish`, `consensus.update_positions`, `consensus.accept` | Number of proposers | +| `disputes_count` | int64 | `consensus.establish`, `consensus.update_positions` | Number of disputed transactions | +| `tx_id` | string | `consensus.update_positions` | Disputed transaction id (per-dispute event) | +| `dispute_our_vote` | boolean | `consensus.update_positions` | Our vote on the disputed tx | +| `dispute_yays` | int64 | `consensus.update_positions` | Yes votes on the disputed tx | +| `dispute_nays` | int64 | `consensus.update_positions` | No votes on the disputed tx | +| `avalanche_threshold` | int64 | `consensus.update_positions` | Escalated weight needed to change our vote | +| `close_time_threshold` | int64 | `consensus.update_positions` | Close-time agreement threshold percentage | +| `agree_count` | int64 | `consensus.check` | Agreeing proposer count | +| `disagree_count` | int64 | `consensus.check` | Disagreeing proposer count | +| `threshold_percent` | int64 | `consensus.check` | Agreement threshold percentage | +| `have_close_time_consensus` | boolean | `consensus.update_positions`, `consensus.check` | Whether the close time reached consensus | +| `proposers_finished` | int64 | `consensus.check` | Proposers that have already validated the next ledger | +| `consensus_stalled` | boolean | `consensus.check` | Whether `checkConsensus` reported a stall | +| `consensus_result` | string | `consensus.check` | Check outcome | +| `quorum` | int64 | `consensus.accept` | Quorum required | +| `round_time_ms` | int64 | `consensus.accept`, `consensus.accept.apply` | Total consensus round duration in milliseconds | +| `consensus_state` | string | `consensus.accept.apply` | Consensus outcome: `"finished"` or `"moved_on"` | +| `close_time` | int64 | `consensus.accept.apply` | Agreed-upon ledger close time (epoch seconds) | +| `close_time_correct` | boolean | `consensus.accept.apply` | Whether validators agreed on close time | +| `close_resolution_ms` | int64 | `consensus.accept.apply` | Close-time rounding granularity in milliseconds | +| `proposing` | boolean | `consensus.accept.apply`, `consensus.validation.send` | Whether this node was a proposer | +| `parent_close_time` | int64 | `consensus.accept.apply` | Parent ledger close time | +| `close_time_self` | int64 | `consensus.accept.apply` | This node's close-time vote | +| `close_time_vote_bins` | string | `consensus.accept.apply` | Distribution of close-time votes | +| `resolution_direction` | string | `consensus.accept.apply` | Whether close resolution increased/decreased/unchanged | +| `tx_count` | int64 | `consensus.accept.apply` | Transactions in the accepted set | +| `ledger_hash` | string | `consensus.validation.send` | Full hash of the validated ledger (shared with peer) | +| `full_validation` | boolean | `consensus.validation.send` | Whether this is a full validation | +| `validation_sign_time` | int64 | `consensus.validation.send` | Validation signing time | +| `mode_old` | string | `consensus.mode_change` | Operating mode before the transition | +| `mode_new` | string | `consensus.mode_change` | Operating mode after the transition | > **`quorum` is on `consensus.accept` only.** Its single set site is > `RCLConsensus::Adaptor::makeAcceptSpan()` @@ -598,15 +606,26 @@ These are system-level metrics emitted by xrpld's `beast::insight` framework via [insight] server=otel endpoint=http://localhost:4318/v1/metrics -prefix=xrpld ``` +`server=otel` is the only key here that changes what gets exported. No `prefix` is +shown because it would do nothing on this path: `OTelCollector` routes every +instrument name through its `static formatName()`, which only lowercases the raw +name and maps `.` and space to `_`, and the only place the class reads `prefix_` +is its startup log line. Exported names are therefore the lowercased raw names +(`jobq_job_count`, `rpc_requests_total`) and the service is identified by the OTel +resource `service.name`, not by a name prefix. `endpoint` is read from this section +but likewise reaches only that log line — the real exporter URL is derived inside +`Telemetry::initMetrics()` from `[telemetry] endpoint`, by swapping the trailing +`/v1/traces` for `/v1/metrics`. + Fallback (StatsD). `StatsDCollector` is still selected by this value, but the stack in `docker/telemetry/` no longer receives it: using this path also requires re-adding the `statsd` receiver to `otel-collector-config.yaml` and uncommenting port 8125 in `docker-compose.yml`, otherwise the metrics go to a port nothing -listens on. Note also that `StatsDCollector` applies `prefix` to the metric name -while `OTelCollector` does not, so switching transports renames every series. +listens on. `prefix` does appear below, because `StatsDCollector` really does +prepend it to every metric name it serializes — so switching transports renames +every series. ```ini [insight] @@ -677,21 +696,17 @@ prefix=xrpld | Prometheus Metric | Source File | Unit | Description | | ----------------- | ----------------- | ---- | ------------------------------ | | `rpc_time` | ServerHandler.cpp | ms | RPC response time distribution | -| `rpc_size` | ServerHandler.cpp | ms\* | RPC response size (see note) | +| `rpc_size` | ServerHandler.cpp | By\* | RPC response size (see note) | | `ios_latency` | Application.cpp | ms | I/O service loop latency | | `pathfind_fast` | PathRequests.h | ms | Fast pathfinding duration | | `pathfind_full` | PathRequests.h | ms | Full pathfinding duration | Quantiles collected: 0th, 50th, 90th, 95th, 99th, 100th percentile. -\* **`rpc_size` now records bytes as bytes (fixed).** It used to go through the -millisecond-scaled event histogram and export as `rpc_size_milliseconds_bucket` -on a ladder topping out at 5000, so the 24.9% of responses larger than 5 kB all -landed in the last bucket and every percentile read back as a flat 5000 — a -plausible-looking constant rather than a byte size. `beast::insight::Event` now +\* **`rpc_size` records bytes, not a duration.** `beast::insight::Event` declares a `Unit`, so this instrument is created with unit `By` and exports as **`rpc_size_bytes_bucket`** on `kByteBuckets` (512 B to 1 MiB, placed from the -measured distribution). Queries and panels must use the new name. +measured distribution). Queries and panels must use that name. **Grafana dashboards**: _Node Health_ (`ios_latency`), _RPC & Pathfinding_ (`rpc_time`, `rpc_size`, `pathfind_*`) @@ -1202,23 +1217,38 @@ docker/telemetry/workload/benchmark.sh --xrpld .build/xrpld --duration 300 > below as **families currently emitting** (idle nodes under-report — workload-gated metrics such as > per-RPC/error counters appear only once exercised, which is Phase 10's purpose). -| Category | Expected Count | Validation Method | Config File | -| ------------------------------ | ------------------- | -------------------------------- | ----------------------- | -| Trace spans | 40 of 41 emitted | Tempo API query | `expected_spans.json` | -| Span attributes | 67 required | Per-span attribute assertion | `expected_spans.json` | -| Legacy beast::insight families | ~270 (≈224 traffic) | Prometheus `__name__` query | `expected_metrics.json` | -| Native MetricsRegistry | 35 instruments | Prometheus query | `expected_metrics.json` | -| Call-site `XRPL_METRIC_*` | 7 instruments | Prometheus query | `expected_metrics.json` | -| Per-job-type gauges | 105 (35 types × 3) | Prometheus `__name__` query | `expected_metrics.json` | -| SpanMetrics RED | 4 per span | Prometheus query | `expected_metrics.json` | -| Grafana dashboards | all 15 on disk | Dashboard API load + panel count | `expected_metrics.json` | -| Log-trace links | Present | Loki query + Tempo reverse check | — | +| Category | Expected Count | Validation Method | Config File | +| ------------------------------ | ------------------- | ------------------------------------------- | ----------------------- | +| Trace spans | 40 of 41 emitted | Tempo API query | `expected_spans.json` | +| Span attributes | 67 required | Per-span attribute assertion | `expected_spans.json` | +| Legacy beast::insight families | ~270 (≈224 traffic) | Named subset asserted; rest regex-accounted | `expected_metrics.json` | +| Native MetricsRegistry | 35 instruments | Prometheus query | `expected_metrics.json` | +| Call-site `XRPL_METRIC_*` | 7 instruments | Prometheus query | `expected_metrics.json` | +| Per-job-type gauges | 105 (35 types × 3) | 6 by literal name; rest regex-accounted | `expected_metrics.json` | +| SpanMetrics RED | 4 per span | Prometheus query | `expected_metrics.json` | +| Grafana dashboards | all 15 on disk | Dashboard API load + panel count | `expected_metrics.json` | +| Log-trace links | Present | Loki query + Tempo reverse check | — | > **These are the harness's numbers, not the code's, and two of them differ.** > `docker/telemetry/workload/expected_spans.json` carries 40 span entries against > the **41** families the code emits ([§1.1](#11-complete-span-inventory-41-spans)) — > `rpc.ws_upgrade` has no entry — and 67 distinct required attributes (the > manifest's own `total_unique_attributes: 58` field is stale). +> The **Validation Method** column for the two bulk beast rows used to read +> "Prometheus `__name__` query", which never described anything real: +> `expected_metrics.json` contains no `__name__` query and +> `validate_telemetry.py` issues none. The single `__name__` string in that file +> is prose quoting a `ledger-data-sync` dashboard query, not a check. What is +> actually asserted in both rows is a subset named literally — +> `overlay_traffic` asserts the four `total_*` families out of ~228, and +> `job_queue_per_type_gauges` asserts 6 of the 105 per-job-type gauges. The +> remainder is now **accounted for** rather than asserted: anchored regexes +> under the top-level `accounted_patterns` list declare both cross products, so +> the `metric.reverse_coverage` check does not report them, while any name +> outside those shapes surfaces as unaccounted. That check warns and never +> fails; see the reverse-coverage row in +> [`docs/telemetry-runbook.md`](../docs/telemetry-runbook.md). +> > `expected_metrics.json` lists all **15** dashboard uids in > `docker/telemetry/grafana/dashboards/`, so dashboard coverage does not differ; > `log-derived-insights` is listed for the provisioning check only, and its panel @@ -2193,7 +2223,6 @@ enabled=1 [insight] server=otel endpoint=http://localhost:4318/v1/metrics -prefix=xrpld ``` ### Production Setup @@ -2209,9 +2238,12 @@ max_queue_size=4096 [insight] server=otel endpoint=http://otel-collector:4318/v1/metrics -prefix=xrpld ``` +Neither block sets `[insight] prefix`: on the `server=otel` path it is inert and +would only mislead — see [§2 Configuration](#configuration). It applies solely to +`server=statsd`. + ### Trace Category Toggle | Config Key | Default | Controls | diff --git a/OpenTelemetryPlan/Phase4_taskList.md b/OpenTelemetryPlan/Phase4_taskList.md index e5e380a9d6..beed519c96 100644 --- a/OpenTelemetryPlan/Phase4_taskList.md +++ b/OpenTelemetryPlan/Phase4_taskList.md @@ -204,6 +204,8 @@ - `converge_percent` — on `consensus.establish`, `consensus.update_positions`, `consensus.check` - `tx_count` — on `consensus.accept.apply` span (in `doAccept()`) - `disputes_count` — on `consensus.update_positions` span (in `updateOurPositions()`) +- `close_reason` — on `consensus.phase.open`, from `whyCloseLedger()` (`anomaly`, `others_closed`, `idle`, `normal`) +- `close_time_avalanche_state` — on `consensus.establish`, terminal regime (`init`, `mid`, `late`, `stuck`) **Design notes**: @@ -872,13 +874,14 @@ and OFF, and don't affect consensus timing. ### New Spans (Phase 4a) -| Span Name | Location | Key Attributes (actually set) | -| ---------------------------- | ------------------ | ----------------------------------------------------------------------------------------------------------------------------- | -| `consensus.round` | `RCLConsensus.cpp` | `consensus_round_id`, `consensus_ledger_id`, `ledger_seq`, `consensus_mode`, `trace_strategy` | -| `consensus.establish` | `Consensus.h` | `converge_percent`, `establish_count`, `proposers` | -| `consensus.update_positions` | `Consensus.h` | `converge_percent`, `proposers`, `have_close_time_consensus`, `close_time_threshold`, `disputes_count`, `avalanche_threshold` | -| `consensus.check` | `Consensus.h` | `agree_count`, `disagree_count`, `converge_percent`, `have_close_time_consensus`, `threshold_percent`, `consensus_result` | -| `consensus.mode_change` | `RCLConsensus.cpp` | `mode_old`, `mode_new` | +| Span Name | Location | Key Attributes (actually set) | +| ---------------------------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `consensus.round` | `RCLConsensus.cpp` | `consensus_round_id`, `consensus_ledger_id`, `ledger_seq`, `consensus_mode`, `trace_strategy` | +| `consensus.phase.open` | `Consensus.h` | `start_reason`, `previous_close_agree`, `peer_positions_at_open`, `early_close_triggered` (at start); `open_duration_ms`, `peer_positions_at_close`, `tx_sets_acquired`, `close_reason`, `proposers_validated` (at close) | +| `consensus.establish` | `Consensus.h` | `converge_percent`, `establish_count`, `proposers`, `disputes_count`, `close_time_avalanche_state` | +| `consensus.update_positions` | `Consensus.h` | `converge_percent`, `proposers`, `have_close_time_consensus`, `close_time_threshold`, `disputes_count`, `avalanche_threshold` | +| `consensus.check` | `Consensus.h` | `agree_count`, `disagree_count`, `converge_percent`, `have_close_time_consensus`, `threshold_percent`, `consensus_result` | +| `consensus.mode_change` | `RCLConsensus.cpp` | `mode_old`, `mode_new` | ### New Events (Phase 4a) diff --git a/cfg/xrpld-example.cfg b/cfg/xrpld-example.cfg index 843126386a..a8dcf56868 100644 --- a/cfg/xrpld-example.cfg +++ b/cfg/xrpld-example.cfg @@ -1741,6 +1741,11 @@ validators.txt # Path to a PEM-encoded CA certificate bundle for TLS verification. # Only used when use_tls=1. Default: empty (system CA store). # +# Leaving this empty stays valid and selects the system CA store. A path +# that is set is checked like the client paths below: with enabled=1 and +# use_tls=1, one that does not exist or cannot be read makes xrpld fail +# to start. +# # tls_client_cert= # # Path to this node's PEM-encoded client certificate, presented to the @@ -1750,15 +1755,19 @@ validators.txt # To enable mTLS, both tls_client_cert and tls_client_key must be # specified. If only one is provided, xrpld will fail to start. Providing # them while use_tls=0 also fails to start, rather than being ignored. -# Both checks apply only when enabled=1; with telemetry disabled these -# settings are read but never validated. +# With use_tls=1 each path is opened at startup, so one that does not +# exist or cannot be read fails to start too, rather than failing later +# as an opaque TLS handshake error. All three checks apply only when +# enabled=1; with telemetry disabled these settings are read but never +# validated. # # tls_client_key= # # Path to the PEM-encoded private key for tls_client_cert. Required -# whenever tls_client_cert is set. Requires use_tls=1. Both conditions -# are enforced exactly as described under tls_client_cert above: when -# enabled=1, breaking either one makes xrpld fail to start. +# whenever tls_client_cert is set. Requires use_tls=1, and must be +# readable. All three conditions are enforced exactly as described under +# tls_client_cert above: when enabled=1, breaking any one of them makes +# xrpld fail to start. # Default: empty. # # Head sampling is intentionally fixed at 1.0 (sample everything) and is diff --git a/docker/telemetry/grafana/dashboards/ledger-data-sync.json b/docker/telemetry/grafana/dashboards/ledger-data-sync.json index 8f9e33efc5..5b2505ecfe 100644 --- a/docker/telemetry/grafana/dashboards/ledger-data-sync.json +++ b/docker/telemetry/grafana/dashboards/ledger-data-sync.json @@ -57,8 +57,8 @@ "links": [], "panels": [ { - "title": "Overlay Traffic Heatmap (All Categories, Bytes In) [$xrpl_network_type]", - "description": "###### What this is:\n*All overlay traffic categories ranked by inbound bytes, giving an at-a-glance view of which message types consume the most receive bandwidth.*\n\n###### How it's computed:\n*Top categories by latest inbound byte value across all traffic categories. Each bar is labelled with its traffic category followed by the node identity; the shared `_bytes_in` suffix is dropped from the category name because the panel already reports inbound bytes.*\n\n###### Reading it:\n*The longest bars are the biggest bandwidth consumers; on a synced node transactions, proposals, and validations usually lead.*\n\n###### Healthy range:\n*workload-dependent.*\n\n###### Watch for:\n*A single ledger-data or fetch category dominating (ongoing sync) or an unexpected category topping the list.*\n\n###### Keywords:\n- **Overlay** *(network-wide)* \u2014 the peer-to-peer mesh xrpld nodes form to gossip transactions, proposals and validations.\n- **Peer** *(per node)* \u2014 another server this node holds a protocol connection to.\n\n###### Computation boundary:\n*Result: Per node \u2014 each series is one server's own value.*\n*Recorded in xrpld code as a native metric (beast::insight); the collector only forwards it; the Grafana query selects and aggregates it.*\n\n###### Source:\n[OverlayImpl.cpp](https://github.com/XRPLF/rippled/blob/develop/src/xrpld/overlay/detail/OverlayImpl.cpp)\n\n###### Function:\n`OverlayImpl ctor (TrafficGauges)`\n\n###### References:\n[Telemetry glossary](https://github.com/XRPLF/rippled/blob/develop/docs/telemetry-glossary.md#overlay)", + "title": "Overlay Traffic by Category (Bytes In Rate) [$xrpl_network_type]", + "description": "###### What this is:\n*All overlay traffic categories ranked by inbound bytes, giving an at-a-glance view of which message types consume the most receive bandwidth.*\n\n###### How it's computed:\n*Top categories by inbound byte rate across all traffic categories. The category name is recovered from the metric name before `rate()` is applied, because `rate()` drops `__name__`; a subquery is used so the renamed series can then be rated. Each bar is labelled with its traffic category followed by the node identity; the shared `_bytes_in` suffix is dropped from the category name because the panel already reports inbound bytes.*\n\n###### Reading it:\n*The longest bars are the biggest bandwidth consumers; on a synced node transactions, proposals, and validations usually lead.*\n\n###### Healthy range:\n*workload-dependent.*\n\n###### Watch for:\n*A single ledger-data or fetch category dominating (ongoing sync) or an unexpected category topping the list.*\n\n###### Keywords:\n- **Overlay** *(network-wide)* \u2014 the peer-to-peer mesh xrpld nodes form to gossip transactions, proposals and validations.\n- **Peer** *(per node)* \u2014 another server this node holds a protocol connection to.\n\n###### Computation boundary:\n*Result: Per node \u2014 each series is one server's own value.*\n*Recorded in xrpld code as a native metric (beast::insight); the collector only forwards it; the Grafana query selects and aggregates it.*\n\n###### Source:\n[OverlayImpl.cpp](https://github.com/XRPLF/rippled/blob/develop/src/xrpld/overlay/detail/OverlayImpl.cpp)\n\n###### Function:\n`OverlayImpl ctor (TrafficGauges)`\n\n###### References:\n[Telemetry glossary](https://github.com/XRPLF/rippled/blob/develop/docs/telemetry-glossary.md#overlay)", "type": "bargauge", "gridPos": { "h": 10, @@ -85,13 +85,13 @@ "datasource": { "type": "prometheus" }, - "expr": "label_replace(topk(20, {service_instance_id=~\"$node\", deployment_environment=~\"$deployment_environment\", xrpl_network_type=~\"$xrpl_network_type\", service_name=~\"$service_name\", xrpl_work_item=~\"$xrpl_work_item\", xrpl_branch=~\"$xrpl_branch\", xrpl_node_role=~\"$xrpl_node_role\", __name__=~\".*_bytes_in\", __name__!~\"total_.*\"}), \"series\", \"$1\", \"__name__\", \"(.*)_bytes_in\")" + "expr": "topk(20, rate(label_replace({service_instance_id=~\"$node\", deployment_environment=~\"$deployment_environment\", xrpl_network_type=~\"$xrpl_network_type\", service_name=~\"$service_name\", xrpl_work_item=~\"$xrpl_work_item\", xrpl_branch=~\"$xrpl_branch\", xrpl_node_role=~\"$xrpl_node_role\", __name__=~\".*_bytes_in\", __name__!~\"total_.*\"}, \"series\", \"$1\", \"__name__\", \"(.*)_bytes_in\")[$__rate_interval:]))" } ], "fieldConfig": { "defaults": { "displayName": "${__field.labels.series} [${__field.labels.service_instance_id} ${__field.labels.xrpl_branch} ${__field.labels.xrpl_node_role} ${__field.labels.xrpl_work_item}]", - "unit": "decbytes", + "unit": "Bps", "thresholds": { "mode": "absolute", "steps": [ diff --git a/docker/telemetry/grafana/dashboards/rpc-pathfinding.json b/docker/telemetry/grafana/dashboards/rpc-pathfinding.json index 8feac10762..7da901c382 100644 --- a/docker/telemetry/grafana/dashboards/rpc-pathfinding.json +++ b/docker/telemetry/grafana/dashboards/rpc-pathfinding.json @@ -154,7 +154,7 @@ }, { "title": "RPC Response Size", - "description": "\u26a0 Instrument mismatch \u2014 values unreliable. Response size is recorded through the millisecond-scaled event histogram (rpc_size_bytes_bucket), so byte values saturate at the top time bucket (5000) and the percentiles are not true byte sizes. A dedicated byte-unit histogram is needed to fix this; tracked separately. Treat this panel as indicative only until then.\n\n###### What this is:\n*The 95th-percentile size of RPC response payloads in bytes.*\n\n###### How it's computed:\n*95th-percentile of response payload sizes over the dashboard rate interval, per node.*\n\n###### Reading it:\n*Smaller is cheaper; large responses cost bandwidth and memory.*\n\n###### Healthy range:\n*Workload-dependent; small for status queries, large for bulk data queries.*\n\n###### Watch for:\n*Growth in large responses, consistent with expensive queries or API misuse.*\n\n###### Keywords:\n- **RPC command / method** *(per node)* \u2014 a named API request served by the node (e.g. account_info, ledger, submit), the unit RPC panels break down by.\n\n###### Computation boundary:\n*Result: Per node \u2014 each series is one server's own value.*\n*Recorded in xrpld code as a native metric (beast::insight); the collector only forwards it; the Grafana query selects and aggregates it.*\n\n###### Source:\n[ServerHandler.cpp](https://github.com/XRPLF/rippled/blob/develop/src/xrpld/rpc/detail/ServerHandler.cpp)\n\n###### Function:\n`ServerHandler ctor`\n\n###### References:\n[RPC command / method](https://xrpl.org/docs/references/http-websocket-apis/public-api-methods) \u00b7 [Telemetry glossary](https://github.com/XRPLF/rippled/blob/develop/docs/telemetry-glossary.md#rpc-command-method)", + "description": "###### What this is:\n*The 95th-percentile size of RPC response payloads in bytes.*\n\n###### How it's computed:\n*95th-percentile of response payload sizes over 5 minutes, per node. The instrument declares a byte unit, so it exports as `rpc_size_bytes_bucket` and is recorded on the byte bucket ladder (512 B to 1 MiB); the panel unit `decbytes` therefore shows a true size.*\n\n###### Reading it:\n*Smaller is cheaper; large responses cost bandwidth and memory.*\n\n###### Healthy range:\n*Workload-dependent; small for status queries, large for bulk data queries.*\n\n###### Watch for:\n*Growth in large responses, consistent with expensive queries or API misuse.*\n\n###### Keywords:\n- **RPC command / method** *(per node)* \u2014 a named API request served by the node (e.g. account_info, ledger, submit), the unit RPC panels break down by.\n\n###### Computation boundary:\n*Result: Per node \u2014 each series is one server's own value.*\n*Recorded in xrpld code as a native metric (beast::insight); the collector only forwards it; the Grafana query selects and aggregates it.*\n\n###### Source:\n[ServerHandler.cpp](https://github.com/XRPLF/rippled/blob/develop/src/xrpld/rpc/detail/ServerHandler.cpp)\n\n###### Function:\n`ServerHandler ctor`\n\n###### References:\n[RPC command / method](https://xrpl.org/docs/references/http-websocket-apis/public-api-methods) \u00b7 [Telemetry glossary](https://github.com/XRPLF/rippled/blob/develop/docs/telemetry-glossary.md#rpc-command-method)", "type": "timeseries", "gridPos": { "h": 10, diff --git a/docker/telemetry/integration-test.sh b/docker/telemetry/integration-test.sh index 2254ac4399..b24d90015a 100755 --- a/docker/telemetry/integration-test.sh +++ b/docker/telemetry/integration-test.sh @@ -397,13 +397,26 @@ trace_ledger=1 metrics_endpoint=http://localhost:4318/v1/metrics [insight] +# server=otel is the only load-bearing key here -- it selects OTelCollector so +# beast::insight metrics leave over OTLP. No prefix is set on purpose: on this +# path it is inert, because OTelCollector's formatName() only lowercases the raw +# instrument name and the one place the class reads prefix_ is its startup log +# line. The service is identified by the OTel resource service.name. server=otel endpoint=http://localhost:4318/v1/metrics -prefix=rippled service_instance_id=Node-${i} [rpc_startup] -{ "command": "log_level", "severity": "warning" } +# info, not warning: check_log_correlation() below greps these nodes' debug.log +# for "trace_id= span_id=" and fails if it finds none. A line only +# carries trace context when it is emitted while a span is current, and the one +# line guaranteed to be is the consensus accept pair in RCLConsensus.cpp, which +# logs at info inside the activation doAccept makes over its whole body. At +# warning that pair is suppressed, leaving only whatever warn-or-worse line +# happens to fire inside some active span -- so the check passed incidentally +# rather than by construction. debug is deliberately not used here either, to +# stay consistent with the workload harness. +{ "command": "log_level", "severity": "info" } [ssl_verify] 0 diff --git a/docker/telemetry/workload/README.md b/docker/telemetry/workload/README.md index e34608192a..c5573f5f25 100644 --- a/docker/telemetry/workload/README.md +++ b/docker/telemetry/workload/README.md @@ -213,9 +213,9 @@ python3 tx_submitter.py --endpoint ws://localhost:6006 \ Automated validation that all expected telemetry data exists. Every metric in `expected_metrics.json` is required — if it doesn't fire, the validation fails. Spans are required unless the entry carries `"optional": true`. -- **Span validation**: All span types from `expected_spans.json` with required attributes and parent-child hierarchies. Entries marked `"optional": true` only fire under traffic the harness may not produce (HTTP/JSON-RPC client, gRPC client, missing-ledger fetch, mode transitions); their absence is recorded as a passing skip, not a failure. +- **Span validation**: All span types from `expected_spans.json` with required attributes and parent-child hierarchies. Entries marked `"optional": true` only fire under traffic the harness may not produce (HTTP/JSON-RPC client, gRPC client, path-finding RPC — see [Pathfinding is not exercised](#pathfinding-is-not-exercised) — missing-ledger fetch, mode transitions); their absence is recorded as a passing skip, not a failure. - **Metric validation**: All metrics from `expected_metrics.json` — SpanMetrics, `beast::insight` gauges/counters/histograms, `MetricsRegistry` OTLP metrics. Every listed metric must have > 0 series. Uses the Prometheus `/api/v1/series` endpoint (not instant queries), polled until the metric appears or the poll window elapses, so a late-populating or quiet series is not a false negative. -- **Log-trace correlation**: trace_id/span_id in Loki logs (requires Loki) +- **Log-trace correlation**: trace_id/span_id in Loki logs (requires Loki). The two checks are `log.trace_id_present` and `log.trace_id_cross_reference`, and they exist only when `--skip-loki` is **not** passed — `run_validation()` builds them inside an `if not skip_loki` branch, so with the flag they are absent from the report rather than reported as skipped. **CI always passes `--skip-loki`, so these two are never exercised there** — see [CI Integration](#ci-integration). - **Dashboard validation**: Every dashboard uid listed under `grafana_dashboards.uids` in `expected_metrics.json` loads with panels. That list currently covers **all 16** dashboards provisioned in `docker/telemetry/grafana/dashboards/`. Note the scope of this check: it asks the Grafana API whether the dashboard exists and returns a panel count — it does **not** run the panels' queries, so a dashboard can pass here while individual panels render empty. ```bash @@ -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: @@ -264,7 +265,17 @@ 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. +- 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. @@ -396,6 +407,55 @@ Of the five `workflow_dispatch` inputs, only `run_benchmark` changes behaviour. them again — load shape comes entirely from `--profile` and `workload-profiles.json`. Their `description:` fields say so. +### Log-trace correlation in CI + +The workflow no longer passes `--skip-loki`, so `log.trace_id_present` and +`log.trace_id_cross_reference` are constructed and gated on every CI run. A green +`Telemetry Validation` is now evidence that log lines carry trace context and +that a logged trace id resolves to an exported trace. `integration-test.sh` has +its own `check_log_correlation()`, but no workflow runs that script. + +Correlation depends on four independent legs, and a failed check on its own names +none of them: the node must write a `debug.log` line carrying trace ids, the +collector container must see that file, its `filelog` receiver must parse and +export the line, and Loki must return it for the validator's LogQL. +`run-full-validation.sh` prints a per-leg diagnostic after the suite whenever the +Loki checks are enabled — per-node correlated-line counts and severity mix, the +container-side view of `/var/log/xrpld`, the receiver's watched files and +internal log-record counters, and Loki's own entry counts for the selector with +and without the line filter. Read that block first; it identifies the broken leg +without reproducing anything. + +The same block prints locally: + +```bash +docker/telemetry/workload/run-full-validation.sh --xrpld .build/xrpld +``` + +Re-run it after any change to log formatting, span activation, the collector's +`filelog` receiver, or the Loki exporter. + +### Pathfinding is not exercised + +`rpc_load_generator.py` stopped issuing `ripple_path_find` on 2026-08-25 — the weight, the request-builder branch and the docstring line went together. + +**Why.** Pathfinding is disabled on every node this harness starts, so those calls could only ever fail: + +- `src/xrpld/core/detail/Config.cpp:725-726` sets `pathSearchMax = 0` whenever a `[validation_seed]` or `[validator_token]` section is present — "by default, validators don't have pathfinding enabled". +- `run-full-validation.sh:308` writes `[validation_seed]` into every generated node cfg, and that script carries no `[path_search]`, `[path_search_fast]` or `[path_search_max]` section to put the default back. +- `src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp:48-49` therefore returns `rpcNOT_SUPPORTED`; `PathFind.cpp:39` does the same for `path_find`. + +**What removing it fixes.** The refusals were not silent. `pathfind.request` is opened at `RipplePathFind.cpp:35`, **above** that guard, so every refused call still exported a span, and the enclosing `rpc.command.ripple_path_find` span carried `rpc_status=error`. At a 3% weight that manufactured a steady ~3% error floor in `span_calls_total{status_code="STATUS_CODE_ERROR"}`. **Any error-rate threshold derived from harness data before this change was measuring the harness, not xrpld** — re-derive it. + +**What it costs.** Pathfinding now has no coverage here at all. Four spans (`pathfind.request`, `.compute`, `.discover`, `.update_all`) and two histograms (`pathfind_fast_milliseconds`, `pathfind_full_milliseconds`) go unexercised, and `pathfind.request` moved from required to `"optional": true` in `expected_spans.json` for that reason. Until the load returns, verify pathfinding by hand: the **PathFind** row of [`../TESTING.md`](../TESTING.md) carries a `curl` recipe, and `../xrpld-telemetry.cfg` is a non-validator config that already enables pathfinding. + +**Putting it back.** All four steps are required. The first two alone just restore the error floor: + +1. Add a `[path_search_max]` section to the node cfg `run-full-validation.sh` generates — or drop `[validation_seed]` and run a non-validator node. The `[path_search*]` block in `../xrpld-telemetry.cfg` is a working example. +2. Restore the `ripple_path_find` weight in `DEFAULT_WEIGHTS` and its branch in `build_rpc_request()`. `path_find` is a streaming subscription and needs its own phase instead — the generator is strictly one request, one reply. +3. Set `pathfind.request` back to required in `expected_spans.json`. Step 1 also makes `pathfind.compute` reachable, so the `pathfind.request -> pathfind.compute` relationship can lose its `"skip": true`. +4. **Re-capture `baselines/baseline-timings.json`.** Restoring the load changes the RPC mix, and `span.rpc.ws_message.{p50,p95,p99}` is a gated key — a baseline captured under a different mix is stale. See [OTel Timings Regression Gate](#otel-timings-regression-gate). + ## Configuration Files | File | Purpose | @@ -416,7 +476,8 @@ them again — load shape comes entirely from `--profile` and "description": "Top-level doc string — skipped by the validator.", "category_name": { "description": "Human-readable description.", - "metrics": ["metric_1", "metric_2"] + "metrics": ["metric_1", "metric_2"], + "required_labels": ["label_1"] }, "grafana_dashboards": { "uids": ["rpc-performance", "node-health"] @@ -424,25 +485,79 @@ them again — load shape comes entirely from `--profile` and "not_asserted": { "description": "Why these are excluded.", "metrics_excluded": { "metric_3": "reason" } - } + }, + "accounted_patterns": [ + { + "pattern": "^family_[a-z]+_(a|b)$", + "family": "Short label.", + "reason": "Why." + } + ] } ``` Every metric listed under a `metrics` array must produce > 0 Prometheus series during the validation run. If a metric doesn't fire, the workload generators need to produce enough load to trigger it. -Three top-level keys are not metric categories: +`required_labels` is optional and read for every category that declares one. Each label becomes one additional check, named `metric..label.