Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill

include/xrpl/telemetry/SpanGuard.h

@@ -167,6 +167,7 @@
 #include <exception>
 #include <initializer_list>
 #include <memory>
+#include <optional>
 #include <string_view>
 #include <utility>
 
@@ -541,6 +542,43 @@ public:
     operator bool() const;
 };
 
+// --- Detach-in-place helpers -----------------------------------------------
+
+/**
+ * Detach an active `std::optional<SpanGuard>` member in place.
+ *
+ * Equivalent to `guard.emplace(std::move(*guard).detached())`, which is the
+ * required idiom for detaching a live guard stored in an `optional` (move
+ * assignment is deleted because the underlying Scope cannot be re-scoped in
+ * place). No-op if `guard` is empty or already null.
+ *
+ * @param guard  The optional guard to detach. Must be called on the thread
+ *               that constructed the active guard inside it (same rule as
+ *               `SpanGuard::detached()`).
+ * @note Must run BEFORE any `captureContext()` snapshot is taken if the
+ *       caller also needs the pre-detach context — capture first, then call
+ *       this helper (same ordering rule `detached()` itself has).
+ */
+void
+detachInPlace(std::optional<SpanGuard>& guard);
+
+/**
+ * Detach an active `std::shared_ptr<SpanGuard>`, returning the detached
+ * guard as a new `shared_ptr`.
+ *
+ * Equivalent to
+ * `guard = std::make_shared<SpanGuard>(std::move(*guard).detached())`.
+ * No-op (returns the input unchanged) if `guard` is null or points at a
+ * null guard.
+ *
+ * @param guard  The guard to detach, taken by value (the caller's pointer
+ *               is consumed; assign the return value back).
+ * @return A `shared_ptr` to the detached guard (new allocation), or the
+ *         input pointer unchanged if it was null/inactive.
+ */
+[[nodiscard]] std::shared_ptr<SpanGuard>
+detachInPlace(std::shared_ptr<SpanGuard> guard);
+
 // ---------------------------------------------------------------------------
 // No-op stub (all inline, zero overhead, no OTel dependency)
 // ---------------------------------------------------------------------------
@@ -691,6 +729,19 @@ public:
     }
 };
 
+// --- Detach-in-place helpers (no-op stubs) ---------------------------------
+
+inline void
+detachInPlace(std::optional<SpanGuard>&)
+{
+}
+
+[[nodiscard]] inline std::shared_ptr<SpanGuard>
+detachInPlace(std::shared_ptr<SpanGuard> guard)
+{
+    return guard;
+}
+
 #endif  // XRPL_ENABLE_TELEMETRY
 
 }  // namespace xrpl::telemetry

src/libxrpl/telemetry/SpanGuard.cpp

@@ -336,6 +336,24 @@ SpanGuard::detached() &&
     return SpanGuard(std::make_unique<Impl>(std::move(s), Impl::Detached{}));
 }
 
+// ===== Detach-in-place helpers =============================================
+
+void
+detachInPlace(std::optional<SpanGuard>& guard)
+{
+    if (!guard || !*guard)
+        return;
+    guard.emplace(std::move(*guard).detached());
+}
+
+std::shared_ptr<SpanGuard>
+detachInPlace(std::shared_ptr<SpanGuard> guard)
+{
+    if (!guard || !*guard)
+        return guard;
+    return std::make_shared<SpanGuard>(std::move(*guard).detached());
+}
+
 // ===== Hash-derived span (category-gated) ==================================
 
 SpanGuard

src/tests/libxrpl/telemetry/SpanGuardScope.cpp

@@ -38,6 +38,7 @@
 
 #include <cstddef>
 #include <memory>
+#include <optional>
 #include <string>
 #include <string_view>
 #include <thread>
@@ -441,6 +442,108 @@ TEST_F(SpanGuardScopeTest, non_detached_cross_thread_corrupts_origin_stack)
     EXPECT_EQ(after->GetTraceId(), build->GetTraceId());
 }
 
+// detachInPlace(optional&) must detach the live guard the same way the
+// hand-rolled emplace(std::move(*opt).detached()) idiom does: after the call
+// the guard can be moved to and ended on a worker thread without leaving the
+// origin thread's context stack holding it.
+TEST_F(SpanGuardScopeTest, detach_in_place_optional_detaches_active_guard)
+{
+    // Scoped optional guard is active on this (origin) thread.
+    std::optional<SpanGuard> opt = SpanGuard::span(TraceCategory::Ledger, "ledger", "build");
+    ASSERT_TRUE(opt.has_value());
+    ASSERT_TRUE(static_cast<bool>(*opt));
+
+    // Strip the thread-local Scope here on the origin thread via the helper.
+    detachInPlace(opt);
+    ASSERT_TRUE(opt.has_value());
+    ASSERT_TRUE(static_cast<bool>(*opt));
+
+    // End the detached span on a worker thread.
+    std::thread worker([d = std::move(*opt)]() mutable {});
+    worker.join();
+
+    // Back on the origin thread: a new span must be a fresh root, proving the
+    // origin stack top was restored by detachInPlace() (not left holding
+    // ledger.build).
+    {
+        auto after = SpanGuard::span(TraceCategory::Rpc, "rpc", "command");
+        ASSERT_TRUE(static_cast<bool>(after));
+    }
+
+    auto spans = spanData()->GetSpans();
+    auto* build = findSpan(spans, "ledger.build");
+    auto* after = findSpan(spans, "rpc.command");
+    ASSERT_NE(build, nullptr);
+    ASSERT_NE(after, nullptr);
+
+    // 'after' is a new root: zero parent and a different trace than
+    // ledger.build.
+    EXPECT_FALSE(after->GetParentSpanId().IsValid());
+    EXPECT_NE(after->GetTraceId(), build->GetTraceId());
+}
+
+// detachInPlace(optional&) on an empty optional is a no-op: it must not crash,
+// must leave the optional empty, and must export nothing.
+TEST_F(SpanGuardScopeTest, detach_in_place_optional_noop_on_empty)
+{
+    std::optional<SpanGuard> opt;
+    ASSERT_FALSE(opt.has_value());
+
+    detachInPlace(opt);
+
+    // Still empty, no span was created or exported.
+    EXPECT_FALSE(opt.has_value());
+    EXPECT_TRUE(spanData()->GetSpans().empty());
+}
+
+// detachInPlace(shared_ptr) must detach the live guard and return it as a new
+// shared_ptr, so the returned guard can be moved to and ended on a worker
+// thread without corrupting the origin thread's context stack.
+TEST_F(SpanGuardScopeTest, detach_in_place_shared_detaches_active_guard)
+{
+    // Scoped shared guard is active on this (origin) thread.
+    auto span =
+        std::make_shared<SpanGuard>(SpanGuard::span(TraceCategory::Ledger, "ledger", "build"));
+    ASSERT_NE(span, nullptr);
+    ASSERT_TRUE(static_cast<bool>(*span));
+
+    // Strip the thread-local Scope here on the origin thread via the helper.
+    span = detachInPlace(std::move(span));
+    ASSERT_NE(span, nullptr);
+    ASSERT_TRUE(static_cast<bool>(*span));
+
+    // End the detached span on a worker thread.
+    std::thread worker([d = std::move(span)]() mutable {});
+    worker.join();
+
+    // Back on the origin thread: a new span must be a fresh root, proving the
+    // origin stack top was restored by detachInPlace() (not left holding
+    // ledger.build).
+    {
+        auto after = SpanGuard::span(TraceCategory::Rpc, "rpc", "command");
+        ASSERT_TRUE(static_cast<bool>(after));
+    }
+
+    auto spans = spanData()->GetSpans();
+    auto* build = findSpan(spans, "ledger.build");
+    auto* after = findSpan(spans, "rpc.command");
+    ASSERT_NE(build, nullptr);
+    ASSERT_NE(after, nullptr);
+
+    // 'after' is a new root: zero parent and a different trace than
+    // ledger.build.
+    EXPECT_FALSE(after->GetParentSpanId().IsValid());
+    EXPECT_NE(after->GetTraceId(), build->GetTraceId());
+}
+
+// detachInPlace(shared_ptr) on a null pointer is a no-op: it must not crash and
+// must return null unchanged.
+TEST_F(SpanGuardScopeTest, detach_in_place_shared_noop_on_null)
+{
+    auto result = detachInPlace(std::shared_ptr<SpanGuard>{});
+    EXPECT_EQ(result, nullptr);
+}
+
 }  // namespace
 }  // namespace xrpl::telemetry
 

src/xrpld/app/consensus/RCLConsensus.cpp

@@ -541,8 +541,8 @@ RCLConsensus::Adaptor::makeAcceptSpan(Result const& result)
         // above, so it is not popped on the wrong thread. accept.apply parents
         // via acceptSpanContext_ (captured above), so no ambient child is
         // orphaned on either the sync (onForceAccept) or async (onAccept) path.
-        // Rebuild the shared_ptr from the detached rvalue (move-assign deleted).
-        span = std::make_shared<telemetry::SpanGuard>(std::move(*span).detached());
+        // detachInPlace rebuilds the shared_ptr (move-assign is deleted).
+        span = telemetry::detachInPlace(std::move(span));
     }
     return span;
 }
@@ -1342,9 +1342,9 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
     // roundSpanContext_ (captured above) is the durable handle that child
     // spans on other threads link to. The guard itself is reset() on a
     // different worker than it was emplaced on, so detach its Scope now --
-    // AFTER the context capture -- to avoid a wrong-thread Scope pop. Re-emplace
-    // the detached guard in place (SpanGuard move-assignment is deleted).
-    roundSpan_.emplace(std::move(*roundSpan_).detached());
+    // AFTER the context capture -- to avoid a wrong-thread Scope pop.
+    // detachInPlace re-emplaces it (move-assignment is deleted).
+    telemetry::detachInPlace(roundSpan_);
 }
 
 std::optional<telemetry::SpanGuard>

src/xrpld/consensus/Consensus.h

@@ -2116,7 +2116,7 @@ Consensus<Adaptor>::startEstablishTracing()
     if (*establishSpan_)
     {
         establishSpanContext_ = establishSpan_->captureContext();
-        establishSpan_.emplace(std::move(*establishSpan_).detached());
+        telemetry::detachInPlace(establishSpan_);
     }
 }