From f8fc808d190c0c2f2fd868b1009e6ec98a8ba372 Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Sun, 23 Aug 2026 19:29:43 +0330 Subject: [PATCH 1/2] test(sdk): assert samplers receive the parent's tracestate Sampler.should_sample declares a trace_state parameter and the consistent probability sampling design reads the parent threshold out of it. Assert that a sampler driven through Tracer.start_span actually receives it, that ParentBased forwards it to its delegate, and that a root span correctly gets None. Also assert the composite sampler preserves vendor tracestate entries rather than replacing the whole tracestate, and that SamplingIntent.update_trace_state is applied to root spans as well as children. These tests fail against the current implementation. --- .../tests/trace/test_sampler_tracestate.py | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 opentelemetry-sdk/tests/trace/test_sampler_tracestate.py diff --git a/opentelemetry-sdk/tests/trace/test_sampler_tracestate.py b/opentelemetry-sdk/tests/trace/test_sampler_tracestate.py new file mode 100644 index 0000000000..f77977b4b3 --- /dev/null +++ b/opentelemetry-sdk/tests/trace/test_sampler_tracestate.py @@ -0,0 +1,147 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + +"""Samplers must actually receive the parent's tracestate. + +`Sampler.should_sample` declares a `trace_state` parameter and the consistent +probability sampling design in `_sampling_experimental` reads the parent +threshold out of it, so dropping it on the way in makes that machinery inert +and lets the composite sampler discard vendor tracestate entries. +""" + +import unittest + +from opentelemetry.sdk.trace import TracerProvider +from opentelemetry.sdk.trace._sampling_experimental import ( + ComposableSampler, + SamplingIntent, + composable_always_on, + composable_parent_threshold, + composite_sampler, +) +from opentelemetry.sdk.trace._sampling_experimental._util import MIN_THRESHOLD +from opentelemetry.sdk.trace.sampling import ( + ALWAYS_ON, + Decision, + ParentBased, + Sampler, + SamplingResult, +) +from opentelemetry.trace import set_span_in_context +from opentelemetry.trace.propagation.tracecontext import ( + TraceContextTextMapPropagator, +) + +_CARRIER = { + "traceparent": "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", + "tracestate": "vendora=alpha,ot=th:8", +} + + +class _SpySampler(Sampler): + """Records the trace_state it was handed.""" + + def __init__(self): + self.seen = [] + + def should_sample( + self, + parent_context, + trace_id, + name, + kind=None, + attributes=None, + links=None, + trace_state=None, + ): + self.seen.append(trace_state) + return SamplingResult(Decision.RECORD_AND_SAMPLE, attributes, trace_state) + + def get_description(self): + return "Spy" + + +def _remote_context(): + return TraceContextTextMapPropagator().extract(dict(_CARRIER)) + + +class TestSamplerReceivesTraceState(unittest.TestCase): + def _run(self, sampler): + provider = TracerProvider(sampler=sampler, shutdown_on_exit=False) + provider.get_tracer(__name__).start_span("child", context=_remote_context()) + provider.shutdown() + + def test_sampler_receives_parent_tracestate(self): + spy = _SpySampler() + self._run(spy) + self.assertIsNotNone(spy.seen[0]) + self.assertEqual(spy.seen[0].get("vendora"), "alpha") + self.assertEqual(spy.seen[0].get("ot"), "th:8") + + def test_parent_based_forwards_tracestate_to_its_delegate(self): + spy = _SpySampler() + self._run(ParentBased(root=ALWAYS_ON, remote_parent_sampled=spy)) + self.assertIsNotNone(spy.seen[0]) + self.assertEqual(spy.seen[0].get("vendora"), "alpha") + + def test_root_span_receives_no_tracestate(self): + """A root span has no parent, so None is correct here.""" + spy = _SpySampler() + provider = TracerProvider(sampler=spy, shutdown_on_exit=False) + provider.get_tracer(__name__).start_span("root") + provider.shutdown() + self.assertIsNone(spy.seen[0]) + + +class TestCompositeSamplerPreservesTraceState(unittest.TestCase): + def _outgoing_tracestate(self, sampler): + provider = TracerProvider(sampler=sampler, shutdown_on_exit=False) + span = provider.get_tracer(__name__).start_span("child", context=_remote_context()) + carrier = {} + TraceContextTextMapPropagator().inject(carrier, context=set_span_in_context(span)) + provider.shutdown() + return carrier.get("tracestate") + + def test_vendor_entries_survive_the_composite_sampler(self): + outgoing = self._outgoing_tracestate( + composite_sampler(composable_parent_threshold(composable_always_on())) + ) + self.assertIsNotNone(outgoing) + self.assertIn("vendora=alpha", outgoing) + + def test_default_sampler_still_preserves_tracestate(self): + """Control: the default sampler already got this right.""" + outgoing = self._outgoing_tracestate(None) + self.assertIn("vendora=alpha", outgoing) + + +class TestSamplingIntentTraceStateUpdate(unittest.TestCase): + """`SamplingIntent.update_trace_state` must run for root spans too.""" + + class _TaggingSampler(ComposableSampler): + def sampling_intent(self, parent_ctx, name, span_kind, attributes, links, trace_state): + return SamplingIntent( + threshold=MIN_THRESHOLD, + update_trace_state=lambda ts: ts.add("vendorb", "beta"), + ) + + def get_description(self): + return "Tagging" + + def setUp(self): + self.sampler = composite_sampler(self._TaggingSampler()) + self.trace_id = 0x0AF7651916CD43DD8448EB211C80319C + + def test_applied_when_incoming_tracestate_is_absent(self): + result = self.sampler.should_sample(None, self.trace_id, "op", trace_state=None) + self.assertIsNotNone(result.trace_state) + self.assertEqual(result.trace_state.get("vendorb"), "beta") + + def test_applied_when_incoming_tracestate_is_present(self): + from opentelemetry.trace import TraceState + + result = self.sampler.should_sample( + None, self.trace_id, "op", trace_state=TraceState([("other", "1")]) + ) + self.assertEqual(result.trace_state.get("vendorb"), "beta") + self.assertEqual(result.trace_state.get("other"), "1") From 82aeba1b0b712f237a8e80f576a03de0e4ed6c32 Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Sun, 23 Aug 2026 19:29:43 +0330 Subject: [PATCH 2/2] fix(sdk): pass the parent's tracestate through to the sampler Tracer.start_span never passed the trace_state argument, and ParentBased accepted it without forwarding it to its delegate, so no sampler ever received a tracestate despite the Sampler ABC declaring the parameter. This left the consistent probability sampling design inert: _ComposableParentThreshold reads the parent threshold out of tracestate and always saw None, falling back to the sampled flag with threshold_reliable false. Worse, _CompositeSampler rebuilds the outgoing tracestate from that parameter, so given None it emitted a fresh tracestate containing only `ot` and discarded every vendor entry from the incoming request. Pass it from start_span, guarding for root spans, and forward it through ParentBased. Also apply SamplingIntent.update_trace_state before the emptiness check in _update_trace_state, so a composable sampler that stamps tracestate is no longer skipped for root spans. --- .changelog/5567.fixed | 1 + .../src/opentelemetry/sdk/trace/__init__.py | 10 +++++++++- .../sdk/trace/_sampling_experimental/_sampler.py | 7 +++++-- .../src/opentelemetry/sdk/trace/sampling.py | 1 + 4 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 .changelog/5567.fixed diff --git a/.changelog/5567.fixed b/.changelog/5567.fixed new file mode 100644 index 0000000000..4521fbda7d --- /dev/null +++ b/.changelog/5567.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: pass the parent span's `trace_state` to `Sampler.should_sample`, forward it through `ParentBased`, and stop the composite sampler discarding vendor tracestate entries diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 14d6573d0c..ce62521147 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -1168,7 +1168,15 @@ 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 - sampling_result = self.sampler.should_sample(context, trace_id, name, kind, attributes, links) + sampling_result = self.sampler.should_sample( + context, + trace_id, + name, + kind, + attributes, + links, + parent_span_context.trace_state if parent_span_context else None, + ) trace_flags = ( trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/_sampling_experimental/_sampler.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/_sampling_experimental/_sampler.py index 7d74a3fb77..331dc10ed3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/_sampling_experimental/_sampler.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/_sampling_experimental/_sampler.py @@ -68,11 +68,14 @@ def _update_trace_state( intent: SamplingIntent, ) -> TraceState | None: otts = ot_trace_state.serialize() - if not trace_state: + # Apply the sampler's own update even when there is no incoming tracestate, + # otherwise a composable sampler that stamps tracestate is silently skipped + # for every root span. + new_trace_state = intent.update_trace_state(trace_state or TraceState()) + if not new_trace_state: if otts: return TraceState(((OTEL_TRACE_STATE_KEY, otts),)) return None - new_trace_state = intent.update_trace_state(trace_state) if otts: return new_trace_state.update(OTEL_TRACE_STATE_KEY, otts) return new_trace_state diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 9b6401620e..421b6f737b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -361,6 +361,7 @@ def should_sample( kind=kind, attributes=attributes, links=links, + trace_state=trace_state, ) def get_description(self):