mirror of
https://github.com/XRPLF/rippled.git
synced 2026-07-23 23:20:33 +00:00
Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation
# Conflicts: # src/xrpld/telemetry/MetricsRegistry.h # src/xrpld/telemetry/ValidationTracker.h
This commit is contained in:
@@ -1,6 +1,3 @@
|
||||
Loop: xrpl.telemetry xrpld.rpc
|
||||
xrpld.rpc > xrpl.telemetry
|
||||
|
||||
Loop: xrpld.app xrpld.overlay
|
||||
xrpld.app > xrpld.overlay
|
||||
|
||||
|
||||
@@ -322,6 +322,7 @@ xrpld.rpc > xrpl.rdb
|
||||
xrpld.rpc > xrpl.resource
|
||||
xrpld.rpc > xrpl.server
|
||||
xrpld.rpc > xrpl.shamap
|
||||
xrpld.rpc > xrpl.telemetry
|
||||
xrpld.rpc > xrpl.tx
|
||||
xrpld.shamap > xrpl.basics
|
||||
xrpld.shamap > xrpld.core
|
||||
|
||||
45
.github/scripts/otel-naming/check_otel_naming.py
vendored
45
.github/scripts/otel-naming/check_otel_naming.py
vendored
@@ -19,6 +19,16 @@ 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 (Rules D & E):
|
||||
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". perf-iac's alloy pipeline stamps each identity at
|
||||
two layers -- dotted on the OTel resource attribute (xrpl.work.item/
|
||||
.branch/.node.role, checked by Rule E) and underscore on the derived
|
||||
Prometheus metric-datapoint label (xrpl_work_item/_branch/_node_role,
|
||||
checked by Rule D) -- so both forms are exempt from the same constant.
|
||||
|
||||
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
|
||||
@@ -57,7 +67,9 @@ Rules (each FAILS the build, when its inputs are present)
|
||||
native-metric label, or a builtin. TraceQL `span.`/`resource.` scope
|
||||
prefixes are stripped before the L1 lookup.
|
||||
E No dotted `xrpl.<domain>.<field>` attribute key in the runbook (only the
|
||||
L1 resource attrs xrpl.network.* may be dotted). Span names, filenames,
|
||||
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.
|
||||
|
||||
Warnings (printed, but do NOT fail the build)
|
||||
@@ -800,6 +812,24 @@ 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_work_item", # perf-iac: ticket/work-item id for the perf comparison run
|
||||
"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:
|
||||
@@ -825,8 +855,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:
|
||||
@@ -877,10 +908,18 @@ def run_rule_e_runbook(root: Path, l1_keys: Set[str], report: Report) -> None:
|
||||
# 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`).
|
||||
# EXTERNAL_INFRA_LABELS (Rule D) holds the underscore/metric-label form of
|
||||
# the perf-iac identity attrs; the resource-attribute layer stamps the same
|
||||
# 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.
|
||||
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"
|
||||
|
||||
@@ -121,6 +121,14 @@ class RuleERunbook(unittest.TestCase):
|
||||
def test_legit_dotted_resource_attrs_in_l1(self):
|
||||
self.assertEqual(_run_rule_e("`xrpl.network.id` `xrpl.network.type`"), [])
|
||||
|
||||
def test_external_infra_dotted_resource_attrs_not_flagged(self):
|
||||
# perf-iac stamps these as dotted resource attrs (alloy pipeline);
|
||||
# EXTERNAL_INFRA_LABELS (Rule D) holds their underscore metric-label
|
||||
# form -- Rule E must also exempt the dotted resource-attr form.
|
||||
self.assertEqual(
|
||||
_run_rule_e("`xrpl.work.item` `xrpl.branch` `xrpl.node.role`"), []
|
||||
)
|
||||
|
||||
def test_prose_word(self):
|
||||
self.assertEqual(_run_rule_e("the `command` attribute"), [])
|
||||
|
||||
@@ -744,6 +752,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__)`.
|
||||
|
||||
Reference in New Issue
Block a user