diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index 4fd22714cf..2538eb482d 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -597,7 +597,7 @@ def get_traceparent(self, *args: "Any", **kwargs: "Any") -> "Optional[str]": span_streaming = has_span_streaming_enabled(client.options) # If we have an active span, return traceparent from there - if span_streaming and type(self.streamed_span) is StreamedSpan: + if span_streaming and self.streamed_span is not None: return self.streamed_span._to_traceparent() elif not span_streaming and self.span is not None: return self.span._to_traceparent() @@ -617,7 +617,7 @@ def get_baggage(self, *args: "Any", **kwargs: "Any") -> "Optional[Baggage]": span_streaming = has_span_streaming_enabled(client.options) # If we have an active span, return baggage from there - if span_streaming and type(self.streamed_span) is StreamedSpan: + if span_streaming and self.streamed_span is not None: return self.streamed_span._to_baggage() elif not span_streaming and self.span is not None: return self.span._to_baggage() @@ -632,7 +632,7 @@ def get_trace_context(self) -> "Dict[str, Any]": if ( has_tracing_enabled(self.get_client().options) and self._span is not None - and not isinstance(self._span, (NoOpStreamedSpan, NoOpSpan)) + and not isinstance(self._span, NoOpSpan) ): return self._span._get_trace_context() @@ -703,7 +703,7 @@ def iter_trace_propagation_headers( if ( has_tracing_enabled(client.options) and span is not None - and not isinstance(span, (NoOpStreamedSpan, NoOpSpan)) + and not isinstance(span, NoOpSpan) ): for header in span._iter_headers(): yield header @@ -1295,13 +1295,18 @@ def start_streamed_span( parent_span = self.streamed_span # If no eligible parent_span was provided and there is no currently - # active span, this is a segment + # active span, this is a new segment if parent_span is None: propagation_context = self.get_active_propagation_context() if is_ignored_span(name, attributes): return NoOpStreamedSpan( scope=self, + segment=None, + trace_id=propagation_context.trace_id, + parent_span_id=propagation_context.parent_span_id, + parent_sampled=propagation_context.parent_sampled, + baggage=propagation_context.baggage, unsampled_reason="ignored", ) @@ -1314,10 +1319,18 @@ def start_streamed_span( if sample_rate is not None: self._update_sample_rate(sample_rate) - if sampled is False: + if sampled is False or sampled is None: return NoOpStreamedSpan( scope=self, + segment=None, + trace_id=propagation_context.trace_id, + parent_span_id=propagation_context.parent_span_id, + parent_sampled=propagation_context.parent_sampled, + baggage=propagation_context.baggage, + sampled=sampled, unsampled_reason=outcome, + sample_rand=sample_rand, + sample_rate=sample_rate, ) return StreamedSpan( @@ -1338,11 +1351,21 @@ def start_streamed_span( with new_scope(): if is_ignored_span(name, attributes): return NoOpStreamedSpan( + segment=parent_span._segment, + trace_id=parent_span.trace_id, + parent_span_id=parent_span.span_id, + parent_sampled=parent_span.sampled, unsampled_reason="ignored", ) if isinstance(parent_span, NoOpStreamedSpan): - return NoOpStreamedSpan(unsampled_reason=parent_span._unsampled_reason) + return NoOpStreamedSpan( + segment=parent_span._segment, + trace_id=parent_span.trace_id, + parent_span_id=parent_span.span_id, + parent_sampled=parent_span.sampled, + unsampled_reason=parent_span._unsampled_reason, + ) return StreamedSpan( name=name, diff --git a/sentry_sdk/traces.py b/sentry_sdk/traces.py index 5ee7e8460b..4911a8c6f5 100644 --- a/sentry_sdk/traces.py +++ b/sentry_sdk/traces.py @@ -501,9 +501,9 @@ def _dynamic_sampling_context(self) -> "dict[str, str]": return self._segment._get_baggage().dynamic_sampling_context() def _to_traceparent(self) -> str: - if self.sampled is True: + if self._segment.sampled is True: sampled = "1" - elif self.sampled is False: + elif self._segment.sampled is False: sampled = "0" else: sampled = None @@ -610,15 +610,39 @@ def _to_json(self) -> "SpanJSON": class NoOpStreamedSpan(StreamedSpan): __slots__ = ( + "_trace_id", + "_span_id", + "_sampled", + "_segment", "_finished", "_unsampled_reason", ) def __init__( self, + segment: "Optional[StreamedSpan]" = None, + trace_id: "Optional[str]" = None, + parent_span_id: "Optional[str]" = None, + parent_sampled: "Optional[bool]" = None, + baggage: "Optional[Baggage]" = None, + sampled: "Optional[bool]" = False, unsampled_reason: "Optional[str]" = None, scope: "Optional[sentry_sdk.Scope]" = None, + sample_rand: "Optional[float]" = None, + sample_rate: "Optional[float]" = None, ) -> None: + self._span_id: "Optional[str]" = None + + self._sampled = sampled + self._segment = segment or self + + self._trace_id: "Optional[str]" = trace_id + self._parent_span_id = parent_span_id + self._parent_sampled = parent_sampled + self._baggage = baggage + self._sample_rand = sample_rand + self._sample_rate = sample_rate + self._scope = scope # type: ignore[assignment] self._unsampled_reason = unsampled_reason @@ -693,9 +717,6 @@ def set_attributes(self, attributes: "Attributes") -> None: def remove_attribute(self, key: str) -> None: pass - def _is_segment(self) -> bool: - return self._scope is not None - @property def status(self) -> "str": return SpanStatus.OK.value @@ -716,17 +737,9 @@ def name(self, value: str) -> None: def active(self) -> bool: return True - @property - def span_id(self) -> str: - return "0000000000000000" - - @property - def trace_id(self) -> str: - return "00000000000000000000000000000000" - @property def sampled(self) -> "Optional[bool]": - return False + return self._sampled @property def start_timestamp(self) -> "Optional[datetime]": @@ -736,6 +749,14 @@ def start_timestamp(self) -> "Optional[datetime]": def end_timestamp(self) -> "Optional[datetime]": return None + def _get_trace_context(self) -> "dict[str, Any]": + return { + "trace_id": self.trace_id, + "span_id": self.span_id, + "parent_span_id": self._parent_span_id, + "dynamic_sampling_context": self._dynamic_sampling_context(), + } + if TYPE_CHECKING: diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index c2e34a795b..c49f8c7054 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -664,7 +664,7 @@ def _fill_sample_rand(self) -> None: # Get the sample rate and compute the transformation that will map the random value # to the desired range: [0, 1), [0, sample_rate), or [sample_rate, 1). - sample_rate = try_convert(float, self.baggage.sentry_items.get("sample_rate")) + sample_rate = self._sample_rate() lower, upper = _sample_rand_range(self.parent_sampled, sample_rate) try: @@ -689,6 +689,13 @@ def _sample_rand(self) -> "Optional[str]": return self.baggage.sentry_items.get("sample_rand") + def _sample_rate(self) -> "Optional[float]": + """Convenience method to get the sample_ value from the baggage.""" + if self.baggage is None: + return None + + return try_convert(float, self.baggage.sentry_items.get("sample_rate")) + class Baggage: """ @@ -857,7 +864,8 @@ def populate_from_segment(cls, segment: "StreamedSpan") -> "Baggage": options = client.options or {} sentry_items["trace_id"] = segment.trace_id - sentry_items["sample_rand"] = f"{segment._sample_rand:.6f}" # noqa: E231 + if segment._sample_rand is not None: + sentry_items["sample_rand"] = f"{segment._sample_rand:.6f}" if options.get("environment"): sentry_items["environment"] = options["environment"] @@ -873,8 +881,8 @@ def populate_from_segment(cls, segment: "StreamedSpan") -> "Baggage": if ( segment.get_attributes().get("sentry.span.source") not in LOW_QUALITY_SEGMENT_SOURCES - ) and segment._name: - sentry_items["transaction"] = segment._name + ) and segment.name: + sentry_items["transaction"] = segment.name if segment._sample_rate is not None: sentry_items["sample_rate"] = str(segment._sample_rate) @@ -1530,23 +1538,23 @@ def _make_sampling_decision( name: str, attributes: "Optional[Attributes]", scope: "sentry_sdk.Scope", -) -> "tuple[bool, Optional[float], Optional[float], Optional[str]]": +) -> "tuple[Optional[bool], Optional[float], Optional[float], Optional[str]]": """ Decide whether a span should be sampled. Returns a tuple with: - 1. the sampling decision + 1. the sampling decision (sampled, unsampled, deferred) 2. the effective sample rate 3. the sample rand 4. the reason for not sampling the span, if unsampled """ client = sentry_sdk.get_client() - if not has_tracing_enabled(client.options): - return False, None, None, None - propagation_context = scope.get_active_propagation_context() + if not has_tracing_enabled(client.options): + return propagation_context.parent_sampled, None, None, None + sample_rand = None if propagation_context.baggage is not None: sample_rand = propagation_context.baggage._sample_rand() @@ -1572,7 +1580,10 @@ def _make_sampling_decision( sample_rate = client.options["traces_sampler"](sampling_context) else: if propagation_context.parent_sampled is not None: - sample_rate = propagation_context.parent_sampled + if propagation_context._sample_rate() is not None: + sample_rate = propagation_context._sample_rate() + else: + sample_rate = propagation_context.parent_sampled else: sample_rate = client.options["traces_sample_rate"] @@ -1582,7 +1593,7 @@ def _make_sampling_decision( logger.warning(f"[Tracing] Discarding {name} because of invalid sample rate.") return False, None, None, "sample_rate" - sample_rate = float(sample_rate) + sample_rate = float(sample_rate) # type: ignore[arg-type] if not sample_rate: if traces_sampler_defined: reason = "traces_sampler returned 0 or False" @@ -1590,7 +1601,7 @@ def _make_sampling_decision( reason = "traces_sample_rate is set to 0" logger.debug(f"[Tracing] Discarding {name} because {reason}") - return False, 0.0, None, "sample_rate" + return False, 0.0, sample_rand, "sample_rate" # Adjust sample rate if we're under backpressure sample_rate_before_backpressure = sample_rate diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 1723830f4e..64e059fc4c 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -282,9 +282,14 @@ def getresponse(self, *args, **kwargs): headers = { "sentry-trace": "771a43a4192642f0b136d5159a501700-1234567890abcdef-1", "baggage": ( - "other-vendor-value-1=foo;bar;baz, sentry-trace_id=771a43a4192642f0b136d5159a501700, " - "sentry-public_key=49d0f7386ad645858ae85020e393bef3, sentry-sample_rate=0.01337, " - "sentry-user_id=Am%C3%A9lie, sentry-sample_rand=0.132521102938283, other-vendor-value-2=foo;bar;" + "other-vendor-value-1=foo;bar;baz," + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + "sentry-sample_rate=0.01337," + "sentry-user_id=Am%C3%A9lie," + "sentry-sample_rand=0.000005," + "sentry-sampled=true," + "other-vendor-value-2=foo;bar;" ), } @@ -309,6 +314,15 @@ def getresponse(self, *args, **kwargs): parent_span_id=request_span["span_id"], sampled=1, ) + + expected_outgoing_baggage = ( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + "sentry-sample_rate=0.01337," + "sentry-user_id=Am%C3%A9lie," + "sentry-sample_rand=0.000005," + "sentry-sampled=true" + ) else: events = capture_events() transaction = continue_trace(headers) @@ -331,16 +345,18 @@ def getresponse(self, *args, **kwargs): sampled=1, ) - assert request_headers["sentry-trace"] == expected_sentry_trace - - expected_outgoing_baggage = ( - "sentry-trace_id=771a43a4192642f0b136d5159a501700," - "sentry-public_key=49d0f7386ad645858ae85020e393bef3," - "sentry-sample_rate=1.0," - "sentry-user_id=Am%C3%A9lie," - "sentry-sample_rand=0.132521102938283" - ) + # Note: the sample rate here is actually wrong. It's fixed in the + # streaming path + expected_outgoing_baggage = ( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + "sentry-sample_rate=1.0," + "sentry-user_id=Am%C3%A9lie," + "sentry-sample_rand=0.000005," + "sentry-sampled=true" + ) + assert request_headers["sentry-trace"] == expected_sentry_trace assert request_headers["baggage"] == expected_outgoing_baggage diff --git a/tests/tracing/test_span_streaming.py b/tests/tracing/test_span_streaming.py index e90e56d779..bce5997a1a 100644 --- a/tests/tracing/test_span_streaming.py +++ b/tests/tracing/test_span_streaming.py @@ -744,8 +744,8 @@ def test_continue_trace_unsampled(sentry_init, capture_items): assert span.sampled is False assert span.name == "" - assert span.trace_id == "00000000000000000000000000000000" - assert span.span_id == "0000000000000000" + assert span.trace_id == trace_id + assert span.span_id != "0000000000000000" sentry_sdk.get_client().flush() spans = [item.payload for item in items] @@ -908,65 +908,237 @@ def test_continue_trace_no_sample_rand(sentry_init, capture_items): assert segment["trace_id"] == trace_id -def test_outgoing_traceparent_and_baggage(sentry_init, capture_envelopes): +@pytest.mark.parametrize( + "traces_sample_rate", + ( + 0.0, + 1.0, + # traces_sample_rate=None means tracing without performance: don't make + # any sampling decisions; defer downstream + None, + ), +) +def test_outgoing_traceparent_and_baggage_head_sdk(sentry_init, traces_sample_rate): + if traces_sample_rate == 0.0: + expected_sampled = False + elif traces_sample_rate == 1.0: + expected_sampled = True + elif traces_sample_rate is None: + expected_sampled = None + sentry_init( - traces_sample_rate=1.0, + traces_sample_rate=traces_sample_rate, _experiments={"trace_lifecycle": "stream"}, ) sentry_sdk.traces.new_trace() with sentry_sdk.traces.start_span(name="span") as span: - assert span.sampled is True + assert span.sampled is expected_sampled trace_id = span.trace_id - span_id = span.span_id traceparent = sentry_sdk.get_traceparent() - assert traceparent == f"{trace_id}-{span_id}-1" + + if expected_sampled is True: + span_id = span.span_id + assert traceparent == f"{trace_id}-{span_id}-1" + elif expected_sampled is False: + span_id = span.span_id + assert traceparent == f"{trace_id}-{span_id}-0" + elif expected_sampled is None: + # Tracing without performance, span ID comes from the propagation + # context on the scope + span_id = ( + sentry_sdk.get_current_scope().get_active_propagation_context().span_id + ) + assert traceparent == f"{trace_id}-{span_id}" baggage = sentry_sdk.get_baggage() baggage_items = dict(tuple(item.split("=")) for item in baggage.split(",")) + assert "sentry-trace_id" in baggage_items assert baggage_items["sentry-trace_id"] == trace_id - assert "sentry-sampled" in baggage_items - assert baggage_items["sentry-sampled"] == "true" + + if expected_sampled is None: + assert "sentry-sampled" not in baggage_items + else: + assert baggage_items["sentry-sample_rate"] == str(traces_sample_rate) + assert "sentry-sample_rand" in baggage_items + + assert "sentry-sampled" in baggage_items + if expected_sampled is True: + assert baggage_items["sentry-sampled"] == "true" + elif expected_sampled is False: + assert baggage_items["sentry-sampled"] == "false" -def test_outgoing_traceparent_and_baggage_when_noop_span_is_active( - sentry_init, capture_envelopes +@pytest.mark.parametrize( + "traces_sample_rate,parent_sampled", + ( + (0.0, False), + (0.0, True), + (1.0, False), + (1.0, True), + # traces_sample_rate=None means tracing without performance: don't make + # any sampling decisions on our end, propagate existing ones + (None, False), + (None, True), + ), +) +def test_outgoing_traceparent_and_baggage_incoming_trace( + sentry_init, traces_sample_rate, parent_sampled ): + """The SDK respects a positive/negative incoming sampling decision.""" + # The case where the incoming trace has a deferred sampling decision is + # tested separately in + # test_outgoing_traceparent_and_baggage_incoming_trace_deferred sentry_init( - traces_sample_rate=1.0, - _experiments={ - "trace_lifecycle": "stream", - "ignore_spans": ["ignored"], - }, + traces_sample_rate=traces_sample_rate, + _experiments={"trace_lifecycle": "stream"}, ) - sentry_sdk.traces.new_trace() + trace_id = "0af7651916cd43dd8448eb211c80319c" + parent_span_id = "b7ad6b7169203331" - propagation_context = ( - sentry_sdk.get_current_scope().get_active_propagation_context() + incoming_baggage = { + "sentry-trace_id": trace_id, + } + + if parent_sampled is True: + incoming_sentry_trace = f"{trace_id}-{parent_span_id}-1" + incoming_baggage.update( + { + "sentry-sample_rate": "0.75", + "sentry-sample_rand": "0.500000", + "sentry-sampled": "true", + } + ) + elif parent_sampled is False: + incoming_sentry_trace = f"{trace_id}-{parent_span_id}-0" + incoming_baggage.update( + { + "sentry-sample_rate": "0.75", + "sentry-sample_rand": "0.800000", + "sentry-sampled": "false", + } + ) + + sentry_sdk.traces.continue_trace( + { + "sentry-trace": incoming_sentry_trace, + "baggage": ",".join( + sorted([f"{k}={v}" for k, v in incoming_baggage.items()]) + ), + } ) - propagation_trace_id = propagation_context.trace_id - propagation_span_id = propagation_context.span_id - with sentry_sdk.traces.start_span(name="ignored") as span: - assert span.sampled is False + with sentry_sdk.traces.start_span(name="span") as span: + assert span.sampled is parent_sampled + + traceparent = sentry_sdk.get_traceparent() + + if traces_sample_rate is None: + # TWP: Span ID will come from the propagation context in this case + # (it doesn't even make sense to start a span explicitly as we do in + # this test since tracing is turned off, but nothing should break + # either) + span_id = ( + sentry_sdk.get_current_scope().get_active_propagation_context().span_id + ) + assert traceparent == f"{trace_id}-{span_id}" + else: + span_id = span.span_id + assert ( + traceparent == f"{trace_id}-{span_id}-{'1' if parent_sampled else '0'}" + ) + + # As we've received incoming baggage, we mustn't modify it ourselves and + # have to propagate it as-is, unless we + baggage = sentry_sdk.get_baggage() + baggage_items = dict(tuple(item.split("=")) for item in baggage.split(",")) + assert baggage_items == incoming_baggage + - noop_trace_id = span.trace_id - noop_span_id = span.span_id +@pytest.mark.parametrize( + "traces_sample_rate", + ( + 0.0, + 1.0, + # traces_sample_rate=None means tracing without performance: don't make + # any sampling decisions on our end, propagate existing ones + None, + ), +) +def test_outgoing_traceparent_and_baggage_incoming_trace_deferred( + sentry_init, traces_sample_rate +): + """The SDK handles a deferred incoming sampling decision correctly.""" + sentry_init( + traces_sample_rate=traces_sample_rate, + _experiments={"trace_lifecycle": "stream"}, + ) + + if traces_sample_rate == 0.0: + expected_sampled = False + elif traces_sample_rate == 1.0: + expected_sampled = True + elif traces_sample_rate is None: + expected_sampled = None + + trace_id = "0af7651916cd43dd8448eb211c80319c" + parent_span_id = "b7ad6b7169203331" + + incoming_baggage = {"sentry-trace_id": trace_id, "sentry-sample_rand": "0.500000"} + + sentry_sdk.traces.continue_trace( + { + "sentry-trace": f"{trace_id}-{parent_span_id}-", + "baggage": ",".join( + sorted([f"{k}={v}" for k, v in incoming_baggage.items()]) + ), + } + ) + + with sentry_sdk.traces.start_span(name="span") as span: + assert span.sampled is expected_sampled traceparent = sentry_sdk.get_traceparent() - assert traceparent != f"{noop_trace_id}-{noop_span_id}" - assert traceparent == f"{propagation_trace_id}-{propagation_span_id}" + + if traces_sample_rate is None: + # TWP: Span ID will come from the propagation context in this case + # (it doesn't even make sense to start a span explicitly as we do in + # this test since tracing is turned off, but nothing should break + # either) + span_id = ( + sentry_sdk.get_current_scope().get_active_propagation_context().span_id + ) + else: + span_id = span.span_id + + if expected_sampled is True: + assert traceparent == f"{trace_id}-{span_id}-1" + elif expected_sampled is False: + assert traceparent == f"{trace_id}-{span_id}-0" + elif expected_sampled is None: + assert traceparent == f"{trace_id}-{span_id}" baggage = sentry_sdk.get_baggage() baggage_items = dict(tuple(item.split("=")) for item in baggage.split(",")) - assert "sentry-trace_id" in baggage_items - assert baggage_items["sentry-trace_id"] != noop_trace_id - assert baggage_items["sentry-trace_id"] == propagation_trace_id + + if traces_sample_rate is not None: + # If our sample rate is not None, we're expected to have made + # a sampling decision + expected_baggage = incoming_baggage + expected_baggage.update( + { + "sentry-sample_rate": str(traces_sample_rate), + } + ) + assert baggage_items == expected_baggage + else: + # If tracing is off, we should have deferred the decision further + assert baggage_items == incoming_baggage def test_set_span_status(sentry_init, capture_items):