mirror of https://github.com/XRPLF/rippled.git synced 2026-07-24 15:40:26 +00:00

Files

Pratik Mankawde 4d044e6254 ci: Harden OTel naming check — unconditional Rule F, test exemption, Rule H

Three robustness fixes to check_otel_naming.py, all on phase-1c where the
script lives:

- Rule F now runs UNCONDITIONALLY. It is a purely syntactic check on the
  call-sites and does not need the L1 key set, so code that calls
  SpanGuard::span/setAttribute directly without ever defining a *SpanNames.h
  is still caught (previously it was silently skipped when no header existed).
- Exempt test files from Rule F (tests pass arbitrary literal keys to exercise
  the API). The call-site matcher now requires a SpanGuard/`.`/`->` receiver,
  so std::span and bare declarations no longer false-positive.
- Add Rule H (warning, non-fatal): a namespace-qualified constant used at a
  telemetry call-site but not defined in any *SpanNames.h is flagged, catching
  constants defined in-place instead of in the proper header. Bare locals and
  std:: names are not warned to avoid noise.

SpanGuard.h / Telemetry.h @code examples updated to reference constants that
exist on this branch. README documents the new behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

2026-06-11 16:10:08 +01:00

check_otel_naming.py

ci: Harden OTel naming check — unconditional Rule F, test exemption, Rule H

2026-06-11 16:10:08 +01:00

README.md

ci: Harden OTel naming check — unconditional Rule F, test exemption, Rule H

2026-06-11 16:10:08 +01:00

README.md

OTel naming-consistency check

check_otel_naming.py enforces the OpenTelemetry span-attribute naming convention documented in CONTRIBUTING.md across every layer of the telemetry pipeline. The *SpanNames.h constants are the single source of truth (L1); every other layer must agree with them.

Running locally

python .github/scripts/otel-naming/check_otel_naming.py

It takes no arguments, can be run from any directory inside the repo, and uses only the Python standard library (no pip install, matching the levelization check). A non-zero exit code means a violation was found; the output lists each violation as RULE | location | token | expected.

What it checks

The valid key set is derived dynamically from the OTel code — there is no hardcoded allowlist:

L1 keys come from the namespace attr { ... } blocks of every *SpanNames.h, resolving the makeStr("x") / join(seg::a, seg::b) DSL (cross-file, so join(seg::rpc, ...) resolves seg::rpc from the base SpanNames.h).
Legitimate dotted keys = the resource attrs declared in the base SpanNames.h (xrpl.network.*) plus the semconv::service::* keys the code passes to Resource::Create() in Telemetry.cpp (service.*). A dotted key declared in any other header is a violation.

Rules (each fails the build, when its inputs are present)

Rule	Check
A	No stray dotted span-attribute key (only the derived resource keys may be dotted).
G	Attribute keys are `lower_snake_case` (`^[a-z][a-z0-9_]*$` per dot-segment) — no camelCase, UPPERCASE, or spaces.
F	No string literals as attribute keys or span-name arguments in `setAttribute`/`addEvent`/`span`/`childSpan`. Attribute values are exempt (runtime data); `*SpanNames.h` definitions and test files are exempt.
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.

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 SpanGuard::span/setAttribute directly without ever defining a header is still caught.

Warnings (printed, never fail the build)

Rule	Check
H	A namespace-qualified constant (e.g. `foo::bar::myKey`) used at a telemetry call-site is not defined in any `*SpanNames.h`. The constant should live in the proper header; defining it in-place bypasses rules A/G/F. Warns rather than fails — the argument may be a legitimately dynamic value, and the header may live on a later branch. Bare locals and `std::` names are not warned.

Presence-gated

Every rule runs only when the source files it needs are present in the tree and is otherwise skipped (printed as SKIP: <rule> — <reason>), never failed. 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.