diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 6bcc3a785b..51506f44bf 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1327,43 +1327,6 @@ PeerImp::handleTransaction( auto stx = std::make_shared(sit); uint256 const txID = stx->getTransactionID(); - using namespace telemetry; - // 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. - // Left null when telemetry is compiled out: there is no span to own, so - // nothing is allocated for one. Every use below tests it, the job - // capture and activateIfLive() accept a null handle, and the transaction - // pipeline already takes a null span by default. Without this the - // make_shared allocated once per inbound transaction, duplicates - // included, to hold an empty object. - std::shared_ptr span; -#ifdef XRPL_ENABLE_TELEMETRY - span = std::make_shared(txReceiveSpan(txID, *m)); -#endif - // Guarded on the span being live because these values are not free and - // this runs for every inbound transaction, including duplicates: the - // hash string allocates, and the open-ledger index takes the ledger - // master's lock. With telemetry compiled out the span is null; with it - // compiled in the block is skipped when telemetry is disabled at runtime - // or the transaction category is off. - if (span && *span) - { - span->setAttribute(tx_span::attr::txHash, to_string(txID).c_str()); - span->setAttribute(tx_span::attr::peerId, static_cast(id_)); - // The current (open) ledger index when the relayed tx was received - // — the ledger being worked on. Correlates this tx.receive to the - // ledger trace; not yet applied to a specific ledger, so no hash. - span->setAttribute( - tx_span::attr::currentLedgerSeq, - static_cast(app_.getLedgerMaster().getCurrentLedgerIndex())); - if (auto const* fmt = TxFormats::getInstance().findByType(stx->getTxnType())) - span->setAttribute(tx_span::attr::txType, fmt->getName().c_str()); - if (auto const version = getVersion(); !version.empty()) - span->setAttribute(tx_span::attr::peerVersion, version.c_str()); - } - // Note: suppressed and txStatus are set once at each exit path - // (not as defaults here) to avoid OTel SDK attribute duplication. - // Charge strongly for attempting to relay a txn with tfInnerBatchTxn // LCOV_EXCL_START /* @@ -1384,8 +1347,6 @@ PeerImp::handleTransaction( */ if (stx->isFlag(tfInnerBatchTxn)) { - if (span) - span->setAttribute(tx_span::attr::txStatus, tx_span::val::rejectedInnerBatch); JLOG(pJournal_.warn()) << "Ignoring Network relayed Tx containing " "tfInnerBatchTxn (handleTransaction)."; fee_.update(resource::kFeeModerateBurdenPeer, "inner batch txn"); @@ -1398,24 +1359,14 @@ PeerImp::handleTransaction( if (!app_.getHashRouter().shouldProcess(txID, id_, flags, kTxInterval)) { - if (span) - span->setAttribute(tx_span::attr::suppressed, true); // we have seen this transaction recently if (any(flags & HashRouterFlags::BAD)) { - if (span) - span->setAttribute(tx_span::attr::txStatus, tx_span::val::knownBad); fee_.update(resource::kFeeUselessData, "known bad"); JLOG(pJournal_.debug()) << "Ignoring known bad tx " << txID; } else { - // Recently-seen but not flagged bad — this is the plain - // duplicate-suppression path. Mark it explicitly so the - // span never exits as "new". - if (span) - span->setAttribute(tx_span::attr::txStatus, tx_span::val::suppressed); - // Erase only if the server has seen this tx. If the server // has not seen this tx then the tx could not have been // queued for this peer. @@ -1431,8 +1382,47 @@ PeerImp::handleTransaction( return; } - if (span) - span->setAttribute(tx_span::attr::suppressed, false); + using namespace telemetry; + // The span starts here, once this node has decided to process the + // transaction. A peer relays every transaction it hears, so most + // inbound copies are ones the checks above drop, and tracing those + // costs a span and its attributes to describe work never done. The + // number dropped is reported as the transactions_duplicate traffic + // category, which needs no span. + // + // 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. + // Left null when telemetry is compiled out: there is no span to own, so + // nothing is allocated for one. Every use below tests it, the job + // capture and activateIfLive() accept a null handle, and the transaction + // pipeline already takes a null span by default. + std::shared_ptr span; +#ifdef XRPL_ENABLE_TELEMETRY + span = std::make_shared(txReceiveSpan(txID, *m)); +#endif + // Guarded on the span being live because these values are not free: the + // hash string allocates, and the open-ledger index takes the ledger + // master's lock. With telemetry compiled out the span is null; with it + // compiled in the block is skipped when telemetry is disabled at runtime + // or the transaction category is off. + if (span && *span) + { + span->setAttribute(tx_span::attr::txHash, to_string(txID).c_str()); + span->setAttribute(tx_span::attr::peerId, static_cast(id_)); + // The current (open) ledger index when the relayed tx was received + // — the ledger being worked on. Correlates this tx.receive to the + // ledger trace; not yet applied to a specific ledger, so no hash. + span->setAttribute( + tx_span::attr::currentLedgerSeq, + static_cast(app_.getLedgerMaster().getCurrentLedgerIndex())); + if (auto const* fmt = TxFormats::getInstance().findByType(stx->getTxnType())) + span->setAttribute(tx_span::attr::txType, fmt->getName().c_str()); + if (auto const version = getVersion(); !version.empty()) + span->setAttribute(tx_span::attr::peerVersion, version.c_str()); + } + // Note: txStatus is set once at each exit path below (not as a default + // here) to avoid OTel SDK attribute duplication. + JLOG(pJournal_.debug()) << "Got tx " << txID; bool checkSignature = true; diff --git a/src/xrpld/telemetry/TxSpanNames.h b/src/xrpld/telemetry/TxSpanNames.h index 2a6aa9824c..1b54f3371b 100644 --- a/src/xrpld/telemetry/TxSpanNames.h +++ b/src/xrpld/telemetry/TxSpanNames.h @@ -65,10 +65,6 @@ inline constexpr auto local = makeStr("local"); * "path" — sync or async processing path. */ inline constexpr auto path = makeStr("path"); -/** - * "suppressed" — whether tx was suppressed as duplicate. - */ -inline constexpr auto suppressed = makeStr("suppressed"); /** * "tx_status" — domain-qualified (collides with rpc_status, txq_status). */ @@ -105,15 +101,6 @@ namespace val { inline constexpr auto sync = makeStr("sync"); inline constexpr auto async = makeStr("async"); inline constexpr auto knownBad = makeStr("known_bad"); -/** - * Transaction was suppressed via HashRouter (duplicate, not flagged bad). - */ -inline constexpr auto suppressed = makeStr("suppressed"); -/** - * Transaction was rejected because it carried tfInnerBatchTxn, which - * must never appear in network-relayed traffic. - */ -inline constexpr auto rejectedInnerBatch = makeStr("rejected_inner_batch"); /** * Transaction was dropped because the validated ledger is too old to * confidently apply new transactions (server is out of sync).