oss_17_jaeger_debug_and_parentbased_trace_state

.changelog/33.fixed

@@ -0,0 +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`

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 = (

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):

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)

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(

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