Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make TraceContextExt::span() return an Option #1089

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions examples/actix-http/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
})
}
Expand All @@ -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)
})
})
Expand Down
10 changes: 7 additions & 3 deletions examples/actix-hyper/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
})
}
Expand All @@ -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)
})
})
Expand Down
11 changes: 8 additions & 3 deletions examples/actix-udp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
})
}
Expand All @@ -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)
})
})
Expand Down
3 changes: 2 additions & 1 deletion examples/aws-xray/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())],
);
Expand Down
12 changes: 10 additions & 2 deletions examples/basic-otlp-http/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![]);
Expand Down
12 changes: 10 additions & 2 deletions examples/basic-otlp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![]);
Expand Down
6 changes: 5 additions & 1 deletion examples/datadog/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
12 changes: 10 additions & 2 deletions examples/external-otlp-tonic-tokio/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![]);
Expand Down
3 changes: 2 additions & 1 deletion examples/grpc/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}"))],
);
Expand Down
3 changes: 2 additions & 1 deletion examples/http/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())],
);
Expand Down
8 changes: 6 additions & 2 deletions examples/traceresponse/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
2 changes: 1 addition & 1 deletion examples/traceresponse/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
1 change: 1 addition & 0 deletions opentelemetry-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-api/src/logs/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
45 changes: 12 additions & 33 deletions opentelemetry-api/src/trace/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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.
///
Expand All @@ -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 {
Expand Down Expand Up @@ -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
///
/// ```
Expand All @@ -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! {
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-api/src/trace/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
}
Expand Down
13 changes: 10 additions & 3 deletions opentelemetry-aws/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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]
Expand Down
Loading