From 6c4e0e6ed44d178d8c3f85d42bbb86e2a7a354b2 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:24:38 +0100 Subject: [PATCH] fix(telemetry): detach tx.receive and tx.process spans before job hand-off Both spans are moved into job-queue lambdas and destroyed on a worker thread. Detaching on the origin thread pops the thread-local OTel Scope there, so later spans on the peer/RPC thread no longer inherit these as a leaked ambient parent. Trace_id/parent are unchanged (both are hashSpan). Co-Authored-By: Claude Opus 4.8 (1M context) --- OpenTelemetryPlan/Phase3_taskList.md | 7 +++++++ src/xrpld/app/misc/NetworkOPs.cpp | 6 +++++- src/xrpld/overlay/detail/PeerImp.cpp | 5 ++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index 79bca77bcc..92d7529c82 100644 --- a/OpenTelemetryPlan/Phase3_taskList.md +++ b/OpenTelemetryPlan/Phase3_taskList.md @@ -100,6 +100,10 @@ - Use `SpanGuard::span(TraceCategory::Transactions, "tx", "receive")` factory (Phase 1c replaced macros with the SpanGuard factory pattern) +> **Note**: The `tx.receive` guard is `.detached()` before being moved into the +> `RcvCheckTx` job so its Scope is popped on the peer thread, not leaked to the +> worker (else later peer messages would inherit this transaction's trace). + **Key modified files**: - `src/xrpld/overlay/detail/PeerImp.cpp` @@ -123,6 +127,9 @@ - Create `tx.process` span - Set attributes: `tx_hash`, `tx_type`, `local` (whether from RPC or peer) - Record whether sync or async path is taken + - `.detached()` the guard before storing it in `TransactionStatus::span`, + since it is applied on a batch worker thread — this pops the Scope on the + origin thread and stops later work inheriting this transaction's trace - In `doTransactionAsync()`: - Capture parent context before queuing diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 3dd384d605..ffd38cfe63 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1379,7 +1379,11 @@ NetworkOPsImp::processTransaction( FailHard failType) { using namespace telemetry; - auto span = std::make_shared(txProcessSpan(transaction->getID())); + // 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(txProcessSpan(transaction->getID()).detached()); 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()) diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index ed1d2b5329..54dd80c544 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1307,7 +1307,10 @@ PeerImp::handleTransaction( uint256 const txID = stx->getTransactionID(); using namespace telemetry; - auto span = std::make_shared(txReceiveSpan(txID, *m)); + // 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(txReceiveSpan(txID, *m).detached()); span->setAttribute(tx_span::attr::txHash, to_string(txID).c_str()); span->setAttribute(tx_span::attr::peerId, static_cast(id_)); if (auto const* fmt = TxFormats::getInstance().findByType(stx->getTxnType()))