docs(telemetry): align runbook and plan docs with the shipped phase-9/10 code

The reference docs had drifted from the code in ways that break the reader
rather than merely misinform: PromQL examples that return no data, a rollback
flag that is a no-op, a sampling knob that does not exist, and two span parents
that moved. Code is treated as the truth throughout; where the code is the
defective side, the doc now records it as a known issue instead of describing
the bug as intent.

Renames the docs missed: histogram names gain the exporter's unit suffix
(ios_latency_milliseconds_bucket and four siblings), ledger_history_mismatch
gains _total, the StatsD-era quantile label gives way to le buckets,
rpc.request becomes rpc.http_request, traces_spanmetrics_calls_total becomes
span_calls_total, and the nine dotted xrpl.* span attributes are recorded as
renamed rather than left as live keys.

Re-parenting: consensus.update_positions and consensus.check are children of
consensus.establish, not of consensus.round.

Units and labels: state_accounting_*_duration is microseconds, not seconds;
cache_metrics label values are case-sensitive; object_count carries demangled
C++ type names. Nodestore read and write latency stays microseconds -- the
nanosecond accumulator change did not move the exported unit.

Adds what shipped but was undocumented: the ledger.acquire span, seven
consensus.round events, twelve span attributes, node_writes_duration_us, the
7-day validation-agreement window, the TxQ admission and reduce-relay metric
families, metrics_endpoint, and the phase-10 validation workflow.

Corrects claims that never held: 10% head sampling (it is fixed at 100%),
configurable redaction (it is unconditional), -DXRPL_ENABLE_TELEMETRY=OFF
(the flag is -Dtelemetry=OFF, default ON), FindOpenTelemetry.cmake and the
xrpl_telemetry target (neither exists), Promtail and a StatsD exporter in the
pipeline (neither exists), and Loki stream selection on job= (only
service_name is a stream label).

Phase 9 is marked complete, its provisioned alerting is attributed to the
branch that shipped it, and Phase 11 stays at zero except the one prerequisite
its code closes. Counts are reconciled repo-wide: 41 emitted span families,
15 dashboards on disk with 14 asserted, 13 alert rules in 5 groups.

Hardens the gate that let this drift through: Rule E of the naming check now
covers the reference docs, its allow-dotted marker is key-scoped and warns on
stale or empty use, a missing checked file is reported instead of silently
skipped, the test suite runs in CI, and doc paths trigger the check.

C++ and CMake changes are comment-only: three MetricsRegistry instrument names,
eight OTelCollector claims of a metric-name prefix that formatName never adds,
and the telemetry option's inverted default.
This commit is contained in:
Pratik Mankawde
2026-08-13 16:18:47 +01:00
parent 733af97ce3
commit 3153f3ef56
35 changed files with 4598 additions and 1390 deletions

View File

@@ -47,7 +47,9 @@ Layers
L3 tempo : docker/telemetry/tempo.yaml (span filter tags)
L4 dashboards: docker/telemetry/grafana/dashboards/*.json (PromQL labels;
Loki/LogQL queries exempt -- see LOG_QUERY_DATASOURCES)
L5 runbook : docs/telemetry-runbook.md (attr tables)
L5 docs : every *.md under docs/ and docker/telemetry/ (attr tables;
the set is DISCOVERED, never enumerated -- see
RULE_E_DOC_ROOTS)
L6 metrics : MetricsRegistry.cpp instrument labels (native-metric
label keys, a valid dashboard-label source besides L1)
@@ -71,11 +73,17 @@ Rules (each FAILS the build, when its inputs are present)
captures or an in-query `| regexp` stage, so they have no L1/L6 source to
resolve against (see LOG_QUERY_DATASOURCES). The exemption is per QUERY,
not per file, so a mixed dashboard still has its PromQL panels checked.
E No dotted `xrpl.<domain>.<field>` attribute key in the runbook (only the
L1 resource attrs xrpl.network.* and the EXTERNAL_INFRA_LABELS dotted
form -- xrpl.work.item/.branch/.node.role -- may be dotted). Span names,
filenames,
OTel-standard keys, and metric labels are not flagged.
E No dotted `xrpl.<domain>.<field>` attribute key in any L5 doc. The doc set
is DISCOVERED, not listed: every `*.md` under RULE_E_DOC_ROOTS, so a doc
added or renamed inside a root is covered without editing this script.
Only the L1 resource
attrs xrpl.network.* and the EXTERNAL_INFRA_LABELS dotted form --
xrpl.work.item/.branch/.node.role -- may be dotted. Span names, filenames,
OTel-standard keys, and metric labels are not flagged. A doc that names the
wrong form as a deliberate counter-example opts that mention out with a
key-scoped marker (RULE_E_MARKER_SYNTAX), which exempts ONLY the keys it
lists on the line it appears on -- any other dotted token on that same line
still fails.
Warnings (printed, but do NOT fail the build)
----------------------------------------------
@@ -84,6 +92,13 @@ Warnings (printed, but do NOT fail the build)
*SpanNames.h (single source of truth); defining one in-place bypasses the
naming rules. A warning (not a failure) because the argument may instead
be a legitimately dynamic local (e.g. a computed span-name leaf).
E A RULE_E_REQUIRED_DOCS anchor is not present in the tree (renamed, moved,
or deleted -- so doc discovery can no longer be trusted to have found the
docs that matter), an allow-dotted marker names no key (so it exempts
nothing), or it names a key the line no longer mentions (a stale
exemption). Warnings, not failures, because presence-gating must keep
working on partial branches -- but the skip is now visible instead of
silent.
Exit code is non-zero if any present-and-enforced rule finds a violation.
Warnings never change the exit code.
@@ -482,7 +497,7 @@ def main() -> None:
run_rule_b_collector(root, l1_keys, report)
run_rule_c_tempo(root, l1_keys, report)
run_rule_d_dashboards(root, l1_keys, metric_labels, report)
run_rule_e_runbook(root, l1_keys, report)
run_rule_e_docs(root, l1_keys, report)
report.render_and_exit()
@@ -869,6 +884,107 @@ EXTERNAL_INFRA_LABELS = {
}
# L5 doc layer (Rule E): the doc trees whose markdown publishes span-attribute
# keys a reader is expected to copy into a TraceQL/PromQL query. A dotted
# `xrpl.*` key left in any of them hands out a query that silently matches
# nothing, so the whole tree is checked, not just the operator runbook:
# * docs/ operator runbook, telemetry glossary, and the
# build/telemetry guides (recursively)
# * docker/telemetry/ stack bring-up guide; its TraceQL/PromQL examples
# are meant to be pasted verbatim
#
# The set is DISCOVERED (every `*.md` under these roots), never enumerated. Two
# reasons this beats a hardcoded path list:
# * it cannot go stale -- a telemetry doc added, renamed, or moved within a
# root is picked up with no edit here, which is the same "derive it, do not
# hardcode it" principle the rest of this script follows (design
# principle 1), and coverage therefore grows with the docs; and
# * it names only doc ROOTS that ship on the default branch, so the rule can
# never be wired to a path that does not exist there.
# Each root is presence-gated (design principle 2): a branch that carries only
# one of them still gets that one checked. Both roots are also CI path-triggers
# for this check -- see `.github/workflows/on-pr.yml`.
RULE_E_DOC_ROOTS = (
Path("docs"),
Path("docker") / "telemetry",
)
# The anchor doc(s) discovery is REQUIRED to find. Discovery on its own can pass
# vacuously: if a root were renamed away, or emptied of markdown, Rule E would
# report a clean zero/near-zero-file run and nobody would notice the layer had
# stopped being checked. Demanding the one doc that is the whole reason Rule E
# exists turns that silent green into a visible warning (plus a count on the OK
# line). Deliberately tiny: this is a tripwire, not a second doc list. Every
# entry must live under RULE_E_DOC_ROOTS, or discovery could never find it.
RULE_E_REQUIRED_DOCS = (Path("docs") / "telemetry-runbook.md",)
def rule_e_docs(root: Path) -> List[Path]:
"""Discover the Rule E doc set: every `*.md` under RULE_E_DOC_ROOTS.
Returns repo-relative paths, sorted for stable reporting and de-duplicated
so overlapping roots (say `docs` and `docs/build`) cannot check one file
twice. A root absent from the tree is skipped rather than treated as an
error, so presence gating applies to the roots as it does to every other
layer."""
found: Set[Path] = set()
for rel_root in RULE_E_DOC_ROOTS:
base = root / rel_root
if not base.is_dir():
continue
found.update(p.relative_to(root) for p in base.rglob("*.md") if p.is_file())
return sorted(found)
# Line-scoped AND key-scoped opt-out for Rule E. Put it on a line that names a
# dotted key as a deliberate counter-example ("`tx_hash`, not `xrpl.tx.hash`") —
# a mention, not a published attribute key — and list exactly the keys that line
# is allowed to mention:
#
# ... use `tx_hash`, not `xrpl.tx.hash`.
# <!-- otel-naming:allow-dotted: xrpl.tx.hash -->
#
# The keys are part of the marker so an exemption cannot widen silently: a dotted
# token added to an already-marked line later, and not named in the marker, still
# fails. A marker with an empty key list therefore exempts NOTHING (and warns) —
# it is not a blanket line opt-out. NEVER use the marker to keep a real attribute
# table dotted; fix the table instead.
RULE_E_MARKER_SYNTAX = "<!-- otel-naming:allow-dotted: <key>[, <key>...] -->"
# The marker itself; group(1) is the raw key list (empty for the bare form).
# `[^>]` keeps the match inside one comment, so a marker cannot swallow the rest
# of the line, and the whole thing is matched per line (never across lines).
RULE_E_MARKER = re.compile(r"<!--\s*otel-naming:allow-dotted\s*:?\s*([^>]*?)\s*-->")
# Keys inside the marker are separated by commas and/or whitespace.
RULE_E_MARKER_KEY_SEP = re.compile(r"[,\s]+")
# The only doc form Rule E flags: a backticked dotted `xrpl.<domain>.<field>`.
RULE_E_DOTTED_TOKEN = re.compile(r"`(xrpl\.[a-z][a-z0-9_.]*)`")
def rule_e_allowed_keys(line: str) -> Tuple[Set[str], int]:
"""Parse every Rule-E allow-dotted marker on one line.
Returns `(keys named by the markers, number of markers seen)`. The count is
returned separately so the caller can tell "no marker" (enforce everything,
silently) from "a marker that names no key" (enforce everything, and warn
that the marker does nothing).
Keys may be written bare or backticked and separated by commas and/or
spaces, so both of these parse to the same two keys::
<!-- otel-naming:allow-dotted: xrpl.tx.hash, xrpl.peer.id -->
<!-- otel-naming:allow-dotted: `xrpl.tx.hash` `xrpl.peer.id` -->
"""
keys: Set[str] = set()
count = 0
for m in RULE_E_MARKER.finditer(line):
count += 1
for raw in RULE_E_MARKER_KEY_SEP.split(m.group(1)):
token = raw.strip().strip("`")
if token:
keys.add(token)
return keys, count
# Datasource types whose query language draws its label names from the log
# stream at query time rather than from anything this repo's OTel code emits.
#
@@ -1059,20 +1175,33 @@ def run_rule_d_dashboards(
report.ok(note + ")")
def run_rule_e_runbook(root: Path, l1_keys: Set[str], report: Report) -> None:
path = root / "docs" / "telemetry-runbook.md"
if not path.is_file():
report.skip("E", "runbook not present")
def run_rule_e_docs(root: Path, l1_keys: Set[str], report: Report) -> None:
discovered = rule_e_docs(root)
if not discovered:
roots = ", ".join(str(rel) for rel in RULE_E_DOC_ROOTS)
report.skip("E", f"no doc-layer file present (no *.md under {roots})")
return
# Presence-gating (design principle 2) must stay VISIBLE. Discovery cannot go
# stale, but it can go QUIET: a root renamed away, or stripped of its
# telemetry docs, still yields a clean run over whatever markdown is left, so
# the layer would stop being checked while the rule reported green. Warn per
# missing anchor — non-fatal, so a partial branch still passes — and name the
# count on the OK line, so the doc set cannot quietly shrink.
missing_required = [
rel for rel in RULE_E_REQUIRED_DOCS if not (root / rel).is_file()
]
for rel in missing_required:
report.warning(
"E", str(rel), "absent", "required Rule E doc not in tree (renamed?)"
)
if not l1_keys:
report.skip("E", "no L1 key set to validate against")
return
text = read_source(path)
found = False
# Only the dotted `xrpl.<domain>.<field>` attribute form is a violation. The
# `xrpl.`-with-trailing-dot anchor is the discriminator: it matches the old
# dotted attribute convention being migrated away from, while everything
# else legitimately dotted in the runbook does NOT match it —
# else legitimately dotted in these docs does NOT match it —
# * span names (`consensus.round`, `tx.process`) no `xrpl.` prefix
# * filenames (`xrpld.cfg`, `RCLConsensus.cpp`) `xrpld.`/`.cpp`, not `xrpl.`
# * OTel-standard (`service.name`, `http.method`) no `xrpl.` prefix
@@ -1085,19 +1214,57 @@ def run_rule_e_runbook(root: Path, l1_keys: Set[str], report: Report) -> None:
# identities dotted (xrpl.work.item/.branch/.node.role -- see the alloy
# pipeline that owns them), so also skip a token whose dotted-to-underscore
# form is in that set.
# A doc that TEACHES the convention has to be able to name the wrong form as
# a counter-example ("`tx_hash`, not `xrpl.tx.hash`"). The allow-dotted
# marker is the opt-out for exactly that: the token is a mention, not a
# published attribute key. It follows the repo's existing inline-marker
# precedent (`<!-- cspell:ignore ... -->`), and is scoped BOTH to the line
# and to the keys it names, so it can neither exempt a whole table nor grow
# to cover a violation appended to an already-marked line.
external_infra_dotted = {lbl.replace("_", ".") for lbl in EXTERNAL_INFRA_LABELS}
for m in re.finditer(r"`(xrpl\.[a-z][a-z0-9_.]*)`", text):
token = m.group(1)
if token in l1_keys: # legitimate dotted resource attr (xrpl.network.*)
continue
if token in external_infra_dotted: # perf-iac resource-attribute layer
continue
found = True
report.violation(
"E", str(path.relative_to(root)), token, "underscore, not dotted"
)
for rel in discovered:
for lineno, line in enumerate(read_source(root / rel).splitlines(), start=1):
allowed, markers = rule_e_allowed_keys(line)
if markers and not allowed:
report.warning(
"E",
f"{rel}:{lineno}",
"allow-dotted",
f"marker names no key: exempts nothing. Use {RULE_E_MARKER_SYNTAX}",
)
tokens = [m.group(1) for m in RULE_E_DOTTED_TOKEN.finditer(line)]
for token in tokens:
if token in l1_keys: # legitimate dotted resource attr (xrpl.network.*)
continue
if token in external_infra_dotted: # perf-iac resource-attr layer
continue
if token in allowed: # named counter-example on this line
continue
found = True
report.violation(
"E",
f"{rel}:{lineno}",
token,
"underscore, not dotted",
)
# A key the marker names but the line no longer mentions is a stale
# exemption: harmless today, but it is how a marker silently starts
# covering more than the author reviewed. Flagged, never fatal.
for stale in sorted(allowed.difference(tokens)):
report.warning(
"E", f"{rel}:{lineno}", stale, "allow-dotted key not on this line"
)
if not found:
report.ok("E: runbook attribute references consistent with L1")
note = (
"E: doc attribute references consistent with L1 "
f"({len(discovered)} file(s) checked"
)
if missing_required:
note += (
f", {len(missing_required)} of {len(RULE_E_REQUIRED_DOCS)} "
"required doc(s) absent"
)
report.ok(note + ")")
if __name__ == "__main__":

View File

@@ -9,7 +9,7 @@ Run from anywhere:
Each rule is exercised in isolation against a synthetic tree / synthetic L1 key
set, covering positive (must flag), negative (must not flag), and boundary
cases. Rule E (runbook dotted-attribute detection) has the densest coverage
cases. Rule E (doc-layer dotted-attribute detection) has the densest coverage
because its discriminator — the `xrpl.<domain>.` prefix vs span names,
filenames, OTel-standard keys, and metric labels — is the subtlest.
"""
@@ -19,6 +19,7 @@ import importlib.util
import io
import json
import shutil
import subprocess
import tempfile
import unittest
from pathlib import Path
@@ -47,13 +48,25 @@ L1 = {
def _run_rule_e(runbook_text: str):
"""Run Rule E against a synthetic runbook; return the flagged tokens."""
return _run_rule_e_docs({"docs/telemetry-runbook.md": runbook_text})[0]
def _run_rule_e_docs(docs, l1_keys=None):
"""Run Rule E against a synthetic doc set.
`docs` maps a repo-relative path to that file's text; only the listed files
are created, so per-file presence gating can be exercised. Returns
(sorted flagged tokens, the Report) so location and skip/ok lines can be
asserted as well as the tokens."""
d = Path(tempfile.mkdtemp())
try:
(d / "docs").mkdir()
(d / "docs" / "telemetry-runbook.md").write_text(runbook_text)
for rel, text in docs.items():
path = d / rel
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(text)
report = chk.Report()
chk.run_rule_e_runbook(d, set(L1), report)
return sorted(v[2] for v in report.violations)
chk.run_rule_e_docs(d, set(L1) if l1_keys is None else set(l1_keys), report)
return sorted(v[2] for v in report.violations), report
finally:
shutil.rmtree(d)
@@ -166,7 +179,7 @@ class RuleERunbook(unittest.TestCase):
d = Path(tempfile.mkdtemp())
try:
report = chk.Report()
chk.run_rule_e_runbook(d, set(L1), report)
chk.run_rule_e_docs(d, set(L1), report)
self.assertEqual(report.violations, [])
self.assertTrue(any("SKIP: E" in s for s in report.skips))
finally:
@@ -178,13 +191,593 @@ class RuleERunbook(unittest.TestCase):
(d / "docs").mkdir()
(d / "docs" / "telemetry-runbook.md").write_text("`xrpl.tx.hash`")
report = chk.Report()
chk.run_rule_e_runbook(d, set(), report)
chk.run_rule_e_docs(d, set(), report)
self.assertEqual(report.violations, [])
self.assertTrue(any("SKIP: E" in s for s in report.skips))
finally:
shutil.rmtree(d)
# The doc paths the Rule E tests build synthetic trees from. All four are real,
# committable files that live under RULE_E_DOC_ROOTS, so a test tree has the same
# shape discovery meets in the repo. GLOSSARY and BUILD_GUIDE additionally prove
# discovery is not runbook-shaped: BUILD_GUIDE sits one directory deeper, so a
# non-recursive glob would miss it.
RUNBOOK_DOC = "docs/telemetry-runbook.md"
GLOSSARY_DOC = "docs/telemetry-glossary.md"
BUILD_GUIDE_DOC = "docs/build/telemetry.md"
TESTING_DOC = "docker/telemetry/TESTING.md"
ALL_DOCS = (RUNBOOK_DOC, GLOSSARY_DOC, BUILD_GUIDE_DOC, TESTING_DOC)
# Top-level trees that ship to the default branch, so anything the check is
# configured to read must live inside one of them. A configured path outside
# these would name a tree that is not part of the shipped repository: the rule
# would warn forever about a doc that can never appear, and the matching CI
# path-trigger would be dead weight.
COMMITTABLE_ROOTS = (Path("docs"), Path("docker"))
def _inside_any(rel: Path, roots) -> bool:
"""True if `rel` is one of `roots` or lives underneath one of them."""
return any(root == rel or root in rel.parents for root in roots)
def _git_tracks(root: Path, rel: Path) -> bool:
"""True if git tracks `rel` (a file) or anything under it (a directory).
Evidence that a configured path is genuinely part of the repository rather
than a local scratch or ignored directory."""
out = subprocess.run(
["git", "ls-files", "--", str(rel)],
cwd=str(root),
capture_output=True,
text=True,
)
return bool(out.stdout.strip())
class RuleEDocSet(unittest.TestCase):
"""Rule E scans every discovered doc, not just the runbook. The glossary, the
build/telemetry guide and the stack testing guide all publish attribute keys
readers copy queries from, so a dotted key left in any of them is as
operator-breaking as one in the runbook."""
RUNBOOK = RUNBOOK_DOC
GLOSSARY = GLOSSARY_DOC
BUILD_GUIDE = BUILD_GUIDE_DOC
TESTING = TESTING_DOC
def test_flags_dotted_attr_in_testing_guide(self):
# TESTING.md publishes copy-paste TraceQL/PromQL, so a dotted key there
# hands the reader a query that silently matches nothing.
tokens, report = _run_rule_e_docs(
{self.TESTING: '{name="tx.process" && span.`xrpl.tx.hash`="AB"}'}
)
self.assertEqual(tokens, ["xrpl.tx.hash"])
self.assertEqual(report.violations[0][1], f"{self.TESTING}:1")
self.assertEqual(report.violations[0][3], "underscore, not dotted")
def test_testing_guide_checked_when_every_other_doc_absent(self):
# Per-file presence gating: TESTING.md alone must not skip the rule.
tokens, report = _run_rule_e_docs({self.TESTING: "`xrpl.peer.version`"})
self.assertEqual(tokens, ["xrpl.peer.version"])
self.assertEqual(report.skips, [])
self.assertEqual(report.violations[0][1], f"{self.TESTING}:1")
# A violating run emits the violation, not an "ok" line.
self.assertEqual(report.checked, [])
def test_testing_guide_alone_reports_one_clean_file(self):
tokens, report = _run_rule_e_docs({self.TESTING: "`tx_hash`"})
self.assertEqual(tokens, [])
self.assertTrue(any("E:" in c and "1 file(s)" in c for c in report.checked))
def test_clean_testing_guide_passes_with_bare_keys(self):
# The real docker/telemetry/TESTING.md shape: bare/underscore attr keys
# plus dotted SPAN names, which must not be flagged.
tokens, report = _run_rule_e_docs(
{self.TESTING: '{name="tx.process" && span.tx_hash!=""} `ledger_seq`'}
)
self.assertEqual(tokens, [])
self.assertEqual(report.violations, [])
def test_exemptions_apply_in_testing_guide_too(self):
tokens, _ = _run_rule_e_docs(
{self.TESTING: "`xrpl.network.type` `xrpl.work.item`"}
)
self.assertEqual(tokens, [])
def test_flags_dotted_attr_in_glossary(self):
tokens, report = _run_rule_e_docs({self.GLOSSARY: "| `xrpl.tx.hash` | h |"})
self.assertEqual(tokens, ["xrpl.tx.hash"])
self.assertEqual(report.violations[0][1], f"{self.GLOSSARY}:1")
def test_flags_dotted_attr_in_nested_build_guide(self):
# One directory below the root: proves discovery recurses rather than
# globbing only the root's own children.
tokens, report = _run_rule_e_docs({self.BUILD_GUIDE: "`xrpl.consensus.mode`"})
self.assertEqual(tokens, ["xrpl.consensus.mode"])
self.assertEqual(report.violations[0][1], f"{self.BUILD_GUIDE}:1")
def test_location_carries_the_offending_line_number(self):
tokens, report = _run_rule_e_docs(
{self.GLOSSARY: "clean\nstill clean\n| `xrpl.tx.hash` |"}
)
self.assertEqual(tokens, ["xrpl.tx.hash"])
self.assertEqual(report.violations[0][1], f"{self.GLOSSARY}:3")
def test_flags_across_all_four_docs(self):
tokens, report = _run_rule_e_docs(
{
self.RUNBOOK: "`xrpl.tx.hash`",
self.GLOSSARY: "`xrpl.peer.id`",
self.BUILD_GUIDE: "`xrpl.ledger.seq`",
self.TESTING: "`xrpl.rpc.command`",
}
)
self.assertEqual(
tokens,
[
"xrpl.ledger.seq",
"xrpl.peer.id",
"xrpl.rpc.command",
"xrpl.tx.hash",
],
)
self.assertEqual(
sorted(v[1] for v in report.violations),
sorted(
[
f"{self.RUNBOOK}:1",
f"{self.GLOSSARY}:1",
f"{self.BUILD_GUIDE}:1",
f"{self.TESTING}:1",
]
),
)
def test_other_doc_checked_when_runbook_absent(self):
# Presence gating is per discovered file: a branch without the runbook
# still gets the docs it does carry checked, instead of the rule skipping.
tokens, report = _run_rule_e_docs({self.GLOSSARY: "`xrpl.tx.hash`"})
self.assertEqual(tokens, ["xrpl.tx.hash"])
self.assertEqual(report.skips, [])
def test_absent_doc_is_not_an_error(self):
tokens, report = _run_rule_e_docs({self.RUNBOOK: "`tx_hash`"})
self.assertEqual(tokens, [])
self.assertTrue(any("1 file(s)" in c for c in report.checked))
def test_clean_doc_set_reports_file_count(self):
tokens, report = _run_rule_e_docs(
{
self.RUNBOOK: "`tx_hash` `consensus.round`",
self.GLOSSARY: "`xrpl.network.id` `ledger_seq`",
self.BUILD_GUIDE: "`service.name` `peer_id`",
self.TESTING: "`rpc_command` `tx.process`",
}
)
self.assertEqual(tokens, [])
self.assertEqual(report.violations, [])
self.assertTrue(any("E:" in c and "4 file(s)" in c for c in report.checked))
def test_skips_when_no_doc_present(self):
tokens, report = _run_rule_e_docs({})
self.assertEqual(tokens, [])
self.assertEqual(report.checked, [])
self.assertTrue(any("SKIP: E" in s for s in report.skips))
def test_exemptions_apply_in_every_discovered_doc(self):
# The L1 resource attrs and perf-iac dotted identities must stay exempt
# in every discovered doc, exactly as in the runbook.
tokens, _ = _run_rule_e_docs(
{
self.GLOSSARY: "`xrpl.network.id` `xrpl.network.type`",
self.BUILD_GUIDE: "`xrpl.work.item` `xrpl.branch` `xrpl.node.role`",
}
)
self.assertEqual(tokens, [])
def _marker(*keys: str) -> str:
"""Build an allow-dotted marker naming `keys` (no key = the bare form)."""
body = ": " + ", ".join(keys) if keys else ""
return f"<!-- otel-naming:allow-dotted{body} -->"
def _marker_warnings(report):
"""The marker-related warnings only.
A synthetic doc set that does not create the runbook leaves that required
anchor absent, which is warned about in its own right (see
RuleERequiredDocPresence). Filtering those out keeps the marker tests
asserting the marker's own behaviour."""
return [tuple(w) for w in report.warnings if w[2] != "absent"]
class RuleEAllowDottedMarkerParsing(unittest.TestCase):
"""`rule_e_allowed_keys` — the marker parser. The key list is what bounds the
exemption, so its parsing (and the marker COUNT, which distinguishes "no
marker" from "a marker that names nothing") is asserted directly."""
def test_no_marker_yields_no_keys_and_no_marker_count(self):
self.assertEqual(
chk.rule_e_allowed_keys("plain `xrpl.tx.hash` line"), (set(), 0)
)
def test_bare_marker_counts_but_names_no_key(self):
self.assertEqual(chk.rule_e_allowed_keys(_marker()), (set(), 1))
def test_single_key(self):
self.assertEqual(
chk.rule_e_allowed_keys(_marker("xrpl.tx.hash")), ({"xrpl.tx.hash"}, 1)
)
def test_comma_separated_keys(self):
self.assertEqual(
chk.rule_e_allowed_keys(_marker("xrpl.tx.hash", "xrpl.node.server_state")),
({"xrpl.tx.hash", "xrpl.node.server_state"}, 1),
)
def test_space_separated_and_backticked_keys(self):
self.assertEqual(
chk.rule_e_allowed_keys(
"<!-- otel-naming:allow-dotted: `xrpl.tx.hash` `xrpl.peer.id` -->"
),
({"xrpl.tx.hash", "xrpl.peer.id"}, 1),
)
def test_no_surrounding_whitespace(self):
self.assertEqual(
chk.rule_e_allowed_keys("<!--otel-naming:allow-dotted:xrpl.tx.hash-->"),
({"xrpl.tx.hash"}, 1),
)
def test_two_markers_on_one_line_union(self):
line = _marker("xrpl.tx.hash") + " text " + _marker("xrpl.peer.id")
self.assertEqual(
chk.rule_e_allowed_keys(line), ({"xrpl.tx.hash", "xrpl.peer.id"}, 2)
)
def test_prose_around_marker_is_not_absorbed_as_a_key(self):
line = "use `tx_hash`, not `xrpl.tx.hash`. " + _marker("xrpl.tx.hash") + " ok"
self.assertEqual(chk.rule_e_allowed_keys(line), ({"xrpl.tx.hash"}, 1))
def test_trailing_comma_does_not_yield_empty_key(self):
self.assertEqual(
chk.rule_e_allowed_keys("<!-- otel-naming:allow-dotted: xrpl.tx.hash, -->"),
({"xrpl.tx.hash"}, 1),
)
class RuleEAllowDottedMarkerEnforcement(unittest.TestCase):
"""The marker exempts ONLY the keys it names, only on its own line. A
blanket line opt-out would let a genuine violation appended to any marked
line ride in silently, which is exactly what these tests forbid."""
REFERENCE = GLOSSARY_DOC
def test_named_key_is_exempt(self):
tokens, report = _run_rule_e_docs(
{
self.REFERENCE: "use `tx_hash`, not `xrpl.tx.hash`. "
+ _marker("xrpl.tx.hash")
}
)
self.assertEqual(tokens, [])
self.assertEqual(report.violations, [])
self.assertEqual(_marker_warnings(report), [])
def test_unlisted_key_on_marked_line_still_fails(self):
# The hole this closes: one marker must not cover a second dotted key
# someone appends to the line later.
tokens, report = _run_rule_e_docs(
{
self.REFERENCE: "not `xrpl.tx.hash` and also `xrpl.peer.id` "
+ _marker("xrpl.tx.hash")
}
)
self.assertEqual(tokens, ["xrpl.peer.id"])
self.assertEqual(report.violations[0][0], "E")
self.assertEqual(report.violations[0][1], f"{self.REFERENCE}:1")
self.assertEqual(report.violations[0][3], "underscore, not dotted")
def test_bare_marker_exempts_nothing_and_warns(self):
tokens, report = _run_rule_e_docs(
{self.REFERENCE: "not `xrpl.tx.hash`. " + _marker()}
)
self.assertEqual(tokens, ["xrpl.tx.hash"])
self.assertEqual(report.violations[0][1], f"{self.REFERENCE}:1")
warnings = _marker_warnings(report)
self.assertEqual(
[w[:3] for w in warnings], [("E", f"{self.REFERENCE}:1", "allow-dotted")]
)
self.assertIn(chk.RULE_E_MARKER_SYNTAX, warnings[0][3])
def test_marker_does_not_leak_to_adjacent_lines(self):
tokens, report = _run_rule_e_docs(
{
self.REFERENCE: "| `xrpl.tx.hash` | before |\n"
+ "not `xrpl.tx.hash`. "
+ _marker("xrpl.tx.hash")
+ "\n| `xrpl.tx.hash` | after |"
}
)
self.assertEqual(tokens, ["xrpl.tx.hash", "xrpl.tx.hash"])
self.assertEqual(
sorted(v[1] for v in report.violations),
[f"{self.REFERENCE}:1", f"{self.REFERENCE}:3"],
)
def test_all_named_keys_exempt_on_a_table_row(self):
# The real shape in the plan docs: a "was -> is now" row naming two old
# dotted keys, with both listed in the marker.
tokens, _ = _run_rule_e_docs(
{
self.REFERENCE: "| `xrpl.validation.full`, `xrpl.peer.validation.full`"
" | one bare `full_validation` | "
+ _marker("xrpl.validation.full", "xrpl.peer.validation.full")
}
)
self.assertEqual(tokens, [])
def test_partially_listed_table_row_fails_on_the_unlisted_key(self):
tokens, report = _run_rule_e_docs(
{
self.REFERENCE: "| `xrpl.validation.full`, `xrpl.peer.validation.full`"
" | one bare `full_validation` | " + _marker("xrpl.validation.full")
}
)
self.assertEqual(tokens, ["xrpl.peer.validation.full"])
self.assertEqual(report.violations[0][1], f"{self.REFERENCE}:1")
def test_key_prefix_does_not_exempt_a_longer_key(self):
# Exemption is an exact token match, not a prefix match.
tokens, _ = _run_rule_e_docs(
{self.REFERENCE: "`xrpl.tx.hash` " + _marker("xrpl.tx")}
)
self.assertEqual(tokens, ["xrpl.tx.hash"])
def test_dotted_prefix_token_can_be_listed_verbatim(self):
# The docs grep for prefixes such as `xrpl.node.` (trailing dot); the
# marker must accept that exact token.
tokens, report = _run_rule_e_docs(
{
self.REFERENCE: "a grep for `xrpl.node.` and `xrpl.peer.` returns "
"nothing " + _marker("xrpl.node.", "xrpl.peer.")
}
)
self.assertEqual(tokens, [])
self.assertEqual(_marker_warnings(report), [])
def test_stale_key_warns_but_does_not_fail(self):
tokens, report = _run_rule_e_docs(
{self.REFERENCE: "all clean now. " + _marker("xrpl.tx.hash")}
)
self.assertEqual(tokens, [])
self.assertEqual(report.violations, [])
self.assertEqual(
_marker_warnings(report),
[
(
"E",
f"{self.REFERENCE}:1",
"xrpl.tx.hash",
"allow-dotted key not on this line",
)
],
)
def test_marker_naming_an_l1_key_is_not_reported_stale(self):
# `xrpl.network.id` is exempt via L1, so a marker naming it is redundant
# but not stale — the key IS on the line.
tokens, report = _run_rule_e_docs(
{self.REFERENCE: "`xrpl.network.id` " + _marker("xrpl.network.id")}
)
self.assertEqual(tokens, [])
self.assertEqual(_marker_warnings(report), [])
def test_marker_works_in_every_doc_of_the_set(self):
docs = {
rel: "not `xrpl.tx.hash`. " + _marker("xrpl.tx.hash") for rel in ALL_DOCS
}
tokens, report = _run_rule_e_docs(docs)
self.assertEqual(tokens, [])
self.assertEqual(report.violations, [])
class RuleEDocDiscovery(unittest.TestCase):
"""`rule_e_docs` — the discovery that replaced a hardcoded doc list. What has
to hold: it finds the real docs in this repo (a rule scanning an empty set
passes without checking anything), it recurses into subdirectories, it never
checks a file twice, and every path it is configured with lies inside a tree
that actually ships."""
def test_discovery_finds_more_than_one_doc_in_this_repo(self):
# The vacuous-pass guard, measured against the real tree: one file (or
# none) would mean discovery had collapsed and Rule E was reporting green
# over almost nothing.
found = chk.rule_e_docs(chk.repo_root())
self.assertGreater(len(found), 1, found)
def test_discovery_returns_no_duplicates(self):
# Roots may overlap (a root and a subdirectory of it); a file must still
# be checked once, so line numbers are not reported twice.
found = chk.rule_e_docs(chk.repo_root())
self.assertEqual(len(found), len(set(found)))
def test_discovery_finds_the_known_telemetry_docs(self):
# Named here as an expectation of DISCOVERY, not as the checker's config:
# if any of these is renamed, discovery still covers it under its new
# name and only this assertion needs updating.
found = set(chk.rule_e_docs(chk.repo_root()))
for rel in (RUNBOOK_DOC, GLOSSARY_DOC, TESTING_DOC):
self.assertIn(Path(rel), found)
def test_discovery_recurses_into_subdirectories(self):
d = Path(tempfile.mkdtemp())
try:
nested = d / "docs" / "build" / "deep"
nested.mkdir(parents=True)
(nested / "telemetry.md").write_text("`tx_hash`")
self.assertEqual(
chk.rule_e_docs(d), [Path("docs") / "build" / "deep" / "telemetry.md"]
)
finally:
shutil.rmtree(d)
def test_discovery_ignores_non_markdown(self):
d = Path(tempfile.mkdtemp())
try:
(d / "docs").mkdir()
(d / "docs" / "notes.txt").write_text("`xrpl.tx.hash`")
(d / "docs" / "runbook.md").write_text("`tx_hash`")
self.assertEqual(chk.rule_e_docs(d), [Path("docs") / "runbook.md"])
finally:
shutil.rmtree(d)
def test_discovery_tolerates_an_absent_root(self):
# Presence gating applies to the roots too: a tree carrying only one of
# them must not raise, and must still discover that one.
d = Path(tempfile.mkdtemp())
try:
(d / "docker" / "telemetry").mkdir(parents=True)
(d / "docker" / "telemetry" / "TESTING.md").write_text("`tx_hash`")
self.assertEqual(
chk.rule_e_docs(d), [Path("docker") / "telemetry" / "TESTING.md"]
)
finally:
shutil.rmtree(d)
def test_no_configured_path_lies_outside_the_committable_roots(self):
# The check may only be wired to trees that ship. A configured path
# outside them would make Rule E warn about a doc that can never appear
# and leave the matching CI path-trigger dead.
for rel in chk.RULE_E_DOC_ROOTS + chk.RULE_E_REQUIRED_DOCS:
self.assertTrue(_inside_any(rel, COMMITTABLE_ROOTS), rel)
def test_configured_paths_exist_and_are_version_controlled(self):
# "Committable" is not just a naming claim: git must actually track the
# contents of every configured path in this repo.
root = chk.repo_root()
for rel in chk.RULE_E_DOC_ROOTS:
self.assertTrue((root / rel).is_dir(), rel)
self.assertTrue(_git_tracks(root, rel), rel)
for rel in chk.RULE_E_REQUIRED_DOCS:
self.assertTrue((root / rel).is_file(), rel)
self.assertTrue(_git_tracks(root, rel), rel)
def test_every_required_doc_is_discoverable(self):
# An anchor outside every root could never be found, so the tripwire
# would fire on every run and stop meaning anything.
for rel in chk.RULE_E_REQUIRED_DOCS:
self.assertTrue(_inside_any(rel, chk.RULE_E_DOC_ROOTS), rel)
self.assertIn(rel, chk.rule_e_docs(chk.repo_root()))
class RuleERequiredDocPresence(unittest.TestCase):
"""A RULE_E_REQUIRED_DOCS anchor that is not in the tree must be VISIBLE.
Discovery cannot go stale, but it can go quiet — a root emptied of telemetry
docs still yields a clean run — so the anchor's absence is warned about and
counted, while staying non-fatal so a partial branch still passes."""
RUNBOOK = RUNBOOK_DOC
def test_every_required_doc_resolves_in_this_repo(self):
# If the runbook is renamed and RULE_E_REQUIRED_DOCS is not updated, the
# anchor warning would fire on every run; catch it here instead.
root = chk.repo_root()
self.assertEqual(
[
str(rel)
for rel in chk.RULE_E_REQUIRED_DOCS
if not (root / rel).is_file()
],
[],
)
def test_missing_anchor_is_warned_once(self):
# Discovery finds the glossary, so the rule runs — but the anchor it is
# required to find is gone, which must be said out loud.
tokens, report = _run_rule_e_docs({GLOSSARY_DOC: "`tx_hash`"})
self.assertEqual(tokens, [])
self.assertEqual(
[tuple(w) for w in report.warnings],
[
(
"E",
self.RUNBOOK,
"absent",
"required Rule E doc not in tree (renamed?)",
)
],
)
def test_ok_line_names_the_absent_anchor_count(self):
tokens, report = _run_rule_e_docs({GLOSSARY_DOC: "`tx_hash`"})
self.assertEqual(tokens, [])
self.assertTrue(
any(
"1 file(s) checked" in c
and f"1 of {len(chk.RULE_E_REQUIRED_DOCS)} required doc(s) absent" in c
for c in report.checked
),
report.checked,
)
def test_full_doc_set_warns_nothing_and_reports_the_full_count(self):
docs = {rel: "`tx_hash`" for rel in ALL_DOCS}
tokens, report = _run_rule_e_docs(docs)
self.assertEqual(tokens, [])
self.assertEqual(report.warnings, [])
self.assertTrue(
any(
f"{len(ALL_DOCS)} file(s) checked" in c and "absent" not in c
for c in report.checked
),
report.checked,
)
def test_anchor_present_warns_nothing_even_with_other_docs_absent(self):
# Only the ANCHOR is required. A tree carrying just the runbook is a
# legitimate partial branch, not a shrinking doc set.
tokens, report = _run_rule_e_docs({self.RUNBOOK: "`tx_hash`"})
self.assertEqual(tokens, [])
self.assertEqual(report.warnings, [])
self.assertTrue(
any("1 file(s) checked" in c and "absent" not in c for c in report.checked),
report.checked,
)
def test_absent_anchor_is_warned_even_when_l1_is_empty(self):
# The anchor can be verified without an L1 key set, so the visibility
# signal must not depend on the (separately gated) key comparison.
tokens, report = _run_rule_e_docs({GLOSSARY_DOC: "`xrpl.tx.hash`"}, l1_keys=[])
self.assertEqual(tokens, [])
self.assertEqual(len(report.warnings), len(chk.RULE_E_REQUIRED_DOCS))
self.assertTrue(any("SKIP: E" in s for s in report.skips))
def test_no_anchor_warning_when_discovery_finds_nothing(self):
# Nothing to check at all is already reported by the SKIP line, which
# names the roots it searched, so an anchor warning would be noise.
tokens, report = _run_rule_e_docs({})
self.assertEqual(tokens, [])
self.assertEqual(report.warnings, [])
roots = ", ".join(str(rel) for rel in chk.RULE_E_DOC_ROOTS)
self.assertEqual(
report.skips,
[f"SKIP: E — no doc-layer file present (no *.md under {roots})"],
)
def test_doc_outside_every_root_is_not_discovered(self):
# The counterpart of the roots being committable: markdown parked outside
# them is not part of the L5 layer, so a dotted key there cannot fail the
# build — and equally cannot make the rule look like it checked something.
tokens, report = _run_rule_e_docs({"elsewhere/notes.md": "`xrpl.tx.hash`"})
self.assertEqual(tokens, [])
self.assertEqual(report.checked, [])
self.assertTrue(any("SKIP: E" in s for s in report.skips))
class DslParser(unittest.TestCase):
"""The makeStr/join/seg:: constexpr DSL resolver — the foundation of the
L1 key set. Covers flat, nested, cross-file, alias, and multi-line forms."""
@@ -1064,7 +1657,7 @@ class RuleEReportTuple(unittest.TestCase):
(d / "docs").mkdir()
(d / "docs" / "telemetry-runbook.md").write_text("`xrpl.tx.hash`")
report = chk.Report()
chk.run_rule_e_runbook(d, {"xrpl.network.id"}, report)
chk.run_rule_e_docs(d, {"xrpl.network.id"}, report)
self.assertEqual(len(report.violations), 1)
rule, _loc, token, expected = report.violations[0]
self.assertEqual(rule, "E")
@@ -1081,7 +1674,7 @@ class RuleEReportTuple(unittest.TestCase):
"`tx_hash` `consensus.round`"
)
report = chk.Report()
chk.run_rule_e_runbook(d, {"tx_hash"}, report)
chk.run_rule_e_docs(d, {"tx_hash"}, report)
self.assertEqual(report.violations, [])
self.assertTrue(any("E:" in c for c in report.checked))
finally: