From 68a69d9064f9b45523ee46b655a8f2410cec51ae Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Fri, 29 May 2026 14:50:24 +0100 Subject: [PATCH] updated as per latest clang-tidy Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- include/xrpl/telemetry/DiscardFlag.h | 10 ++--- include/xrpl/telemetry/Telemetry.h | 12 ++--- src/libxrpl/telemetry/NullTelemetry.cpp | 12 +++-- src/libxrpl/telemetry/SpanGuard.cpp | 24 +++++----- src/libxrpl/telemetry/Telemetry.cpp | 55 ++++++++++++----------- src/libxrpl/telemetry/TelemetryConfig.cpp | 2 +- src/xrpld/app/main/Application.cpp | 4 +- 7 files changed, 61 insertions(+), 58 deletions(-) diff --git a/include/xrpl/telemetry/DiscardFlag.h b/include/xrpl/telemetry/DiscardFlag.h index 991b9b8f6a..e1c81c3319 100644 --- a/include/xrpl/telemetry/DiscardFlag.h +++ b/include/xrpl/telemetry/DiscardFlag.h @@ -2,7 +2,7 @@ /** Thread-local flag for span discard signaling. - SpanGuard::discard() sets tl_discardCurrentSpan to true before calling + SpanGuard::discard() sets gTlDiscardCurrentSpan to true before calling Span::End(). The OTel SDK calls SpanProcessor::OnEnd() synchronously on the same thread, so FilteringSpanProcessor checks and clears this flag in OnEnd() to drop the span before it enters the batch export queue. @@ -16,12 +16,10 @@ @see SpanGuard::discard(), FilteringSpanProcessor (Telemetry.cpp) */ -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { /** When true, the FilteringSpanProcessor drops the current span in OnEnd(). Set by SpanGuard::discard(), cleared by OnEnd(). */ -inline thread_local bool tl_discardCurrentSpan = false; +inline thread_local bool gTlDiscardCurrentSpan = false; -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index ae9303a1db..84a01858f5 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -4,7 +4,7 @@ Provides the Telemetry base class that all components use to create trace spans. Two concrete implementations exist, selected at construction time - by make_Telemetry(): + by makeTelemetry(): - TelemetryImpl (Telemetry.cpp): real OTel SDK integration, compiled only when XRPL_ENABLE_TELEMETRY is defined and enabled at runtime. @@ -98,7 +98,7 @@ class Telemetry the initialization thread, factory methods load on worker threads. @see setInstance(), getInstance() */ - inline static std::atomic instance_{nullptr}; + inline static std::atomic instance{nullptr}; public: /** Get the global Telemetry instance. @@ -107,7 +107,7 @@ public: static Telemetry* getInstance() { - return instance_.load(std::memory_order_acquire); + return instance.load(std::memory_order_acquire); } /** Set the global Telemetry instance. @@ -118,7 +118,7 @@ public: static void setInstance(Telemetry* t) { - instance_.store(t, std::memory_order_release); + instance.store(t, std::memory_order_release); } /** Configuration parsed from the [telemetry] section of xrpld.cfg. @@ -300,7 +300,7 @@ public: @param journal Journal for log output during initialization. */ std::unique_ptr -make_Telemetry(Telemetry::Setup const& setup, beast::Journal journal); +makeTelemetry(Telemetry::Setup const& setup, beast::Journal journal); /** Parse the [telemetry] config section into a Setup struct. @@ -312,7 +312,7 @@ make_Telemetry(Telemetry::Setup const& setup, beast::Journal journal); @return A populated Setup struct with defaults for missing values. */ Telemetry::Setup -setup_Telemetry( +setupTelemetry( Section const& section, std::string const& nodePublicKey, std::string const& version, diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index 4a1b901614..7cbfd24019 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -1,25 +1,23 @@ /** No-op implementation of the Telemetry interface. Always compiled (regardless of XRPL_ENABLE_TELEMETRY). Provides the - make_Telemetry() factory when telemetry is compiled out (#ifndef), which + makeTelemetry() factory when telemetry is compiled out (#ifndef), which unconditionally returns a NullTelemetry that does nothing. When XRPL_ENABLE_TELEMETRY IS defined, the OTel virtual methods - (getTracer, startSpan) return noop tracers/spans. The make_Telemetry() + (getTracer, startSpan) return noop tracers/spans. The makeTelemetry() factory in this file is not used in that case -- Telemetry.cpp provides its own factory that can return the real TelemetryImpl. */ -#include #include +#include + #ifdef XRPL_ENABLE_TELEMETRY #include #endif -#include -#include - namespace xrpl::telemetry { namespace { @@ -122,7 +120,7 @@ public: */ #ifndef XRPL_ENABLE_TELEMETRY std::unique_ptr -make_Telemetry(Telemetry::Setup const& setup, beast::Journal) +makeTelemetry(Telemetry::Setup const& setup, beast::Journal) { return std::make_unique(setup); } diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index 90bb139b97..b698d8d4b9 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -34,12 +34,15 @@ #include #include +#include +#include +#include #include +#include #include #include -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { namespace otel_trace = opentelemetry::trace; @@ -163,7 +166,7 @@ SpanGuard SpanGuard::span(TraceCategory cat, std::string_view prefix, std::string_view name) { auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled() || !isCategoryEnabled(*tel, cat)) + if ((tel == nullptr) || !tel->isEnabled() || !isCategoryEnabled(*tel, cat)) return {}; auto fullName = std::string(prefix) + "." + std::string(name); return SpanGuard(std::make_unique(tel->startSpan(fullName, categoryToSpanKind(cat)))); @@ -177,7 +180,7 @@ SpanGuard::childSpan(std::string_view name) const if (!impl_) return {}; auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled()) + if ((tel == nullptr) || !tel->isEnabled()) return {}; auto ctx = opentelemetry::context::RuntimeContext::GetCurrent(); return SpanGuard(std::make_unique(tel->startSpan(name, ctx))); @@ -189,7 +192,7 @@ SpanGuard::childSpan(std::string_view name, SpanContext const& parentCtx) if (!parentCtx.isValid()) return {}; auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled()) + if ((tel == nullptr) || !tel->isEnabled()) return {}; return SpanGuard(std::make_unique(tel->startSpan(name, parentCtx.impl_->ctx))); } @@ -200,7 +203,7 @@ SpanGuard::linkedSpan(std::string_view name) const if (!impl_) return {}; auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled()) + if ((tel == nullptr) || !tel->isEnabled()) return {}; auto tracer = tel->getTracer("xrpld"); @@ -224,7 +227,7 @@ SpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx) if (!linkCtx.isValid()) return {}; auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled()) + if ((tel == nullptr) || !tel->isEnabled()) return {}; auto tracer = tel->getTracer("xrpld"); @@ -266,9 +269,11 @@ void SpanGuard::setAttribute(std::string_view key, std::string_view value) { if (impl_) + { impl_->span->SetAttribute( opentelemetry::nostd::string_view(key.data(), key.size()), opentelemetry::nostd::string_view(value.data(), value.size())); + } } void @@ -337,14 +342,13 @@ SpanGuard::discard() { if (impl_) { - tl_discardCurrentSpan = true; + gTlDiscardCurrentSpan = true; impl_->span->End(); impl_->span = nullptr; // prevent ~Impl from calling End() again impl_.reset(); } } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry #endif // XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 3b212dc4fa..016d9bbf58 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -10,7 +10,7 @@ trace-ID-ratio sampler, and resource attributes. - NullTelemetryOtel: no-op fallback used when telemetry is compiled in but disabled at runtime (enabled=0 in config). - - make_Telemetry(): factory that selects the appropriate implementation. + - makeTelemetry(): factory that selects the appropriate implementation. */ #ifdef XRPL_ENABLE_TELEMETRY @@ -33,8 +33,12 @@ #include #include -namespace xrpl { -namespace telemetry { +#include +#include +#include +#include + +namespace xrpl::telemetry { namespace { @@ -46,7 +50,7 @@ namespace resource = opentelemetry::sdk::resource; /** SpanProcessor decorator that drops discarded spans. Wraps a delegate processor (typically BatchSpanProcessor). In OnEnd(), - checks the tl_discardCurrentSpan thread-local flag. If set (by + checks the gTlDiscardCurrentSpan thread-local flag. If set (by SpanGuard::discard()), the span is silently dropped — never entering the batch queue, never sent over the network, never stored. @@ -72,12 +76,12 @@ namespace resource = opentelemetry::sdk::resource; +---------------------+ @note Thread safety: OnEnd() may be called concurrently from multiple - threads. The tl_discardCurrentSpan flag is thread-local, so each + threads. The gTlDiscardCurrentSpan flag is thread-local, so each thread's discard state is independent — no synchronization needed. */ class FilteringSpanProcessor : public trace_sdk::SpanProcessor { - std::unique_ptr delegate_; + std::unique_ptr delegate_{}; public: explicit FilteringSpanProcessor(std::unique_ptr delegate) @@ -102,12 +106,12 @@ public: void OnEnd(std::unique_ptr&& span) noexcept override { - if (tl_discardCurrentSpan) + if (gTlDiscardCurrentSpan) { // SpanGuard::discard() set the flag on this thread just before // calling Span::End(), which invokes OnEnd() synchronously. // Clear the flag and drop the span. - tl_discardCurrentSpan = false; + gTlDiscardCurrentSpan = false; return; } delegate_->OnEnd(std::move(span)); @@ -140,7 +144,7 @@ class NullTelemetryOtel : public Telemetry Setup const setup_; public: - explicit NullTelemetryOtel(Setup const& setup) : setup_(setup) + explicit NullTelemetryOtel(Setup setup) : setup_(std::move(setup)) { } @@ -156,37 +160,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; @@ -235,10 +239,10 @@ class TelemetryImpl : public Telemetry Held as std::shared_ptr so we can call ForceFlush() on shutdown. Wrapped in a nostd::shared_ptr when registered as the global provider. */ - std::shared_ptr sdkProvider_; + std::shared_ptr sdkProvider_{}; public: - TelemetryImpl(Setup const& setup, beast::Journal journal) : setup_(setup), journal_(journal) + TelemetryImpl(Setup setup, beast::Journal journal) : setup_(std::move(setup)), journal_(journal) { } @@ -330,37 +334,37 @@ public: JLOG(journal_.info()) << "Telemetry stopped"; } - bool + [[nodiscard]] bool isEnabled() const override { return true; } - bool + [[nodiscard]] bool shouldTraceTransactions() const override { return setup_.traceTransactions; } - bool + [[nodiscard]] bool shouldTraceConsensus() const override { return setup_.traceConsensus; } - bool + [[nodiscard]] bool shouldTraceRpc() const override { return setup_.traceRpc; } - bool + [[nodiscard]] bool shouldTracePeer() const override { return setup_.tracePeer; } - bool + [[nodiscard]] bool shouldTraceLedger() const override { return setup_.traceLedger; @@ -400,14 +404,13 @@ public: } // namespace std::unique_ptr -make_Telemetry(Telemetry::Setup const& setup, beast::Journal journal) +makeTelemetry(Telemetry::Setup const& setup, beast::Journal journal) { if (setup.enabled) return std::make_unique(setup, journal); return std::make_unique(setup); } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry #endif // XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index 9ab7bb5cd6..6d642ee177 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -41,7 +41,7 @@ networkTypeFromId(std::uint32_t networkId) } // namespace Telemetry::Setup -setup_Telemetry( +setupTelemetry( Section const& section, std::string const& nodePublicKey, std::string const& version, diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 29fb2dd0e9..803c54dd13 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -324,8 +324,8 @@ public: [this] { signalStop("PerfLog"); })) , telemetry_( - telemetry::make_Telemetry( - telemetry::setup_Telemetry( + telemetry::makeTelemetry( + telemetry::setupTelemetry( config_->section("telemetry"), "", // Updated later via setServiceInstanceId() BuildInfo::getVersionString(),