refactor(telemetry): tx spans thread-free (no detach); txq spans scoped for nesting

SpanGuard is now thread-free (holds no Scope), so the tx.receive (PeerImp)
and tx.process (NetworkOPs) handoff sites no longer need .detached() before
being stored and ended on a worker thread — just construct the guard. The
stale "Scope leak" comments are replaced accordingly.

Make the six txq.* spans ScopedSpanGuard so their sub-spans nest via the
ambient context: txq.apply_direct/batch_clear under txq.enqueue, and
txq.accept_tx under txq.accept. All six are verified synchronous, ended at
scope, and never moved/handed off, so scoping is safe. applyDirect's span
is pure RAII (no method calls), so it is declared const.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-07-21 20:15:31 +01:00
parent d63d5bd45e
commit deecae0f01
3 changed files with 13 additions and 18 deletions

View File

@@ -1384,11 +1384,10 @@ NetworkOPsImp::processTransaction(
FailHard failType)
{
using namespace telemetry;
// Detached: this span is stored in TransactionStatus and applied on a
// batch worker thread, so it must not leave its Scope bound to this
// thread's context stack (that leak would adopt later work into this
// transaction's trace).
auto span = std::make_shared<SpanGuard>(txProcessSpan(transaction->getID()).detached());
// SpanGuard is thread-free (holds no Scope), so it is safe to store here
// and end on the batch worker thread that later applies this transaction —
// no detach step is needed.
auto span = std::make_shared<SpanGuard>(txProcessSpan(transaction->getID()));
span->setAttribute(tx_span::attr::txHash, to_string(transaction->getID()).c_str());
span->setAttribute(tx_span::attr::local, bLocal);
if (auto const& stx = transaction->getSTransaction())

View File

@@ -545,7 +545,7 @@ TxQ::tryClearAccountQueueUpThruTx(
beast::Journal j)
{
using namespace telemetry;
[[maybe_unused]] auto span = SpanGuard::span(
[[maybe_unused]] ScopedSpanGuard span(
TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::batchClear);
SeqProxy const tSeqProx{tx.getSeqProxy()};
@@ -752,8 +752,7 @@ TxQ::apply(
beast::Journal j)
{
using namespace telemetry;
auto span =
SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::enqueue);
ScopedSpanGuard span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::enqueue);
span.setAttribute(txq_span::attr::txHash, to_string(tx->getTransactionID()).c_str());
if (auto const* fmt = TxFormats::getInstance().findByType(tx->getTxnType()))
span.setAttribute(txq_span::attr::txType, fmt->getName().c_str());
@@ -1374,8 +1373,7 @@ void
TxQ::processClosedLedger(Application& app, ReadView const& view, bool timeLeap)
{
using namespace telemetry;
auto span =
SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::cleanup);
ScopedSpanGuard span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::cleanup);
span.setAttribute(txq_span::attr::ledgerSeq, static_cast<int64_t>(view.header().seq));
std::scoped_lock const lock(mutex_);
@@ -1466,8 +1464,7 @@ TxQ::accept(Application& app, OpenView& view)
// Create the span and read byFee_.size() only after taking the lock, since
// byFee_ is guarded by mutex_.
auto span =
SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::accept);
ScopedSpanGuard span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::accept);
span.setAttribute(txq_span::attr::queueSize, static_cast<int64_t>(byFee_.size()));
auto const metricsSnapshot = feeMetrics_.getSnapshot();
@@ -1497,7 +1494,7 @@ TxQ::accept(Application& app, OpenView& view)
JLOG(j_.trace()) << "Applying queued transaction " << candidateIter->txID
<< " to open ledger.";
auto txSpan = SpanGuard::span(
ScopedSpanGuard txSpan(
TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::acceptTx);
txSpan.setAttribute(txq_span::attr::txHash, to_string(candidateIter->txID).c_str());
txSpan.setAttribute(
@@ -1718,7 +1715,7 @@ TxQ::tryDirectApply(
beast::Journal j)
{
using namespace telemetry;
[[maybe_unused]] auto span = SpanGuard::span(
[[maybe_unused]] ScopedSpanGuard const span(
TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::applyDirect);
auto const account = (*tx)[sfAccount];

View File

@@ -1311,10 +1311,9 @@ PeerImp::handleTransaction(
uint256 const txID = stx->getTransactionID();
using namespace telemetry;
// Detached: this span is handed to a job-queue worker and must not
// leave its Scope bound to this peer thread's context stack (that
// leak would adopt later peer messages into this transaction's trace).
auto span = std::make_shared<SpanGuard>(txReceiveSpan(txID, *m).detached());
// SpanGuard is thread-free (holds no Scope), so it is safe to hand to
// a job-queue worker and end on that thread — no detach step is needed.
auto span = std::make_shared<SpanGuard>(txReceiveSpan(txID, *m));
span->setAttribute(tx_span::attr::txHash, to_string(txID).c_str());
span->setAttribute(tx_span::attr::peerId, static_cast<int64_t>(id_));
if (auto const* fmt = TxFormats::getInstance().findByType(stx->getTxnType()))