The integration test's span assertions never actually ran. check_span()
built a Tempo /api/search call with --data-urlencode but no -G, so curl
POSTed the params as a body; Tempo answers 200 and ignores the query, so
every span name looked present. Verified against a live Tempo 2.9.4: the
buggy form returns the store's total trace count for any name, including
"zzz.does.not.exist"; with -G a real name returns 1 and a bogus one 0.
Fixed alongside it: the RPC check asserted "rpc.request", which is never
emitted (ServerHandler.cpp builds "rpc.http_request"). These two had to
change together, since -G turns the bogus name from a silent pass into a
hard failure.
Also in the script: a consensus timeout logged two failures and counted
two, because a post-loop else re-reported what the timeout branch had
already reported; and three unguarded curl calls aborted the whole script
under set -euo pipefail, making the ACCOUNT_ZERO fallback dead code with
no cleanup. Guarded the curls and wired an EXIT trap to the existing
cleanup(). The trap deliberately fires only before the summary, so a
completed run still leaves the stack up as the header documents.
Docs corrections, all re-derived from code:
- span inventory heading 35 -> 38, attribute heading 83 -> 89 rows
(78 unique keys), and the section 6 header table now carries the
missing TxApplySpanNames.h row so its columns sum to the same figures
- two stale paths: ConsensusSpanNames.h is under include/xrpl/consensus/,
TxSpanNames.h under src/xrpld/telemetry/
- consensus_round_id is int64, not string (RCLConsensus.cpp sets
prevLgr.seq() + 1); the runbook's TraceQL examples now use a numeric
literal instead of an unparseable bare <round_id>
- state-accounting duration gauges are cumulative MICROSECONDS, not
seconds (NetworkOPs.cpp declares std::chrono::microseconds and
publishes dur.count() raw)
- sampling_ratio is not a config key; head sampling is fixed at 1.0 and
the shipped collector has no tail sampling, so the caveat was rewritten
- the plan blurb referenced Jaeger; this stack is Tempo
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.
Add reference entries for the observability surface introduced on
phase-9: the `handler` label on the job instruments, the five
`getobject_*` request metrics, and the per-job-type queue saturation
gauges.
Names here follow this branch's StatsD pipeline, which preserves case
and carries the `xrpld_` prefix, so they differ from the lowercased
OTel-native names used from phase-7 onward. The sections state where
the implementing code lives, since it is introduced downstream.
Also correct pre-existing entries: `job_count` exports as
`jobq_job_count` via the collector group prefix, the non-special job
type count is 35 (not 36), and `JtLedgerData` has five producers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Document the two new ledger-correlation attributes in the data-collection
reference: add them to the transaction and TxQ attribute tables (with per-span
coverage), update the attribute count, and correct the txq.enqueue parent note
(parents to tx.process on the submission path via explicit context; a root on
the open-ledger rebuild path, correlated via current_ledger_seq).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reflects the C1 fix: rpc.command.* stays unscoped (its dispatch wraps
doRipplePathFind which yields), so pathfind.request nests under rpc.process.
The request -> compute -> discover sub-tree nests correctly via ScopedSpanGuard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document that deterministic-trace_id spans (tx.* apply pipeline, tx.process,
tx.receive, consensus.round) are now genuine trace roots with empty
parent_span_id via the custom DeterministicIdGenerator, superseding the old
synthetic-parent behavior that showed 'root span not yet received' in Tempo.
Also update the fresh-root note: peer entry spans use ScopedSpanGuard::freshRoot().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These inbound peer-message entry points (kConsumer) used span(), which
inherits whatever span is active on the peer thread — including a leaked
tx.receive scope — so validations/proposals were wrongly nested under
unrelated transaction traces. rootSpan() starts a fresh trace root.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
Make dashboard uids bare domain names (uid == filename stem) and update
doc references. Grafana links/bookmarks to the old uids will need
refreshing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 09-data-collection-reference: add $service_name, $deployment_environment,
$xrpl_network_type to the template-variable table; note they appear on all
dashboards and point to the runbook's Deployment Tiers section.
- TESTING.md: add a Prometheus check verifying the tier labels
(deployment_environment, xrpl_network_type, service_name) are present on
metric series.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
