Make TraceContextExt::span() return an Option

open-telemetry · Jun 1, 2023 · 6929175 · 6929175
1 parent 8935df0
commit 6929175
Show file tree

Hide file tree

Showing 28 changed files with 228 additions and 136 deletions.
diff --git a/examples/actix-http/src/main.rs b/examples/actix-http/src/main.rs
@@ -28,7 +28,9 @@ fn init_tracer() -> Result<sdktrace::Tracer, TraceError> {
 async fn index() -> &'static str {
     let tracer = global::tracer("request");
     tracer.in_span("index", |ctx| {
-        ctx.span().set_attribute(Key::new("parameter").i64(10));
+        if let Some(span) = ctx.span() {
+            span.set_attribute(Key::new("parameter").i64(10));
+        }
         "Index"
     })
 }
@@ -45,8 +47,9 @@ async fn main() -> std::io::Result<()> {
             .wrap(Logger::default())
             .wrap_fn(move |req, srv| {
                 tracer.in_span("middleware", move |cx| {
-                    cx.span()
-                        .set_attribute(Key::new("path").string(req.path().to_string()));
+                    if let Some(span) = cx.span() {
+                        span.set_attribute(Key::new("path").string(req.path().to_string()));
+                    }
                     srv.call(req).with_context(cx)
                 })
             })

diff --git a/examples/actix-hyper/src/main.rs b/examples/actix-hyper/src/main.rs
@@ -20,7 +20,9 @@ fn init_tracer() -> Result<sdktrace::Tracer, TraceError> {
 async fn index() -> &'static str {
     let tracer = global::tracer("request");
     tracer.in_span("index", |ctx| {
-        ctx.span().set_attribute(Key::new("parameter").i64(10));
+        if let Some(span) = ctx.span() {
+            span.set_attribute(Key::new("parameter").i64(10));
+        }
         "Index"
     })
 }
@@ -37,8 +39,10 @@ async fn main() -> std::io::Result<()> {
             .wrap(Logger::default())
             .wrap_fn(move |req, srv| {
                 tracer.in_span("middleware", move |cx| {
-                    cx.span()
-                        .set_attribute(Key::new("path").string(req.path().to_string()));
+                    if let Some(span) = cx.span() {
+                        span.set_attribute(Key::new("path").string(req.path().to_string()));
+                    }
+
                     srv.call(req).with_context(cx)
                 })
             })

diff --git a/examples/actix-udp/src/main.rs b/examples/actix-udp/src/main.rs
@@ -25,7 +25,10 @@ fn init_tracer() -> Result<sdktrace::Tracer, TraceError> {
 async fn index() -> &'static str {
     let tracer = global::tracer("request");
     tracer.in_span("index", |ctx| {
-        ctx.span().set_attribute(Key::new("parameter").i64(10));
+        if let Some(span) = ctx.span() {
+            span.set_attribute(Key::new("parameter").i64(10));
+        }
+
         "Index"
     })
 }
@@ -42,8 +45,10 @@ async fn main() -> std::io::Result<()> {
             .wrap_fn(|req, srv| {
                 let tracer = global::tracer("request");
                 tracer.in_span("middleware", move |cx| {
-                    cx.span()
-                        .set_attribute(Key::new("path").string(req.path().to_string()));
+                    if let Some(span) = cx.span() {
+                        span.set_attribute(Key::new("path").string(req.path().to_string()));
+                    };
+
                     srv.call(req).with_context(cx)
                 })
             })

diff --git a/examples/aws-xray/src/client.rs b/examples/aws-xray/src/client.rs
@@ -45,7 +45,8 @@ async fn main() -> std::result::Result<(), Box<dyn std::error::Error + Send + Sy
 
     let res = client.request(req.body(Body::from("Hallo!"))?).await?;
 
-    cx.span().add_event(
+    // `cx` initialized with span above, so unwrapping is safe
+    cx.span().unwrap().add_event(
         "Got response!".to_string(),
         vec![KeyValue::new("status", res.status().to_string())],
     );

diff --git a/examples/basic-otlp-http/src/main.rs b/examples/basic-otlp-http/src/main.rs
@@ -62,15 +62,23 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
     histogram.record(&cx, 5.5, COMMON_ATTRIBUTES.as_ref());
 
     tracer.in_span("operation", |cx| {
-        let span = cx.span();
+        let span = match cx.span() {
+            Some(span) => span,
+            None => return,
+        };
+
         span.add_event(
             "Nice operation!".to_string(),
             vec![Key::new("bogons").i64(100)],
         );
         span.set_attribute(ANOTHER_KEY.string("yes"));
 
         tracer.in_span("Sub operation...", |cx| {
-            let span = cx.span();
+            let span = match cx.span() {
+                Some(span) => span,
+                None => return,
+            };
+
             span.set_attribute(LEMONS_KEY.string("five"));
 
             span.add_event("Sub span event", vec![]);

diff --git a/examples/basic-otlp/src/main.rs b/examples/basic-otlp/src/main.rs
@@ -80,15 +80,23 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
     histogram.record(&cx, 5.5, COMMON_ATTRIBUTES.as_ref());
 
     tracer.in_span("operation", |cx| {
-        let span = cx.span();
+        let span = match cx.span() {
+            Some(span) => span,
+            None => return,
+        };
+
         span.add_event(
             "Nice operation!".to_string(),
             vec![Key::new("bogons").i64(100)],
         );
         span.set_attribute(ANOTHER_KEY.string("yes"));
 
         tracer.in_span("Sub operation...", |cx| {
-            let span = cx.span();
+            let span = match cx.span() {
+                Some(span) => span,
+                None => return,
+            };
+
             span.set_attribute(LEMONS_KEY.string("five"));
 
             span.add_event("Sub span event", vec![]);

diff --git a/examples/datadog/src/main.rs b/examples/datadog/src/main.rs
@@ -24,7 +24,11 @@ fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
         .install_simple()?;
 
     tracer.in_span("foo", |cx| {
-        let span = cx.span();
+        let span = match cx.span() {
+            Some(span) => span,
+            None => return,
+        };
+
         span.set_attribute(Key::new("span.type").string("web"));
         span.set_attribute(Key::new("http.url").string("http://localhost:8080/foo"));
         span.set_attribute(Key::new("http.method").string("GET"));

diff --git a/examples/external-otlp-tonic-tokio/src/main.rs b/examples/external-otlp-tonic-tokio/src/main.rs
@@ -85,15 +85,23 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
     let tracer = tracer("ex.com/basic");
 
     tracer.in_span("operation", |cx| {
-        let span = cx.span();
+        let span = match cx.span() {
+            Some(span) => span,
+            None => return,
+        };
+
         span.add_event(
             "Nice operation!".to_string(),
             vec![Key::new("bogons").i64(100)],
         );
         span.set_attribute(ANOTHER_KEY.string("yes"));
 
         tracer.in_span("Sub operation...", |cx| {
-            let span = cx.span();
+            let span = match cx.span() {
+                Some(span) => span,
+                None => return,
+            };
+
             span.set_attribute(LEMONS_KEY.string("five"));
 
             span.add_event("Sub span event", vec![]);

diff --git a/examples/grpc/src/client.rs b/examples/grpc/src/client.rs
@@ -52,7 +52,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>
 
     let response = client.say_hello(request).await?;
 
-    cx.span().add_event(
+    // `cx` initialized with span above, so unwrapping is safe
+    cx.span().unwrap().add_event(
         "response-received".to_string(),
         vec![KeyValue::new("response", format!("{response:?}"))],
     );

diff --git a/examples/http/src/client.rs b/examples/http/src/client.rs
@@ -35,7 +35,8 @@ async fn main() -> std::result::Result<(), Box<dyn std::error::Error + Send + Sy
     });
     let res = client.request(req.body(Body::from("Hallo!"))?).await?;
 
-    cx.span().add_event(
+    // `cx` initialized with span above, so unwrapping is safe
+    cx.span().unwrap().add_event(
         "Got response!".to_string(),
         vec![KeyValue::new("status", res.status().to_string())],
     );

diff --git a/examples/traceresponse/src/client.rs b/examples/traceresponse/src/client.rs
@@ -48,9 +48,13 @@ async fn main() -> std::result::Result<(), Box<dyn std::error::Error + Send + Sy
     let response_cx =
         response_propagator.extract_with_context(&cx, &HeaderExtractor(res.headers()));
 
-    let response_span = response_cx.span();
+    let response_span = match response_cx.span() {
+        Some(response_span) => response_span,
+        None => return Ok(()),
+    };
 
-    cx.span().add_event(
+    // `cx` initialized with span above, so unwrapping is safe
+    cx.span().unwrap().add_event(
         "Got response!".to_string(),
         vec![
             KeyValue::new("status", res.status().to_string()),

diff --git a/examples/traceresponse/src/server.rs b/examples/traceresponse/src/server.rs
@@ -24,7 +24,7 @@ async fn handle(req: Request<Body>) -> Result<Response<Body>, Infallible> {
 
     let cx = Context::current_with_span(span);
 
-    cx.span().add_event("handling this...", Vec::new());
+    cx.span().unwrap().add_event("handling this...", Vec::new());
 
     let mut res = Response::new("Hello, World!".into());
 

diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Fixed
 
+- Change `TraceContextExt::span()` to yield an `Option` and remove `has_active_span()`. [#1089](https://github.com/open-telemetry/opentelemetry-rust/pull/1089)
 - Fix `SpanRef::set_attributes` mutability requirement. [#1038](https://github.com/open-telemetry/opentelemetry-rust/pull/1038)
 - Move OrderMap module to root of otel-api crate. [#1061](https://github.com/open-telemetry/opentelemetry-rust/pull/1061)
 

diff --git a/opentelemetry-api/src/logs/record.rs b/opentelemetry-api/src/logs/record.rs
@@ -286,8 +286,8 @@ impl LogRecordBuilder {
     where
         T: TraceContextExt,
     {
-        if context.has_active_span() {
-            self.with_span_context(context.span().span_context())
+        if let Some(span) = context.span() {
+            self.with_span_context(span.span_context())
         } else {
             self
         }

diff --git a/opentelemetry-api/src/trace/context.rs b/opentelemetry-api/src/trace/context.rs
@@ -5,7 +5,6 @@ use crate::{
     Context, ContextGuard, KeyValue,
 };
 use futures_util::{sink::Sink, stream::Stream};
-use once_cell::sync::Lazy;
 use pin_project_lite::pin_project;
 use std::{
     borrow::Cow,
@@ -15,11 +14,6 @@ use std::{
     task::{Context as TaskContext, Poll},
 };
 
-static NOOP_SPAN: Lazy<SynchronizedSpan> = Lazy::new(|| SynchronizedSpan {
-    span_context: SpanContext::empty_context(),
-    inner: None,
-});
-
 /// A reference to the currently active span in this context.
 #[derive(Debug)]
 pub struct SpanRef<'a>(&'a SynchronizedSpan);
@@ -220,29 +214,19 @@ pub trait TraceContextExt {
     ///
     fn with_span<T: crate::trace::Span + Send + Sync + 'static>(&self, span: T) -> Self;
 
-    /// Returns a reference to this context's span, or the default no-op span if
-    /// none has been set.
+    /// Returns a reference to this context's span if it has been set.
     ///
     /// # Examples
     ///
     /// ```
     /// use opentelemetry_api::{trace::TraceContextExt, Context};
     ///
     /// // Add an event to the currently active span
-    /// Context::current().span().add_event("An event!", vec![]);
-    /// ```
-    fn span(&self) -> SpanRef<'_>;
-
-    /// Returns whether or not an active span has been set.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use opentelemetry_api::{trace::TraceContextExt, Context};
-    ///
-    /// assert!(!Context::current().has_active_span());
+    /// if let Some(span) = Context::current().span() {
+    ///     span.add_event("An event!", vec![]);
+    /// }
     /// ```
-    fn has_active_span(&self) -> bool;
+    fn span(&self) -> Option<SpanRef<'_>>;
 
     /// Returns a copy of this context with the span context included.
     ///
@@ -265,16 +249,8 @@ impl TraceContextExt for Context {
         })
     }
 
-    fn span(&self) -> SpanRef<'_> {
-        if let Some(span) = self.get::<SynchronizedSpan>() {
-            SpanRef(span)
-        } else {
-            SpanRef(&NOOP_SPAN)
-        }
-    }
-
-    fn has_active_span(&self) -> bool {
-        self.get::<SynchronizedSpan>().is_some()
+    fn span(&self) -> Option<SpanRef<'_>> {
+        self.get::<SynchronizedSpan>().map(SpanRef)
     }
 
     fn with_remote_span_context(&self, span_context: crate::trace::SpanContext) -> Self {
@@ -323,6 +299,9 @@ pub fn mark_span_as_active<T: crate::trace::Span + Send + Sync + 'static>(span:
 
 /// Executes a closure with a reference to this thread's current span.
 ///
+/// If the current thread has an active span, calls the closure `f` on it and yields `Some` with
+/// the result of the closure. Otherwise, yields `None` without calling `f`.
+///
 /// # Examples
 ///
 /// ```
@@ -344,11 +323,11 @@ pub fn mark_span_as_active<T: crate::trace::Span + Send + Sync + 'static>(span:
 ///     })
 /// }
 /// ```
-pub fn get_active_span<F, T>(f: F) -> T
+pub fn get_active_span<F, T>(f: F) -> Option<T>
 where
     F: FnOnce(SpanRef<'_>) -> T,
 {
-    f(Context::current().span())
+    Context::current().span().map(f)
 }
 
 pin_project! {

diff --git a/opentelemetry-api/src/trace/noop.rs b/opentelemetry-api/src/trace/noop.rs
@@ -143,12 +143,12 @@ impl trace::Tracer for NoopTracer {
     /// If the span builder or the context's current span contains a valid span context, it is
     /// propagated.
     fn build_with_context(&self, _builder: trace::SpanBuilder, parent_cx: &Context) -> Self::Span {
-        if parent_cx.has_active_span() {
+        if let Some(span) = parent_cx.span() {
             NoopSpan {
-                span_context: parent_cx.span().span_context().clone(),
+                span_context: span.span_context().clone(),
             }
         } else {
-            NoopSpan::new()
+            NoopSpan::default()
         }
     }
 }

diff --git a/opentelemetry-aws/src/lib.rs b/opentelemetry-aws/src/lib.rs
@@ -211,7 +211,11 @@ pub mod trace {
 
     impl TextMapPropagator for XrayPropagator {
         fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) {
-            let span = cx.span();
+            let span = match cx.span() {
+                Some(span) => span,
+                None => return,
+            };
+
             let span_context = span.span_context();
             if let Some(header_value) = span_context_to_string(span_context) {
                 injector.set(AWS_XRAY_TRACE_HEADER, header_value);
@@ -352,7 +356,7 @@ pub mod trace {
 
                 let propagator = XrayPropagator::default();
                 let context = propagator.extract(&map);
-                assert_eq!(context.span().span_context(), &expected);
+                assert_eq!(context.span().unwrap().span_context(), &expected);
             }
         }
 
@@ -361,7 +365,10 @@ pub mod trace {
             let map: HashMap<String, String> = HashMap::new();
             let propagator = XrayPropagator::default();
             let context = propagator.extract(&map);
-            assert_eq!(context.span().span_context(), &SpanContext::empty_context())
+            assert_eq!(
+                context.span().unwrap().span_context(),
+                &SpanContext::empty_context()
+            )
         }
 
         #[test]