perf(context): move active span to flag and make invalid spans none

Author: bantonsson

opentelemetry/src/context.rs

@@ -425,47 +425,44 @@ impl Context {
     }
 
     #[cfg(feature = "trace")]
-    pub(crate) fn current_with_synchronized_span(value: SynchronizedSpan) -> Self {
-        Self::map_current(|cx| {
-            if let Some(inner) = &cx.inner {
+    pub(crate) fn current_with_synchronized_span(span: Option<Arc<SynchronizedSpan>>) -> Self {
+        Self::map_current(|cx| cx.with_synchronized_span(span))
+    }
+
+    #[cfg(feature = "trace")]
+    pub(crate) fn with_synchronized_span(&self, span: Option<Arc<SynchronizedSpan>>) -> Self {
+        let active = span.is_some();
+        if let Some(inner) = &self.inner {
+            if span.is_none() && inner.span.is_none() {
+                self.clone()
+            } else {
                 Context {
                     inner: Some(Arc::new(InnerContext {
-                        span: Some(Arc::new(value)),
+                        span,
                         entries: inner.entries.clone(),
                     })),
-                    flags: cx.flags,
+                    flags: self.flags.with_active_span(active),
                 }
+            }
+        } else {
+            if span.is_none() {
+                self.clone()
             } else {
                 Context {
                     inner: Some(Arc::new(InnerContext {
-                        span: Some(Arc::new(value)),
+                        span,
                         entries: None,
                     })),
-                    flags: ContextFlags::new(),
+                    flags: self.flags.with_active_span(active),
                 }
             }
-        })
+        }
     }
 
     #[cfg(feature = "trace")]
-    pub(crate) fn with_synchronized_span(&self, value: SynchronizedSpan) -> Self {
-        if let Some(inner) = &self.inner {
-            Context {
-                inner: Some(Arc::new(InnerContext {
-                    span: Some(Arc::new(value)),
-                    entries: inner.entries.clone(),
-                })),
-                flags: self.flags,
-            }
-        } else {
-            Context {
-                inner: Some(Arc::new(InnerContext {
-                    span: Some(Arc::new(value)),
-                    entries: None,
-                })),
-                flags: ContextFlags::new(),
-            }
-        }
+    #[inline(always)]
+    pub(crate) const fn has_active_span_flag(&self) -> bool {
+        self.flags.is_span_active()
     }
 }
 
@@ -505,12 +502,8 @@ struct ContextFlags(u16);
 
 impl ContextFlags {
     const SUPPRESS_TELEMETRY: u16 = 1 << 0;
-
-    /// Creates a new ContextFlags with all flags cleared.
-    #[inline(always)]
-    const fn new() -> Self {
-        ContextFlags(0)
-    }
+    #[cfg(feature = "trace")]
+    const ACTIVE_SPAN: u16 = 1 << 1;
 
     /// Returns true if telemetry suppression is enabled.
     #[inline(always)]
@@ -523,13 +516,44 @@ impl ContextFlags {
     const fn with_telemetry_suppressed(self) -> Self {
         ContextFlags(self.0 | Self::SUPPRESS_TELEMETRY)
     }
+
+    /// Returns true if the active span flag is set.
+    #[cfg(feature = "trace")]
+    #[inline(always)]
+    const fn is_span_active(self) -> bool {
+        (self.0 & Self::ACTIVE_SPAN) != 0
+    }
+
+    /// Returns a new ContextFlags with the active span flag set.
+    #[cfg(feature = "trace")]
+    #[inline(always)]
+    const fn with_active_span(self, active: bool) -> Self {
+        if active {
+            ContextFlags(self.0 | Self::ACTIVE_SPAN)
+        } else {
+            ContextFlags(self.0 & !Self::ACTIVE_SPAN)
+        }
+    }
 }
 
 impl fmt::Debug for ContextFlags {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        #[cfg(feature = "trace")]
+        let mut comma = false;
         f.write_str("ContextFlags(")?;
         if self.is_telemetry_suppressed() {
             f.write_str("TELEMETRY_SUPPRESSED")?;
+            #[cfg(feature = "trace")]
+            {
+                comma = true;
+            }
+        }
+        #[cfg(feature = "trace")]
+        if self.is_span_active() {
+            if comma {
+                f.write_str(", ")?;
+            }
+            f.write_str("ACTIVE_SPAN")?;
         }
         f.write_str(")")
     }

opentelemetry/src/trace/context.rs

@@ -4,7 +4,11 @@ use crate::{
     trace::{Span, SpanContext, Status},
     Context, ContextGuard, KeyValue,
 };
-use std::{borrow::Cow, error::Error, sync::Mutex};
+use std::{
+    borrow::Cow,
+    error::Error,
+    sync::{Arc, Mutex},
+};
 
 // Re-export for compatability. This used to be contained here.
 pub use crate::context::{FutureExt, WithContext};
@@ -30,22 +34,29 @@ impl SynchronizedSpan {
     pub(crate) fn span_context(&self) -> &SpanContext {
         &self.span_context
     }
-}
 
-impl From<SpanContext> for SynchronizedSpan {
-    fn from(value: SpanContext) -> Self {
-        Self {
-            span_context: value,
-            inner: None,
+    fn maybe_from_span<T>(value: T) -> Option<Arc<SynchronizedSpan>>
+    where
+        T: Span + Send + Sync + 'static,
+    {
+        if !value.span_context().is_valid() {
+            None
+        } else {
+            Some(Arc::new(SynchronizedSpan {
+                span_context: value.span_context().clone(),
+                inner: Some(Mutex::new(global::BoxedSpan::new(value))),
+            }))
         }
     }
-}
 
-impl<T: Span + Send + Sync + 'static> From<T> for SynchronizedSpan {
-    fn from(value: T) -> Self {
-        Self {
-            span_context: value.span_context().clone(),
-            inner: Some(Mutex::new(global::BoxedSpan::new(value))),
+    fn maybe_from_span_context(value: SpanContext) -> Option<Arc<SynchronizedSpan>> {
+        if !value.is_valid() {
+            None
+        } else {
+            Some(Arc::new(SynchronizedSpan {
+                span_context: value,
+                inner: None,
+            }))
         }
     }
 }
@@ -300,11 +311,11 @@ pub trait TraceContextExt {
 
 impl TraceContextExt for Context {
     fn current_with_span<T: crate::trace::Span + Send + Sync + 'static>(span: T) -> Self {
-        Context::current_with_synchronized_span(span.into())
+        Context::current_with_synchronized_span(SynchronizedSpan::maybe_from_span(span))
     }
 
     fn with_span<T: crate::trace::Span + Send + Sync + 'static>(&self, span: T) -> Self {
-        self.with_synchronized_span(span.into())
+        self.with_synchronized_span(SynchronizedSpan::maybe_from_span(span))
     }
 
     fn span(&self) -> SpanRef<'_> {
@@ -316,11 +327,11 @@ impl TraceContextExt for Context {
     }
 
     fn has_active_span(&self) -> bool {
-        self.inner.as_ref().map_or(false, |i| i.span.is_some())
+        self.has_active_span_flag()
     }
 
     fn with_remote_span_context(&self, span_context: crate::trace::SpanContext) -> Self {
-        self.with_synchronized_span(span_context.into())
+        self.with_synchronized_span(SynchronizedSpan::maybe_from_span_context(span_context))
     }
 }
 

opentelemetry/src/trace/span_context.rs

@@ -446,18 +446,27 @@ mod tests {
             format!("{cx:?}"),
             "Context { span: \"None\", entries count: 0, flags: ContextFlags() }"
         );
-        let cx = Context::current().with_remote_span_context(SpanContext::NONE).with_telemetry_suppressed();
+        let remote_span_cx = SpanContext::new(
+            0x123456789abcdef0123456789abcdef0.into(),
+            0x876543210fedcba0.into(),
+            TraceFlags::NOT_SAMPLED,
+            false,
+            TraceState::NONE,
+        );
+        let cx = Context::current()
+            .with_remote_span_context(remote_span_cx)
+            .with_telemetry_suppressed();
         assert_eq!(
             format!("{cx:?}"),
             "Context { \
                span: SpanContext { \
-                       trace_id: 00000000000000000000000000000000, \
-                       span_id: 0000000000000000, \
+                       trace_id: 123456789abcdef0123456789abcdef0, \
+                       span_id: 876543210fedcba0, \
                        trace_flags: TraceFlags(0), \
                        is_remote: false, \
                        trace_state: TraceState(None) \
                      }, \
-               entries count: 1, flags: ContextFlags(TELEMETRY_SUPPRESSED) \
+               entries count: 1, flags: ContextFlags(TELEMETRY_SUPPRESSED, ACTIVE_SPAN) \
              }"
         );
     }