The rename script rewrites "Ripple epoch" to "XRPL epoch", so the old
spelling in a tracked .md makes the check-rename job fail on a dirty tree.
The attribute keys are left alone: the script's pattern needs a space, and
those keys are a cross-layer contract.
The rename script rewrites "Ripple epoch" to "XRPL epoch", so the old
spelling in a tracked .md makes the check-rename job fail on a dirty tree.
The attribute key close_time_ripple_epoch_s is left alone: the script's
pattern needs a space, and that key is a cross-layer contract.
The plan doc offered `"attribute"` as the alternative to `"deterministic"`
for consensus_trace_strategy. The parser accepts `"random"`; "attribute"
described the correlation mechanism rather than the setting's value. Note
also that the alternative is experimental and not used.
retries_remaining is stamped on the txq.accept_tx span before the
transaction is applied and before the retry counter is decremented, so a
span with txq_status="retried" always shows a non-zero count and exhaustion
shows up as txq_status="failed" with zero. The attribute comment said only
"retries left before discard", which reads as a post-decrement value and
led to a runbook query that could never match.
Also rename the drifted consensus_trace_strategy value in the plan docs
from "attribute" to "random", the spelling the parser accepts, and note that
it is experimental and not used.
consensus_trace_strategy was read as a std::string and compared against the
literal "attribute" in startRoundTracing(), while the runbook documented
"deterministic" and "random". The documented value "random" therefore fell
through to the default and did nothing.
Parse the setting once into ConsensusTraceStrategy, so the consensus code
branches on a type. The accepted spellings are now "deterministic" and
"random"; anything else fails at startup instead of silently defaulting.
The behaviour behind the old "attribute" name is unchanged and is now
reached by "random".
Document consensus_trace_strategy in xrpld-example.cfg, stating that
"random" is experimental and not used: it gives each node its own trace id,
so one round arrives as one trace per node.
Also state on the tx.included event that it covers the agreed consensus set
before the ledger is built, so it is a superset of the accepted ledger.
One conflict, in 06-implementation-phases.md: this branch had rewritten the
phase-4 task table with a Status column, a descoping note and a Spans Produced
section, while upstream corrected the class name in the old plain table. This
branch's section is kept and the name correction re-applied to its 4.1 row.
One conflict, in SpanGuard.h: this branch added struct TraceBytes and upstream
added enum SpanRole at the same position after TraceCategory. Unrelated
declarations, so both are kept.
Guards the validation the parser gained upstream: zero rejected for all three
keys, a non-numeric value raising std::runtime_error rather than leaking
boost::bad_lexical_cast, a negative value rejected instead of wrapping to
4294967295, both bounds accepted exactly, and batch_size held at or below
max_queue_size.
The catch is std::runtime_error, not std::exception, on purpose: if the parser
ever stops wrapping, a bad_cast escapes and the suite fails loudly instead of
swallowing it.
Two conflicts, both resolved by composing the sides rather than taking one.
cfg/xrpld-example.cfg: this branch had moved the batch-processor keys under
their own heading while upstream edited them in place, so a merge-both would
have documented them twice. Upstream's range sentences are applied to the
relocated block and the head-sampling note keeps its position.
02-design-decisions.md: the summary table changed on both sides for different
reasons. Upstream renamed ledger_index to current_ledger_seq and ledger_seq;
this branch had corrected the PathFinding row to the keys it actually emits.
Both are kept.
The category mapped every Rpc span to kServer, so one inbound request emitted
several nested server spans. Per the trace spec, SERVER covers server-side
handling of a remote request the client awaits, while INTERNAL is an operation
with a local parent. rpc.process and both rpc.command sites have a local parent,
so they now pass SpanRole::Internal.
The four transport-edge roots keep the category default: rpc.http_request,
rpc.ws_upgrade, rpc.ws_message and the gRPC span each begin a remote call. This
matters to Tempo's service-graph and span-metrics generators, which pair server
spans with client spans and leave a surplus one unpaired.
childSpan() takes the span name verbatim, so a bare op:: suffix names the span
"process" rather than "rpc.process". The same defect was corrected in the
SpanGuard and Telemetry examples; this is the last copy.
startRoundTracing runs as an argument to Consensus::startRound, so it creates
consensus.round before startRoundInternal applies the new mode. Reading mode_
there recorded the previous round's value, and a validator switching from
observing to proposing got a round span labelled observing that nothing corrected.
The attribute is now written in onModeChange, from the mode being applied. All
three MonitoredMode::set paths funnel through there, so round start, a wrong-ledger
switch and a bow-out all correct the parent span with one statement. Every path
reaches it under RCLConsensus::mutex_ on the thread that created the span.
The stale write is removed rather than kept alongside: neither Consensus::startRound
nor startRoundInternal has an early return before mode_.set, so every round span is
stamped. If a future path ever skipped it the attribute would be absent, which reads
as a gap, instead of confidently wrong. onClose also sets consensus_mode, from the
engine's own parameter, and is correct as it stands.
Also tests addEvent's attribute overload on a live span, reading the exported event
name and each value back off the in-memory exporter. It was previously only ever
called on a null guard, so a dropped attribute exported nothing and failed nothing.
tryDirectApply returns an engaged optional whenever the fee bar was cleared,
including when xrpl::apply() failed, so testing the optional labelled failures as
applied. TxQ_test's fail-in-preclaim case hits exactly this: the fee clears the
bar and preclaim then rejects with terINSUF_FEE_B.
The stamp now branches on ApplyResult::applied. A failure reports failed rather
than falling through to the default rejected, because rejected means the
transaction got nowhere, while this one cleared the fee bar and ran through
apply(). ter_code is recorded either way, so the failure is diagnosable. Both
values already existed and are used the same way by the queued-apply path in this
file, so the vocabulary is unchanged.
The value set in the phase-3 task list is updated to match, including a ter_code
row for txq.accept_tx that was already emitted but undocumented.
Span kind was derived from TraceCategory alone, so every Rpc-category span was
kServer. A category cannot tell an inbound handler from the internal work under
it, and trace backends pair kServer with kClient, so internal spans left as
kServer become unpaired edges in a service graph and read as extra inbound
requests.
SpanRole is a new xrpl-owned enum, orthogonal to TraceCategory: the category
names the subsystem and gates the span on config, the role says whether the span
handles a remote call. It is a defaulted fourth parameter on span(), freshRoot()
and the ScopedSpanGuard equivalents, defaulting to SpanRole::FromCategory, so no
existing call site changes. resolveSpanKind() applies an explicit role and falls
back to the category map, which keeps its single responsibility. The
telemetry-disabled stubs mirror all four signatures.
No call site passes a role yet. The two that need it are on a later branch.
Also fixes a ScopedSpanGuard example that passed a bare op:: suffix to
childSpan(), which takes the name verbatim. Naming the child rpc.command made it
a child of rpc.command.<cmd>, inverting the hierarchy, so the example's parent is
now rpc.process and the command attribute moved onto the command span.
Review feedback on the RPC integration PR.
The childSpan examples could not work as written. childSpan() takes its parent
from the ambient context and uses impl_ only as a liveness gate, so an unscoped
SpanGuard parent produced two siblings rather than a parent and child. The parent
is now a ScopedSpanGuard, the child no longer reuses the parent's name, and the
examples pass a full dotted constant because childSpan() takes the name verbatim.
Five of the ten Rule D tests could not fail. Four passed an empty L1 key set,
which makes the rule skip validation altogether; the fifth asserted an empty
result against an escaped-quote selector that extracted no labels at all. Each
now passes a nonempty L1 set and carries a known-bad label in the same
expression, so it asserts both that the intended labels are accepted and that
Rule D ran. Verified by disabling the rule: the old tests stay green, the new
ones all fail.
Span kind is not fixed here. categoryToSpanKind and the span factories belong to
the telemetry library, so the role parameter is routed to that branch, and the
two call sites here follow once it exists.
Two review findings on the telemetry library.
SpanContext::isValid() returned impl_ != nullptr, so it answered true for a
context holding no span. threadLocalContext() wraps whatever GetCurrent()
returns, and that is an empty Context on a thread with no active span, which
contradicted the documented "invalid context if none is active". It now asks the
Context for its span. childSpan(name, ctx) is the one caller whose behaviour
changes: a context with no span used to produce a new root span, and now returns
a null guard as its @return already promised.
The three batch settings went to the OTel BatchSpanProcessor unchecked. Three
ways that failed: zero was accepted for all of them; batch_size could exceed
max_queue_size, which the SDK documents as a precondition and does not enforce;
and a mistyped value let boost::bad_lexical_cast escape, which derives from
std::bad_cast rather than std::runtime_error, so the operator saw a bare "bad
cast" naming no key. Reading unsigned also turned "-1" into 4294967295 instead
of failing, so the value is parsed signed and negatives are rejected.
xrpld-example.cfg now states the ranges.
Review feedback on the plan documents. Four kinds of error:
- Symbols that do not exist: ConsensusProposal::prevLedger_ (it is
previousLedger_), RCLConsensusAdaptor (it is RCLConsensus::Adaptor, and
startRound() is on RCLConsensus itself), and RPCHandler::doCommand (a free
function, xrpl::rpc::doCommand).
- Attribute keys: the tables used ledger_index, which no telemetry code emits.
Same concept as ledger_seq but a different referent, so the code disambiguates
by prefix: current_ledger_seq for the open ledger a transaction targeted,
ledger_seq for a closed or validated one. A note now states which is which.
- TraceQL that does not parse: span-field predicates need braces, status.code
is not an intrinsic (status = error), and avg(duration) does not take a by
clause (avg_over_time does). All five re-tested against Tempo.
- The StatsD comparison omitted the Histogram instrument, which aggregates at
the point of measure, and the when-to-use table had no row for a metric that
spans cannot afford to carry.
The emitted keys are close_time_ripple_epoch_s,
parent_close_time_ripple_epoch_s and close_time_self_ripple_epoch_s.
Update the consensus.accept.apply attribute tables and the close-time
attribute descriptions to match.
The emitted key is close_time_ripple_epoch_s, which names its unit and
epoch. Update the ledger attribute table to match.
ledger_index and ledger_tx_count in the same table belong to a separate
rename and are left as they are.
