diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index 577d0d6a93..94de0e9682 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 { @@ -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/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/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index c74fc3bb7b..92f87f7a70 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 { @@ -229,27 +228,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; /** @return The configured consensus trace correlation strategy. */ @@ -329,5 +328,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 51c134075e..a957330a1a 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; @@ -131,5 +134,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 be93476ae3..0f4894556d 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 { @@ -81,5 +83,4 @@ setup_Telemetry( return setup; } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp index 59ea205a69..d09220b778 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp @@ -8,7 +8,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"); @@ -30,28 +30,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/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 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 edaae48397..3dd83399c9 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1443,11 +1443,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 @@ -1481,11 +1481,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; } @@ -1543,7 +1543,8 @@ PeerImp::handleTransaction( flags, checkSignature, batch, - stx]() { + stx, + sp = std::move(span)]() { if (auto peer = weak.lock()) peer->checkTransaction(flags, checkSignature, stx, batch); }); 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