Skip to content

Commit

Permalink
Use current span for SDK-less context propagation (#510)
Browse files Browse the repository at this point in the history
In the absence of an SDK the API should not record spans anymore but it
should propagate the span context. In our case absence of an SDK means
we're using the `NoopTracer`.

There are different ways to create new spans and they provide different
ways of specifying the parent context:

- Tracer::start: uses Context::current()
- Tracer::build: uses the context from the span builder or falls back to
  the current context. BUT it did not have the fallback in the
  NoopTracer.
- Tracer::start_with_context: uses the specified context

This changes the `SpanBuilder` to make the `parent_context` in it
non-optional. That way the current context fallback is done on
`SpanBuilder` creation. Tracer implementations just need to read the
`parparent_context` from the span builder.
  • Loading branch information
frigus02 committed Apr 6, 2021
1 parent 4d58d4b commit f7db050
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 40 deletions.
51 changes: 27 additions & 24 deletions opentelemetry/src/sdk/trace/tracer.rs
Expand Up @@ -139,10 +139,7 @@ impl crate::trace::Tracer for Tracer {
where
T: Into<Cow<'static, str>>,
{
let mut builder = self.span_builder(name);
builder.parent_context = Some(cx);

self.build(builder)
self.build(SpanBuilder::from_name_with_context(name, cx))
}

/// Creates a span builder
Expand Down Expand Up @@ -181,12 +178,8 @@ impl crate::trace::Tracer for Tracer {
let mut flags = 0;
let mut span_trace_state = Default::default();

let parent_cx = builder
.parent_context
.take()
.unwrap_or_else(Context::current);
let parent_span = if parent_cx.has_active_span() {
Some(parent_cx.span())
let parent_span = if builder.parent_context.has_active_span() {
Some(builder.parent_context.span())
} else {
None
};
Expand Down Expand Up @@ -220,10 +213,10 @@ impl crate::trace::Tracer for Tracer {
// * There is no parent or a remote parent, in which case make decision now
// * There is a local parent, in which case defer to the parent's decision
let sampling_decision = if let Some(sampling_result) = builder.sampling_result.take() {
self.process_sampling_result(sampling_result, &parent_cx)
self.process_sampling_result(sampling_result, &builder.parent_context)
} else if no_parent || remote_parent {
self.make_sampling_decision(
&parent_cx,
&builder.parent_context,
trace_id,
&builder.name,
&span_kind,
Expand All @@ -245,6 +238,16 @@ impl crate::trace::Tracer for Tracer {
};

// Build optional inner context, `None` if not recording.
let SpanBuilder {
parent_context,
name,
start_time,
end_time,
message_events,
status_code,
status_message,
..
} = builder;
let inner = sampling_decision.map(|(trace_flags, mut extra_attrs, trace_state)| {
flags = trace_flags;
span_trace_state = trace_state;
Expand All @@ -258,23 +261,23 @@ impl crate::trace::Tracer for Tracer {
if let Some(link_options) = &mut link_options {
links.append_vec(link_options);
}
let start_time = builder.start_time.unwrap_or_else(crate::time::now);
let end_time = builder.end_time.unwrap_or(start_time);
let mut message_events = EvictedQueue::new(config.max_events_per_span);
if let Some(mut events) = builder.message_events {
message_events.append_vec(&mut events);
let start_time = start_time.unwrap_or_else(crate::time::now);
let end_time = end_time.unwrap_or(start_time);
let mut message_events_queue = EvictedQueue::new(config.max_events_per_span);
if let Some(mut events) = message_events {
message_events_queue.append_vec(&mut events);
}
let status_code = builder.status_code.unwrap_or(StatusCode::Unset);
let status_message = builder.status_message.unwrap_or(Cow::Borrowed(""));
let status_code = status_code.unwrap_or(StatusCode::Unset);
let status_message = status_message.unwrap_or(Cow::Borrowed(""));

SpanData {
parent_span_id,
span_kind,
name: builder.name,
name,
start_time,
end_time,
attributes,
message_events,
message_events: message_events_queue,
links,
status_code,
status_message,
Expand All @@ -286,7 +289,7 @@ impl crate::trace::Tracer for Tracer {

// Call `on_start` for all processors
for processor in provider.span_processors() {
processor.on_start(&span, &parent_cx)
processor.on_start(&span, &parent_context)
}

span
Expand Down Expand Up @@ -346,13 +349,13 @@ mod tests {
let tracer = tracer_provider.get_tracer("test", None);
let trace_state = TraceState::from_key_value(vec![("foo", "bar")]).unwrap();
let span_builder = SpanBuilder {
parent_context: Some(Context::current_with_span(TestSpan(SpanContext::new(
parent_context: Context::new().with_span(TestSpan(SpanContext::new(
TraceId::from_u128(128),
SpanId::from_u64(64),
TRACE_FLAG_SAMPLED,
true,
trace_state,
)))),
))),
..Default::default()
};

Expand Down
27 changes: 14 additions & 13 deletions opentelemetry/src/trace/noop.rs
Expand Up @@ -6,7 +6,7 @@
use crate::{
sdk::export::trace::{ExportResult, SpanData, SpanExporter},
trace,
trace::{TraceContextExt, TraceState},
trace::{SpanBuilder, TraceContextExt, TraceState},
Context, KeyValue,
};
use async_trait::async_trait;
Expand Down Expand Up @@ -131,14 +131,12 @@ impl trace::Tracer for NoopTracer {

/// Starts a new `NoopSpan` with a given context.
///
/// If the context contains a valid span context, it is propagated.
/// If the context contains a valid span, it's span context is propagated.
fn start_with_context<T>(&self, name: T, cx: Context) -> Self::Span
where
T: Into<std::borrow::Cow<'static, str>>,
{
let mut builder = self.span_builder(name);
builder.parent_context = Some(cx);
self.build(builder)
self.build(SpanBuilder::from_name_with_context(name, cx))
}

/// Starts a `SpanBuilder`.
Expand All @@ -151,13 +149,16 @@ impl trace::Tracer for NoopTracer {

/// Builds a `NoopSpan` from a `SpanBuilder`.
///
/// If the span builder or context contains a valid span context, it is propagated.
fn build(&self, mut builder: trace::SpanBuilder) -> Self::Span {
match builder.parent_context.take() {
Some(cx) if cx.has_active_span() => trace::NoopSpan {
/// If the span builder or the context's current span contains a valid span context, it is
/// propagated.
fn build(&self, builder: trace::SpanBuilder) -> Self::Span {
let cx = builder.parent_context;
if cx.has_active_span() {
trace::NoopSpan {
span_context: cx.span().span_context().clone(),
},
_ => self.invalid(),
}
} else {
self.invalid()
}
}
}
Expand Down Expand Up @@ -212,13 +213,13 @@ mod tests {
let tracer = NoopTracer::new();
let builder = tracer
.span_builder("foo")
.with_parent_context(Context::current_with_span(TestSpan(valid_span_context())));
.with_parent_context(Context::new().with_span(TestSpan(valid_span_context())));
let span = tracer.build(builder);
assert!(span.span_context().is_valid());
}

#[test]
fn noop_tracer_propagates_valid_span_context_from_span() {
fn noop_tracer_propagates_valid_span_context_from_explicitly_specified_context() {
let tracer = NoopTracer::new();
let cx = Context::new().with_span(NoopSpan {
span_context: valid_span_context(),
Expand Down
14 changes: 11 additions & 3 deletions opentelemetry/src/trace/tracer.rs
Expand Up @@ -337,7 +337,7 @@ pub trait Tracer: fmt::Debug + 'static {
#[derive(Clone, Debug, Default)]
pub struct SpanBuilder {
/// Parent `Context`
pub parent_context: Option<Context>,
pub parent_context: Context,
/// Trace id, useful for integrations with external tracing systems.
pub trace_id: Option<TraceId>,
/// Span id, useful for integrations with external tracing systems.
Expand Down Expand Up @@ -368,8 +368,16 @@ pub struct SpanBuilder {
impl SpanBuilder {
/// Create a new span builder from a span name
pub fn from_name<T: Into<Cow<'static, str>>>(name: T) -> Self {
Self::from_name_with_context(name, Context::current())
}

/// Create a new span builder from a span name with the specified context
pub(crate) fn from_name_with_context<T: Into<Cow<'static, str>>>(
name: T,
parent_context: Context,
) -> Self {
SpanBuilder {
parent_context: None,
parent_context,
trace_id: None,
span_id: None,
span_kind: None,
Expand All @@ -388,7 +396,7 @@ impl SpanBuilder {
/// Assign parent context
pub fn with_parent_context(self, parent_context: Context) -> Self {
SpanBuilder {
parent_context: Some(parent_context),
parent_context,
..self
}
}
Expand Down

0 comments on commit f7db050

Please sign in to comment.