From fb25d97077867ead7764b25dec1ef0eadf786eaa Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 28 Apr 2026 18:01:50 +0100 Subject: [PATCH 1/4] fix: extend tx span lifetimes across async job boundaries - tx.receive span in PeerImp: convert to shared_ptr, capture in checkTransaction lambda so it measures actual processing, not just message parsing - tx.process span in NetworkOPs: convert to shared_ptr, store in TransactionStatus so it lives until the batch job processes the entry; sync path unchanged (span destructs on function return) Co-Authored-By: Claude Opus 4.6 --- src/xrpld/app/misc/NetworkOPs.cpp | 31 ++++++++++++++++++---------- src/xrpld/overlay/detail/PeerImp.cpp | 15 +++++++------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index d75de3344e..17972c8fa6 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -172,9 +172,16 @@ class NetworkOPsImp final : public NetworkOPs FailHard const failType; bool applied = false; TER result; + /// Keeps the tx.process span alive until the batch processes this entry. + std::shared_ptr span; - TransactionStatus(std::shared_ptr t, bool a, bool l, FailHard f) - : transaction(std::move(t)), admin(a), local(l), failType(f) + TransactionStatus( + std::shared_ptr t, + bool a, + bool l, + FailHard f, + std::shared_ptr s = nullptr) + : transaction(std::move(t)), admin(a), local(l), failType(f), span(std::move(s)) { XRPL_ASSERT( local || failType == FailHard::no, @@ -397,7 +404,8 @@ public: doTransactionAsync( std::shared_ptr transaction, bool bUnlimited, - FailHard failtype); + FailHard failtype, + std::shared_ptr span = nullptr); private: bool @@ -1315,9 +1323,9 @@ NetworkOPsImp::processTransaction( FailHard failType) { using namespace telemetry; - auto span = txProcessSpan(transaction->getID()); - span.setAttribute(tx_span::attr::hash, to_string(transaction->getID()).c_str()); - span.setAttribute(tx_span::attr::local, bLocal); + auto span = std::make_shared(txProcessSpan(transaction->getID())); + span->setAttribute(tx_span::attr::hash, to_string(transaction->getID()).c_str()); + span->setAttribute(tx_span::attr::local, bLocal); auto ev = m_job_queue.makeLoadEvent(jtTXN_PROC, "ProcessTXN"); @@ -1327,13 +1335,13 @@ NetworkOPsImp::processTransaction( if (bLocal) { - span.setAttribute(tx_span::attr::path, tx_span::val::sync); + span->setAttribute(tx_span::attr::path, tx_span::val::sync); doTransactionSync(transaction, bUnlimited, failType); } else { - span.setAttribute(tx_span::attr::path, tx_span::val::async); - doTransactionAsync(transaction, bUnlimited, failType); + span->setAttribute(tx_span::attr::path, tx_span::val::async); + doTransactionAsync(transaction, bUnlimited, failType, std::move(span)); } } @@ -1341,14 +1349,15 @@ void NetworkOPsImp::doTransactionAsync( std::shared_ptr transaction, bool bUnlimited, - FailHard failType) + FailHard failType, + std::shared_ptr span) { std::lock_guard const lock(mMutex); if (transaction->getApplying()) return; - mTransactions.emplace_back(transaction, bUnlimited, false, failType); + mTransactions.emplace_back(transaction, bUnlimited, false, failType, std::move(span)); transaction->setApplying(); if (mDispatchState == DispatchState::none) diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 16f8484243..97040698a2 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1442,11 +1442,11 @@ PeerImp::handleTransaction( uint256 const txID = stx->getTransactionID(); using namespace telemetry; - auto span = txReceiveSpan(txID, *m); - span.setAttribute(tx_span::attr::hash, to_string(txID).c_str()); - span.setAttribute(tx_span::attr::peerId, static_cast(id_)); + auto span = std::make_shared(txReceiveSpan(txID, *m)); + span->setAttribute(tx_span::attr::hash, to_string(txID).c_str()); + span->setAttribute(tx_span::attr::peerId, static_cast(id_)); if (auto const version = getVersion(); !version.empty()) - span.setAttribute(tx_span::attr::peerVersion, version.c_str()); + span->setAttribute(tx_span::attr::peerVersion, version.c_str()); // Charge strongly for attempting to relay a txn with tfInnerBatchTxn // LCOV_EXCL_START @@ -1480,11 +1480,11 @@ PeerImp::handleTransaction( if (!app_.getHashRouter().shouldProcess(txID, id_, flags, tx_interval)) { - span.setAttribute(tx_span::attr::suppressed, true); + span->setAttribute(tx_span::attr::suppressed, true); // we have seen this transaction recently if (any(flags & HashRouterFlags::BAD)) { - span.setAttribute(tx_span::attr::status, tx_span::val::knownBad); + span->setAttribute(tx_span::attr::status, tx_span::val::knownBad); fee_.update(Resource::feeUselessData, "known bad"); JLOG(p_journal_.debug()) << "Ignoring known bad tx " << txID; } @@ -1542,7 +1542,8 @@ PeerImp::handleTransaction( flags, checkSignature, batch, - stx]() { + stx, + sp = std::move(span)]() { if (auto peer = weak.lock()) peer->checkTransaction(flags, checkSignature, stx, batch); }); From d4e91b462e6f45944b43013dffed8572e526b65c Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 29 Apr 2026 11:16:21 +0100 Subject: [PATCH 2/4] fix(telemetry): resolve clang-tidy warnings in Telemetry interfaces Use C++17 concatenated namespaces, add [[nodiscard]] to query methods, add missing direct includes, and use pass-by-value + std::move in NullTelemetry constructor. Co-Authored-By: Claude Opus 4.6 --- include/xrpl/telemetry/Telemetry.h | 18 ++++++++--------- src/libxrpl/telemetry/NullTelemetry.cpp | 24 ++++++++++++----------- src/libxrpl/telemetry/TelemetryConfig.cpp | 9 +++++---- src/xrpld/app/main/Application.cpp | 1 + 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index d3b729815a..1d69e01a43 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -85,8 +85,7 @@ #include #endif -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { class Telemetry { @@ -222,27 +221,27 @@ public: stop() = 0; /** @return true if this instance is actively exporting spans. */ - virtual bool + [[nodiscard]] virtual bool isEnabled() const = 0; /** @return true if transaction processing should be traced. */ - virtual bool + [[nodiscard]] virtual bool shouldTraceTransactions() const = 0; /** @return true if consensus rounds should be traced. */ - virtual bool + [[nodiscard]] virtual bool shouldTraceConsensus() const = 0; /** @return true if RPC request handling should be traced. */ - virtual bool + [[nodiscard]] virtual bool shouldTraceRpc() const = 0; /** @return true if peer-to-peer messages should be traced. */ - virtual bool + [[nodiscard]] virtual bool shouldTracePeer() const = 0; /** @return true if ledger close/accept should be traced. */ - virtual bool + [[nodiscard]] virtual bool shouldTraceLedger() const = 0; #ifdef XRPL_ENABLE_TELEMETRY @@ -318,5 +317,4 @@ setup_Telemetry( std::string const& version, std::uint32_t networkId); -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index 6d57f77c69..4a1b901614 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -10,14 +10,17 @@ its own factory that can return the real TelemetryImpl. */ +#include #include #ifdef XRPL_ENABLE_TELEMETRY #include #endif -namespace xrpl { -namespace telemetry { +#include +#include + +namespace xrpl::telemetry { namespace { @@ -32,7 +35,7 @@ class NullTelemetry : public Telemetry Setup const setup_; public: - explicit NullTelemetry(Setup const& setup) : setup_(setup) + explicit NullTelemetry(Setup setup) : setup_(std::move(setup)) { } @@ -48,37 +51,37 @@ public: Telemetry::setInstance(nullptr); } - bool + [[nodiscard]] bool isEnabled() const override { return false; } - bool + [[nodiscard]] bool shouldTraceTransactions() const override { return false; } - bool + [[nodiscard]] bool shouldTraceConsensus() const override { return false; } - bool + [[nodiscard]] bool shouldTraceRpc() const override { return false; } - bool + [[nodiscard]] bool shouldTracePeer() const override { return false; } - bool + [[nodiscard]] bool shouldTraceLedger() const override { return false; @@ -125,5 +128,4 @@ make_Telemetry(Telemetry::Setup const& setup, beast::Journal) } #endif -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index 16a1461286..9ab7bb5cd6 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -7,12 +7,14 @@ See cfg/xrpld-example.cfg for the full list of available options. */ +#include #include #include +#include +#include -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { namespace { @@ -78,5 +80,4 @@ setup_Telemetry( return setup; } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index f9a70725ec..e222660c39 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -82,6 +82,7 @@ #include #include #include +#include #include #include #include From bd6e58a20e542d359fd33711c7c228be9ff57f53 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 29 Apr 2026 11:21:13 +0100 Subject: [PATCH 3/4] fix(telemetry): add missing span constants, fix test API, update levelization Add unknownCommand and wsUpgrade span name constants to RpcSpanNames.h, fix SpanGuardFactory tests to use the 3-argument SpanGuard::span() API, update levelization results, and apply rename script to docs. Co-Authored-By: Claude Opus 4.6 --- .github/scripts/levelization/results/loops.txt | 2 +- .github/scripts/levelization/results/ordering.txt | 1 + OpenTelemetryPlan/Phase3_taskList.md | 2 +- OpenTelemetryPlan/Phase5_taskList.md | 4 ++-- src/tests/libxrpl/telemetry/SpanGuardFactory.cpp | 10 +++++----- src/xrpld/rpc/detail/RpcSpanNames.h | 2 ++ 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.github/scripts/levelization/results/loops.txt b/.github/scripts/levelization/results/loops.txt index 181cbec44a..358aa387eb 100644 --- a/.github/scripts/levelization/results/loops.txt +++ b/.github/scripts/levelization/results/loops.txt @@ -5,7 +5,7 @@ Loop: test.jtx test.unit_test test.unit_test ~= test.jtx Loop: xrpl.telemetry xrpld.rpc - xrpld.rpc ~= xrpl.telemetry + xrpld.rpc > xrpl.telemetry Loop: xrpld.app xrpld.overlay xrpld.app > xrpld.overlay diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index b908b4a64c..3c23c8ff68 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -194,6 +194,7 @@ tests.libxrpl > xrpl.json tests.libxrpl > xrpl.net tests.libxrpl > xrpl.protocol tests.libxrpl > xrpl.protocol_autogen +tests.libxrpl > xrpl.telemetry xrpl.conditions > xrpl.basics xrpl.conditions > xrpl.protocol xrpl.core > xrpl.basics diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index 09bc8f975c..1e93d4fd4c 100644 --- a/OpenTelemetryPlan/Phase3_taskList.md +++ b/OpenTelemetryPlan/Phase3_taskList.md @@ -23,7 +23,7 @@ **What to do**: -- Edit `include/xrpl/proto/xrpl.proto` (or `src/ripple/proto/ripple.proto`, wherever the proto is): +- Edit `include/xrpl/proto/xrpl.proto` (or `src/xrpld/proto/ripple.proto`, wherever the proto is): - Add `TraceContext` message definition: ```protobuf message TraceContext { diff --git a/OpenTelemetryPlan/Phase5_taskList.md b/OpenTelemetryPlan/Phase5_taskList.md index 644c842e40..c2d0aa9c60 100644 --- a/OpenTelemetryPlan/Phase5_taskList.md +++ b/OpenTelemetryPlan/Phase5_taskList.md @@ -175,7 +175,7 @@ **What to do**: - Create `docs/telemetry-runbook.md`: - - **Setup**: How to enable telemetry in rippled + - **Setup**: How to enable telemetry in xrpld - **Configuration**: All config options with descriptions - **Collector Deployment**: Docker Compose vs. Kubernetes vs. bare metal - **Troubleshooting**: Common issues and resolutions @@ -199,7 +199,7 @@ **What to do**: 1. Start full Docker stack (Collector, Tempo, Grafana, Prometheus) -2. Build rippled with `telemetry=ON` +2. Build xrpld with `telemetry=ON` 3. Run in standalone mode with telemetry enabled 4. Generate RPC traffic and verify traces in Tempo 5. Verify dashboards populate in Grafana diff --git a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp index 89f6283bca..373705d2b4 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp @@ -7,7 +7,7 @@ using namespace xrpl::telemetry; TEST(SpanGuardFactory, null_guard_methods_are_safe) { - auto span = SpanGuard::span("nonexistent.span"); + auto span = SpanGuard::span(TraceCategory::Rpc, "rpc", "nonexistent"); EXPECT_FALSE(span); span.setAttribute("key", "value"); @@ -29,28 +29,28 @@ TEST(SpanGuardFactory, category_span_returns_null_when_disabled) TEST(SpanGuardFactory, child_span_null_when_no_parent) { - auto span = SpanGuard::span("parent.test"); + auto span = SpanGuard::span(TraceCategory::Rpc, "rpc", "parent"); auto child = span.childSpan("child.test"); EXPECT_FALSE(child); } TEST(SpanGuardFactory, linked_span_null_when_no_context) { - auto span = SpanGuard::span("source.test"); + auto span = SpanGuard::span(TraceCategory::Rpc, "rpc", "source"); auto linked = span.linkedSpan("linked.test"); EXPECT_FALSE(linked); } TEST(SpanGuardFactory, capture_context_returns_invalid_on_null) { - auto span = SpanGuard::span("ctx.test"); + auto span = SpanGuard::span(TraceCategory::Rpc, "rpc", "ctx"); auto ctx = span.captureContext(); EXPECT_FALSE(ctx.isValid()); } TEST(SpanGuardFactory, move_construction_transfers_ownership) { - auto span = SpanGuard::span("move.test"); + auto span = SpanGuard::span(TraceCategory::Rpc, "rpc", "move"); auto moved = std::move(span); EXPECT_FALSE(span); moved.setAttribute("key", "value"); diff --git a/src/xrpld/rpc/detail/RpcSpanNames.h b/src/xrpld/rpc/detail/RpcSpanNames.h index a10fd1af3e..ef46c79782 100644 --- a/src/xrpld/rpc/detail/RpcSpanNames.h +++ b/src/xrpld/rpc/detail/RpcSpanNames.h @@ -37,6 +37,7 @@ inline constexpr auto command = join(seg::rpc, makeStr("command")); namespace op { inline constexpr auto wsMessage = makeStr("ws_message"); +inline constexpr auto wsUpgrade = makeStr("ws_upgrade"); inline constexpr auto httpRequest = makeStr("http_request"); inline constexpr auto process = makeStr("process"); } // namespace op @@ -65,6 +66,7 @@ using telemetry::attr_val::error; using telemetry::attr_val::success; inline constexpr auto admin = makeStr("admin"); inline constexpr auto user = makeStr("user"); +inline constexpr auto unknownCommand = makeStr("unknown_command"); } // namespace val } // namespace rpc_span From 8e97c7329ae650abf541583591f76c1d82d37cc3 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 29 Apr 2026 11:23:43 +0100 Subject: [PATCH 4/4] fix(telemetry): fix include ordering, levelization, and rename for phase 3 Move TxQSpanNames.h include to correct alphabetical position, update levelization results for new xrpld.telemetry module dependencies, and apply rename script to docs. Co-Authored-By: Claude Opus 4.6 --- .github/scripts/levelization/results/loops.txt | 3 +++ .github/scripts/levelization/results/ordering.txt | 4 +++- OpenTelemetryPlan/Phase3_taskList.md | 8 ++++---- src/xrpld/app/misc/detail/TxQ.cpp | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.github/scripts/levelization/results/loops.txt b/.github/scripts/levelization/results/loops.txt index 358aa387eb..66906f48c6 100644 --- a/.github/scripts/levelization/results/loops.txt +++ b/.github/scripts/levelization/results/loops.txt @@ -19,6 +19,9 @@ Loop: xrpld.app xrpld.rpc Loop: xrpld.app xrpld.shamap xrpld.shamap > xrpld.app +Loop: xrpld.app xrpld.telemetry + xrpld.telemetry == xrpld.app + Loop: xrpld.overlay xrpld.rpc xrpld.rpc ~= xrpld.overlay diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 9f1c7b943b..c0f6877714 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -238,7 +238,6 @@ xrpld.app > xrpl.basics xrpld.app > xrpl.core xrpld.app > xrpld.consensus xrpld.app > xrpld.core -xrpld.app > xrpld.telemetry xrpld.app > xrpl.json xrpld.app > xrpl.ledger xrpld.app > xrpl.net @@ -271,6 +270,7 @@ xrpld.overlay > xrpl.protocol xrpld.overlay > xrpl.resource xrpld.overlay > xrpl.server xrpld.overlay > xrpl.shamap +xrpld.overlay > xrpl.telemetry xrpld.overlay > xrpl.tx xrpld.peerfinder > xrpl.basics xrpld.peerfinder > xrpld.core @@ -298,3 +298,5 @@ xrpld.shamap > xrpl.basics xrpld.shamap > xrpld.core xrpld.shamap > xrpl.protocol xrpld.shamap > xrpl.shamap +xrpld.telemetry > xrpl.basics +xrpld.telemetry > xrpl.telemetry diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index c52adb49fc..94de0e9682 100644 --- a/OpenTelemetryPlan/Phase3_taskList.md +++ b/OpenTelemetryPlan/Phase3_taskList.md @@ -224,7 +224,7 @@ > **Upstream**: Phase 2 (RPC span infrastructure must exist). > **Downstream**: Phase 10 (validation checks for this attribute). -**Objective**: Add the relaying peer's rippled version to `tx.receive` spans so operators can correlate transaction issues with peer version mismatches during network upgrades. +**Objective**: Add the relaying peer's xrpld version to `tx.receive` spans so operators can correlate transaction issues with peer version mismatches during network upgrades. **What to do**: @@ -235,9 +235,9 @@ **New span attribute**: -| Attribute | Type | Source | Example | -| ------------------- | ------ | -------------------- | ----------------- | -| `xrpl.peer.version` | string | `peer->getVersion()` | `"rippled-2.4.0"` | +| Attribute | Type | Source | Example | +| ------------------- | ------ | -------------------- | --------------- | +| `xrpl.peer.version` | string | `peer->getVersion()` | `"xrpld-2.4.0"` | **Rationale**: Transaction relay is where version mismatches cause subtle serialization or validation bugs. Tracing "this tx came from a v2.3.0 peer" helps diagnose compatibility issues. The community dashboard tracks peer versions externally; this brings version awareness into the trace itself. diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 51a5e1e386..32842ab9ad 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -1,8 +1,8 @@ #include -#include #include #include +#include #include #include