diff --git a/.changelog/23.fixed b/.changelog/23.fixed new file mode 100644 index 00000000000..747ce92e0bb --- /dev/null +++ b/.changelog/23.fixed @@ -0,0 +1 @@ +B3 propagators now preserve and re-propagate a received debug flag (`d` in the single header, `x-b3-flags: 1` in the multi header) diff --git a/propagator/opentelemetry-propagator-b3/src/opentelemetry/propagators/b3/__init__.py b/propagator/opentelemetry-propagator-b3/src/opentelemetry/propagators/b3/__init__.py index 025a2b5dc5b..4e66f316b5e 100644 --- a/propagator/opentelemetry-propagator-b3/src/opentelemetry/propagators/b3/__init__.py +++ b/propagator/opentelemetry-propagator-b3/src/opentelemetry/propagators/b3/__init__.py @@ -7,7 +7,7 @@ from typing_extensions import deprecated from opentelemetry import trace -from opentelemetry.context import Context +from opentelemetry.context import Context, create_key, get_value, set_value from opentelemetry.propagators.textmap import ( CarrierT, Getter, @@ -18,6 +18,8 @@ ) from opentelemetry.trace import format_span_id, format_trace_id +_DEBUG_FLAG_KEY = create_key("b3-debug-flag") + class B3MultiFormat(TextMapPropagator): """Propagator for the B3 HTTP multi-header format. @@ -102,6 +104,14 @@ def extract( if sampled in self._SAMPLE_PROPAGATE_VALUES or flags == "1": options |= trace.TraceFlags.SAMPLED + # The b3 debug flag ("d" in the single header, "x-b3-flags: 1" in + # the multi header) implies sampling but is distinct from it and, + # per the b3 spec, must be preserved and re-propagated. The trace + # flags cannot carry it, so store it in the context to be re-emitted + # on inject. + if sampled == "d" or flags == "1": + context = set_value(_DEBUG_FLAG_KEY, True, context) + return trace.set_span_in_context( trace.NonRecordingSpan( trace.SpanContext( @@ -137,7 +147,13 @@ def inject( setter.set( carrier, self.SPAN_ID_KEY, format_span_id(span_context.span_id) ) - setter.set(carrier, self.SAMPLED_KEY, "1" if sampled else "0") + # A received debug flag must be preserved and re-propagated as + # "x-b3-flags: 1". Debug implies sampling, so the sampled header is + # omitted, matching the b3 spec and other implementations. + if get_value(_DEBUG_FLAG_KEY, context): + setter.set(carrier, self.FLAGS_KEY, "1") + else: + setter.set(carrier, self.SAMPLED_KEY, "1" if sampled else "0") @property def fields(self) -> set[str]: @@ -145,6 +161,7 @@ def fields(self) -> set[str]: self.TRACE_ID_KEY, self.SPAN_ID_KEY, self.SAMPLED_KEY, + self.FLAGS_KEY, } @@ -169,10 +186,17 @@ def inject( sampled = (trace.TraceFlags.SAMPLED & span_context.trace_flags) != 0 + # A received debug flag must be preserved and re-propagated as the + # "d" value in the sampling position of the single header. + if get_value(_DEBUG_FLAG_KEY, context): + sampling_field = "d" + else: + sampling_field = "1" if sampled else "0" + fields = [ format_trace_id(span_context.trace_id), format_span_id(span_context.span_id), - "1" if sampled else "0", + sampling_field, ] setter.set(carrier, self.SINGLE_HEADER_KEY, "-".join(fields)) diff --git a/propagator/opentelemetry-propagator-b3/tests/test_b3_format.py b/propagator/opentelemetry-propagator-b3/tests/test_b3_format.py index 172ba2b4ad2..67a4c1ead08 100644 --- a/propagator/opentelemetry-propagator-b3/tests/test_b3_format.py +++ b/propagator/opentelemetry-propagator-b3/tests/test_b3_format.py @@ -82,6 +82,25 @@ def assertSampled(self, carrier): def assertNotSampled(self, carrier): pass + def assertDebug(self, carrier): + pass + + def assertNotDebug(self, carrier): + pass + + @classmethod + @abstractmethod + def debug_carrier(cls, trace_id, span_id): + """Return a carrier that carries the b3 debug flag.""" + + def roundtrip(self, carrier): + """Extract then inject, preserving the extracted context.""" + propagator = self.get_propagator() + ctx = propagator.extract(carrier) + new_carrier = {} + propagator.inject(new_carrier, context=ctx) + return new_carrier + def test_extract_multi_header(self): """Test the extraction of B3 headers.""" propagator = self.get_propagator() @@ -214,6 +233,28 @@ def test_flags_and_sampling(self): self.assertSampled(new_carrier) + def test_debug_flag_roundtrip(self): + """A received debug flag is preserved and re-propagated on inject.""" + new_carrier = self.roundtrip( + self.debug_carrier( + self.serialized_trace_id, self.serialized_span_id + ) + ) + self.assertDebug(new_carrier) + + def test_non_debug_context_does_not_emit_debug(self): + """A normal sampled context does not emit the debug flag on inject.""" + propagator = self.get_propagator() + new_carrier = self.roundtrip( + { + propagator.TRACE_ID_KEY: self.serialized_trace_id, + propagator.SPAN_ID_KEY: self.serialized_span_id, + propagator.SAMPLED_KEY: "1", + } + ) + self.assertSampled(new_carrier) + self.assertNotDebug(new_carrier) + def test_derived_ctx_is_returned_for_success(self): """Ensure returned context is derived from the given context.""" old_ctx = Context({"k1": "v1"}) @@ -414,6 +455,15 @@ def test_fields(self): with tracer.start_as_current_span("child"): propagator.inject({}, setter=mock_setter) + # The debug flag is only injected when a debug context was extracted, + # so exercise that path as well to cover every possible field. + debug_ctx = propagator.extract( + self.debug_carrier( + self.serialized_trace_id, self.serialized_span_id + ) + ) + propagator.inject({}, context=debug_ctx, setter=mock_setter) + inject_fields = set() for call in mock_setter.mock_calls: @@ -445,6 +495,23 @@ def assertSampled(self, carrier): def assertNotSampled(self, carrier): self.assertEqual(carrier[self.get_propagator().SAMPLED_KEY], "0") + def assertDebug(self, carrier): + self.assertEqual(carrier[self.get_propagator().FLAGS_KEY], "1") + # Debug implies sampling, so the sampled header is omitted. + self.assertNotIn(self.get_propagator().SAMPLED_KEY, carrier) + + def assertNotDebug(self, carrier): + self.assertNotIn(self.get_propagator().FLAGS_KEY, carrier) + + @classmethod + def debug_carrier(cls, trace_id, span_id): + propagator = cls.get_propagator() + return { + propagator.TRACE_ID_KEY: trace_id, + propagator.SPAN_ID_KEY: span_id, + propagator.FLAGS_KEY: "1", + } + class TestB3SingleFormat(AbstractB3FormatTestCase, unittest.TestCase): @classmethod @@ -464,3 +531,20 @@ def assertNotSampled(self, carrier): self.assertEqual( carrier[self.get_propagator().SINGLE_HEADER_KEY].split("-")[2], "0" ) + + def assertDebug(self, carrier): + self.assertEqual( + carrier[self.get_propagator().SINGLE_HEADER_KEY].split("-")[2], "d" + ) + + def assertNotDebug(self, carrier): + self.assertNotEqual( + carrier[self.get_propagator().SINGLE_HEADER_KEY].split("-")[2], "d" + ) + + @classmethod + def debug_carrier(cls, trace_id, span_id): + propagator = cls.get_propagator() + return { + propagator.SINGLE_HEADER_KEY: f"{trace_id}-{span_id}-d", + }