mirror of
https://github.com/XRPLF/rippled.git
synced 2026-07-30 18:40:28 +00:00
4d044e6254
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>
66 lines
5.3 KiB
Markdown
66 lines
5.3 KiB
Markdown
# OTel naming-consistency check
|
|
|
|
`check_otel_naming.py` enforces the OpenTelemetry span-attribute naming
|
|
convention documented in
|
|
[CONTRIBUTING.md](../../../CONTRIBUTING.md#telemetry-span-attribute-naming)
|
|
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.
|