The accept.apply factory no longer falls back to the round context, so the
harness contract lists consensus.accept alone. Note rewritten to describe the
scoped child as it is now.
The contract's parent field was documentation only, so no check could fail a
span for being parented when it should be a root. A live sweep found three
spans nested one level off while every harness check passed. The field becomes
allowed_parents, a list derived per span from its creation factory and every
call path that reaches it, and is asserted per span.
A parent on another node and a parent absent from the trace are both
inconclusive rather than violations: the receive spans are parented cross-node
by design, and a rotation in flight has not exported its root yet. Spans
reached by two call paths list every lawful parent.
A second gate reads a round as a unit: every required phase child under the
round span on the same node, their start times in protocol order, and no
mode_change recording mode_old == mode_new. It selects traces that already hold
the accept span, since that span always ends after the round span and can land
in a later export batch.
num_cleared leaves txq.batch_clear's required attributes: the code writes it
only after the batch actually clears.
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.
clang-tidy's readability-math-missing-parentheses flags seven mixed
precedence expressions in this file, and warnings are errors in CI.
The parentheses match the precedence the compiler already applied, so every
static_assert value and the loop bound are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The $xrpl_work_item variable's description carried a real ticket id as its
example. The filter needs no example, so the id is gone and the wording stays.
The $xrpl_work_item variable's description carried a real ticket id as its
example. The filter needs no example, so the id is gone and the wording stays.
The harness checked parentage only within a node. Every declared
relationship named two spans from one process, so a receive span that
lost its cross-node parent still satisfied the contract: it is created
either way, with all its normal attributes, and only its parent differs.
Declare the three pairs that cross a node boundary, so a send side that
stops propagating context fails the run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
StatsDCollector test kept both sides: this branch's onCollectionReady() call,
which enables polling and only exists from here on, followed by the upstream
branch's control assertion that reads the resulting datagram.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both doc indexes kept this branch's 09-data-collection-reference.md rows,
which only exist here, and dropped every secure-OTel.md reference because
the upstream branch removed that file. No dangling link remains.
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>
SetRuntimeContextStorage() writes a process-global shared_ptr that every log
line reads through RuntimeContext::GetCurrent(). Neither side is atomic, and
assigning the wrapper destroys it and placement-news a replacement over the
same buffer, so a concurrent reader could make a virtual call through an
indeterminate vptr. Install it in main() while the process is still
single-threaded instead, and drop the member that held it.
Gated on the telemetry section being enabled. Only that one key is read
here, because parsing the whole section can throw on a contradictory TLS
combination and that error belongs where it already reports.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pending_ is a hash map, so one reconcile pass handed its entries over in no
useful order. The windows only move forward, so an event applied after a
later one landed in a bucket the grid had already rolled past and was
counted into the wrong window. Collect the decisions, sort by minute, then
apply.
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.
Both workload scripts decoded JSON and went straight to .get() or .items().
An array or a scalar decodes fine and then raises AttributeError, which is not a
ValueError, so the handler around the weights parsing could not catch it and the
operator saw a traceback naming neither the command nor what arrived.
Four sites across the two files, all four guarded, and the weights handler now
also catches TypeError for a non-numeric weight value.
Negative weights are left alone: they do not raise, the type is dropped from the
mix silently, and changing that changes accepted input.
This board's own copy of the Success series selected status_code=STATUS_CODE_OK.
Successful rpc.command spans no longer carry Ok: the specification reserves it
for an operator asserting verified success and warns a tool may read it as
suppressing errors, so instrumentation leaves a successful span Unset.
The blob differs from phase-9's, so merging forward would not have corrected it.
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.
onTimer read polling_ before taking metricsLock_. A tick that read the flag as
set could be preempted before acquiring the lock, letting onCollectionStopping()
clear the flag, take the uncontended lock and return. The tick then resumed and
ran every hook handler, which read the ledger master, the network operations,
the peer finder, the job queue and the overlay after shutdown had been told
polling stopped.
Collector::onCollectionStopping() promises polling has stopped by the time it
returns, and the shutdown ordering in ApplicationImp depends on that.
Reading the gate inside the lock makes seeing it set imply holding the lock, so
either the tick holds it across the handlers and the stop waits, or the stop
wins and the tick polls nothing. The old comment assumed that invariant rather
than establishing it.
The startup path caught std::exception only. initMetrics() reaches the OTel SDK,
which can throw something outside that hierarchy, and escaping a member
initializer would stop the node starting.
The section states the rule it must not break: a telemetry failure never stops
the node, the global provider stays a no-op and every instrument call remains
valid. The new clause drops the half-built provider and logs, exactly as the
std::exception clause does.
~ApplicationImp() already carries the same pairing for the same reason.
