From 6c4c3e104960c3b7ebe068cebed15a4830e9d453 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:01:18 +0100 Subject: [PATCH 1/2] layering Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- .github/scripts/levelization/results/loops.txt | 2 +- .github/scripts/levelization/results/ordering.txt | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/scripts/levelization/results/loops.txt b/.github/scripts/levelization/results/loops.txt index 181cbec44a..a108f7b465 100644 --- a/.github/scripts/levelization/results/loops.txt +++ b/.github/scripts/levelization/results/loops.txt @@ -5,7 +5,7 @@ Loop: test.jtx test.unit_test test.unit_test ~= test.jtx Loop: xrpl.telemetry xrpld.rpc - xrpld.rpc ~= xrpl.telemetry + xrpld.rpc == xrpl.telemetry Loop: xrpld.app xrpld.overlay xrpld.app > xrpld.overlay diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 0e86ca8525..c480165983 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -252,7 +252,6 @@ xrpl.shamap > xrpl.basics xrpl.shamap > xrpl.nodestore xrpl.shamap > xrpl.protocol xrpl.telemetry > xrpl.config -xrpl.telemetry > xrpld.consensus xrpl.tx > xrpl.basics xrpl.tx > xrpl.core xrpl.tx > xrpl.ledger From 59030e5d61661bbbe2bd3fa599c53198f3fa570f Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:21:42 +0100 Subject: [PATCH 2/2] fixed a rule in otel naming check file. added tests for it. Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- .github/scripts/otel-naming/README.md | 4 +- .../scripts/otel-naming/check_otel_naming.py | 238 +++++--- .../otel-naming/test_check_otel_naming.py | 571 ++++++++++++++++++ 3 files changed, 735 insertions(+), 78 deletions(-) create mode 100644 .github/scripts/otel-naming/test_check_otel_naming.py diff --git a/.github/scripts/otel-naming/README.md b/.github/scripts/otel-naming/README.md index 421a108931..5f185cec21 100644 --- a/.github/scripts/otel-naming/README.md +++ b/.github/scripts/otel-naming/README.md @@ -41,7 +41,7 @@ hardcoded allowlist: | B | Every collector `spanmetrics.dimensions` name exists in the L1 key set. | | C | Every Tempo span-filter tag exists in the L1 key set. | | D | Every dashboard PromQL label (non-builtin) exists in the L1 key set. | -| E | Every runbook attribute reference exists in the L1 key set. | +| E | No dotted `xrpl..` attribute key in the runbook (only the L1 resource attrs `xrpl.network.*` may be dotted). Span names, filenames, OTel-standard keys, and metric labels are not flagged. | Rule F runs **unconditionally** (it is a purely syntactic check on the call-sites and needs no `*SpanNames.h`), so a code path that calls @@ -62,4 +62,4 @@ This keeps the check correct no matter how telemetry work is split across PRs a stacked chain, one large PR, or independent per-stage PRs where (for example) the collector config lands before the dashboards. The collector/Tempo/dashboard/ runbook layers are introduced in later phases; on a branch without them, only -the L1-intrinsic rules (A, F) run. +the L1-intrinsic rules (A, G, F) run. diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index 13b79237dc..aeaa7e003c 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """ -Usage: check_naming.py +Usage: check_otel_naming.py This script takes no parameters and can be called from any directory inside the repository (it locates the repo root via `git rev-parse`). @@ -51,7 +51,9 @@ Rules (each FAILS the build, when its inputs are present) B Every collector spanmetrics dimension exists in the L1 key set. C Every tempo span-filter tag exists in the L1 key set. D Every dashboard PromQL label (non-builtin) exists in the L1 key set. - E Every attribute named in the runbook tables exists in the L1 key set. + E No dotted `xrpl..` attribute key in the runbook (only the + L1 resource attrs xrpl.network.* may be dotted). Span names, filenames, + OTel-standard keys, and metric labels are not flagged. Warnings (printed, but do NOT fail the build) ---------------------------------------------- @@ -65,7 +67,6 @@ Exit code is non-zero if any present-and-enforced rule finds a violation. Warnings never change the exit code. """ -import json import re import subprocess import sys @@ -78,16 +79,32 @@ from typing import Dict, List, Optional, Set, Tuple def repo_root() -> Path: - """Return the repository root, so the script works from any CWD.""" - out = subprocess.run( - ["git", "rev-parse", "--show-toplevel"], - capture_output=True, - text=True, - check=True, - ) + """Return the repository root, so the script works from any CWD. + + Exits with a readable message (not a traceback) if git is unavailable or the + CWD is outside a repository.""" + try: + out = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=True, + ) + except (subprocess.CalledProcessError, FileNotFoundError): + print( + "error: check_otel_naming.py must be run inside the git repository.", + file=sys.stderr, + ) + sys.exit(2) return Path(out.stdout.strip()) +def read_source(path: Path) -> str: + """Read a file as UTF-8, tolerating stray non-UTF-8 bytes rather than + crashing the whole check on one bad byte.""" + return path.read_text(encoding="utf-8", errors="ignore") + + # --------------------------------------------------------------------------- # Regexes (compiled once) # --------------------------------------------------------------------------- @@ -97,6 +114,8 @@ CONST_DEF = re.compile(r"inline\s+constexpr\s+auto\s+(\w+)\s*=\s*(.+?);", re.DOT MAKESTR = re.compile(r'makeStr\(\s*"([^"]*)"\s*\)') # A `namespace {` opener, to track which namespace a constant lives in. NS_OPEN = re.compile(r"namespace\s+([\w:]+)\s*\{") +# A `using ::a::b::field;` re-export inside an attr block; captures the leaf. +USING_DECL = re.compile(r"using\s+(?:::)?[\w:]*::(\w+)\s*;") # Telemetry call-sites whose string arguments must be constants, not literals. # Require a receiver so we match real SpanGuard calls, not std::span / a math # `span(...)` / a bare method declaration: @@ -109,19 +128,26 @@ CALLSITE = re.compile( ) # A C++ string literal (used to flag literals inside call-site argument lists). STRING_LITERAL = re.compile(r'"((?:[^"\\]|\\.)*)"') -# Dotted token that looks like `a.b` or `a.b.c` (lowercase + underscore words). -DOTTED_TOKEN = re.compile(r"\b([a-z][a-z0-9_]*(?:\.[a-z][a-z0-9_]*)+)\b") +# A C++ line comment (`//` ... end of line) and a block comment (`/* ... */`). +LINE_COMMENT = re.compile(r"//[^\n]*") +BLOCK_COMMENT = re.compile(r"/\*.*?\*/", re.DOTALL) -def strip_block_comments(text: str) -> str: - """Remove /* ... */ comments but KEEP /// and // line comments. +def strip_comments(text: str) -> str: + """Remove C/C++ `//` line comments and `/* ... */` block comments. - Doxygen @code examples live in /** ... */ blocks, and Rule F must still see - the call-sites inside them, so we do NOT strip /** */ wholesale. Instead we - only strip C-style comments that are clearly not doc blocks is unsafe, so we - keep all comments and rely on call-site detection. This function currently - returns text unchanged and exists as a single, documented control point. + Used only for L1 attribute-key extraction so that a commented-out or + illustrative `makeStr("...")` inside a `namespace attr` block does not leak + into the authoritative key set. Rule F deliberately does NOT strip comments + — it must still see `@code` doc-comment examples so their call-site + arguments are held to the constant-only convention. + + String literals are not specially handled; a `//` or `/*` appearing inside a + string is vanishingly rare in the *SpanNames.h headers and would at worst + drop a constant from L1 (a conservative direction). """ + text = BLOCK_COMMENT.sub("", text) + text = LINE_COMMENT.sub("", text) return text @@ -189,17 +215,34 @@ def build_global_symbols(headers: List[Path]) -> Dict[str, str]: symbols: Dict[str, str] = {} ordered = sorted(headers, key=lambda p: (p.name != "SpanNames.h", str(p))) # Two passes: the first seeds segments, the second resolves dependents. + # Comments are stripped so a commented-out constant cannot seed the table. for _ in range(2): for h in ordered: - resolve_constants(h.read_text(), symbols) + resolve_constants(strip_comments(read_source(h)), symbols) return symbols def split_top_level_args(s: str) -> List[str]: - """Split a comma-separated arg list, respecting nested parentheses.""" + """Split a comma-separated arg list, respecting nested parentheses and + ignoring parens/commas that appear inside a "string literal" (so a value + like `setAttribute(k, ",")` does not get mis-split).""" args, depth, cur = [], 0, "" + in_str = False + escaped = False for ch in s: - if ch == "(": + if in_str: + cur += ch + if escaped: + escaped = False + elif ch == "\\": + escaped = True + elif ch == '"': + in_str = False + continue + if ch == '"': + in_str = True + cur += ch + elif ch == "(": depth += 1 cur += ch elif ch == ")": @@ -215,38 +258,54 @@ def split_top_level_args(s: str) -> List[str]: return args +def attr_namespace_spans(text: str) -> List[str]: + """Return the source text of each `namespace attr { ... }` block in `text`. + + Brace-matched over the whole (comment-stripped) text, so a definition that + wraps across several physical lines is contained in one span. Nested braces + inside the block are balanced correctly.""" + spans: List[str] = [] + for opener in NS_OPEN.finditer(text): + if opener.group(1).split("::")[-1] != "attr": + continue + # Walk from the opening brace, balancing nesting to the matching close. + i = opener.end() # one char past the namespace's `{` + depth = 1 + start = i + while i < len(text) and depth > 0: + c = text[i] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + i += 1 + spans.append(text[start : i - 1]) + return spans + + def attr_keys_from_header(path: Path, symbols: Dict[str, str]) -> Set[str]: """Return the set of attribute-key strings declared in a header's `namespace attr { ... }` block(s). `symbols` is the global cross-file - table so `join(seg::rpc, ...)` style dotted attrs resolve correctly.""" - text = path.read_text() + table so `join(seg::rpc, ...)` style dotted attrs resolve correctly. + + Comments are stripped first (a commented constant must not enter L1), and + each attr block is brace-matched over the whole text so multi-line + `inline constexpr auto NAME = join(\\n ...);` definitions are captured.""" + text = strip_comments(read_source(path)) keys: Set[str] = set() - # Walk the file tracking namespace nesting to find `namespace attr` blocks. - depth = 0 - attr_depth: Optional[int] = None - for line in text.splitlines(): - opener = NS_OPEN.search(line) - if opener: - if opener.group(1).split("::")[-1] == "attr": - attr_depth = depth - depth += 1 - continue - # Count brace nesting crudely for namespace close detection. - depth += line.count("{") - line.count("}") - if attr_depth is not None and depth <= attr_depth: - attr_depth = None - continue - if attr_depth is not None: - md = CONST_DEF.search(line) - if md: - # Resolve the named constant against the global symbol table; - # this captures both makeStr("x") and join(seg::y, ...) forms. - val = symbols.get(md.group(1)) - if val is not None: - keys.add(val) - else: - for ms in MAKESTR.finditer(line): - keys.add(ms.group(1)) + for block in attr_namespace_spans(text): + for md in CONST_DEF.finditer(block): + # Resolve the named constant against the global symbol table; this + # captures makeStr("x"), join(seg::y, ...), and `using ::a::b` forms. + val = symbols.get(md.group(1)) + if val is not None: + keys.add(val) + # `using ::ns::attr::field;` re-exports a base constant into this attr + # block (e.g. RpcSpanNames imports txHash). Resolve the imported name. + for um in USING_DECL.finditer(block): + val = symbols.get(um.group(1)) + if val is not None: + keys.add(val) return keys @@ -373,20 +432,28 @@ def derive_dotted_resource_keys( allow |= {k for k in keys if "." in k} tele = root / "src" / "libxrpl" / "telemetry" / "Telemetry.cpp" if tele.is_file(): - text = tele.read_text() - # semconv::service::kServiceName -> service.name, etc. + text = read_source(tele) + # semconv::::k -> the dotted OTel-standard key. The + # CamelKey already embeds the group, e.g. service::kServiceInstanceId + # -> service.instance.id. Split the CamelCase name into dotted lowercase + # segments; if it does not lead with the group, prepend the group. for m in re.finditer(r"semconv::(\w+)::k(\w+)", text): - group = m.group(1) # e.g. "service" - field = camel_to_snake(m.group(2)).replace("service_", "", 1) - allow.add(f"{group}.{field.replace('_', '.')}") + group, camel = m.group(1), m.group(2) + segments = camel_to_dotsegments(camel) + if segments and segments[0] == group: + allow.add(".".join(segments)) + else: + allow.add(group + "." + ".".join(segments)) report.ok(f"resource dotted-key allowlist derived: {sorted(allow)}") else: report.skip("resource-derive", "Telemetry.cpp not present") return allow -def camel_to_snake(s: str) -> str: - return re.sub(r"(? List[str]: + """Split a CamelCase identifier into lowercase dot-segment parts, e.g. + `ServiceInstanceId` -> ['service', 'instance', 'id'].""" + return [w.lower() for w in re.findall(r"[A-Z][a-z0-9]*", s)] def run_rule_a( @@ -460,7 +527,7 @@ def spanname_symbol_names(headers: List[Path]) -> Set[str]: constant referenced at a call-site actually lives in a SpanNames header.""" names: Set[str] = set() for h in headers: - for m in CONST_DEF.finditer(h.read_text(errors="ignore")): + for m in CONST_DEF.finditer(strip_comments(read_source(h))): names.add(m.group(1)) return names @@ -487,7 +554,7 @@ def run_rule_f(root: Path, report: Report, header_symbols: Set[str]) -> None: for path in sorted(sources): if path.name.endswith("SpanNames.h") or is_test_path(path): continue - text = path.read_text(errors="ignore") + text = read_source(path) rel = path.relative_to(root) for call, arglist, lineno in iter_calls(text): positions = CONSTANT_ARG_POSITIONS.get(call, set()) @@ -538,11 +605,24 @@ def iter_calls(text: str): for m in CALLSITE.finditer(text): name = m.group(1) # Walk from the opening paren, balancing nesting to find the close. + # Parens inside a "string literal" are ignored so a value such as + # `setAttribute(k, ")")` does not close the call early. i = m.end() # one char past the '(' depth = 1 + in_str = False + escaped = False while i < len(text) and depth > 0: c = text[i] - if c == "(": + if in_str: + if escaped: + escaped = False + elif c == "\\": + escaped = True + elif c == '"': + in_str = False + elif c == '"': + in_str = True + elif c == "(": depth += 1 elif c == ")": depth -= 1 @@ -557,7 +637,7 @@ def run_rule_b_collector(root: Path, l1_keys: Set[str], report: Report) -> None: if not path.is_file(): report.skip("B", "collector config not present") return - text = path.read_text() + text = read_source(path) if "spanmetrics" not in text: report.skip("B", "no spanmetrics block in collector config") return @@ -593,7 +673,7 @@ def run_rule_c_tempo(root: Path, l1_keys: Set[str], report: Report) -> None: if not path.is_file(): report.skip("C", "tempo.yaml not present") return - text = path.read_text() + text = read_source(path) tags = re.findall(r"(?:tag|attribute)\s*:\s*([A-Za-z0-9_.]+)", text) span_tags = [t for t in tags if not t.startswith(("service.", "span."))] if not span_tags: @@ -632,7 +712,7 @@ def run_rule_d_dashboards(root: Path, l1_keys: Set[str], report: Report) -> None found = False for f in files: try: - text = f.read_text() + text = read_source(f) except OSError: continue # PromQL `sum by (a, b)` and `{label="..."}` references. @@ -660,21 +740,27 @@ def run_rule_e_runbook(root: Path, l1_keys: Set[str], report: Report) -> None: if not l1_keys: report.skip("E", "no L1 key set to validate against") return - text = path.read_text() + text = read_source(path) found = False - # Attribute names appear as `code` spans in the runbook's reference tables. - for m in re.finditer(r"`([a-z][a-z0-9_]*(?:\.[a-z0-9_]+)*)`", text): + # Only the dotted `xrpl..` 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 — + # * 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 + # * metric labels (`xrpl_rpc_command`) underscore, no dot + # Legitimate dotted resource attrs (`xrpl.network.id`/`.type`) are in L1 and + # are skipped. A dotted `xrpl.` token absent from L1 is a genuine doc/code + # mismatch (e.g. `xrpl.tx.hash` where the code emits `tx_hash`). + for m in re.finditer(r"`(xrpl\.[a-z][a-z0-9_.]*)`", text): token = m.group(1) - # Only consider tokens that look like attribute keys (underscore form or - # a dotted form), and skip obvious span names / prose words. - if "_" in token or "." in token: - if token in l1_keys: - continue - if "." in token: # dotted: must be a resource key in L1 - found = True - report.violation( - "E", str(path.relative_to(root)), token, "underscore, not dotted" - ) + if token in l1_keys: # legitimate dotted resource attr (xrpl.network.*) + continue + found = True + report.violation( + "E", str(path.relative_to(root)), token, "underscore, not dotted" + ) if not found: report.ok("E: runbook attribute references consistent with L1") diff --git a/.github/scripts/otel-naming/test_check_otel_naming.py b/.github/scripts/otel-naming/test_check_otel_naming.py new file mode 100644 index 0000000000..e2e9e1fe88 --- /dev/null +++ b/.github/scripts/otel-naming/test_check_otel_naming.py @@ -0,0 +1,571 @@ +#!/usr/bin/env python3 + +"""Unit tests for check_otel_naming.py. + +Stdlib-only (unittest), matching the dependency-free policy of the check itself. +Run from anywhere: + + python .github/scripts/otel-naming/test_check_otel_naming.py + +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 +because its discriminator — the `xrpl..` prefix vs span names, +filenames, OTel-standard keys, and metric labels — is the subtlest. +""" + +import contextlib +import importlib.util +import io +import shutil +import tempfile +import unittest +from pathlib import Path + +# Load the check module by path (it is not an importable package). +_spec = importlib.util.spec_from_file_location( + "check_otel_naming", str(Path(__file__).with_name("check_otel_naming.py")) +) +chk = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(chk) + + +# A controlled L1 set used across tests: the two legitimate dotted resource +# attrs plus a handful of underscore span-attribute keys. +L1 = { + "xrpl.network.id", + "xrpl.network.type", + "tx_hash", + "peer_id", + "consensus_mode", + "command", + "rpc_status", + "ledger_seq", +} + + +def _run_rule_e(runbook_text: str): + """Run Rule E against a synthetic runbook; return the flagged tokens.""" + d = Path(tempfile.mkdtemp()) + try: + (d / "docs").mkdir() + (d / "docs" / "telemetry-runbook.md").write_text(runbook_text) + report = chk.Report() + chk.run_rule_e_runbook(d, set(L1), report) + return sorted(v[2] for v in report.violations) + finally: + shutil.rmtree(d) + + +class RuleERunbook(unittest.TestCase): + """Rule E: only dotted `xrpl..` attribute keys are flagged.""" + + # ----- positive: genuine dotted attribute-key violations ----- + def test_single_dotted_attr(self): + self.assertEqual(_run_rule_e("`xrpl.tx.hash`"), ["xrpl.tx.hash"]) + + def test_multiple_dotted_attrs(self): + self.assertEqual( + _run_rule_e("`xrpl.tx.hash` and `xrpl.consensus.mode`"), + ["xrpl.consensus.mode", "xrpl.tx.hash"], + ) + + def test_deep_dotted_three_segments(self): + self.assertEqual( + _run_rule_e("`xrpl.consensus.ledger.seq`"), ["xrpl.consensus.ledger.seq"] + ) + + def test_dotted_attr_with_underscore_field(self): + self.assertEqual( + _run_rule_e("`xrpl.consensus.round_id`"), ["xrpl.consensus.round_id"] + ) + + def test_repeated_token_reported_each_occurrence(self): + self.assertEqual( + _run_rule_e("`xrpl.tx.hash` ... `xrpl.tx.hash`"), + ["xrpl.tx.hash", "xrpl.tx.hash"], + ) + + def test_resource_attr_not_in_l1_is_flagged(self): + self.assertEqual( + _run_rule_e("`xrpl.network.unknown`"), ["xrpl.network.unknown"] + ) + + # ----- negative: legitimately-dotted tokens that must NOT be flagged ----- + def test_span_name_single(self): + self.assertEqual(_run_rule_e("`consensus.round`"), []) + + def test_span_name_multi_segment(self): + self.assertEqual( + _run_rule_e("`consensus.phase.open` `rpc.command.server_info`"), [] + ) + + def test_filename_cfg(self): + self.assertEqual(_run_rule_e("`xrpld.cfg`"), []) + + def test_filename_cpp(self): + self.assertEqual(_run_rule_e("`RCLConsensus.cpp`"), []) + + def test_otel_standard_service_name(self): + self.assertEqual(_run_rule_e("`service.name`"), []) + + def test_otel_standard_http_method(self): + self.assertEqual(_run_rule_e("`http.method`"), []) + + def test_metric_label_underscore(self): + self.assertEqual(_run_rule_e("`xrpl_rpc_command`"), []) + + def test_bare_underscore_attrs(self): + self.assertEqual(_run_rule_e("`tx_hash` `consensus_mode`"), []) + + def test_legit_dotted_resource_attrs_in_l1(self): + self.assertEqual(_run_rule_e("`xrpl.network.id` `xrpl.network.type`"), []) + + def test_prose_word(self): + self.assertEqual(_run_rule_e("the `command` attribute"), []) + + def test_plain_prose_no_backticks(self): + self.assertEqual(_run_rule_e("xrpl.tx.hash without backticks is prose"), []) + + # ----- boundary ----- + def test_empty_runbook(self): + self.assertEqual(_run_rule_e(""), []) + + def test_lookalike_prefix_xrpld(self): + # `xrpld.` is NOT `xrpl.` — must not match. + self.assertEqual(_run_rule_e("`xrpld.foo`"), []) + + def test_lookalike_prefix_underscore(self): + # `xrpl_rpc.command` starts with `xrpl_`, not `xrpl.`. + self.assertEqual(_run_rule_e("`xrpl_rpc.command`"), []) + + def test_uppercase_segment_not_matched(self): + # The pattern requires a lowercase char after `xrpl.`; uppercase keys are + # caught by Rule G at the L1 layer, not by the runbook text scan. + self.assertEqual(_run_rule_e("`xrpl.TX.hash`"), []) + + def test_token_touching_table_pipes(self): + self.assertEqual(_run_rule_e("| `xrpl.tx.hash` | desc |"), ["xrpl.tx.hash"]) + + def test_mixed_line_only_xrpl_dotted_flagged(self): + self.assertEqual( + _run_rule_e("`consensus.round` uses `xrpl.tx.hash` and `service.name`"), + ["xrpl.tx.hash"], + ) + + def test_skips_when_runbook_absent(self): + d = Path(tempfile.mkdtemp()) + try: + report = chk.Report() + chk.run_rule_e_runbook(d, set(L1), report) + self.assertEqual(report.violations, []) + self.assertTrue(any("SKIP: E" in s for s in report.skips)) + finally: + shutil.rmtree(d) + + def test_skips_when_l1_empty(self): + d = Path(tempfile.mkdtemp()) + try: + (d / "docs").mkdir() + (d / "docs" / "telemetry-runbook.md").write_text("`xrpl.tx.hash`") + report = chk.Report() + chk.run_rule_e_runbook(d, set(), report) + self.assertEqual(report.violations, []) + self.assertTrue(any("SKIP: E" in s for s in report.skips)) + finally: + shutil.rmtree(d) + + +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.""" + + def test_flat_join(self): + syms = chk.resolve_constants( + 'inline constexpr auto a = makeStr("xrpl");\n' + 'inline constexpr auto b = makeStr("network");\n' + "inline constexpr auto c = join(a, b);\n" + ) + self.assertEqual(syms["c"], "xrpl.network") + + def test_nested_join_three_segments(self): + syms = chk.resolve_constants( + 'inline constexpr auto xrpl = makeStr("xrpl");\n' + 'inline constexpr auto network = makeStr("network");\n' + "inline constexpr auto networkId = " + 'join(join(xrpl, network), makeStr("id"));\n' + ) + self.assertEqual(syms["networkId"], "xrpl.network.id") + + def test_qualified_seg_reference(self): + # `seg::rpc` resolves by its bare leaf `rpc`. + syms = chk.resolve_constants('inline constexpr auto rpc = makeStr("rpc");\n') + syms2 = chk.resolve_constants( + 'inline constexpr auto command = join(seg::rpc, makeStr("command"));\n', + syms, + ) + self.assertEqual(syms2["command"], "rpc.command") + + def test_alias_reference(self): + syms = chk.resolve_constants('inline constexpr auto rpc = makeStr("rpc");\n') + chk.resolve_constants("inline constexpr auto alias = seg::rpc;\n", syms) + self.assertEqual(syms["alias"], "rpc") + + def test_unresolvable_expr_omitted(self): + syms = chk.resolve_constants("inline constexpr auto x = join(unknown, y);\n") + self.assertNotIn("x", syms) + + def test_split_top_level_args_respects_nesting(self): + self.assertEqual( + chk.split_top_level_args("join(seg::a, b), c"), + ["join(seg::a, b)", " c"], + ) + + def test_split_top_level_args_ignores_comma_in_string(self): + self.assertEqual( + chk.split_top_level_args('key, ","'), + ["key", ' ","'], + ) + + def test_strip_comments_removes_line_and_block(self): + self.assertEqual( + chk.strip_comments("a // line\nb /* blk */ c").split(), + ["a", "b", "c"], + ) + + +def _write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + +def _header(ns_attr_body: str, prefix_seg: str = "") -> str: + """A minimal *SpanNames.h body: optional seg defs + a namespace attr block.""" + return ( + "#pragma once\n" + + prefix_seg + + "namespace xrpl::telemetry::demo::span {\n" + + "namespace attr {\n" + + ns_attr_body + + "} // namespace attr\n" + + "}\n" + ) + + +class AttrKeyExtraction(unittest.TestCase): + """attr_keys_from_header: comment-stripping + multi-line + using re-export.""" + + def _l1(self, header_text): + d = Path(tempfile.mkdtemp()) + try: + h = d / "src" / "DemoSpanNames.h" + _write(h, header_text) + syms = chk.build_global_symbols([h]) + return chk.attr_keys_from_header(h, syms) + finally: + shutil.rmtree(d) + + def test_single_line_makestr(self): + keys = self._l1(_header('inline constexpr auto k = makeStr("tx_hash");\n')) + self.assertIn("tx_hash", keys) + + def test_multiline_constexpr_captured(self): + keys = self._l1( + _header("inline constexpr auto k =\n" ' makeStr("round_time_ms");\n') + ) + self.assertIn("round_time_ms", keys) + + def test_commented_makestr_not_leaked(self): + keys = self._l1( + _header( + 'inline constexpr auto k = makeStr("good");\n' + '// inline constexpr auto bad = makeStr("old.dotted");\n' + ) + ) + self.assertIn("good", keys) + self.assertNotIn("old.dotted", keys) + + def test_block_commented_makestr_not_leaked(self): + keys = self._l1( + _header( + 'inline constexpr auto k = makeStr("good");\n' + '/* makeStr("blockbad") */\n' + ) + ) + self.assertNotIn("blockbad", keys) + + +class CamelToDotSegments(unittest.TestCase): + """semconv CamelCase -> dotted OTel-standard key derivation.""" + + def test_service_instance_id(self): + self.assertEqual( + chk.camel_to_dotsegments("ServiceInstanceId"), + ["service", "instance", "id"], + ) + + def test_service_name(self): + self.assertEqual(chk.camel_to_dotsegments("ServiceName"), ["service", "name"]) + + def test_derive_keys_from_telemetry_cpp(self): + d = Path(tempfile.mkdtemp()) + try: + tele = d / "src" / "libxrpl" / "telemetry" / "Telemetry.cpp" + _write( + tele, + "resource::Resource::Create({\n" + " {semconv::service::kServiceName, x},\n" + " {semconv::service::kServiceInstanceId, y},\n" + "});\n", + ) + report = chk.Report() + allow = chk.derive_dotted_resource_keys(d, {}, report) + self.assertIn("service.name", allow) + self.assertIn("service.instance.id", allow) + finally: + shutil.rmtree(d) + + +def _run_rule_a(keys_by_header, allow): + report = chk.Report() + chk.run_rule_a(keys_by_header, allow, report) + return sorted(v[2] for v in report.violations) + + +class RuleADotted(unittest.TestCase): + def test_dotted_attr_not_in_allow_flagged(self): + kbh = {Path("src/RpcSpanNames.h"): {"xrpl.tx.hash", "command"}} + self.assertEqual(_run_rule_a(kbh, {"xrpl.network.id"}), ["xrpl.tx.hash"]) + + def test_resource_attr_in_allow_passes(self): + kbh = {Path("src/SpanNames.h"): {"xrpl.network.id"}} + self.assertEqual(_run_rule_a(kbh, {"xrpl.network.id"}), []) + + def test_bare_key_never_flagged(self): + kbh = {Path("src/TxSpanNames.h"): {"tx_hash", "command"}} + self.assertEqual(_run_rule_a(kbh, set()), []) + + +def _run_rule_g(keys_by_header): + report = chk.Report() + chk.run_rule_g(keys_by_header, report) + return sorted(v[2] for v in report.violations) + + +class RuleGSnakeCase(unittest.TestCase): + def test_camelcase_flagged(self): + self.assertEqual(_run_rule_g({Path("h"): {"txHash"}}), ["txHash"]) + + def test_uppercase_flagged(self): + self.assertEqual(_run_rule_g({Path("h"): {"TX_HASH"}}), ["TX_HASH"]) + + def test_space_flagged(self): + self.assertEqual(_run_rule_g({Path("h"): {"bad key"}}), ["bad key"]) + + def test_snake_case_passes(self): + self.assertEqual(_run_rule_g({Path("h"): {"tx_hash", "rpc_status"}}), []) + + def test_dotted_resource_segments_pass(self): + self.assertEqual(_run_rule_g({Path("h"): {"xrpl.network.id"}}), []) + + def test_dotted_with_bad_segment_flagged(self): + self.assertEqual( + _run_rule_g({Path("h"): {"xrpl.Network.id"}}), ["xrpl.Network.id"] + ) + + +class RuleFAndH(unittest.TestCase): + """run_rule_f: literal keys/span-names flagged; values & tests exempt. + Rule H: qualified constant not in any header warns (non-fatal).""" + + def _run(self, rel_path, source, header_symbols=frozenset()): + d = Path(tempfile.mkdtemp()) + try: + _write(d / rel_path, source) + report = chk.Report() + chk.run_rule_f(d, report, set(header_symbols)) + return ( + sorted(v[2] for v in report.violations), + sorted(w[2] for w in report.warnings), + ) + finally: + shutil.rmtree(d) + + def test_literal_key_flagged(self): + v, _ = self._run("src/Foo.cpp", 'g.setAttribute("lit_key", v);\n') + self.assertEqual(v, ['setAttribute arg0 "lit_key"']) + + def test_literal_value_exempt(self): + v, _ = self._run("src/Foo.cpp", 'g.setAttribute(attr::command, "submit");\n') + self.assertEqual(v, []) + + def test_span_name_args_flagged(self): + v, _ = self._run("src/Foo.cpp", 'SpanGuard::span(cat, "rpc", "command");\n') + self.assertEqual(v, ['span arg1 "rpc"', 'span arg2 "command"']) + + def test_test_path_exempt(self): + v, _ = self._run("src/test/Foo.cpp", 'g.setAttribute("lit_key", v);\n') + self.assertEqual(v, []) + + def test_spannames_header_exempt(self): + v, _ = self._run("src/DemoSpanNames.h", 'g.setAttribute("lit_key", v);\n') + self.assertEqual(v, []) + + def test_bare_span_call_not_matched(self): + # No SpanGuard/./-> receiver -> not a telemetry call-site. + v, _ = self._run("src/Foo.cpp", 'auto s = span("not", "telemetry");\n') + self.assertEqual(v, []) + + def test_multiline_call_reports_first_line(self): + v, _ = self._run("src/Foo.cpp", 'g.setAttribute(\n "k",\n v);\n') + self.assertEqual(v, ['setAttribute arg0 "k"']) + + def test_paren_in_string_value_does_not_break_parsing(self): + # The ")" inside the value must not end the call early; key still seen. + v, _ = self._run("src/Foo.cpp", 'g.setAttribute("k", ")");\n') + self.assertEqual(v, ['setAttribute arg0 "k"']) + + def test_rule_h_qualified_constant_warns(self): + v, w = self._run( + "src/Foo.cpp", + "g.setAttribute(consensus::span::accept, v);\n", + header_symbols={"command"}, + ) + self.assertEqual(v, []) + self.assertEqual(w, ["setAttribute arg0 consensus::span::accept"]) + + def test_rule_h_known_constant_no_warning(self): + _, w = self._run( + "src/Foo.cpp", + "g.setAttribute(rpc_span::attr::command, v);\n", + header_symbols={"command"}, + ) + self.assertEqual(w, []) + + def test_rule_h_bare_local_no_warning(self): + _, w = self._run( + "src/Foo.cpp", "g.setAttribute(myLeaf, v);\n", header_symbols={"command"} + ) + self.assertEqual(w, []) + + +class RuleBCollector(unittest.TestCase): + def _run(self, yaml_text, l1): + d = Path(tempfile.mkdtemp()) + try: + _write(d / "docker" / "telemetry" / "otel-collector-config.yaml", yaml_text) + report = chk.Report() + chk.run_rule_b_collector(d, set(l1), report) + return sorted(v[2] for v in report.violations), report.skips + finally: + shutil.rmtree(d) + + def test_dimension_not_in_l1_flagged(self): + y = "spanmetrics:\n dimensions:\n - name: bogus_dim\n - name: command\n" + v, _ = self._run(y, {"command"}) + self.assertEqual(v, ["bogus_dim"]) + + def test_all_dimensions_in_l1_pass(self): + y = "spanmetrics:\n dimensions:\n - name: command\n - name: rpc_status\n" + v, _ = self._run(y, {"command", "rpc_status"}) + self.assertEqual(v, []) + + def test_skip_when_no_spanmetrics_block(self): + v, skips = self._run("receivers:\n otlp:\n", {"command"}) + self.assertEqual(v, []) + self.assertTrue(any("SKIP: B" in s for s in skips)) + + +class RuleDDashboards(unittest.TestCase): + def _run(self, json_text, l1): + d = Path(tempfile.mkdtemp()) + try: + _write( + d / "docker" / "telemetry" / "grafana" / "dashboards" / "x.json", + json_text, + ) + report = chk.Report() + chk.run_rule_d_dashboards(d, set(l1), report) + return sorted(v[2] for v in report.violations) + finally: + shutil.rmtree(d) + + def test_unknown_promql_label_flagged(self): + self.assertEqual( + self._run('"expr": "sum by (bogus_label) (x)"', {"command"}), + ["bogus_label"], + ) + + def test_builtin_labels_not_flagged(self): + self.assertEqual( + self._run('"expr": "sum by (le, span_name, exported_instance) (x)"', set()), + [], + ) + + def test_l1_label_passes(self): + self.assertEqual(self._run('"q": "{command=\\"x\\"}"', {"command"}), []) + + +class ReportExitContract(unittest.TestCase): + @staticmethod + def _exit_code(report): + """Call render_and_exit (which prints + raises SystemExit), swallowing + its stdout, and return the exit code.""" + with contextlib.redirect_stdout(io.StringIO()): + try: + report.render_and_exit() + except SystemExit as e: + return e.code + return None # pragma: no cover - render_and_exit always exits + + def test_violation_exits_nonzero(self): + r = chk.Report() + r.violation("A", "f", "tok", "exp") + self.assertEqual(self._exit_code(r), 1) + + def test_clean_exits_zero(self): + r = chk.Report() + r.ok("all good") + self.assertEqual(self._exit_code(r), 0) + + def test_warning_only_exits_zero(self): + r = chk.Report() + r.warning("H", "f", "tok", "note") + self.assertEqual(self._exit_code(r), 0) + + +class RuleEReportTuple(unittest.TestCase): + """Assert Rule E records the full (rule, expected) tuple, not just token.""" + + def test_violation_tuple_fields(self): + d = Path(tempfile.mkdtemp()) + try: + (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) + self.assertEqual(len(report.violations), 1) + rule, _loc, token, expected = report.violations[0] + self.assertEqual(rule, "E") + self.assertEqual(token, "xrpl.tx.hash") + self.assertEqual(expected, "underscore, not dotted") + finally: + shutil.rmtree(d) + + def test_clean_runbook_records_ok(self): + d = Path(tempfile.mkdtemp()) + try: + (d / "docs").mkdir() + (d / "docs" / "telemetry-runbook.md").write_text( + "`tx_hash` `consensus.round`" + ) + report = chk.Report() + chk.run_rule_e_runbook(d, {"tx_hash"}, report) + self.assertEqual(report.violations, []) + self.assertTrue(any("E:" in c for c in report.checked)) + finally: + shutil.rmtree(d) + + +if __name__ == "__main__": + unittest.main(verbosity=2)