From 7c8a3906a19743b5b7285d6bc70993d051b44846 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Fri, 17 Apr 2026 17:40:37 +0100 Subject: [PATCH] refactor(telemetry): replace per-category factory methods with TraceCategory enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace rpcSpan(), txSpan(), consensusSpan(), peerSpan(), ledgerSpan() with a single span(TraceCategory, prefix, name) factory method. Adding a new traceable subsystem now requires only a new enum value and one switch case — no new methods or header changes. Co-Authored-By: Claude Opus 4.6 --- include/xrpl/telemetry/SpanGuard.h | 121 ++++++++++++---------------- src/libxrpl/telemetry/SpanGuard.cpp | 73 +++++++---------- 2 files changed, 82 insertions(+), 112 deletions(-) diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index b4c03c7730..030c8f5829 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -9,27 +9,23 @@ Dependency diagram: - +-----------------------------------------+ - | SpanGuard | - +-----------------------------------------+ - | - impl_ : unique_ptr (pimpl) | - +-----------------------------------------+ - | + rpcSpan(name) : SpanGuard [static] | - | + txSpan(name) : SpanGuard [static] | - | + consensusSpan(name) [static] | - | + peerSpan(name) [static] | - | + ledgerSpan(name) [static] | - | + span(name) [static] | - | + childSpan(name) : SpanGuard | - | + linkedSpan(name) : SpanGuard | - | + captureContext() : SpanContext | - | + setAttribute(key, value) | - | + setOk() / setError(desc) | - | + addEvent(name) | - | + recordException(e) | - | + discard() | - | + operator bool() | - +-----------------------------------------+ + +-------------------------------------------+ + | SpanGuard | + +-------------------------------------------+ + | - impl_ : unique_ptr (pimpl) | + +-------------------------------------------+ + | + span(name) : SpanGuard [static] | + | + span(cat, prefix, name) [static] | + | + childSpan(name) : SpanGuard | + | + linkedSpan(name) : SpanGuard | + | + captureContext() : SpanContext | + | + setAttribute(key, value) | + | + setOk() / setError(desc) | + | + addEvent(name) | + | + recordException(e) | + | + discard() | + | + operator bool() | + +-------------------------------------------+ | hides (pimpl) +-------+-------+ | | @@ -47,9 +43,14 @@ Usage examples: - 1. Basic RPC tracing (factory method): + 1. Basic RPC tracing (factory method with category): @code - auto span = SpanGuard::rpcSpan("rpc.command.submit"); + // Define prefix at class level: + static constexpr std::string_view spanPrefix_ = "rpc.command"; + + // At the call site: + auto span = SpanGuard::span( + TraceCategory::Rpc, spanPrefix_, "submit"); span.setAttribute("xrpl.rpc.command", "submit"); span.setAttribute("xrpl.rpc.status", "success"); // span ended automatically on scope exit @@ -57,7 +58,8 @@ 2. Error recording: @code - auto span = SpanGuard::rpcSpan("rpc.command.submit"); + auto span = SpanGuard::span( + TraceCategory::Rpc, "rpc.command", "submit"); try { doWork(); span.setOk(); @@ -69,7 +71,8 @@ 3. Cross-thread context propagation: @code // Thread A: create span and capture context - auto span = SpanGuard::consensusSpan("consensus.round"); + auto span = SpanGuard::span( + TraceCategory::Consensus, "consensus", "round"); auto ctx = span.captureContext(); // Thread B: create child with captured context @@ -78,7 +81,8 @@ 4. Conditional check (rarely needed — methods are no-ops on null): @code - auto span = SpanGuard::rpcSpan("rpc.request"); + auto span = SpanGuard::span( + TraceCategory::Rpc, "rpc", "request"); if (span) { // expensive attribute computation only when active span.setAttribute("xrpl.rpc.payload_size", computeSize()); @@ -87,7 +91,8 @@ 5. Tail-based filtering via discard(): @code - auto span = SpanGuard::txSpan("tx.process"); + auto span = SpanGuard::span( + TraceCategory::Transactions, "tx", "process"); auto result = preflight(tx); if (result != tesSUCCESS) { span.discard(); // drop span, never exported @@ -119,6 +124,14 @@ namespace xrpl { namespace telemetry { +/** Trace subsystem categories for conditional span creation. + + Each value maps to a runtime config flag (e.g. `trace_rpc=1`). + Used by SpanGuard::span(TraceCategory, prefix, name) to decide + whether to create a real span or return a null guard. +*/ +enum class TraceCategory { Rpc, Transactions, Consensus, Peer, Ledger }; + /** Opaque wrapper for an OTel context snapshot. Used to propagate trace context across threads. Created by @@ -180,32 +193,22 @@ public: operator=(SpanGuard const&) = delete; // --- Static factory methods ---------------------------------------- - // Each checks the global Telemetry instance and the corresponding - // shouldTrace*() flag. Returns a null guard if tracing is off. - /** Create an unconditional span (always created if telemetry is on). */ + /** Create an unconditional span (always created if telemetry is on). + @param name Full span name (e.g. "app.startup"). + */ static SpanGuard span(std::string_view name); - /** Create a span guarded by shouldTraceRpc(). */ + /** Create a span guarded by a TraceCategory flag. + The span name is built as "prefix.name". Returns a null guard + if the category is disabled in config. + @param cat Trace subsystem category. + @param prefix Span name prefix (e.g. "rpc.command"). + @param name Span name suffix (e.g. "submit"). + */ static SpanGuard - rpcSpan(std::string_view name); - - /** Create a span guarded by shouldTraceTransactions(). */ - static SpanGuard - txSpan(std::string_view name); - - /** Create a span guarded by shouldTraceConsensus(). */ - static SpanGuard - consensusSpan(std::string_view name); - - /** Create a span guarded by shouldTracePeer(). */ - static SpanGuard - peerSpan(std::string_view name); - - /** Create a span guarded by shouldTraceLedger(). */ - static SpanGuard - ledgerSpan(std::string_view name); + span(TraceCategory cat, std::string_view prefix, std::string_view name); // --- Child / linked span creation ---------------------------------- @@ -332,27 +335,7 @@ public: return {}; } static SpanGuard - rpcSpan(std::string_view) - { - return {}; - } - static SpanGuard - txSpan(std::string_view) - { - return {}; - } - static SpanGuard - consensusSpan(std::string_view) - { - return {}; - } - static SpanGuard - peerSpan(std::string_view) - { - return {}; - } - static SpanGuard - ledgerSpan(std::string_view) + span(TraceCategory, std::string_view, std::string_view) { return {}; } diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index 8d1aebdd97..6381d8a469 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -4,10 +4,10 @@ The public SpanGuard.h header contains only standard-library types and forward-declares the Impl struct. - Static factory methods (rpcSpan, txSpan, etc.) access the global - Telemetry instance via Telemetry::getInstance(), check the relevant - shouldTrace*() flag, and return either an active guard with a real - Span+Scope or a null guard whose methods are all no-ops. + Static factory methods access the global Telemetry instance via + Telemetry::getInstance(), check whether the requested TraceCategory + is enabled, and return either an active guard with a real Span+Scope + or a null guard whose methods are all no-ops. The Impl struct holds the OTel Span (shared_ptr) and Scope. Scope is non-movable, but since Impl lives behind a unique_ptr, @@ -109,6 +109,28 @@ operator bool() const // ===== Static factory methods ============================================== +/** Check whether the given TraceCategory is enabled on the Telemetry instance. + @return true if the category's shouldTrace*() flag is on. +*/ +static bool +isCategoryEnabled(Telemetry const& tel, TraceCategory cat) +{ + switch (cat) + { + case TraceCategory::Rpc: + return tel.shouldTraceRpc(); + case TraceCategory::Transactions: + return tel.shouldTraceTransactions(); + case TraceCategory::Consensus: + return tel.shouldTraceConsensus(); + case TraceCategory::Peer: + return tel.shouldTracePeer(); + case TraceCategory::Ledger: + return tel.shouldTraceLedger(); + } + return false; // unreachable, silences compiler warning +} + SpanGuard SpanGuard::span(std::string_view name) { @@ -119,48 +141,13 @@ SpanGuard::span(std::string_view name) } SpanGuard -SpanGuard::rpcSpan(std::string_view name) +SpanGuard::span(TraceCategory cat, std::string_view prefix, std::string_view name) { auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled() || !tel->shouldTraceRpc()) + if (!tel || !tel->isEnabled() || !isCategoryEnabled(*tel, cat)) return {}; - return SpanGuard(std::make_unique(tel->startSpan(name))); -} - -SpanGuard -SpanGuard::txSpan(std::string_view name) -{ - auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled() || !tel->shouldTraceTransactions()) - return {}; - return SpanGuard(std::make_unique(tel->startSpan(name))); -} - -SpanGuard -SpanGuard::consensusSpan(std::string_view name) -{ - auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled() || !tel->shouldTraceConsensus()) - return {}; - return SpanGuard(std::make_unique(tel->startSpan(name))); -} - -SpanGuard -SpanGuard::peerSpan(std::string_view name) -{ - auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled() || !tel->shouldTracePeer()) - return {}; - return SpanGuard(std::make_unique(tel->startSpan(name))); -} - -SpanGuard -SpanGuard::ledgerSpan(std::string_view name) -{ - auto* tel = Telemetry::getInstance(); - if (!tel || !tel->isEnabled() || !tel->shouldTraceLedger()) - return {}; - return SpanGuard(std::make_unique(tel->startSpan(name))); + auto fullName = std::string(prefix) + "." + std::string(name); + return SpanGuard(std::make_unique(tel->startSpan(fullName))); } // ===== Child / linked span creation ========================================