From 16648be085d8736a2cfa67e91e6324d6d546105f Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 22 Jul 2026 01:18:05 -0500 Subject: [PATCH 1/2] Fix Jaeger inject debug flag and ParentBased trace_state forwarding Jaeger propagator inject no longer ORs in the DEBUG (forced-keep) flag for a normally-sampled span; it now propagates only the SAMPLED flag, avoiding forcing downstream Jaeger components into a forced-keep decision. ParentBased.should_sample now forwards the incoming trace_state to its delegate sampler, and the SDK Tracer now passes the parent span context's trace_state into should_sample, so the documented trace_state parameter is actually delivered to custom delegate samplers. --- .changelog/0001.fixed | 1 + .changelog/0002.fixed | 1 + .../src/opentelemetry/sdk/trace/__init__.py | 13 ++++- .../src/opentelemetry/sdk/trace/sampling.py | 1 + .../tests/trace/test_sampling.py | 49 +++++++++++++++++++ .../propagators/jaeger/__init__.py | 9 ++-- .../tests/test_jaeger_propagator.py | 12 ++--- 7 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 .changelog/0001.fixed create mode 100644 .changelog/0002.fixed diff --git a/.changelog/0001.fixed b/.changelog/0001.fixed new file mode 100644 index 00000000000..d08c98d26bb --- /dev/null +++ b/.changelog/0001.fixed @@ -0,0 +1 @@ +Jaeger propagator no longer forces the DEBUG flag on inject; a normally-sampled span now propagates only the SAMPLED flag diff --git a/.changelog/0002.fixed b/.changelog/0002.fixed new file mode 100644 index 00000000000..4238e1a503c --- /dev/null +++ b/.changelog/0002.fixed @@ -0,0 +1 @@ +`ParentBased` sampler now forwards the incoming `trace_state` to its delegate sampler, and the SDK now passes the parent's `trace_state` into `should_sample` diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index fe800c3695a..910ebb926d1 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -1221,8 +1221,19 @@ def start_span( # pylint: disable=too-many-locals # The sampler may also add attributes to the newly-created span, e.g. # to include information about the sampling result. # The sampler may also modify the parent span context's tracestate + trace_state = ( + parent_span_context.trace_state + if parent_span_context is not None + else None + ) sampling_result = self.sampler.should_sample( - context, trace_id, name, kind, attributes, links + context, + trace_id, + name, + kind, + attributes, + links, + trace_state, ) trace_flags = ( diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 1652220dfc9..b1b52e4172a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -363,6 +363,7 @@ def should_sample( kind=kind, attributes=attributes, links=links, + trace_state=trace_state, ) def get_description(self): diff --git a/opentelemetry-sdk/tests/trace/test_sampling.py b/opentelemetry-sdk/tests/trace/test_sampling.py index 1d33a1a2c2c..c9e3151a9a9 100644 --- a/opentelemetry-sdk/tests/trace/test_sampling.py +++ b/opentelemetry-sdk/tests/trace/test_sampling.py @@ -524,3 +524,52 @@ def implicit_parent_context(span: trace.Span): context_api.detach(token) self.exec_parent_based(implicit_parent_context) + + def test_parent_based_forwards_trace_state_to_delegate(self): + # ParentBased must forward the trace_state it receives to the delegate + # sampler it selects, so that custom delegate samplers can act on it. + class _RecordingSampler(sampling.Sampler): + def __init__(self): + self.received_trace_state = "not-called" + + def should_sample( + self, + parent_context, + trace_id, + name, + kind=None, + attributes=None, + links=None, + trace_state=None, + ): + self.received_trace_state = trace_state + return sampling.ALWAYS_ON.should_sample( + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ) + + def get_description(self): + return "RecordingSampler" + + trace_state = trace.TraceState([("key", "value")]) + delegate = _RecordingSampler() + sampler = sampling.ParentBased( + root=sampling.ALWAYS_OFF, + remote_parent_sampled=delegate, + ) + context = self._create_parent( + TO_SAMPLED, is_remote=True, trace_state=trace_state + ) + sampler.should_sample( + context, + 0x8000000000000000, + "remote sampled parent", + trace.SpanKind.INTERNAL, + trace_state=trace_state, + ) + self.assertEqual(delegate.received_trace_state, trace_state) diff --git a/propagator/opentelemetry-propagator-jaeger/src/opentelemetry/propagators/jaeger/__init__.py b/propagator/opentelemetry-propagator-jaeger/src/opentelemetry/propagators/jaeger/__init__.py index 79bb1d89ea8..155c76eafa5 100644 --- a/propagator/opentelemetry-propagator-jaeger/src/opentelemetry/propagators/jaeger/__init__.py +++ b/propagator/opentelemetry-propagator-jaeger/src/opentelemetry/propagators/jaeger/__init__.py @@ -73,9 +73,12 @@ def inject( span_parent_id = ( span.parent.span_id if span.is_recording() and span.parent else 0 ) - trace_flags = span_context.trace_flags - if trace_flags.sampled: - trace_flags |= self.DEBUG_FLAG + # Only propagate the SAMPLED flag. The DEBUG flag must not be set on + # inject for a normally-sampled span, since that would force downstream + # Jaeger components into a forced-keep decision. + trace_flags = trace.TraceFlags( + span_context.trace_flags & trace.TraceFlags.SAMPLED + ) # set span identity setter.set( diff --git a/propagator/opentelemetry-propagator-jaeger/tests/test_jaeger_propagator.py b/propagator/opentelemetry-propagator-jaeger/tests/test_jaeger_propagator.py index 481b56b9196..e55303132fc 100644 --- a/propagator/opentelemetry-propagator-jaeger/tests/test_jaeger_propagator.py +++ b/propagator/opentelemetry-propagator-jaeger/tests/test_jaeger_propagator.py @@ -93,14 +93,14 @@ def test_sampled_flag_set(self): ) self.assertEqual(1, sample_flag_value) - def test_debug_flag_set(self): + def test_debug_flag_not_set_for_sampled_span(self): + # Injecting a normally-sampled span must set only the SAMPLED bit and + # must never force the Jaeger DEBUG (forced-keep) flag. old_carrier = {FORMAT.TRACE_ID_KEY: self.serialized_uber_trace_id} _, new_carrier = get_context_new_carrier(old_carrier) - debug_flag_value = ( - int(new_carrier[FORMAT.TRACE_ID_KEY].split(":")[3]) - & FORMAT.DEBUG_FLAG - ) - self.assertEqual(FORMAT.DEBUG_FLAG, debug_flag_value) + flags = int(new_carrier[FORMAT.TRACE_ID_KEY].split(":")[3]) + self.assertEqual(1, flags & 0x01) + self.assertEqual(0, flags & FORMAT.DEBUG_FLAG) def test_sample_debug_flags_unset(self): uber_trace_id = jaeger._format_uber_trace_id( # pylint: disable=protected-access From c80022c1ea298248eabacfe77928c54557fdd204 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 22 Jul 2026 08:37:54 -0500 Subject: [PATCH 2/2] Rename changelog fragment to match PR number --- .changelog/0001.fixed | 1 - .changelog/{0002.fixed => 33.fixed} | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 .changelog/0001.fixed rename .changelog/{0002.fixed => 33.fixed} (56%) diff --git a/.changelog/0001.fixed b/.changelog/0001.fixed deleted file mode 100644 index d08c98d26bb..00000000000 --- a/.changelog/0001.fixed +++ /dev/null @@ -1 +0,0 @@ -Jaeger propagator no longer forces the DEBUG flag on inject; a normally-sampled span now propagates only the SAMPLED flag diff --git a/.changelog/0002.fixed b/.changelog/33.fixed similarity index 56% rename from .changelog/0002.fixed rename to .changelog/33.fixed index 4238e1a503c..0f8aae6aa67 100644 --- a/.changelog/0002.fixed +++ b/.changelog/33.fixed @@ -1 +1,2 @@ +Jaeger propagator no longer forces the DEBUG flag on inject; a normally-sampled span now propagates only the SAMPLED flag `ParentBased` sampler now forwards the incoming `trace_state` to its delegate sampler, and the SDK now passes the parent's `trace_state` into `should_sample`