diff --git a/include/xrpl/telemetry/DiscardFlag.h b/include/xrpl/telemetry/DiscardFlag.h index e1c81c3319..05ba047b9c 100644 --- a/include/xrpl/telemetry/DiscardFlag.h +++ b/include/xrpl/telemetry/DiscardFlag.h @@ -1,25 +1,86 @@ #pragma once -/** Thread-local flag for span discard signaling. +/** Thread-local discard signaling between SpanGuard and the span processor. - 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. + SpanGuard::discard() wants to drop a span without sending it to the + exporter. The OTel SDK calls SpanProcessor::OnEnd() synchronously on the + same thread that calls Span::End(), so a thread-local flag set just before + End() and read inside OnEnd() lets FilteringSpanProcessor drop the span + before it enters the batch export queue. - This side-channel avoids inspecting the Recordable's internals (which - vary by exporter type — SpanData vs OtlpRecordable). + This side-channel avoids inspecting the Recordable's internals (which vary + by exporter type — SpanData vs OtlpRecordable). + + The raw flag lives in `detail` and is mutated only through DiscardScope, a + RAII guard that sets it on construction and clears it on destruction. This + keeps the set/clear lifetime bound to a scope (so the flag cannot leak onto + the next span even if End() were to throw) and prevents any class which includes this header + from flipping the flag directly. FilteringSpanProcessor reads it through + isDiscardingCurrentSpan(). Kept in a separate header to avoid transitive include bloat: SpanGuard.h - only needs this flag, not the full Telemetry.h with BasicConfig/Journal. + only needs this signaling, not the full Telemetry.h with BasicConfig/Journal. + + Usage: + @code + // In SpanGuard::discard(): + { + DiscardScope discardScope; // flag set for this scope only + span->End(); // OnEnd() runs synchronously, sees flag + } // flag cleared here, unconditionally + @endcode + + @note Thread safety: the flag is thread-local, so each thread observes only + its own discard signal — no synchronization is required. @see SpanGuard::discard(), FilteringSpanProcessor (Telemetry.cpp) */ namespace xrpl::telemetry { -/** When true, the FilteringSpanProcessor drops the current span in - OnEnd(). Set by SpanGuard::discard(), cleared by OnEnd(). */ +namespace detail { + +/** Internal thread-local discard flag. Mutate only via DiscardScope; read + only via isDiscardingCurrentSpan(). Not intended for direct use. */ inline thread_local bool gTlDiscardCurrentSpan = false; +} // namespace detail + +/** RAII guard that marks the current thread's span for discard. + + Sets the thread-local discard flag on construction and clears it on + destruction, so a span ended within the guard's scope is dropped by + FilteringSpanProcessor::OnEnd() while the flag stays confined to that scope. + Non-copyable and non-movable — its sole purpose is the scoped flag lifetime. +*/ +class DiscardScope +{ +public: + DiscardScope() noexcept + { + detail::gTlDiscardCurrentSpan = true; + } + + ~DiscardScope() + { + detail::gTlDiscardCurrentSpan = false; + } + + DiscardScope(DiscardScope const&) = delete; + DiscardScope& + operator=(DiscardScope const&) = delete; + DiscardScope(DiscardScope&&) = delete; + DiscardScope& + operator=(DiscardScope&&) = delete; +}; + +/** @return true if the current thread is inside a DiscardScope, i.e. the span + ending now should be dropped rather than exported. Read by + FilteringSpanProcessor::OnEnd(). */ +[[nodiscard]] inline bool +isDiscardingCurrentSpan() noexcept +{ + return detail::gTlDiscardCurrentSpan; +} + } // namespace xrpl::telemetry diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index c59f96ac4f..0da37dbfe1 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -88,6 +88,13 @@ namespace xrpl::telemetry { +#ifdef XRPL_ENABLE_TELEMETRY +/** OTel instrumentation scope (tracer) name. Identifies this library as the + source of spans; distinct from the `service.name` resource attribute + (Setup::serviceName), which is config-overridable. */ +inline constexpr std::string_view kTracerName{"xrpld"}; +#endif + class Telemetry { /** Global singleton pointer, set by start()/stop() in the active @@ -165,7 +172,7 @@ public: std::uint32_t batchSize = 512; /** Delay between batch exports. */ - std::chrono::milliseconds batchDelay{5000}; + std::chrono::milliseconds batchDelay = std::chrono::milliseconds{5000}; /** Maximum number of spans queued before dropping. */ std::uint32_t maxQueueSize = 2048; @@ -255,7 +262,7 @@ public: @return A shared pointer to the Tracer. */ virtual opentelemetry::nostd::shared_ptr - getTracer(std::string_view name = "xrpld") = 0; + getTracer(std::string_view name = kTracerName) = 0; /** Start a new span on the current thread's context. diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index 41d7aed635..fc824c2bb8 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -41,7 +41,7 @@ class NullTelemetry : public Telemetry Setup const setup_; public: - explicit NullTelemetry(Setup setup) : setup_(std::move(setup)) + explicit NullTelemetry(Setup setup) : setup_{std::move(setup)} { } diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index 87739ae7b8..10778a0a7a 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ #include #include +#include #include #include #include @@ -138,6 +140,13 @@ isCategoryEnabled(Telemetry const& tel, TraceCategory cat) namespace { +// Span-link attribute marking a "follows-from" (causal, non-parent) link, +// emitted by both linkedSpan() overloads. Custom xrpl attribute — not part +// of the OTel semantic conventions, so defined here rather than pulled from +// . +constexpr char const* kLinkTypeKey = "xrpl.link.type"; +constexpr char const* kLinkTypeFollowsFrom = "follows_from"; + // Map a TraceCategory to an OTel SpanKind so Tempo's service-graph / // RED metrics see the correct direction. RPC spans are emitted at the // server entry point (handler dispatch), Peer spans at inbound-message @@ -168,7 +177,7 @@ SpanGuard::span(TraceCategory cat, std::string_view prefix, std::string_view nam auto* tel = Telemetry::getInstance(); if ((tel == nullptr) || !tel->isEnabled() || !isCategoryEnabled(*tel, cat)) return {}; - auto fullName = std::string(prefix) + "." + std::string(name); + auto fullName = std::format("{}.{}", prefix, name); return SpanGuard(std::make_unique(tel->startSpan(fullName, categoryToSpanKind(cat)))); } @@ -206,19 +215,17 @@ SpanGuard::linkedSpan(std::string_view name) const if ((tel == nullptr) || !tel->isEnabled()) return {}; - auto tracer = tel->getTracer("xrpld"); + auto tracer = tel->getTracer(); auto spanCtx = impl_->span->GetContext(); // Mark as root span so it starts a new trace sub-tree rather than // inheriting the current thread's active span as parent. otel_trace::StartSpanOptions opts; - opentelemetry::context::Context rootCtx; - rootCtx = rootCtx.SetValue(otel_trace::kIsRootSpanKey, true); - opts.parent = rootCtx; + opts.parent = opentelemetry::context::Context{otel_trace::kIsRootSpanKey, true}; return SpanGuard( std::make_unique(tracer->StartSpan( - std::string(name), {}, {{spanCtx, {{"xrpl.link.type", "follows_from"}}}}, opts))); + std::string(name), {}, {{spanCtx, {{kLinkTypeKey, kLinkTypeFollowsFrom}}}}, opts))); } SpanGuard @@ -230,7 +237,7 @@ SpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx) if ((tel == nullptr) || !tel->isEnabled()) return {}; - auto tracer = tel->getTracer("xrpld"); + auto tracer = tel->getTracer(); // Extract the span from the captured context to get its SpanContext. auto linkSpan = otel_trace::GetSpan(linkCtx.impl_->ctx); @@ -240,15 +247,13 @@ SpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx) // Mark as root span so it starts a new trace sub-tree rather than // inheriting the current thread's active span as parent. otel_trace::StartSpanOptions opts; - opentelemetry::context::Context rootCtx; - rootCtx = rootCtx.SetValue(otel_trace::kIsRootSpanKey, true); - opts.parent = rootCtx; + opts.parent = opentelemetry::context::Context{otel_trace::kIsRootSpanKey, true}; return SpanGuard( std::make_unique(tracer->StartSpan( std::string(name), {}, - {{linkSpan->GetContext(), {{"xrpl.link.type", "follows_from"}}}}, + {{linkSpan->GetContext(), {{kLinkTypeKey, kLinkTypeFollowsFrom}}}}, opts))); } @@ -260,7 +265,7 @@ SpanGuard::captureContext() const if (!impl_) return {}; auto ctx = opentelemetry::context::RuntimeContext::GetCurrent(); - return SpanContext(std::make_shared(ctx)); + return SpanContext(std::make_shared(std::move(ctx))); } // ===== Attribute setters =================================================== @@ -331,9 +336,13 @@ SpanGuard::recordException(std::exception const& e) { if (!impl_) return; + namespace semconv_exc = opentelemetry::semconv::exception; + // Event name "exception" and the attribute keys follow the OTel semantic + // conventions; the keys come from semconv constants rather than literals. impl_->span->AddEvent( "exception", - {{"exception.type", typeid(e).name()}, {"exception.message", std::string(e.what())}}); + {{semconv_exc::kExceptionType, typeid(e).name()}, + {semconv_exc::kExceptionMessage, std::string(e.what())}}); impl_->span->SetStatus(otel_trace::StatusCode::kError, e.what()); } @@ -342,16 +351,19 @@ SpanGuard::discard() { if (impl_) { - gTlDiscardCurrentSpan = true; - impl_->span->End(); - // Clear here so discard() owns the flag's whole lifetime - // (set -> End -> clear) in one scope, rather than relying on - // FilteringSpanProcessor::OnEnd() to clear it. Today every valid guard - // wraps a recording span (head sampling is 1.0), so OnEnd() always runs - // and clearing here is equivalent — but colocating set and clear keeps - // the flag leak-proof if a later phase can hand back a non-recording - // span (e.g. honoring a non-sampled remote parent during propagation). - gTlDiscardCurrentSpan = false; + { + // DiscardScope owns the flag's whole lifetime: it sets the flag, + // and clears it on scope exit — even if End() were to throw. The + // SDK invokes FilteringSpanProcessor::OnEnd() synchronously from + // End() on this thread, so the flag is observed while still set. + // Today every valid guard wraps a recording span (head sampling is + // 1.0), so OnEnd() always runs — but scoping set/clear keeps the + // flag leak-proof if a later phase can hand back a non-recording + // span (e.g. honoring a non-sampled remote parent during + // propagation), so it can never spill onto the next span. + DiscardScope discardScope; + impl_->span->End(); + } impl_->span = nullptr; // prevent ~Impl from calling End() again impl_.reset(); } diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 9e55a40f4e..259e0febfc 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -62,9 +62,10 @@ namespace resource = opentelemetry::sdk::resource; /** SpanProcessor decorator that drops discarded spans. Wraps a delegate processor (typically BatchSpanProcessor). In OnEnd(), - 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. + calls isDiscardingCurrentSpan(). If the calling thread is inside a + DiscardScope (entered by SpanGuard::discard()), the span is silently + dropped — never entering the batch queue, never sent over the network, + never stored. Uses a thread-local flag rather than inspecting Recordable attributes because the Recordable type varies by exporter (SpanData for simple @@ -88,7 +89,7 @@ namespace resource = opentelemetry::sdk::resource; +---------------------+ @note Thread safety: OnEnd() may be called concurrently from multiple - threads. The gTlDiscardCurrentSpan flag is thread-local, so each + threads. The discard flag behind DiscardScope is thread-local, so each thread's discard state is independent — no synchronization needed. */ class FilteringSpanProcessor : public trace_sdk::SpanProcessor @@ -118,11 +119,11 @@ public: void OnEnd(std::unique_ptr&& span) noexcept override { - if (gTlDiscardCurrentSpan) + if (isDiscardingCurrentSpan()) { - // SpanGuard::discard() set the flag on this thread just before - // calling Span::End(), which invokes OnEnd() synchronously. - // Drop the span. + // SpanGuard::discard() is inside a DiscardScope on this thread, + // which it entered just before calling Span::End() — and End() + // invokes OnEnd() synchronously. Drop the span. return; } delegate_->OnEnd(std::move(span)); @@ -389,7 +390,7 @@ public: } opentelemetry::nostd::shared_ptr - getTracer(std::string_view name) override + getTracer(std::string_view name = kTracerName) override { if (!sdkProvider_) return trace_api::Provider::GetTracerProvider()->GetTracer(std::string(name)); @@ -399,7 +400,7 @@ public: opentelemetry::nostd::shared_ptr startSpan(std::string_view name, trace_api::SpanKind kind) override { - auto tracer = getTracer("xrpld"); + auto tracer = getTracer(); trace_api::StartSpanOptions opts; opts.kind = kind; return tracer->StartSpan(std::string(name), opts); @@ -411,7 +412,7 @@ public: opentelemetry::context::Context const& parentContext, trace_api::SpanKind kind) override { - auto tracer = getTracer("xrpld"); + auto tracer = getTracer(); trace_api::StartSpanOptions opts; opts.kind = kind; opts.parent = parentContext;