Static constants take the k prefix (readability-identifier-naming), and
SpanGuardScope.cpp no longer names anything from <opentelemetry/metrics/noop.h>
since it calls noopMeter(). Both fail CI under warnings-as-errors.
The helper's docstring also claimed NoopMeterProvider hides the base
two-argument GetMeter; it declares that overload itself, so the only
detail worth sharing is the version.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
resolveNodePublicKey() returned std::nullopt in three real cases: a first boot
with no wallet database, a standalone run (its wallet is a private temporary
database), and --newnodeid. Telemetry's resources are built during
ApplicationImp's member-init list and are immutable, so on those runs the node
reported an empty service.instance.id and no xrpl.node.id for the whole run,
while setup() minted a key moments later and patched only the tracer.
Replace it with resolveNodeIdentity(), which always returns a keypair: derived
from a configured seed, else read from an existing wallet database, else
minted. Main.cpp passes that pair to makeApplication(), ApplicationImp stores
it in nodeIdentity_ -- now declared before telemetry_ and no longer an optional,
because it is always set -- and builds the telemetry resource from it.
setup() calls getNodeIdentity(), which now persists rather than mints: it
stores the resolved pair when the wallet holds no identity, adopts the stored
one when it does, and clears first for --newnodeid. The write stays in setup()
because that is where the database exists; a standalone run has no persistent
wallet to write to, which is why the pair has to be decided before
construction rather than read back afterwards. Wallet gains storeNodeIdentity()
for that write, and getNodeIdentity(session) now uses it instead of repeating
the insert.
The three-argument makeApplication() mints a keypair, so jtx::Env and any other
test Application behave as a standalone run always did.
Also fold the three hand-rolled "meter from a NoopMeterProvider" copies into
telemetry::noopMeter(): the base-pointer call and the kMeterVersion argument are
both easy to get wrong alone, and the meter identity has to match the one the
histogram views select on.
The new gtest covers the wallet half: store-then-read, store not replacing an
existing identity, clear-then-store, and that the mint path persists. It adds
the tests.libxrpl > xrpl.rdb levelization edge, regenerated here.
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.
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.
Brings coroutine-aware context storage + tx/consensus worker-body activation.
Resolved: Telemetry.cpp keeps both meterProvider_ (phase-7) and contextStorage_
(coro-aware); doc-09 keeps phase-7 structure and applies the pathfind.request →
rpc.command.<name> correction to phase-7's own PathFind section.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add two GTests on the SpanGuardScopeTest fixture and install a
CoroAwareContextStorage so the tests exercise coro-aware ambient context:
- scopedGuard_survives_localvalue_store_swap: a ScopedSpanGuard's scope
is hidden when its LocalValue store is swapped out and visible again
when swapped back in, and pops cleanly under the owning store.
- activate_sets_ambient_without_owning: SpanGuard::activate() makes the
span ambient for the activation's lifetime without ending it; the
owning guard ends it exactly once.
Fix the cross-store death test to match the store-identity assertion
message ("constructing context store", not "constructing thread") after
the A3 refactor; the fixture's storage install is what lets the assert
fire at all.
Revert RipplePathFind's pathfind.request from freshRoot SpanGuard back to
ScopedSpanGuard: the coro-aware storage moves the scope with the
coroutine across yield, so it nests under rpc.command and stays
trace-correlated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite SpanGuardScope.cpp for the unscoped SpanGuard / scoped
ScopedSpanGuard split and the DeterministicIdGenerator: install the
generator in the TestTelemetry mock (4-arg TracerProviderFactory), drop the
obsolete detached()/detachInPlace() tests, and add freshRoot, scoped-ambient,
scope-handoff, ends-once, deterministic-root, and cross-thread death tests
with exact assertions.
Fix the last detached() caller: RipplePathFind now uses SpanGuard::freshRoot
(SpanGuard is thread-free, so it is held across the coroutine yield and ended
on resume with no scope to strip).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getMeter() became a pure virtual on THIS branch (native-metrics), which
makes the TestTelemetry mock abstract and would break a telemetry=ON
build of phase7/phase8 in isolation. The override was previously only on
phase9 (where it was mis-attributed); relocate it to phase7 where the
pure virtual is introduced, so every branch from here forward builds.
Mirrors NullTelemetry: an inert meter from a process-wide noop provider.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-review fixes for the SpanGuard scope-leak change:
- SpanGuard.h: restore the injectCurrentContextToProtobuf real-class
declaration, its #else no-op stub, and the protocol::TraceContext
forward-declaration dropped during the 3->4 union merge (compile break).
- Re-parent consensus.phase.open and consensus.establish under the round
span via a new RCLConsensus::Adaptor::roundSpanContext() accessor: the
round span is now detached, so ambient parenting no longer works; the
phase spans link explicitly to the captured round context. csf::Peer
gains a matching no-op accessor so the generic engine still compiles.
- Switch five childSpan(op::X, ctx) sites to the full consensus::span::X
constants: childSpan(name, ctx) takes the name verbatim, so the suffix-
only op:: constants emitted short, non-dotted span names.
- TestTelemetry mock: add getConsensusTraceStrategy() override (base pure
virtual introduced on this branch) so the mock is not abstract.
- validate(): rewrite the trace_context comment to drop attack-surface
framing and the PR-discussion reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
doRipplePathFind holds its pathfind.request span across context.coro->yield();
the coroutine resumes on a different JobQueue worker. rootSpan() gives it a
fresh trace root (no stale ambient parent), and detached() strips the
thread-local Scope so the guard neither lingers on the worker's context stack
across the yield nor pops the wrong stack on resume.
Adds SpanGuardScope.cpp: an in-memory-exporter test for rootSpan()/detached()
including the rootSpan().detached() combination used here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
