From 48747bab4cc085a5ef4df25b6682a20441278153 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:49:44 +0100 Subject: [PATCH 1/3] ci(telemetry): recognize perf-iac identity labels in otel-naming Rule D xrpl_branch and xrpl_node_role are collector/infra-injected resource labels used by perf-iac dashboards to identify the build under test and its role in the perf cluster. They have no L1 span-attribute source or L6 native-metric-label source (like the existing job/instance/job_type builtins), so Rule D flagged them across every perf-iac dashboard. Register them in the builtins set, matching the existing pattern for infra-level dashboard labels. Add test coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/otel-naming/check_otel_naming.py | 2 ++ .github/scripts/otel-naming/test_check_otel_naming.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index 41e8748075..a5ed07c0e4 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -822,6 +822,8 @@ def run_rule_d_dashboards( "service_instance_id", "job", "instance", + "xrpl_branch", # perf-iac identity label: git ref of the build under test + "xrpl_node_role", # perf-iac identity label: validator/peer role in the perf cluster } # A dashboard label is valid if it is a span attribute (L1), a native-metric # label (L6), or a Prometheus/Grafana builtin. diff --git a/.github/scripts/otel-naming/test_check_otel_naming.py b/.github/scripts/otel-naming/test_check_otel_naming.py index cf042b877b..e22c31c54e 100644 --- a/.github/scripts/otel-naming/test_check_otel_naming.py +++ b/.github/scripts/otel-naming/test_check_otel_naming.py @@ -744,6 +744,12 @@ class RuleDDashboards(unittest.TestCase): [], ) + def test_perf_iac_identity_labels_not_flagged(self): + self.assertEqual( + self._run('"expr": "sum by (xrpl_branch, xrpl_node_role) (x)"', set()), + [], + ) + def test_prometheus_name_label_not_flagged(self): # `__name__` is the Prometheus reserved metric-name label; the renamed # system-*.json dashboards use `sum by (le, __name__)`. From c5f419121fa3d5eb59bb160b659dd2e98ff9e195 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:50:19 +0100 Subject: [PATCH 2/3] Revert "ci(telemetry): recognize perf-iac identity labels in otel-naming Rule D" This reverts commit 48747bab4cc085a5ef4df25b6682a20441278153. --- .github/scripts/otel-naming/check_otel_naming.py | 2 -- .github/scripts/otel-naming/test_check_otel_naming.py | 6 ------ 2 files changed, 8 deletions(-) diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index a5ed07c0e4..41e8748075 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -822,8 +822,6 @@ def run_rule_d_dashboards( "service_instance_id", "job", "instance", - "xrpl_branch", # perf-iac identity label: git ref of the build under test - "xrpl_node_role", # perf-iac identity label: validator/peer role in the perf cluster } # A dashboard label is valid if it is a span attribute (L1), a native-metric # label (L6), or a Prometheus/Grafana builtin. diff --git a/.github/scripts/otel-naming/test_check_otel_naming.py b/.github/scripts/otel-naming/test_check_otel_naming.py index e22c31c54e..cf042b877b 100644 --- a/.github/scripts/otel-naming/test_check_otel_naming.py +++ b/.github/scripts/otel-naming/test_check_otel_naming.py @@ -744,12 +744,6 @@ class RuleDDashboards(unittest.TestCase): [], ) - def test_perf_iac_identity_labels_not_flagged(self): - self.assertEqual( - self._run('"expr": "sum by (xrpl_branch, xrpl_node_role) (x)"', set()), - [], - ) - def test_prometheus_name_label_not_flagged(self): # `__name__` is the Prometheus reserved metric-name label; the renamed # system-*.json dashboards use `sum by (le, __name__)`. From 8d8375101102479c3208827b4a1aa2f2f20afe42 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:54:30 +0100 Subject: [PATCH 3/3] ci(telemetry): recognize external-infra identity labels in otel-naming Rule D xrpl_branch and xrpl_node_role are stamped by the perf-iac harness -- infrastructure outside this repo's OTel code -- so they have no L1 (*SpanNames.h), L2 (collector config), or L6 (MetricsRegistry.cpp) source Rule D can derive them from. Unlike the generic builtins set (Prometheus/ Grafana mechanics every OTel setup has), these are repo-specific and narrow, so they get their own EXTERNAL_INFRA_LABELS constant: a visible, documented, deliberately narrow exception to the 'no hardcoded allowlist' design principle, not a silent workaround. Add test coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../scripts/otel-naming/check_otel_naming.py | 28 +++++++++++++++++-- .../otel-naming/test_check_otel_naming.py | 6 ++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/.github/scripts/otel-naming/check_otel_naming.py b/.github/scripts/otel-naming/check_otel_naming.py index 41e8748075..1899fe0c52 100644 --- a/.github/scripts/otel-naming/check_otel_naming.py +++ b/.github/scripts/otel-naming/check_otel_naming.py @@ -19,6 +19,12 @@ Design principles and the `join(seg::..., ...)` dotted resource compositions), and * the keys the code passes to `Resource::Create({ ... })` in Telemetry.cpp (the standard `semconv::service::*` keys -> service.name/version/...). + The one narrow, explicit exception is EXTERNAL_INFRA_LABELS (Rule D): + identity labels stamped by infrastructure outside this repo's OTel code + (the perf-iac harness), which by definition have no source in-tree to + derive from. Kept separate from the generic Prometheus/Grafana builtins + set so the exception stays visible rather than blending into "things + every OTel setup has". 2. Presence-gated enforcement. Every rule runs ONLY when the source files it needs are present in the tree, and is otherwise skipped (never failed). This @@ -800,6 +806,23 @@ def metric_label_names(root: Path) -> Set[str]: return labels +# Identity labels stamped by EXTERNAL infrastructure the OTel pipeline in this +# repo does not own: the perf-iac harness attaches these to every metric it +# scrapes so dashboards can filter by which build/role produced a series. They +# have no L1 (*SpanNames.h), L2 (collector config), or L6 (MetricsRegistry.cpp) +# source to derive from, so — unlike every other dashboard label — they cannot +# be validated dynamically. This is a deliberate, narrow exception to the "no +# hardcoded allowlist" design principle, kept separate from the generic +# Prometheus/Grafana `builtins` set below so it stays visible and auditable. +# Add a label here ONLY if it is genuinely injected by infra outside this +# repo's OTel code (never as a workaround for a dashboard querying a label +# that nothing actually emits — that is a real Rule D violation). +EXTERNAL_INFRA_LABELS = { + "xrpl_branch", # perf-iac: git ref of the xrpld build under test + "xrpl_node_role", # perf-iac: validator/peer role in the perf cluster +} + + def run_rule_d_dashboards( root: Path, l1_keys: Set[str], metric_labels: Set[str], report: Report ) -> None: @@ -824,8 +847,9 @@ def run_rule_d_dashboards( "instance", } # A dashboard label is valid if it is a span attribute (L1), a native-metric - # label (L6), or a Prometheus/Grafana builtin. - valid = l1_keys | metric_labels | builtins + # label (L6), a Prometheus/Grafana builtin, or an external-infra identity + # label (EXTERNAL_INFRA_LABELS). + valid = l1_keys | metric_labels | builtins | EXTERNAL_INFRA_LABELS found = False for f in files: try: diff --git a/.github/scripts/otel-naming/test_check_otel_naming.py b/.github/scripts/otel-naming/test_check_otel_naming.py index cf042b877b..c24bc063b4 100644 --- a/.github/scripts/otel-naming/test_check_otel_naming.py +++ b/.github/scripts/otel-naming/test_check_otel_naming.py @@ -744,6 +744,12 @@ class RuleDDashboards(unittest.TestCase): [], ) + def test_external_infra_labels_not_flagged(self): + # EXTERNAL_INFRA_LABELS (perf-iac identity labels with no in-tree + # source) must be recognized as valid, distinct from `builtins`. + expr = "sum by (" + ", ".join(sorted(chk.EXTERNAL_INFRA_LABELS)) + ") (x)" + self.assertEqual(self._run(f'"expr": "{expr}"', set()), []) + def test_prometheus_name_label_not_flagged(self): # `__name__` is the Prometheus reserved metric-name label; the renamed # system-*.json dashboards use `sum by (le, __name__)`.