From a104140a517c6c43617a4ab3bc30a0e6d3e24f92 Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Wed, 27 May 2026 16:46:35 +0100
Subject: [PATCH] addressing code review comments

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
---
 docs/build/telemetry.md                     |  3 +-
 include/xrpl/protocol/detail/features.macro |  1 -
 include/xrpl/telemetry/Telemetry.h          | 33 +++++++++++----------
 src/libxrpl/telemetry/SpanGuard.cpp         |  3 +-
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/docs/build/telemetry.md b/docs/build/telemetry.md
index e8d7fb2dd5..eff1a6848b 100644
--- a/docs/build/telemetry.md
+++ b/docs/build/telemetry.md
@@ -36,7 +36,8 @@ such as Grafana Tempo.
 Telemetry is **off by default** at both compile time and runtime:
 
 - **Compile time**: The Conan option `telemetry` and CMake option `telemetry` must be set to `True`/`ON`.
-  When disabled, all tracing macros compile to `((void)0)` with zero overhead.
+  When disabled, all `SpanGuard` calls compile to inline no-ops (defined in `SpanGuard.h`)
+  with zero overhead — no OTel SDK dependency required.
 - **Runtime**: The `[telemetry]` config section must set `enabled=1`.
   When disabled at runtime, a no-op implementation is used.
 
diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro
index bad43dd6ed..494b3fa6cd 100644
--- a/include/xrpl/protocol/detail/features.macro
+++ b/include/xrpl/protocol/detail/features.macro
@@ -15,7 +15,6 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
-XRPL_FIX    (Cleanup3_2_0,                Supported::no, VoteBehavior::DefaultNo)
 XRPL_FEATURE(MPTokensV2,                  Supported::no, VoteBehavior::DefaultNo)
 XRPL_FIX    (Security3_1_3,               Supported::no, VoteBehavior::DefaultNo)
 XRPL_FIX    (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h
index 1d69e01a43..ae9303a1db 100644
--- a/include/xrpl/telemetry/Telemetry.h
+++ b/include/xrpl/telemetry/Telemetry.h
@@ -35,32 +35,33 @@
 
     Usage examples:
 
-    1. Check before tracing (typical guard pattern):
+    1. Root span at a subsystem entry point (typical usage):
     @code
-        auto& telemetry = registry.getTelemetry();
-        if (telemetry.isEnabled() && telemetry.shouldTraceRpc())
-        {
-            auto span = telemetry.startSpan("rpc.command.server_info");
-            // ... do work, span ends when shared_ptr refcount drops to 0
-        }
+        // In an RPC handler dispatch:
+        auto guard = SpanGuard::span(TraceCategory::Rpc, "rpc", commandName);
+        guard.setAttribute("xrpl.rpc.command", commandName);
+        // ... process request
+        // guard destructor automatically ends the span on scope exit
     @endcode
 
-    2. RAII tracing with SpanGuard (preferred):
+    2. Child span for a sub-operation (cross-scope):
     @code
-        if (telemetry.isEnabled() && telemetry.shouldTraceRpc())
+        auto parent = SpanGuard::span(TraceCategory::Transactions, "tx", "process");
         {
-            SpanGuard guard(telemetry.startSpan("rpc.command.submit"));
-            guard.setAttribute("xrpl.rpc.command", "submit");
-            // ... guard ends span automatically on scope exit
+            auto child = parent.childSpan("tx.apply");
+            child.setAttribute("xrpl.tx.type", txType);
+            // child ends here
         }
+        // parent continues, then ends here
     @endcode
 
     3. Cross-thread context propagation:
     @code
-        // On thread A: capture context
-        auto ctx = guard.context();
-        // On thread B: create child span with explicit parent
-        auto child = telemetry.startSpan("async.work", ctx);
+        // Thread A: capture the active context while span is in scope
+        auto ctx = parentGuard.captureContext();
+
+        // Thread B: create child span with explicit parent
+        auto child = SpanGuard::childSpan("async.work", ctx);
     @endcode
 
     @note Thread safety: The Telemetry interface is safe for concurrent reads
diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp
index 11eda56ed6..90bb139b97 100644
--- a/src/libxrpl/telemetry/SpanGuard.cpp
+++ b/src/libxrpl/telemetry/SpanGuard.cpp
@@ -35,6 +35,7 @@
 #include <opentelemetry/trace/tracer.h>
 
 #include <string>
+#include <typeinfo>
 #include <utility>
 
 namespace xrpl {
@@ -327,7 +328,7 @@ SpanGuard::recordException(std::exception const& e)
         return;
     impl_->span->AddEvent(
         "exception",
-        {{"exception.type", "std::exception"}, {"exception.message", std::string(e.what())}});
+        {{"exception.type", typeid(e).name()}, {"exception.message", std::string(e.what())}});
     impl_->span->SetStatus(otel_trace::StatusCode::kError, e.what());
 }