Skip to content

Commit

Permalink
Remove remote span context (#508)
Browse files Browse the repository at this point in the history 
Storing a Span's SpanContext two separate ways in a Context (one for
local another for remote) is unnecessary and adds complexity
throughout the project when determining heredity of a Span. This moves
to storing the Span directly in the Context uniformly (for both local
and remote) as current Span.
  • Loading branch information
@jtescher
jtescher committed Apr 4, 2021
1 parent 155d03d commit 49b2654
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 142 deletions.
7 changes: 2 additions & 5 deletions opentelemetry-aws/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ pub mod trace {

let propagator = XrayPropagator::default();
let context = propagator.extract(&map);
assert_eq!(context.remote_span_context(), Some(&expected));
assert_eq!(context.span().span_context(), &expected);
}
}

Expand All @@ -333,10 +333,7 @@ pub mod trace {
let map: HashMap<String, String> = HashMap::new();
let propagator = XrayPropagator::default();
let context = propagator.extract(&map);
assert_eq!(
context.remote_span_context(),
Some(&SpanContext::empty_context())
)
assert_eq!(context.span().span_context(), &SpanContext::empty_context())
}

#[test]
Expand Down
7 changes: 2 additions & 5 deletions opentelemetry-datadog/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ mod propagator {

let propagator = DatadogPropagator::default();
let context = propagator.extract(&map);
assert_eq!(context.remote_span_context(), Some(&expected));
assert_eq!(context.span().span_context(), &expected);
}
}

Expand All @@ -352,10 +352,7 @@ mod propagator {
let map: HashMap<String, String> = HashMap::new();
let propagator = DatadogPropagator::default();
let context = propagator.extract(&map);
assert_eq!(
context.remote_span_context(),
Some(&SpanContext::empty_context())
)
assert_eq!(context.span().span_context(), &SpanContext::empty_context())
}

#[test]
Expand Down
23 changes: 7 additions & 16 deletions opentelemetry-jaeger/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,7 @@ mod propagator {
let map: HashMap<String, String> = HashMap::new();
let propagator = Propagator::new();
let context = propagator.extract(&map);
assert_eq!(
context.remote_span_context(),
Some(&SpanContext::empty_context())
)
assert_eq!(context.span().span_context(), &SpanContext::empty_context())
}

#[test]
Expand All @@ -513,7 +510,7 @@ mod propagator {
);
let propagator = Propagator::new();
let context = propagator.extract(&map);
assert_eq!(context.remote_span_context(), Some(&expected));
assert_eq!(context.span().span_context(), &expected);
}
}

Expand All @@ -526,10 +523,7 @@ mod propagator {
);
let propagator = Propagator::new();
let context = propagator.extract(&map);
assert_eq!(
context.remote_span_context(),
Some(&SpanContext::empty_context())
);
assert_eq!(context.span().span_context(), &SpanContext::empty_context());
}

#[test]
Expand All @@ -541,10 +535,7 @@ mod propagator {
);
let propagator = Propagator::new();
let context = propagator.extract(&map);
assert_eq!(
context.remote_span_context(),
Some(&SpanContext::empty_context())
);
assert_eq!(context.span().span_context(), &SpanContext::empty_context());
}

#[test]
Expand All @@ -557,14 +548,14 @@ mod propagator {
let propagator = Propagator::new();
let context = propagator.extract(&map);
assert_eq!(
context.remote_span_context(),
Some(&SpanContext::new(
context.span().span_context(),
&SpanContext::new(
TraceId::from_u128(TRACE_ID),
SpanId::from_u64(SPAN_ID),
1,
true,
TraceState::default(),
))
)
);
}

Expand Down
81 changes: 44 additions & 37 deletions opentelemetry-zipkin/src/propagator/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,33 +319,33 @@ mod tests {
const SPAN_ID_HEX: u64 = 0x00f0_67aa_0ba9_02b7;

#[rustfmt::skip]
fn single_header_extract_data() -> Vec<(&'static str, Option<SpanContext>)> {
fn single_header_extract_data() -> Vec<(&'static str, SpanContext)> {
vec![
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEFERRED, true, TraceState::default()))), // deferred
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default()))), // not sampled
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default()))), // sampled
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-d", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEBUG, true, TraceState::default()))), // debug
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1-00000000000000cd", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), 1, true, TraceState::default()))), // with parent span id
("a3ce929d0e0e4736-00f067aa0ba902b7-1-00000000000000cd", Some(SpanContext::new(TraceId::from_u128(0x0000_0000_0000_0000_a3ce_929d_0e0e_4736), SpanId::from_u64(SPAN_ID_HEX), 1, true, TraceState::default()))), // padding 64 bit traceID
("0", None),
("-", None),
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEFERRED, true, TraceState::default())), // deferred
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default())), // not sampled
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default())), // sampled
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-d", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEBUG, true, TraceState::default())), // debug
("4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1-00000000000000cd", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), 1, true, TraceState::default())), // with parent span id
("a3ce929d0e0e4736-00f067aa0ba902b7-1-00000000000000cd", SpanContext::new(TraceId::from_u128(0x0000_0000_0000_0000_a3ce_929d_0e0e_4736), SpanId::from_u64(SPAN_ID_HEX), 1, true, TraceState::default())), // padding 64 bit traceID
("0", SpanContext::empty_context()),
("-", SpanContext::empty_context()),
]
}

#[rustfmt::skip]
#[allow(clippy::type_complexity)]
fn multi_header_extract_data() -> Vec<((Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>), Option<SpanContext>)> {
fn multi_header_extract_data() -> Vec<((Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>), SpanContext)> {
// (TraceId, SpanId, Sampled, FlagId, ParentSpanId)
vec![
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), None, None, None), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEFERRED, true, TraceState::default()))), // deferred
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("0"), None, None), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default()))), // not sampled
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("1"), None, None), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default()))), // sampled
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("true"), None, None), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default()))),
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("false"), None, None), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default()))), // use true/false to set sample
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), None, Some("1"), None), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEBUG | TRACE_FLAG_SAMPLED, true, TraceState::default()))), // debug
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("0"), Some("1"), Some("00f067aa0ba90200")), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEBUG | TRACE_FLAG_SAMPLED, true, TraceState::default()))), // debug flag should override sample flag
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("1"), Some("2"), Some("00f067aa0ba90200")), Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default()))), // invalid debug flag, should ignore
((None, None, Some("0"), None, None), None),
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), None, None, None), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEFERRED, true, TraceState::default())), // deferred
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("0"), None, None), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default())), // not sampled
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("1"), None, None), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default())), // sampled
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("true"), None, None), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default())),
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("false"), None, None), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default())), // use true/false to set sample
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), None, Some("1"), None), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEBUG | TRACE_FLAG_SAMPLED, true, TraceState::default())), // debug
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("0"), Some("1"), Some("00f067aa0ba90200")), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_DEBUG | TRACE_FLAG_SAMPLED, true, TraceState::default())), // debug flag should override sample flag
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("1"), Some("2"), Some("00f067aa0ba90200")), SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_SAMPLED, true, TraceState::default())), // invalid debug flag, should ignore
((None, None, Some("0"), None, None), SpanContext::empty_context()),
]
}

Expand Down Expand Up @@ -388,13 +388,13 @@ mod tests {

#[rustfmt::skip]
#[allow(clippy::type_complexity)]
fn single_multi_header_extract_data() -> Vec<((Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>), &'static str, Option<SpanContext>)> {
fn single_multi_header_extract_data() -> Vec<((Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>, Option<&'static str>), &'static str, SpanContext)> {
// (TraceId, SpanId, Sampled, FlagId, ParentSpanId), b3
vec![
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), None, None, None), "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0",
Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default()))), // single header take precedence
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("0"), None, None), "-", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default()))), // when single header is invalid, fall back to multiple headers
((Some("0"), Some("0"), Some("0"), None, None), "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0", Some(SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default()))) // invalid multiple header should go unnoticed since single header take precedence.
SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default())), // single header take precedence
((Some(TRACE_ID_STR), Some(SPAN_ID_STR), Some("0"), None, None), "-", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default())), // when single header is invalid, fall back to multiple headers
((Some("0"), Some("0"), Some("0"), None, None), "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0", SpanContext::new(TraceId::from_u128(TRACE_ID_HEX), SpanId::from_u64(SPAN_ID_HEX), TRACE_FLAG_NOT_SAMPLED, true, TraceState::default())) // invalid multiple header should go unnoticed since single header take precedence.
]
}

Expand Down Expand Up @@ -478,8 +478,9 @@ mod tests {
assert_eq!(
single_header_propagator
.extract(&extractor)
.remote_span_context()
.cloned(),
.span()
.span_context()
.clone(),
expected_context
)
}
Expand All @@ -490,15 +491,17 @@ mod tests {
assert_eq!(
multi_header_propagator
.extract(&extractor)
.remote_span_context()
.cloned(),
.span()
.span_context()
.clone(),
expected_context
);
assert_eq!(
unspecific_header_propagator
.extract(&extractor)
.remote_span_context()
.cloned(),
.span()
.span_context()
.clone(),
expected_context
)
}
Expand All @@ -512,15 +515,17 @@ mod tests {
assert_eq!(
single_header_propagator
.extract(&extractor)
.remote_span_context()
.cloned(),
.span()
.span_context()
.clone(),
expected_context
);
assert_eq!(
single_multi_propagator
.extract(&extractor)
.remote_span_context()
.cloned(),
.span()
.span_context()
.clone(),
expected_context
)
}
Expand All @@ -534,8 +539,9 @@ mod tests {
assert_eq!(
single_header_propagator
.extract(&extractor)
.remote_span_context(),
None
.span()
.span_context(),
&SpanContext::empty_context(),
)
}

Expand All @@ -545,8 +551,9 @@ mod tests {
assert_eq!(
multi_header_propagator
.extract(&extractor)
.remote_span_context(),
None
.span()
.span_context(),
&SpanContext::empty_context(),
)
}
}
Expand Down
12 changes: 7 additions & 5 deletions opentelemetry/src/sdk/propagation/composite.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,9 @@ mod tests {
assert_eq!(
composite_propagator
.extract(&extractor)
.remote_span_context(),
None
.span()
.span_context(),
&SpanContext::empty_context()
);
}
}
Expand Down Expand Up @@ -254,14 +255,15 @@ mod tests {
assert_eq!(
composite_propagator
.extract(&extractor)
.remote_span_context(),
Some(&SpanContext::new(
.span()
.span_context(),
&SpanContext::new(
TraceId::from_u128(1),
SpanId::from_u64(1),
0,
true,
TraceState::default(),
))
)
);
}
}
Expand Down
12 changes: 6 additions & 6 deletions opentelemetry/src/sdk/propagation/trace_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ mod tests {
extractor.insert(TRACESTATE_HEADER.to_string(), trace_state.to_string());

assert_eq!(
propagator.extract(&extractor).remote_span_context(),
Some(&expected_context)
propagator.extract(&extractor).span().span_context(),
&expected_context
)
}
}
Expand All @@ -233,8 +233,8 @@ mod tests {
assert_eq!(
propagator
.extract(&extractor)
.remote_span_context()
.unwrap()
.span()
.span_context()
.trace_state()
.header(),
state
Expand All @@ -250,8 +250,8 @@ mod tests {
extractor.insert(TRACEPARENT_HEADER.to_string(), invalid_header.to_string());

assert_eq!(
propagator.extract(&extractor).remote_span_context(),
None,
propagator.extract(&extractor).span().span_context(),
&SpanContext::empty_context(),
"{}",
reason
)
Expand Down

0 comments on commit 49b2654

Please sign in to comment.