ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-27 23:38:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(telemetry): nest the accept work under consensus.accept.apply

Browse Source 
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.
This commit is contained in:
Pratik Mankawde
2026-09-23 13:49:42 +01:00
parent 7d21baf558
commit 59bae37688
3 changed files with 63 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
include/xrpl/consensus/ConsensusSpanNames.h
View File

@@ -59,7 +59,9 @@
* | Attrs: proposers, round_time_ms, quorum
* | |
* | +-- consensus.accept.apply [jtACCEPT thread, child of accept]
* | Created: Adaptor::doAccept()
* | Created: Adaptor::doAccept(), scoped: the txq spans doAccept
* | goes on to create nest under it; the tx apply-stage
* | spans are hash-derived roots and do not
* | Attrs: ledger_seq, close_time_ripple_epoch_s, close_time_correct,
* | close_resolution_ms, consensus_state, proposing, round_time_ms,
* | parent_close_time_ripple_epoch_s, close_time_self_ripple_epoch_s,

53
src/tests/libxrpl/telemetry/SpanGuardScope.cpp
View File

@@ -57,6 +57,7 @@
#include <opentelemetry/trace/trace_id.h>
#include <opentelemetry/trace/tracer.h>
#include <algorithm>
#include <array>
#include <cstddef>
#include <cstdint>
@@ -688,6 +689,58 @@ TEST_F(SpanGuardScopeTest, scopedGuard_addEvent_records_name_and_attribute_value
EXPECT_EQ(eventAttribute(events.front(), kTxIdKey), std::string(kTxId));
}
// A scoped child of a captured context is the ambient parent of the spans
// created after it on the same thread. A hash-derived root created inside that
// scope stays a root. consensus.accept.apply relies on both.
TEST_F(SpanGuardScopeTest, scopedChildOfCapturedContextIsAmbientForLaterSpans)
{
namespace cs = consensus::span;
auto const h = makeTraceIdBytes();
{
// consensus.accept: unscoped, thread-free, context captured.
auto accept =
SpanGuard::freshRoot(TraceCategory::Consensus, seg::consensus, cs::op::accept);
ASSERT_TRUE(static_cast<bool>(accept));
auto const acceptCtx = accept.spanContext();
// consensus.accept.apply: scoped child of that context.
ScopedSpanGuard const apply = ScopedSpanGuard::childSpan(cs::acceptApply, acceptCtx);
ASSERT_TRUE(static_cast<bool>(apply));
// ledger.build: a plain ambient scoped guard.
{
ScopedSpanGuard const build(TraceCategory::Ledger, seg::ledger, "build");
ASSERT_TRUE(static_cast<bool>(build));
}
// ledger.store: hash-derived, so a deterministic root.
{
auto store =
SpanGuard::hashSpan(TraceCategory::Ledger, "ledger.store", h.data(), h.size());
ASSERT_TRUE(static_cast<bool>(store));
}
}
auto spans = spanData()->GetSpans();
auto* accept = findSpan(spans, cs::accept);
auto* apply = findSpan(spans, cs::acceptApply);
auto* build = findSpan(spans, "ledger.build");
auto* store = findSpan(spans, "ledger.store");
ASSERT_NE(accept, nullptr);
ASSERT_NE(apply, nullptr);
ASSERT_NE(build, nullptr);
ASSERT_NE(store, nullptr);
EXPECT_EQ(apply->GetParentSpanId(), accept->GetSpanId());
// build nests under apply, not beside it.
EXPECT_EQ(build->GetParentSpanId(), apply->GetSpanId());
EXPECT_EQ(build->GetTraceId(), apply->GetTraceId());
// The hash-derived span is a root on its own pinned trace id.
EXPECT_FALSE(store->GetParentSpanId().IsValid());
EXPECT_TRUE(std::ranges::equal(store->GetTraceId().Id(), h));
}
// A forced-root span started while a PendingTraceId is active adopts that
// pinned 16-byte trace_id and remains a true root (no parent).
TEST_F(SpanGuardScopeTest, deterministicIdGenerator_forced_root_gets_pending_trace_id)

18
src/xrpld/app/consensus/RCLConsensus.cpp
View File

@@ -595,10 +595,9 @@ RCLConsensus::Adaptor::doAccept(
{
namespace cs = telemetry::consensus::span;
// Make the accept span ambient for the whole accept so doAccept's log lines
// (and any spans created here) correlate to it. Non-owning: acceptSpan still
// owns/ends the span. doAccept runs to completion on the JtAccept worker
// (no coroutine yield), so this scope is thread-local and safe.
// Make the accept span ambient until accept.apply opens below. Non-owning:
// acceptSpan still owns and ends the span. doAccept runs to completion on
// one thread, so the scope pops on the thread that pushed it.
auto acceptActivation = telemetry::activateIfLive(acceptSpan);
prevProposers_ = result.proposers;
@@ -627,13 +626,10 @@ RCLConsensus::Adaptor::doAccept(
closeTimeCorrect = true;
}
// Parent accept.apply via the captured accept context (acceptSpanContext_):
// the accept span is a thread-free SpanGuard, so an explicit context is
// used for both the sync (onForceAccept) and async (onAccept) paths. Falls
// back to the round context if the accept span was null.
auto doAcceptSpan = acceptSpanContext_.isValid()
? telemetry::SpanGuard::childSpan(cs::acceptApply, acceptSpanContext_)
: telemetry::SpanGuard::childSpan(cs::acceptApply, roundSpanContext_);
// Scoped: accept.apply is the ambient parent of every span doAccept creates
// from here on. Parented through acceptSpanContext_ because the accept span
// is a thread-free SpanGuard; the context is valid whenever that span is live.
auto doAcceptSpan = telemetry::ScopedSpanGuard::childSpan(cs::acceptApply, acceptSpanContext_);
doAcceptSpan.setAttribute(cs::attr::ledgerSeq, static_cast<int64_t>(prevLedger.seq()) + 1);
doAcceptSpan.setAttribute(
cs::attr::closeTimeRippleEpochS,