Skip to content

Commit

Permalink
Fixed B3 Propagator
Browse files Browse the repository at this point in the history
B3 propagator was returning `None` instead of invalid context after last
change. This resulted in many apps crashing when B3 failed to extract a
valid context from carrier. This PR patches it to not make apps crash
and return an invalid context when one cannot be extracted from the
carrier.
  • Loading branch information
owais committed Apr 6, 2021
1 parent cad261e commit bf92d75
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#1721](https://github.com/open-telemetry/opentelemetry-python/pull/1721))
- Update bootstrap cmd to use exact version when installing instrumentation packages.
([#1722](https://github.com/open-telemetry/opentelemetry-python/pull/1722))
- Fix B3 propagator to never return None.
([#1750](https://github.com/open-telemetry/opentelemetry-python/pull/1750))


## [1.0.0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.0.0) - 2021-03-26
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ def extract(
or self._trace_id_regex.fullmatch(trace_id) is None
or self._span_id_regex.fullmatch(span_id) is None
):
if context is None:
return trace.set_span_in_context(trace.INVALID_SPAN, context)
return context

trace_id = int(trace_id, 16)
Expand All @@ -119,7 +121,8 @@ def extract(
trace_flags=trace.TraceFlags(options),
trace_state=trace.TraceState(),
)
)
),
context,
)

def inject(
Expand Down
34 changes: 34 additions & 0 deletions propagator/opentelemetry-propagator-b3/tests/test_b3_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,31 @@ def test_flags_and_sampling(self):

self.assertEqual(new_carrier[FORMAT.SAMPLED_KEY], "1")

def test_derived_ctx_is_returned_for_success(self):
"""Ensure returned context is derived from the given context."""
old_ctx = {"k1": "v1"}
new_ctx = FORMAT.extract(
{
FORMAT.TRACE_ID_KEY: self.serialized_trace_id,
FORMAT.SPAN_ID_KEY: self.serialized_span_id,
FORMAT.FLAGS_KEY: "1",
},
old_ctx,
)
self.assertIn("current-span", new_ctx)
for k, v in old_ctx.items():
self.assertIn(k, new_ctx)
self.assertEqual(new_ctx[k], v)

def test_derived_ctx_is_returned_for_failure(self):
"""Ensure returned context is derived from the given context."""
old_ctx = {"k2": "v2"}
new_ctx = FORMAT.extract({}, old_ctx)
self.assertNotIn("current-span", new_ctx)
for k, v in old_ctx.items():
self.assertIn(k, new_ctx)
self.assertEqual(new_ctx[k], v)

def test_64bit_trace_id(self):
"""64 bit trace ids should be padded to 128 bit trace ids."""
trace_id_64_bit = self.serialized_trace_id[:16]
Expand Down Expand Up @@ -334,3 +359,12 @@ def test_fields(self):
inject_fields.add(call[1][1])

self.assertEqual(FORMAT.fields, inject_fields)

def test_extract_none_context(self):
"""Given no trace ID, do not modify context"""
old_ctx = None

carrier = {}
new_ctx = FORMAT.extract(carrier, old_ctx)
self.assertIsNotNone(new_ctx)
self.assertEqual(new_ctx["current-span"], trace_api.INVALID_SPAN)

0 comments on commit bf92d75

Please sign in to comment.