develop's new fix-gtest-names pre-commit hook requires a snake_case test-case
name. CI runs it with --all-files, so the existing CamelCase names fail the
check on every branch that carries these files.
develop's new fix-gtest-names pre-commit hook requires a snake_case test-case
name. CI runs it with --all-files, so the existing CamelCase names fail the
check on every branch that carries this file.
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.
The scheme guard only ran when tls_client_cert was set, so use_tls=1 against an
http endpoint booted: the CA file was checked for readability, then the exporter
read TLS off the URL scheme alone and sent spans in the clear. An operator who
asks for TLS should get TLS or a startup error.
Widen the guard to every use_tls=1 node. tls_client_cert already requires
use_tls=1, so this strictly widens the old condition. The error message and the
function's description now name use_tls rather than the certificate key.
one_way_tls_on_a_plain_http_endpoint_is_accepted pinned the old behaviour and is
flipped to _throws, asserting the scheme message, the endpoint key and the URL.
Three cases that sent use_tls=1 at an http endpoint while asserting a file error
now set an https endpoint, so the scheme guard cannot be what throws.
All three stop() overrides cleared the global Telemetry instance
unconditionally, which assumes one live Application per process. The unit-test
binary breaks that whenever two environments run, and makeTelemetry() hands it
a NullTelemetryOtel, so the null implementations matter as much as the real one.
Compare against this before clearing, in all three.
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.
