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.
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 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>
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>
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 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.
CMake sets the define with add_compile_definitions, which is scoped to this
build, so it reached no consumer. Four public telemetry headers change class
layout on it: with the define SpanGuard holds a unique_ptr and its methods are
out of line in the library, without it SpanGuard is empty and all-inline.
A package compiling against the installed headers therefore built a different
SpanGuard than the one it linked. Exporting the define through cpp_info keeps
the two in step, and it stays inside the existing telemetry option so the Conan
option remains the only switch.
The C-string overload of setAttribute handed its pointer straight to
std::string_view, whose pointer constructor reads the pointer to find the
length. A null one is undefined behaviour.
A null pointer carries no text, and an empty value already means something on
this class, so nothing is recorded rather than an empty string.
The overload itself has to stay. Without it a string literal would bind to the
bool overload, because pointer-to-bool is a standard conversion and beats the
string_view one, so every string attribute would silently record true. The
header now records that.
Four labels in a flowchart wrote a backslash and an n to break a line.
Mermaid has no escape for that, so it printed the two characters inside the
node instead of wrapping. The labels are now markdown strings carrying a real
line break, which also auto-wrap.
The <br/> tags elsewhere in these diagrams are left alone. Mermaid enables HTML
labels by default and this repository overrides no Mermaid config, so they
render correctly.
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 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.
The steps described setting a suppressed attribute on a dropped duplicate, in
the pre-rename dotted spelling. The span is now created after the duplicate
check, so there is no span on that path in either spelling.
The tx.receive span is created after the duplicate check, so it carries no
suppressed attribute and a dropped copy produces no span at all. How many were
dropped is the transactions_duplicate traffic category.
