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 1/2] 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 2/2] 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