From 2644179a42cbbe0e2d6c7bbd4f36497c9ba588fe Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 11 Jun 2026 19:28:24 +0100 Subject: [PATCH] =?UTF-8?q?ci:=20Rule=20C=20=E2=80=94=20read=20Grafana=20t?= =?UTF-8?q?empo=20datasource,=20enforce=20span-scope=20filter=20tags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule C was reading docker/telemetry/tempo.yaml (the Tempo server config), which has no filter tags, so it always SKIPped — L3 was silently unenforced. The trace-search filter tags actually live in the Grafana datasource provisioning file (docker/telemetry/grafana/provisioning/datasources/tempo.yaml) as search.filters[].{tag,scope}. Point Rule C there (server file as fallback), pair each tag with its scope, validate only span-scope tags against L1 (resource/ intrinsic tags like service.*/name/status/duration are exempt), and strip the TraceQL span. prefix. On phase-9 this turns "SKIP: C" into "OK: C: 24 tempo span-filter tags all in L1" — L3 is now genuinely guarded. Adds a RuleCTempo test class (4 cases: span-tag-not-in-L1 flagged, span-tags-pass, resource/intrinsic ignored, skip when datasource absent). 83 tests total. Co-Authored-By: Claude Opus 4.8 --- .../scripts/otel-naming/check_otel_naming.py | 47 ++++++++++---- .../otel-naming/test_check_otel_naming.py | 63 +++++++++++++++++++ 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index 5558eaf95c..3b5ad1fe5b 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -685,24 +685,49 @@ def extract_spanmetrics_dimensions(text: str) -> List[str]: def run_rule_c_tempo(root: Path, l1_keys: Set[str], report: Report) -> None: - path = root / "docker" / "telemetry" / "tempo.yaml" - if not path.is_file(): - report.skip("C", "tempo.yaml not present") - return - 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: - report.skip("C", "no span-attribute tags in tempo.yaml") + # The trace-search filter tags live in the Grafana Tempo DATASOURCE + # provisioning file (search.filters[].{tag,scope}); the Tempo server + # tempo.yaml has no such tags. Prefer the datasource file; fall back to the + # server file so the rule still does something if the layout changes. + candidates = [ + root / "docker/telemetry/grafana/provisioning/datasources/tempo.yaml", + root / "docker/telemetry/tempo.yaml", + ] + path = next((p for p in candidates if p.is_file()), None) + if path is None: + report.skip("C", "tempo datasource provisioning not present") return if not l1_keys: report.skip("C", "no L1 key set to validate against") return + # Pair each filter's `tag:` with its `scope:` (a few lines below it) and + # validate only span-scope tags — resource/intrinsic tags (service.*, name, + # status, duration) are not span attributes. Strip a TraceQL span. prefix. + lines = read_source(path).splitlines() + span_tags: List[str] = [] + for i, line in enumerate(lines): + m = re.search(r"^\s*tag:\s*(\S+)", line) + if not m: + continue + scope = next( + ( + sm.group(1) + for j in range(i, min(i + 4, len(lines))) + for sm in [re.search(r"scope:\s*(\S+)", lines[j])] + if sm + ), + "", + ) + if scope == "span": + span_tags.append(TRACEQL_SCOPE.sub("", m.group(1))) + if not span_tags: + report.skip("C", "no span-scope filter tags in tempo datasource") + return miss = [t for t in span_tags if t not in l1_keys] - for t in miss: + for t in sorted(set(miss)): report.violation("C", str(path.relative_to(root)), t, "must exist in L1") if not miss: - report.ok(f"C: {len(span_tags)} tempo tag(s) all in L1") + report.ok(f"C: {len(span_tags)} tempo span-filter tag(s) all in L1") def metric_label_names(root: Path) -> Set[str]: diff --git a/.github/scripts/otel-naming/test_check_otel_naming.py b/.github/scripts/otel-naming/test_check_otel_naming.py index 7504a62ea0..638e72216e 100644 --- a/.github/scripts/otel-naming/test_check_otel_naming.py +++ b/.github/scripts/otel-naming/test_check_otel_naming.py @@ -476,6 +476,69 @@ class RuleBCollector(unittest.TestCase): self.assertTrue(any("SKIP: B" in s for s in skips)) +class RuleCTempo(unittest.TestCase): + """Rule C reads the Grafana Tempo DATASOURCE file's search.filters and + validates only span-scope tags against L1.""" + + DS = "docker/telemetry/grafana/provisioning/datasources/tempo.yaml" + + def _run(self, yaml_text, l1): + d = Path(tempfile.mkdtemp()) + try: + _write(d / self.DS, yaml_text) + report = chk.Report() + chk.run_rule_c_tempo(d, set(l1), report) + return sorted(v[2] for v in report.violations), report.skips + finally: + shutil.rmtree(d) + + def _filter(self, fid, tag, scope): + return ( + f" - id: {fid}\n" + f" tag: {tag}\n" + f' operator: "="\n' + f" scope: {scope}\n" + f" type: static\n" + ) + + def test_span_tag_not_in_l1_flagged(self): + y = "search:\n filters:\n" + self._filter("f1", "bogus_tag", "span") + v, _ = self._run(y, {"command"}) + self.assertEqual(v, ["bogus_tag"]) + + def test_span_tags_in_l1_pass(self): + y = ( + "search:\n filters:\n" + + self._filter("f1", "command", "span") + + self._filter("f2", "tx_hash", "span") + ) + v, _ = self._run(y, {"command", "tx_hash"}) + self.assertEqual(v, []) + + def test_resource_and_intrinsic_tags_ignored(self): + # service.* (resource) and name/status/duration (intrinsic) are not + # span attributes — they must not be validated against L1. + y = ( + "search:\n filters:\n" + + self._filter("f1", "service.instance.id", "resource") + + self._filter("f2", "name", "intrinsic") + + self._filter("f3", "duration", "intrinsic") + ) + v, skips = self._run(y, {"command"}) + self.assertEqual(v, []) + self.assertTrue(any("SKIP: C" in s for s in skips)) + + def test_skip_when_datasource_absent(self): + d = Path(tempfile.mkdtemp()) + try: + report = chk.Report() + chk.run_rule_c_tempo(d, {"command"}, report) + self.assertEqual(report.violations, []) + self.assertTrue(any("SKIP: C" in s for s in report.skips)) + finally: + shutil.rmtree(d) + + class RuleDDashboards(unittest.TestCase): def _run(self, json_text, l1, metric_labels=frozenset()): d = Path(tempfile.mkdtemp())