Telemetry must read state, never change it. Two defects here did change it,
plus three smaller correctness and privacy fixes.
doPathFind and doRipplePathFind read source_account / destination_account off
context.params to hash them into span attributes. context.params is non-const,
so those reads selected json::Value's non-const operator[], which inserts a
null for a missing key. The same object is later validated by
PathRequest::parseJson, whose first checks are isMember(source_account) and
isMember(destination_account) — so a request that omitted either field looked
present and the client received Malformed instead of Missing. Reads now go
through std::as_const, whose overload returns kNull without inserting.
PathRequest::doUpdate emitted pathfind_dest_currency as
to_string(saDstAmount_.asset()). For a non-XRP asset that renders as
"<issuer>/<currency>" with the issuer as a plaintext Base58 address, so a
plain account address reached the span pipeline even though every other
account here is hashed first. The issuer is now redacted and the currency
kept; an MPT asset renders as its issuance ID and carries no address.
PathRequestManager::updateAll created pathfind.update_all with an unscoped
SpanGuard. An unscoped guard takes the ambient span as its own parent but does
not itself become the ambient parent, so the pathfind.compute spans that
doUpdate creates never nested under it, contradicting the documented hierarchy.
It is now a scoped guard, held in std::optional because ScopedSpanGuard is
deliberately non-movable and so cannot be produced by a ternary. The skip when
there are no active subscriptions is preserved. updateAll is dispatched via
addJob and doUpdate runs synchronously, so the guard is constructed and
destroyed under the same context store, as ScopedSpanGuard requires.
The WebSocket entry point emitted the client-supplied command string directly.
That value becomes a Prometheus label, so arbitrary request input could drive
unbounded label cardinality. It is now resolved against the handler registry,
collapsing anything unrecognized to "unknown", matching what the HTTP path
already does.
Also: the pathfind.discover comment claimed future child spans could be
parented off it, which its unscoped guard cannot do — corrected to say what
would be required instead. Config-reference and task-list docs named the
parser setupTelemetry(); the API is makeTelemetrySetup().
Conflict in src/libxrpl/telemetry/SpanGuard.cpp: both sides added a different
include in the same alphabetical slot — phase-4 added <initializer_list>, the
incoming std::format change added <format>. Both are required, so both are
kept in sorted order.
Conflict in OpenTelemetryPlan/03-implementation-strategy.md §3.9: both
branches independently fixed the same defect (stale hand-maintained line
counts) in different ways.
- 1b (0ff947454c) stripped the Lines Added / Lines Changed columns but kept
the §3.9.1 and §3.9.2 tables.
- 1a (3ad525a48a) removed both sections outright.
Resolved in favour of 1a's deletion. The tables were pre-implementation
estimates with no source of truth, so trimming the columns only defers the
next drift; the file/component lists duplicated §3.1's directory tree, which
1b already keeps current (DiscardFlag.h, FilteringSpanProcessor). Keeping the
upstream resolution also stops the same conflict recurring on 1c..10.
1b-specific content verified intact after the merge: §3.1 retains the
DiscardFlag.h entry, the FilteringSpanProcessor and discard() annotations,
and the TracingInstrumentation removal. No references to the deleted
sections remain in any plan doc.
The §3.9.1 "Files Modified Summary" and §3.9.2 "Detailed File Impact"
tables carried hand-maintained per-component line counts that had drifted
from the plan: the Lines Added column summed to 1,565 while the Total row
claimed ~1,670. Files (34) and Lines Changed (120) reconciled, so only the
Added total was stale — residue from expanding Core Telemetry 5 -> 11 files.
Rather than patch one cell, remove both tables. They were pre-implementation
estimates with no source of truth, so any figure in them drifts again on the
next phase. §3.1's directory tree remains the canonical list of the 11 new
telemetry files, and §3.9.3-3.9.7 (risk, architectural impact, backward
compatibility, rollback) carry the assessment without inventing line counts.
No references to the removed sections exist elsewhere in the plan docs.
Both spans are moved into job-queue lambdas and destroyed on a worker
thread. Detaching on the origin thread pops the thread-local OTel Scope
there, so later spans on the peer/RPC thread no longer inherit these as a
leaked ambient parent. Trace_id/parent are unchanged (both are hashSpan).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add PathFinding Spans section to the runbook Span Reference and
remove the incorrect "not emitted / later-phase" annotations:
pathfind.request/compute/discover/update_all are emitted since
Phase 2 (upstream of Phase 5).
- Runbook label names use the bare spanmetrics dimensions
(command, rpc_status, local, consensus_mode); fix stale docs.
- 05-configuration-reference: note tls_client_cert/key require use_tls=1.
- TelemetryConfig: reject mTLS cert paths when use_tls=0 so telemetry
is not silently sent in plaintext.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 05-configuration-reference: service_name now applies to metrics as well
as traces (service.name resource attribute).
- 07-observability-backends: document that collector enrichment includes
deployment-tier tagging (deployment.environment + fallback
xrpl.network.type) for filtering one Grafana stack by tier.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The network-type label is derived from [network_id] in TelemetryConfig.cpp;
unmapped/unset IDs fall through to "unknown", not "standalone". Align the
design-doc resource-attribute table with the code and cfg example.
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.
