Skip to content
Open
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
27 changes: 24 additions & 3 deletions opentelemetry-sdk/benches/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

use criterion::{criterion_group, criterion_main, Criterion};
use opentelemetry::{
trace::{mark_span_as_active, Span, TraceContextExt, Tracer, TracerProvider},
trace::{mark_span_as_active, Span, SpanBuilder, TraceContextExt, Tracer, TracerProvider},
Context, KeyValue,
};
use opentelemetry_sdk::{
Expand Down Expand Up @@ -75,11 +75,32 @@ fn criterion_benchmark(c: &mut Criterion) {
span.set_attribute(KeyValue::new("key1", false));
span.set_attribute(KeyValue::new("key2", "hello"));
span.set_attribute(KeyValue::new("key3", 123.456));
span.set_attribute(KeyValue::new("key4", "world"));
span.set_attribute(KeyValue::new("key5", 123));
if span.is_recording() {
span.set_attribute(KeyValue::new("key4", "world"));
span.set_attribute(KeyValue::new("key5", 123));
}
});
});

trace_benchmark_group(c, "span-creation-tracer-in-span-with-builder", |tracer| {
// This is similar to the simple span creation, but also does the job of activating
// the span in the current context.
tracer.in_span_with_builder(
SpanBuilder::from_name("span-name").with_attributes([
KeyValue::new("key1", false),
KeyValue::new("key2", "hello"),
KeyValue::new("key3", 123.456),
]),
|ctx| {
let span = ctx.span();
if span.is_recording() {
span.set_attribute(KeyValue::new("key4", "world"));
span.set_attribute(KeyValue::new("key5", 123));
}
},
);
});

trace_benchmark_group(c, "span-creation-simple-context-activation", |tracer| {
// This optimizes by bypassing the context activation
// based on sampling decision, and hence it is faster than the
Expand Down
219 changes: 219 additions & 0 deletions opentelemetry-sdk/src/trace/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,223 @@ mod tests {

assert!(!span.span_context().is_sampled());
}

#[test]
fn in_span_with_context_uses_provided_context() {
use crate::trace::{InMemorySpanExporter, SimpleSpanProcessor};

let exporter = InMemorySpanExporter::default();
let tracer_provider = crate::trace::SdkTracerProvider::builder()
.with_sampler(Sampler::AlwaysOn)
.with_span_processor(SimpleSpanProcessor::new(exporter.clone()))
.build();
let tracer = tracer_provider.tracer("test");

// Create a parent context explicitly
let parent_span = tracer.start("parent");
let parent_trace_id = parent_span.span_context().trace_id();
let parent_span_id = parent_span.span_context().span_id();
let parent_cx = Context::current_with_span(parent_span);

// Use in_span_with_context with explicit parent context
let mut child_trace_id = None;
let mut child_span_id = None;
let mut executed = false;

let returned_value = tracer.in_span_with_context("child-span", &parent_cx, |cx| {
let span = cx.span();
child_trace_id = Some(span.span_context().trace_id());
child_span_id = Some(span.span_context().span_id());
executed = true;
"test_result"
});

// Verify child span inherited parent's trace_id
assert_eq!(child_trace_id, Some(parent_trace_id));
// Verify child has a different span_id than parent
assert_ne!(child_span_id, Some(parent_span_id));
// Verify the closure was executed
assert!(executed);
// Verify return value is passed through
assert_eq!(returned_value, "test_result");

// End the parent span to export it
drop(parent_cx);

// Verify parent-child relationship through exporter
let spans = exporter.get_finished_spans().unwrap();
assert_eq!(spans.len(), 2);
let parent = spans.iter().find(|s| s.name == "parent").unwrap();
let child = spans.iter().find(|s| s.name == "child-span").unwrap();
assert_eq!(child.parent_span_id, parent.span_context.span_id());
assert_eq!(
child.span_context.trace_id(),
parent.span_context.trace_id()
);
}

#[test]
fn in_span_with_builder_uses_current_context() {
use crate::trace::{InMemorySpanExporter, SimpleSpanProcessor};

let exporter = InMemorySpanExporter::default();
let tracer_provider = crate::trace::SdkTracerProvider::builder()
.with_sampler(Sampler::AlwaysOn)
.with_span_processor(SimpleSpanProcessor::new(exporter.clone()))
.build();
let tracer = tracer_provider.tracer("test");

// Create a parent span and attach it to the current context
let parent_span = tracer.start("parent");
let parent_trace_id = parent_span.span_context().trace_id();
let parent_span_id = parent_span.span_context().span_id();
let _attached = Context::current_with_span(parent_span).attach();

// Use in_span_with_builder with configured span
let mut child_trace_id = None;

tracer.in_span_with_builder(
tracer
.span_builder("child")
.with_kind(SpanKind::Client)
.with_attributes(vec![KeyValue::new("test_key", "test_value")]),
|cx| {
let span = cx.span();
child_trace_id = Some(span.span_context().trace_id());
},
);

// Verify child span inherited parent's trace_id
assert_eq!(child_trace_id, Some(parent_trace_id));

// End the attached context to export the parent span
drop(_attached);

// Verify parent-child relationship through exporter
let spans = exporter.get_finished_spans().unwrap();
assert_eq!(spans.len(), 2);
let child = spans.iter().find(|s| s.name == "child").unwrap();
assert_eq!(child.parent_span_id, parent_span_id);
assert_eq!(child.span_context.trace_id(), parent_trace_id);
}

#[test]
fn in_span_with_builder_and_context_uses_provided_context() {
use crate::trace::{InMemorySpanExporter, SimpleSpanProcessor};

let exporter = InMemorySpanExporter::default();
let tracer_provider = crate::trace::SdkTracerProvider::builder()
.with_sampler(Sampler::AlwaysOn)
.with_span_processor(SimpleSpanProcessor::new(exporter.clone()))
.build();
let tracer = tracer_provider.tracer("test");

// Create a parent context explicitly
let parent_span = tracer.start("parent");
let parent_trace_id = parent_span.span_context().trace_id();
let parent_span_id = parent_span.span_context().span_id();
let parent_cx = Context::current_with_span(parent_span);

// Use in_span_with_builder_and_context with explicit parent context
let mut child_trace_id = None;
let mut result = 0;

let returned_value = tracer.in_span_with_builder_and_context(
tracer
.span_builder("child")
.with_kind(SpanKind::Server)
.with_attributes(vec![
KeyValue::new("http.method", "GET"),
KeyValue::new("http.url", "/api/test"),
]),
&parent_cx,
|cx| {
let span = cx.span();
child_trace_id = Some(span.span_context().trace_id());
result = 42;
result
},
);

// Verify child span inherited parent's trace_id
assert_eq!(child_trace_id, Some(parent_trace_id));
// Verify return value is passed through
assert_eq!(returned_value, 42);
assert_eq!(result, 42);

// End the parent span to export it
drop(parent_cx);

// Verify parent-child relationship through exporter
let spans = exporter.get_finished_spans().unwrap();
assert_eq!(spans.len(), 2);
let child = spans.iter().find(|s| s.name == "child").unwrap();
assert_eq!(child.parent_span_id, parent_span_id);
assert_eq!(child.span_context.trace_id(), parent_trace_id);
}

#[test]
fn in_span_with_builder_and_context_ignores_active_context() {
use crate::trace::{InMemorySpanExporter, SimpleSpanProcessor};

let exporter = InMemorySpanExporter::default();
let tracer_provider = crate::trace::SdkTracerProvider::builder()
.with_sampler(Sampler::AlwaysOn)
.with_span_processor(SimpleSpanProcessor::new(exporter.clone()))
.build();
let tracer = tracer_provider.tracer("test");

// Create an active context with a specific trace context
let active_span_context = SpanContext::new(
TraceId::from(1u128),
SpanId::from(1u64),
TraceFlags::SAMPLED,
true,
Default::default(),
);
let active_trace_id = active_span_context.trace_id();
let active_span_id = active_span_context.span_id();
let _attached = Context::current_with_span(TestSpan(active_span_context)).attach();

// Create a different parent context with a different trace ID to explicitly provide
let provided_span_context = SpanContext::new(
TraceId::from(2u128),
SpanId::from(2u64),
TraceFlags::SAMPLED,
true,
Default::default(),
);
let provided_trace_id = provided_span_context.trace_id();
let provided_span_id = provided_span_context.span_id();
let provided_cx = Context::current_with_span(TestSpan(provided_span_context));

// Ensure the two parents have different trace IDs
assert_ne!(active_trace_id, provided_trace_id);

// Use in_span_with_builder_and_context with explicit parent context
let mut child_trace_id = None;

tracer.in_span_with_builder_and_context(
tracer.span_builder("child").with_kind(SpanKind::Internal),
&provided_cx,
|cx| {
let span = cx.span();
child_trace_id = Some(span.span_context().trace_id());
},
);

// Verify child uses the provided context, NOT the active context
assert_eq!(child_trace_id, Some(provided_trace_id));
assert_ne!(child_trace_id, Some(active_trace_id));

// Verify parent-child relationship through exporter
let spans = exporter.get_finished_spans().unwrap();
assert_eq!(spans.len(), 1);
let child = &spans[0];
assert_eq!(child.name, "child");
assert_eq!(child.parent_span_id, provided_span_id);
assert_eq!(child.span_context.trace_id(), provided_trace_id);
assert_ne!(child.parent_span_id, active_span_id);
assert_ne!(child.span_context.trace_id(), active_trace_id);
}
}
117 changes: 116 additions & 1 deletion opentelemetry/src/trace/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,126 @@ pub trait Tracer {
N: Into<Cow<'static, str>>,
Self::Span: Send + Sync + 'static,
{
let span = self.start(name);
self.in_span_with_builder(self.span_builder(name), f)
}

/// Start a new span with a given parent context and execute the given closure with
/// reference to the context in which the span is active.
///
/// This method starts a new span as a child of the provided parent context and sets
/// it as the active span for the given function. It then executes the body. It ends
/// the span before returning the execution result.
///
/// # Examples
///
/// ```
/// use opentelemetry::{global, trace::{Span, Tracer, TraceContextExt}, Context, KeyValue};
///
/// fn my_function() {
/// let tracer = global::tracer("my-component");
///
/// // Create a parent context
/// let parent = tracer.start("parent-span");
/// let parent_cx = Context::current_with_span(parent);
///
/// // start an active span with explicit parent context
/// tracer.in_span_with_context("child-span", &parent_cx, |_cx| {
/// // child span is active here
/// })
/// }
/// ```
fn in_span_with_context<T, F, N>(&self, name: N, parent_cx: &Context, f: F) -> T
where
F: FnOnce(Context) -> T,
N: Into<Cow<'static, str>>,
Self::Span: Send + Sync + 'static,
{
self.in_span_with_builder_and_context(self.span_builder(name), parent_cx, f)
}

/// Start a new span from a [`SpanBuilder`] and execute the given closure with
/// reference to the context in which the span is active.
///
/// This method builds and starts a new span from a [`SpanBuilder`] using the current
/// context and sets it as the active span for the given function. It then executes
/// the body. It ends the span before returning the execution result.
///
/// # Examples
///
/// ```
/// use opentelemetry::{global, trace::{Span, SpanKind, Tracer}, KeyValue};
///
/// fn my_function() {
/// let tracer = global::tracer("my-component");
///
/// // start an active span with span configuration
/// tracer.in_span_with_builder(
/// tracer.span_builder("span-name")
/// .with_kind(SpanKind::Client)
/// .with_attributes(vec![KeyValue::new("key", "value")]),
/// |_cx| {
/// // span is active here with configured attributes
/// }
/// )
/// }
/// ```
fn in_span_with_builder<T, F>(&self, builder: SpanBuilder, f: F) -> T
where
F: FnOnce(Context) -> T,
Self::Span: Send + Sync + 'static,
{
let span = self.build(builder);
let cx = Context::current_with_span(span);
let _guard = cx.clone().attach();
f(cx)
}

/// Start a new span from a [`SpanBuilder`] with a given parent context and execute the
/// given closure with reference to the context in which the span is active.
///
/// This method builds and starts a new span from a [`SpanBuilder`] as a child of the
/// provided parent context and sets it as the active span for the given function. It
/// then executes the body. It ends the span before returning the execution result.
///
/// # Examples
///
/// ```
/// use opentelemetry::{global, trace::{Span, SpanKind, Tracer, TraceContextExt}, Context, KeyValue};
///
/// fn my_function() {
/// let tracer = global::tracer("my-component");
///
/// // Create a parent context
/// let parent = tracer.start("parent-span");
/// let parent_cx = Context::current_with_span(parent);
///
/// // start an active span with explicit parent context and span configuration
/// tracer.in_span_with_builder_and_context(
/// tracer.span_builder("child-span")
/// .with_kind(SpanKind::Client)
/// .with_attributes(vec![KeyValue::new("key", "value")]),
/// &parent_cx,
/// |_cx| {
/// // child span is active here with configured attributes
/// }
/// )
/// }
/// ```
fn in_span_with_builder_and_context<T, F>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on our benchmarks (https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-sdk/benches/span.rs#L71) the in_span is not the most performant way to span creation/activation.

Not opposed to this change, just noting the perf implications. Maybe we can improve the implementation to make it better, and/or warn users about perf implication in the doc in follow ups.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look at the benchmarks and extend them with versions for these variants.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the benchmarks are really comparing apples and oranges. Do we want comparable numbers or what are we trying to measure in the benchmarks? Yes, if you don't attach a Span to the Context it will be faster than attaching a Span to the Context. If you don't set an Attribute on a Span it will always be faster than setting an Attribute on a Span.

The bigger question is what type of API do we want to have here. Should we have a method that maybe attaches the Span to the Context and gives you a ref to the Span instead of the Context? This would mean that Context::current() would return a Context that doesn't have this Span attached. This feels weird and I can't really see why you would want that.

&self,
builder: SpanBuilder,
parent_cx: &Context,
f: F,
) -> T
where
F: FnOnce(Context) -> T,
Self::Span: Send + Sync + 'static,
{
let span = self.build_with_context(builder, parent_cx);
let cx = parent_cx.with_span(span);
let _guard = cx.clone().attach();
f(cx)
}
}

/// `SpanBuilder` allows span attributes to be configured before the span
Expand Down