The integration-test.sh trace queries used resource.service.name="rippled",
but the OTel resource attribute is "xrpld" (TelemetryConfig.cpp, MetricsRegistry.cpp),
so every span check and the service-registration check returned no results.
Also restore .github/scripts/rename/README.md, which an overzealous
find-and-replace had corrupted into circular no-ops (e.g. rename "xrpld" to
"xrpld"). The file documents the rename process and must retain the source
names (rippled/ripple); restored to match develop.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The TxQSpanNames.h span-hierarchy comment and the Phase 3 task-list attribute
table showed txq.accept.tx, but the constant emits txq.accept_tx
(op::acceptTx = "accept_tx"). Correct both.
Rule A silently missed a dotted span attribute (xrpl.ledger.hash) because of
two interacting bugs:
1. attr_keys_from_header resolved each constant via a flat global symbol table
keyed by bare name, so a later header defining a same-named constant (e.g.
consensus attr::ledgerHash = "ledger_hash") clobbered the base header's
attr::ledgerHash = "xrpl.ledger.hash", erasing the real dotted key from L1.
Now each constant is resolved against its own header (the global table only
seeds seg::/join() cross-file references); using-re-exports still resolve
globally.
2. derive_dotted_resource_keys allowlisted any dotted key declared in the base
SpanNames.h. Now it allowlists only the keys actually passed to
Resource::Create() in Telemetry.cpp (semconv service.* + the attr:: constants
set there, e.g. xrpl.network.*). A dotted key declared in a header but never
set as a resource attr is a Rule-A violation.
Adds 4 regression tests (collision, using-re-export, allowlist scope, brace
matching). No allowlist exception is added — the check now catches the
violation so the offending code can be fixed.
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 <noreply@anthropic.com>
Phase 9 surfaced two Rule D gaps (false positives, not data errors):
- TraceQL `span.<attr>` / `resource.<attr>` references: the bare attribute is
in L1, but the scope-prefixed form was flagged. Now strip the
span./resource./event./link/instrumentation_scope. prefix before the L1
lookup.
- Native OTel metric labels (e.g. `job_type`, `reason`) emitted by
MetricsRegistry are valid dashboard labels but are not span attributes. Add
an L6 source: parse `Add(.., {{"label", ...}})` instrument calls and accept
those label keys alongside L1 and builtins.
Verified against phase-9's real dashboards: 6 prior false positives -> 0.
79 tests (7 new for span-prefix stripping and metric-label extraction).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rule D (dashboard PromQL labels must exist in L1) flagged `__name__` once the
phase-7 system-*.json dashboards started using `sum by (le, __name__)`.
`__name__` is the Prometheus reserved label for the metric name itself — a
builtin, not a span attribute. Add it to the builtin allowlist and cover it
with a test. (Earlier dashboards only used `__name__` inside `{__name__=~...}`
matchers, which the label regex did not extract, so this surfaced only now.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bring phase-3 forward into phase 4 (consensus tracing). Phase 4 introduces
ConsensusSpanNames.h (and already did the trusted→proposal/validation rename
and underscore-attr conversion in ffc197b914).
Conflict resolution (reviewed by a code-review agent before commit):
- 02-design-decisions.md §2.4 Consensus Attributes: merge-both — kept phase-3's
underscore table form AND folded in phase-4's richer "Phase 4a" attribute set
(round_id, ledger_id, trace_strategy, converge/establish/disputes counts,
agree/disagree counts, threshold_percent, consensus_result, mode_old/new),
each mapped to its authoritative ConsensusSpanNames.h constant. Dropped the
planned-but-unimplemented proposers_agreed/proposers_total (no code constant;
agree_count/disagree_count serve that role).
- SpanGuardFactory.cpp: kept phase-4's explanatory comment about why a libxrpl
test uses literal keys, plus the converted command/rpc_status keys.
Naming check passes (100 keys across 8 *SpanNames.h headers, including consensus).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bring phase-2 forward into phase 3 (transaction tracing). Phase 3 introduces
TxSpanNames.h, TxQSpanNames.h, and TxApplySpanNames.h.
Conflict resolution:
- TxQ.cpp: kept phase-3's txq_span-based instrumentation (phase-2 had none).
Dropped the orphaned `NumberSO{... fixUniversalNumber}` line — develop's
#5962 (Retire fixUniversalNumber) removed that symbol repo-wide; the
conflict block had carried one stale copy that would not compile.
- 05/08/OpenTelemetryPlan.md: dropped the deleted 04-code-samples / POC_taskList
references (carried from phase-2), kept phase-3's new secure-OTel.md doc rows,
section, and Mermaid node/edge/style. Config code block -> prose; merged the
secure-OTel hardening pointer with the authoritative-config prose.
- Phase3_taskList.md: removed the "dotted keys for readability" note that came
from phase-2 — phase 3 already uses the underscore keys.
Reviewed by code-review agents: telemetry instrumentation intact, naming check
green (47 keys across 7 *SpanNames.h headers), no conflict markers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bring the hardened OTel naming check forward from phase-1c: unconditional
Rule F, test-file exemption, and the Rule H in-place-constant warning. The
check passes clean on phase 2 (24 keys across 4 *SpanNames.h headers including
PathFind; the SpanGuardFactory.cpp test is correctly exempt from Rule F).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
The script and its README live on phase-1c (where check_otel_naming.py was
introduced). The test-file Rule-F exemption was mistakenly applied here on
phase-2; revert to phase-1c's version verbatim. The exemption and further
script improvements will land on phase-1c and merge forward, keeping the
script's logic on the branch that owns it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bring the naming convention, code-sample cleanup, and CI naming check into
phase 2 (RPC tracing). Phase 2 introduces PathFindSpanNames.h.
Conflict resolution:
- 04-code-samples.md, POC_taskList.md: deletion wins.
- 02-design-decisions.md: took the convention-applied tables, but kept phase-2's
accurate PathFinding summary row (pathfind_fast/search_level/num_paths/...,
matching the implemented PathFindSpanNames.h).
- 05/08: took the code-block-free prose; kept phase-2's Phase2-5_taskList.md
index rows (dropping only the deleted POC row). Fixed stale setup_Telemetry/
make_Telemetry doc references to the code-correct setupTelemetry/makeTelemetry.
- Telemetry.h auto-merged to the constant-based @code examples.
check_otel_naming.py change: exempt test files from Rule F (tests pass
arbitrary literal keys to exercise the API). The check passes clean on the
merged tree (24 keys across 4 *SpanNames.h headers, including PathFind).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add Rule G to check_otel_naming.py: every span-attribute key must be
lower_snake_case (^[a-z][a-z0-9_]*$ per dot-separated segment). This catches
camelCase, UPPERCASE, and spaces in keys, which the structural (dotted) and
source (literal) rules did not. Document it in the script README and
CONTRIBUTING.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add check_otel_naming.py and wire it into on-pr.yml so every PR validates
that span-attribute names stay consistent across the code, collector, Tempo,
dashboards, and docs.
- The valid key set is derived dynamically from the *SpanNames.h constants and
the resource attributes the code registers in Telemetry.cpp — no hardcoded
allowlist to drift.
- Each rule is presence-gated: it runs only when the file it needs is in the
tree, so the check is correct whether telemetry changes land in one PR or
several (the collector/Tempo/dashboard/runbook layers arrive in later phases).
- Rule A flags dotted span-attribute keys; Rule F flags string-literal
attribute keys and span-name arguments (values may be runtime data).
- stdlib-only, mirroring the levelization check (bare `python`, no pip step).
- Telemetry.h / SpanGuard.h @code examples now use *SpanNames.h constants so
the strict literal check passes.
- CONTRIBUTING.md documents the check and how to run it locally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
