mirror of
https://github.com/XRPLF/rippled.git
synced 2026-09-27 15:28:03 +00:00
Merge branch 'pratik/otel-phase10-workload-validation' into pratik/otel-sync-diagnostics
Brings phase-10 up to 3836078a78 (74 commits). Seven conflicts, resolved
per-hunk; no side was taken wholesale.
validate_telemetry.py, four hunks. The module docstring keeps both category
lists, renumbered. _log_prometheus_metric_names takes phase-10's version: it
returns the family list that the new reverse-coverage check consumes, and
_log_name_list prints every family sorted one per line, which supersedes the
hand-maintained prefix filter this branch had been extending -- that filter
existed only to keep the log readable and listed strictly less. validate_metrics
takes phase-10's _metric_check_targets call. assert_sync_diagnostics_metrics and
phase-10's _check_metric_label both landed at the same place; both are kept.
That third hunk carried the hazard this branch had flagged in advance.
_metric_check_targets selects every group satisfying isinstance(dict) and had no
equivalent of SKIPPED_METRIC_GROUPS, because on phase-10 there was no group that
needed excluding. Merged as-is it would have walked sync_diagnostics while
assert_sync_diagnostics_metrics also walks it, polling and reporting all 61
metrics twice. The exclusion is reinstated inside that function, and its
docstring claim that the isinstance test "selects exactly the same groups the
previous name-based exclusion list did" is corrected -- true on phase-10, false
here, and the reason is ownership, which no structural test can express.
expected_metrics.json: both sides appended to metrics_excluded, so both sets are
kept, 29 entries. expected_spans.json: phase-10's fuller pathfind.compute
skip_reason replaces this branch's, and this branch's ledger.acquire ->
ledger.acquire.astree relationship is kept.
Both docs carried stale counts, and the two sides disagreed with each other --
15 dashboards against 16, and both claiming 41 span types when the contract holds
48. Rather than pick a stale side, every figure is recomputed from the resolved
contract: 48 span types as 28 required and 20 optional, 145 metric checks across
26 asserting categories as 140 names plus 5 required_labels, of which 61 are the
sync_diagnostics names, and 16 dashboards, which matches both the uid list and
the files on disk. The runbook keeps phase-10's table, which adds the reverse-
coverage row.
ConsensusSpanNames.h: both sides added different constants to namespace val;
both kept. Confirmed no identifier is redefined -- the merged file's duplicate
set is identical to this branch's, and those duplicates are distinct namespaces
(op::round against the enclosing span::round), not redefinitions.
ConsensusSpanNames.cpp was an add/add: both branches wrote this file
independently, 9 tests here and 8 on phase-10, with no name in common. All 17
are kept. The guard is dropped rather than applied to the union: SpanNames.h
documents that its constants are deliberately NOT guarded by
XRPL_ENABLE_TELEMETRY, ConsensusSpanNames.h has no guard, and the tests this
branch contributed reference ValStatus only in comments while calling
validationStatusValue with plain ints. So they compile without telemetry, and
unguarding them gains coverage in a -Dtelemetry=OFF build rather than losing it.
One defect belongs to the merge itself, appearing on neither parent. phase-10
added a job_queue_per_type_gauges group holding six jobq_<type>_running/_waiting
names; this branch declared jobq_saturation in MetricNames.h. Rule K checks a
name only when its family is owned, so declaring that constant made jobq_ owned
and turned phase-10's six entries into violations. They are beast::insight gauges
created per job type by JobTypeData's constructor, so no constant can exist for
them -- one triple per job type, minted at runtime. The group joins
NON_OTEL_METRIC_GROUPS alongside statsd_gauges for the same reason.
Verification: no conflict markers repo-wide and no unmerged index entries; both
JSON contracts parse; validate_telemetry.py compiles; every count written into
the docs re-derived from the resolved files and matching; span counters still 48
and 74; check_otel_naming.py exits 0, and Rule K proven still able to fail by
injecting a bogus name in an owned family; levelization baseline clean after
regeneration, with 17 incoming include changes; doxygen style clean across all
tracked C++ at CI scope; pre-commit --all-files clean except cargo-fmt, which
reports "Executable `cargo` not found" and touches none of the 0 Rust files here.
NOT compiled -- no approval to build, so the incoming C++ is unverified by a
compiler on this branch.
This commit is contained in:
20
.github/scripts/otel-naming/check_otel_naming.py
vendored
20
.github/scripts/otel-naming/check_otel_naming.py
vendored
@@ -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(
|
||||
|
||||
359
.github/scripts/telemetry/check_regression_bounds.py
vendored
Normal file
359
.github/scripts/telemetry/check_regression_bounds.py
vendored
Normal file
@@ -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())
|
||||
286
.github/scripts/telemetry/test_check_regression_bounds.py
vendored
Normal file
286
.github/scripts/telemetry/test_check_regression_bounds.py
vendored
Normal file
@@ -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()
|
||||
Reference in New Issue
Block a user