gtest's EXPECT_EQ expands to an if/else, so an unbraced if around it is a
dangling-else error with warnings treated as errors. Found by the local
build of xrpl_tests.
A transaction names one or more accounts: the sender in Account, and by
type a Destination, Owner, Issuer, Holder and so on. tx.process now
carries each top-level account-typed field as its own attribute, keyed
tx_<field> in lower snake case (tx_account, tx_destination, ...), so an
account can be searched for in traces whatever role it played.
Addresses are public ledger identifiers and are emitted raw.
The keys live in TxAccountSpanNames.h in libxrpl, with a field-to-key
table in TxAccountSpanNames.cpp. A library test walks TxFormats and the
SField registry: every account field a transaction can carry has a key,
and no field that only ledger entries carry has one. An empty account
field is skipped rather than rendered as the zero address.
Rewrite the privacy policy (design decisions section 2.4.4) and the two
plan summaries that still described account hashing and a configurable
redaction. Add the two account attributes to the path-finding attribute
table and describe pathfind_dest_currency as the rendered asset.
An XRP account address is a public ledger identifier drawn from an
enumerable set. An unsalted hash of it is reversible by lookup, so it
protected nothing and only broke the join against explorers, RPC
responses and logs that show the same address.
- pathfind_source_account and pathfind_dest_account carry the request's
r-address. A value that does not parse as an r-address is not emitted,
so a malformed or mistaken request value never reaches a span. Both
handlers share setAccountAttribute() in PathFindSpanAttributes.h.
- pathfind_dest_currency is to_string(Asset): "XRP", "<issuer>/<CUR>",
or the MPT issuance id.
- The collector's attributes/hash processor is removed. No layer hashes.
- redactAccount() stays available; its header no longer claims to sit in
the emit path.
An MPT issuance id is a sequence concatenated with the issuer's account id, so
emitting it whole put an account on a span in the clear, while the Issue arm
beside it redacts its issuer. The collector hashes only the two account
attributes, so nothing downstream caught it.
Hash the whole id: one stable token per asset, no issuer. The comment claiming
the id carries no address was wrong and is corrected.
The tx.process span read the fee with STAmount::xrp(), which throws on a
non-native amount. preflight1 rejects such a transaction with temBAD_FEE, but
the attribute runs first, so with tracing on the submit RPC returned
internalSubmit instead. Turning tracing on must not change a response.
Guard on native() and leave the attribute out otherwise. The peer path was
already contained by its own catch.
MonitoredMode::set calls onModeChange on every round start, so the span was
created whether or not the mode moved. A node with a steady mode therefore
emitted one mode_change per round carrying mode_old == mode_new, which a live
sweep confirmed on every round of both instrumented builds. The round span's
own consensus_mode attribute is still written on every call, since that is
where the round learns the mode it is running in.
accept.apply was a plain SpanGuard, so it never became the ambient span of
doAccept. The spans the function goes on to create inherited the activated
accept span instead and came out as accept.apply's siblings, while running
inside its own time window. Every guard was scoped before the SpanGuard split,
so this restores the hierarchy that design had.
Scoped now, so the hierarchy follows the call flow. Drops the parent-context
fallback arm with it: the accept context is captured only while the accept span
is live, and that span is a child of the round context, so an invalid accept
context implies an invalid round context and both arms returned an empty guard.
Only SpanGuard carried addEvent(name, attrs), so a call site holding a scoped
guard could not record an event attribute. Forwarding overload, with the no-op
twin in the telemetry-disabled stub, so a span can be converted between scoped
and unscoped without dropping the attributes on its events.
The find_library hint was pinned to the _RELEASE variable CMakeDeps
generates, so in any other configuration it expanded to nothing. The
archive was then found only via CMAKE_PREFIX_PATH, which can hand a Debug
build the Release archive instead of failing. Derive the suffix from
CMAKE_BUILD_TYPE and ask for the package's lib directory directly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SpanGuardScope.cpp kept both includes: each side added one and both symbols
are used in the merged test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tempo.yaml kept both sides' filter blocks: phase-2's six path-finding
filters ahead of this branch's three transaction filters, matching chain
order, and both header comment lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RPCHandler.cpp composed both sides: phase-1c's null-guard and reply-aware
status logic, keeping this branch's load_type attribute inside that guard
because its argument allocates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
propose() and validate() both injected the ambient span context, but no
span is ever activated on the threads that reach them: the round span is
deliberately non-ambient and the validation span is parented through a
stored context. So no TraceContext was written, every receiving peer took
its standalone-span fallback, and the consensus receive spans arrived as
orphan trace roots.
Inject from the span in hand instead, which is the form the propagation
helper documents and the transaction relay path already uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both pathfind handlers returned on many paths without recording a status,
so a failed request produced a span that read as success. Route every exit
through one helper that reads the rpc error token off the reply, which also
covers the replies built further down the call chain.
The token set is fixed by the error registry, so it is safe as a span
label; raw request text would not be.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rpc_status is a span-metrics dimension, so leaving it unset on this path
emitted a series with a blank label. Any query selecting on error missed
the failure entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The store swap was undone by trailing statements, which a fatal assertion skips.
The thread store is a function-local static, so it outlives every test rather
than being reset between them: a skipped restore left it owning a stack object
from a dead frame, and the crash then landed in whichever test ran next.
The onCoro flag did not protect it either, because the cleanup function reads
that flag out of the freed object to decide not to delete it.
The guard is declared after both stack stores so it is destroyed before them.
This branch emits nine pathfind attributes and the datasource offered a dropdown
for none of them, so the signal was there but not searchable in Explore. The
file's own header states that each phase adds filters for what it introduces.
Six filters, for the attributes that select a request. The three counts are
measurements read off a span rather than things an operator searches by, so they
get none. ledger_index is dynamic because it takes a new value every ledger.
The case had no assertions at all, so it could not fail. Its comment also said
the attribute constants live in an xrpld-level header a libxrpl test cannot
include. ConsensusSpanNames.h is a libxrpl header, two sibling tests already
include it, and a comment fifty lines above says so correctly.
So the seven literal keys were never forced. They now come from the same
constants the emitter uses, and the case asserts the property it was written
for: the factory decides its verdict once at creation, and writing either
close-time outcome leaves the guard inert and publishing no propagation bytes.
The premise is asserted too, so a failure names which exit produced the null
guard the rest of the case rests on.
The status description was the fixed string error, so a trace recorded that a
request failed but not why.
It now carries the error token from the reply, falling back to the status's own
error code and then to the old string. Every source is a compile-time literal
from the error registry, so no request text reaches it. The rpc_status attribute
stays at success and error: that one is a span-metrics dimension, and widening
it would mint a series per token per command per node.
The work is gated on the span being live, so a build with telemetry compiled out
or a disabled guard pays nothing, matching how the neighbouring helper is
handled. asCString() is null-checked as well as type-checked, because it returns
the raw pointer where asString() guards it.
The comment above resolveCommandSpanName now states the invariant rather than
how it could be abused.
callMethod decided the rpc.command span's status from the Status the handler
returned. Handler.cpp registers 70 of its 72 methods through byRef(), which
returns a default Status whatever happened, because an old-style handler
reports its error in the reply body instead. So a failed account_info or a
refused path_find came out with rpc_status=success and span status Ok.
That is the opposite of what the comment above the code claimed it did, and it
left a {status.code=error} query blind to every non-throwing RPC error.
The status now comes from the reply as well as the Status, which covers both
handler styles. byRef() is unchanged and identical to develop: it behaves
correctly for its own purpose, and no RPC reply changes. Only telemetry was
reading the wrong signal.
setOk() is dropped rather than moved. The specification reserves Ok for an
operator asserting verified success and warns that a tool may treat it as
suppressing errors, so a successful call now leaves the status Unset.
No test accompanies this: the Beast tree has no telemetry fixture and the
in-memory span exporter is linked only into xrpl_tests, which cannot reach
daemon code. Asserting the exported status needs that infrastructure first.
resolveCommandSpanName() converted command/method to a string with no type
check. json::Value::asString() throws for an array or an object, so a request
whose nested method is [] reached that conversion and the throw replaced a
clean tooBusy reply with internal.
The overloaded path is the only way in: fillHandler() returns tooBusy before
anything has read those fields, and every other exit either converted them
itself or means neither field is present. The effect is that the error code a
client receives depends on whether telemetry was compiled in, which telemetry
must never do.
The span now falls back to its existing unknown-command label when either
present field is not a string. The WebSocket path already validates both
fields before dispatch, so it is left alone.
The test drives a genuinely overloaded job queue, reading the threshold from
the production constant rather than copying it, and asserts the client still
gets tooBusy. It lives in the Beast tree because doCommand is daemon code and
needs jtx, which the gtest binary cannot reach.
The document is internal and is kept outside the repo instead.
This also removes every link to it: the document-index rows in
OpenTelemetryPlan.md and 08-appendix.md, its node, edge and style lines in the
plan's Mermaid map, its own section in OpenTelemetryPlan.md, and the two
cross-reference notes in 02-design-decisions.md and 05-configuration-reference.md.
The compose ports published on every host interface, so the collector's
unauthenticated OTLP receivers, Tempo, and the anonymous-admin Grafana were
reachable by anything that could route to the host.
Nothing consumed the published ports from off-host: containers address each
other by compose service name, and every doc and script uses localhost.
Resolved src/libxrpl/tx/applySteps.cpp. calculateBaseFee now returns
std::expected<XRPAmount, TER>, so invokePreclaim rejects the transaction when
the fee cannot be computed. Kept that inside this branch's preclaimTer wrapper,
so the returned error also reaches the span's ter_result attribute.
