From 6929175feaa53c19bcece862f63ee66b5b561164 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 1 Jun 2023 10:47:53 +0200 Subject: [PATCH] Make TraceContextExt::span() return an Option --- examples/actix-http/src/main.rs | 9 ++-- examples/actix-hyper/src/main.rs | 10 +++-- examples/actix-udp/src/main.rs | 11 +++-- examples/aws-xray/src/client.rs | 3 +- examples/basic-otlp-http/src/main.rs | 12 ++++- examples/basic-otlp/src/main.rs | 12 ++++- examples/datadog/src/main.rs | 6 ++- .../external-otlp-tonic-tokio/src/main.rs | 12 ++++- examples/grpc/src/client.rs | 3 +- examples/http/src/client.rs | 3 +- examples/traceresponse/src/client.rs | 8 +++- examples/traceresponse/src/server.rs | 2 +- opentelemetry-api/CHANGELOG.md | 1 + opentelemetry-api/src/logs/record.rs | 4 +- opentelemetry-api/src/trace/context.rs | 45 +++++-------------- opentelemetry-api/src/trace/noop.rs | 6 +-- opentelemetry-aws/src/lib.rs | 13 ++++-- .../propagator/trace_context_response.rs | 18 ++++++-- opentelemetry-datadog/src/lib.rs | 13 ++++-- opentelemetry-jaeger/src/lib.rs | 27 ++++++++--- .../tests/integration_test.rs | 16 ++++--- opentelemetry-sdk/src/logs/log_emitter.rs | 3 +- .../src/propagation/composite.rs | 8 +++- .../src/propagation/trace_context.rs | 19 ++++++-- opentelemetry-sdk/src/trace/sampler.rs | 45 ++++++++----------- .../jaeger_remote/sampling_strategy.rs | 4 +- opentelemetry-sdk/src/trace/tracer.rs | 38 ++++++++-------- opentelemetry-zipkin/src/propagator/mod.rs | 13 +++++- 28 files changed, 228 insertions(+), 136 deletions(-) diff --git a/examples/actix-http/src/main.rs b/examples/actix-http/src/main.rs index e9af38a125..58e77b218d 100644 --- a/examples/actix-http/src/main.rs +++ b/examples/actix-http/src/main.rs @@ -28,7 +28,9 @@ fn init_tracer() -> Result { 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 index efafe38d8c..c7bad2b04e 100644 --- a/examples/actix-hyper/src/main.rs +++ b/examples/actix-hyper/src/main.rs @@ -20,7 +20,9 @@ fn init_tracer() -> Result { 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 index 2a741b92a1..e83cd3c939 100644 --- a/examples/actix-udp/src/main.rs +++ b/examples/actix-udp/src/main.rs @@ -25,7 +25,10 @@ fn init_tracer() -> Result { 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 index 08d66858a8..1139da9869 100644 --- 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 Result<(), Box> { 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)], @@ -70,7 +74,11 @@ async fn main() -> Result<(), Box> { 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 index dc15d7cce6..0d47b4d792 100644 --- a/examples/basic-otlp/src/main.rs +++ b/examples/basic-otlp/src/main.rs @@ -80,7 +80,11 @@ async fn main() -> Result<(), Box> { 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)], @@ -88,7 +92,11 @@ async fn main() -> Result<(), Box> { 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 index 5cb1400c5e..e4a9d9a2a4 100644 --- a/examples/datadog/src/main.rs +++ b/examples/datadog/src/main.rs @@ -24,7 +24,11 @@ fn main() -> Result<(), Box> { .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 index d8e29f075b..622562ed03 100644 --- a/examples/external-otlp-tonic-tokio/src/main.rs +++ b/examples/external-otlp-tonic-tokio/src/main.rs @@ -85,7 +85,11 @@ async fn main() -> Result<(), Box> { 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)], @@ -93,7 +97,11 @@ async fn main() -> Result<(), Box> { 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 index a51173a0de..b7760bb07d 100644 --- a/examples/grpc/src/client.rs +++ b/examples/grpc/src/client.rs @@ -52,7 +52,8 @@ async fn main() -> Result<(), Box 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 index 1e16dab77d..ce554de687 100644 --- a/examples/http/src/client.rs +++ b/examples/http/src/client.rs @@ -35,7 +35,8 @@ async fn main() -> std::result::Result<(), Box std::result::Result<(), Box 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 index 275b52d9db..093992f030 100644 --- a/examples/traceresponse/src/server.rs +++ b/examples/traceresponse/src/server.rs @@ -24,7 +24,7 @@ async fn handle(req: Request) -> Result, 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 index 28868bc13e..d874c0c827 100644 --- 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 index 6cd47c25bb..97076e1db1 100644 --- 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 index 62c75234ec..84f6f90c5c 100644 --- 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 = 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,8 +214,7 @@ pub trait TraceContextExt { /// fn with_span(&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 /// @@ -229,20 +222,11 @@ pub trait TraceContextExt { /// 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>; /// 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::() { - SpanRef(span) - } else { - SpanRef(&NOOP_SPAN) - } - } - - fn has_active_span(&self) -> bool { - self.get::().is_some() + fn span(&self) -> Option> { + self.get::().map(SpanRef) } fn with_remote_span_context(&self, span_context: crate::trace::SpanContext) -> Self { @@ -323,6 +299,9 @@ pub fn mark_span_as_active(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(span: /// }) /// } /// ``` -pub fn get_active_span(f: F) -> T +pub fn get_active_span(f: F) -> Option 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 index b1f999e998..6ec79617a0 100644 --- 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 index be08599a45..24e9c849aa 100644 --- 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 = 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] diff --git a/opentelemetry-contrib/src/trace/propagator/trace_context_response.rs b/opentelemetry-contrib/src/trace/propagator/trace_context_response.rs index 68479d6d58..408cb830d7 100644 --- a/opentelemetry-contrib/src/trace/propagator/trace_context_response.rs +++ b/opentelemetry-contrib/src/trace/propagator/trace_context_response.rs @@ -100,7 +100,11 @@ impl TextMapPropagator for TraceContextResponsePropagator { /// Properly encodes the values of the `SpanContext` and injects them /// into the `Injector`. 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 span_context.is_valid() { let header_value = format!( @@ -194,7 +198,11 @@ mod tests { extractor.insert(TRACERESPONSE_HEADER.to_string(), traceresponse.to_string()); assert_eq!( - propagator.extract(&extractor).span().span_context(), + propagator + .extract(&extractor) + .span() + .unwrap() + .span_context(), &expected_context ) } @@ -209,7 +217,11 @@ mod tests { extractor.insert(TRACERESPONSE_HEADER.to_string(), invalid_header.to_string()); assert_eq!( - propagator.extract(&extractor).span().span_context(), + propagator + .extract(&extractor) + .span() + .unwrap() + .span_context(), &SpanContext::empty_context(), "{}", reason diff --git a/opentelemetry-datadog/src/lib.rs b/opentelemetry-datadog/src/lib.rs index 22268c4a37..fa587a12d3 100644 --- a/opentelemetry-datadog/src/lib.rs +++ b/opentelemetry-datadog/src/lib.rs @@ -270,7 +270,11 @@ mod propagator { impl TextMapPropagator for DatadogPropagator { 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 span_context.is_valid() { injector.set( @@ -353,7 +357,7 @@ mod propagator { let propagator = DatadogPropagator::default(); let context = propagator.extract(&map); - assert_eq!(context.span().span_context(), &expected); + assert_eq!(context.span().unwrap().span_context(), &expected); } } @@ -362,7 +366,10 @@ mod propagator { let map: HashMap = HashMap::new(); let propagator = DatadogPropagator::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] diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index e568982e0b..40af1889e5 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -540,7 +540,11 @@ mod propagator { impl TextMapPropagator for Propagator { 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 span_context.is_valid() { let flag: u8 = if span_context.is_sampled() { @@ -705,7 +709,7 @@ mod propagator { let mut map: HashMap = HashMap::new(); map.set(context_key, format!("{}:{}:0:{}", trace_id, span_id, flag)); let context = propagator.extract(&map); - assert_eq!(context.span().span_context(), &expected); + assert_eq!(context.span().unwrap().span_context(), &expected); } } @@ -728,7 +732,10 @@ mod propagator { let map: HashMap = HashMap::new(); let propagator = Propagator::new(); 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] @@ -749,7 +756,7 @@ mod propagator { format!("{}:{}:0:{}", trace_id, span_id, flag), ); let context = propagator.extract(&map); - assert_eq!(context.span().span_context(), &expected); + assert_eq!(context.span().unwrap().span_context(), &expected); } } @@ -762,7 +769,10 @@ mod propagator { ); let propagator = Propagator::new(); 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] @@ -774,7 +784,10 @@ mod propagator { ); let propagator = Propagator::new(); 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] @@ -787,7 +800,7 @@ mod propagator { let propagator = Propagator::new(); let context = propagator.extract(&map); assert_eq!( - context.span().span_context(), + context.span().unwrap().span_context(), &SpanContext::new( TraceId::from_u128(TRACE_ID), SpanId::from_u64(SPAN_ID), diff --git a/opentelemetry-jaeger/tests/integration_test.rs b/opentelemetry-jaeger/tests/integration_test.rs index 08c1315dcf..77a1886dfe 100644 --- a/opentelemetry-jaeger/tests/integration_test.rs +++ b/opentelemetry-jaeger/tests/integration_test.rs @@ -31,16 +31,20 @@ mod tests { tracer.in_span("step-2-1", |_cx| {}); tracer.in_span("step-2-2", |_cx| { tracer.in_span("step-3-1", |cx| { - let span = cx.span(); - span.set_status(Status::error("some err")) + if let Some(span) = cx.span() { + span.set_status(Status::error("some err")); + } }); tracer.in_span("step-3-2", |cx| { - cx.span() - .set_attribute(KeyValue::new("tag-3-2-1", "tag-value-3-2-1")) + if let Some(span) = cx.span() { + span.set_attribute(KeyValue::new("tag-3-2-1", "tag-value-3-2-1")) + } }) }); - cx.span() - .add_event("something happened", vec![KeyValue::new("key1", "value1")]); + + if let Some(span) = cx.span() { + span.add_event("something happened", vec![KeyValue::new("key1", "value1")]); + } }); } } diff --git a/opentelemetry-sdk/src/logs/log_emitter.rs b/opentelemetry-sdk/src/logs/log_emitter.rs index f4a03e89dc..44963b4e2c 100644 --- a/opentelemetry-sdk/src/logs/log_emitter.rs +++ b/opentelemetry-sdk/src/logs/log_emitter.rs @@ -212,8 +212,7 @@ impl opentelemetry_api::logs::Logger for Logger { let mut record = record.clone(); if self.include_trace_context { let ctx = Context::current(); - if ctx.has_active_span() { - let span = ctx.span(); + if let Some(span) = ctx.span() { record.trace_context = Some(span.span_context().into()); } } diff --git a/opentelemetry-sdk/src/propagation/composite.rs b/opentelemetry-sdk/src/propagation/composite.rs index cddd6c934c..5f159e9170 100644 --- a/opentelemetry-sdk/src/propagation/composite.rs +++ b/opentelemetry-sdk/src/propagation/composite.rs @@ -138,7 +138,11 @@ mod tests { impl TextMapPropagator for TestPropagator { 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(); injector.set( "testheader", @@ -210,6 +214,7 @@ mod tests { composite_propagator .extract(&extractor) .span() + .unwrap() .span_context(), &SpanContext::empty_context() ); @@ -256,6 +261,7 @@ mod tests { composite_propagator .extract(&extractor) .span() + .unwrap() .span_context(), &SpanContext::new( TraceId::from_u128(1), diff --git a/opentelemetry-sdk/src/propagation/trace_context.rs b/opentelemetry-sdk/src/propagation/trace_context.rs index 9ada5a3168..2dd568e0db 100644 --- a/opentelemetry-sdk/src/propagation/trace_context.rs +++ b/opentelemetry-sdk/src/propagation/trace_context.rs @@ -110,7 +110,11 @@ impl TextMapPropagator for TraceContextPropagator { /// Properly encodes the values of the `SpanContext` and injects them /// into the `Injector`. 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 span_context.is_valid() { let header_value = format!( @@ -207,7 +211,11 @@ mod tests { extractor.insert(TRACESTATE_HEADER.to_string(), trace_state.to_string()); assert_eq!( - propagator.extract(&extractor).span().span_context(), + propagator + .extract(&extractor) + .span() + .unwrap() + .span_context(), &expected_context ) } @@ -227,6 +235,7 @@ mod tests { propagator .extract(&extractor) .span() + .unwrap() .span_context() .trace_state() .header(), @@ -243,7 +252,11 @@ mod tests { extractor.insert(TRACEPARENT_HEADER.to_string(), invalid_header.to_string()); assert_eq!( - propagator.extract(&extractor).span().span_context(), + propagator + .extract(&extractor) + .span() + .unwrap() + .span_context(), &SpanContext::empty_context(), "{}", reason diff --git a/opentelemetry-sdk/src/trace/sampler.rs b/opentelemetry-sdk/src/trace/sampler.rs index cd8e96a56c..0b8a0e356f 100644 --- a/opentelemetry-sdk/src/trace/sampler.rs +++ b/opentelemetry-sdk/src/trace/sampler.rs @@ -179,31 +179,22 @@ impl ShouldSample for Sampler { // Never sample the trace Sampler::AlwaysOff => SamplingDecision::Drop, // The parent decision if sampled; otherwise the decision of delegate_sampler - Sampler::ParentBased(delegate_sampler) => parent_context - .filter(|cx| cx.has_active_span()) - .map_or_else( - || { - delegate_sampler - .should_sample( - parent_context, - trace_id, - name, - span_kind, - attributes, - links, - ) - .decision - }, - |ctx| { - let span = ctx.span(); - let parent_span_context = span.span_context(); - if parent_span_context.is_sampled() { - SamplingDecision::RecordAndSample - } else { - SamplingDecision::Drop - } - }, - ), + Sampler::ParentBased(delegate_sampler) => match parent_context.and_then(|cx| cx.span()) + { + Some(span) => { + let parent_span_context = span.span_context(); + if parent_span_context.is_sampled() { + SamplingDecision::RecordAndSample + } else { + SamplingDecision::Drop + } + } + None => { + delegate_sampler + .should_sample(parent_context, trace_id, name, span_kind, attributes, links) + .decision + } + }, // Probabilistically sample the trace. Sampler::TraceIdRatioBased(prob) => sample_based_on_probability(prob, trace_id), #[cfg(feature = "jaeger_remote_sampler")] @@ -218,8 +209,8 @@ impl ShouldSample for Sampler { // No extra attributes ever set by the SDK samplers. attributes: Vec::new(), // all sampler in SDK will not modify trace state. - trace_state: match parent_context { - Some(ctx) => ctx.span().span_context().trace_state().clone(), + trace_state: match parent_context.and_then(|ctx| ctx.span()) { + Some(span) => span.span_context().trace_state().clone(), None => TraceState::default(), }, } diff --git a/opentelemetry-sdk/src/trace/sampler/jaeger_remote/sampling_strategy.rs b/opentelemetry-sdk/src/trace/sampler/jaeger_remote/sampling_strategy.rs index bb66233142..53076796fd 100644 --- a/opentelemetry-sdk/src/trace/sampler/jaeger_remote/sampling_strategy.rs +++ b/opentelemetry-sdk/src/trace/sampler/jaeger_remote/sampling_strategy.rs @@ -173,8 +173,8 @@ impl Inner { Some(SamplingResult { decision, attributes: Vec::new(), - trace_state: match parent_context { - Some(ctx) => ctx.span().span_context().trace_state().clone(), + trace_state: match parent_context.and_then(|ctx| ctx.span()) { + Some(span) => span.span_context().trace_state().clone(), None => TraceState::default(), }, }) diff --git a/opentelemetry-sdk/src/trace/tracer.rs b/opentelemetry-sdk/src/trace/tracer.rs index 830d1b0436..ad1070691d 100644 --- a/opentelemetry-sdk/src/trace/tracer.rs +++ b/opentelemetry-sdk/src/trace/tracer.rs @@ -103,7 +103,10 @@ impl Tracer { attributes, trace_state, } => { - let trace_flags = parent_cx.span().span_context().trace_flags(); + let trace_flags = match parent_cx.span() { + Some(span) => span.span_context().trace_flags(), + None => TraceFlags::default(), + }; Some((trace_flags.with_sampled(false), attributes, trace_state)) } SamplingResult { @@ -111,7 +114,10 @@ impl Tracer { attributes, trace_state, } => { - let trace_flags = parent_cx.span().span_context().trace_flags(); + let trace_flags = match parent_cx.span() { + Some(span) => span.span_context().trace_flags(), + None => TraceFlags::default(), + }; Some((trace_flags.with_sampled(true), attributes, trace_state)) } } @@ -150,23 +156,18 @@ impl opentelemetry_api::trace::Tracer for Tracer { let span_kind = builder.span_kind.take().unwrap_or(SpanKind::Internal); let mut attribute_options = builder.attributes.take().unwrap_or_default(); let mut link_options = builder.links.take(); - let mut parent_span_id = SpanId::INVALID; - let trace_id; - let parent_span = if parent_cx.has_active_span() { - Some(parent_cx.span()) - } else { - None - }; - - // Build context for sampling decision - if let Some(sc) = parent_span.as_ref().map(|parent| parent.span_context()) { - parent_span_id = sc.span_id(); - trace_id = sc.trace_id(); - } else { - trace_id = builder - .trace_id - .unwrap_or_else(|| config.id_generator.new_trace_id()); + let (parent_span_id, trace_id) = match parent_cx.span() { + Some(span) => { + let span_context = span.span_context(); + (span_context.span_id(), span_context.trace_id()) + } + None => ( + SpanId::INVALID, + builder + .trace_id + .unwrap_or_else(|| config.id_generator.new_trace_id()), + ), }; // In order to accomodate use cases like `tracing-opentelemetry` we there is the ability @@ -298,6 +299,7 @@ mod tests { let trace_state = parent_context .unwrap() .span() + .unwrap() .span_context() .trace_state() .clone(); diff --git a/opentelemetry-zipkin/src/propagator/mod.rs b/opentelemetry-zipkin/src/propagator/mod.rs index 9f4d1ecb32..52b716bb06 100644 --- a/opentelemetry-zipkin/src/propagator/mod.rs +++ b/opentelemetry-zipkin/src/propagator/mod.rs @@ -221,7 +221,11 @@ impl TextMapPropagator for Propagator { /// Properly encodes the values of the `Context`'s `SpanContext` and injects /// them into the `Injector`. fn inject_context(&self, context: &Context, injector: &mut dyn Injector) { - let span = context.span(); + let span = match context.span() { + Some(span) => span, + None => return, + }; + let span_context = span.span_context(); if span_context.is_valid() { let is_deferred = @@ -489,6 +493,7 @@ mod tests { single_header_propagator .extract(&extractor) .span() + .unwrap() .span_context() .clone(), expected_context @@ -502,6 +507,7 @@ mod tests { multi_header_propagator .extract(&extractor) .span() + .unwrap() .span_context() .clone(), expected_context @@ -510,6 +516,7 @@ mod tests { unspecific_header_propagator .extract(&extractor) .span() + .unwrap() .span_context() .clone(), expected_context @@ -526,6 +533,7 @@ mod tests { single_header_propagator .extract(&extractor) .span() + .unwrap() .span_context() .clone(), expected_context @@ -534,6 +542,7 @@ mod tests { single_multi_propagator .extract(&extractor) .span() + .unwrap() .span_context() .clone(), expected_context @@ -550,6 +559,7 @@ mod tests { single_header_propagator .extract(&extractor) .span() + .unwrap() .span_context(), &SpanContext::empty_context(), ) @@ -562,6 +572,7 @@ mod tests { multi_header_propagator .extract(&extractor) .span() + .unwrap() .span_context(), &SpanContext::empty_context(), )