Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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)
Comment thread
sentry-warden[bot] marked this conversation as resolved.
):
return self._span._get_trace_context()

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Comment thread
sentrivana marked this conversation as resolved.
unsampled_reason="ignored",
)

Expand All @@ -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(
Expand All @@ -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,
Expand Down
53 changes: 37 additions & 16 deletions sentry_sdk/traces.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,16 +501,16 @@ 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

traceparent = "%s-%s" % (self.trace_id, self.span_id)
traceparent = f"{self.trace_id}-{self.span_id}"
if sampled is not None:
traceparent += "-%s" % (sampled,)
traceparent += f"-{sampled}"

return traceparent

Expand Down Expand Up @@ -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,
Comment thread
sentry-warden[bot] marked this conversation as resolved.
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

Expand Down Expand Up @@ -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
Expand All @@ -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]":
Expand All @@ -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:

Expand Down
35 changes: 23 additions & 12 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@

# 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:
Expand All @@ -689,6 +689,13 @@

return self.baggage.sentry_items.get("sample_rand")

def _sample_rate(self) -> "Optional[float]":
"""Convenience method to get the sample_rate value from the baggage."""
if self.baggage is None:
return None

return try_convert(float, self.baggage.sentry_items.get("sample_rate"))


class Baggage:
"""
Expand Down Expand Up @@ -857,7 +864,8 @@
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"]
Expand All @@ -873,8 +881,8 @@
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)
Expand Down Expand Up @@ -1530,23 +1538,23 @@
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):

Check warning on line 1555 in sentry_sdk/tracing_utils.py

View check run for this annotation

@sentry/warden / warden: code-review

Tracing-disabled SDK creates and sends real spans when inheriting a sampled parent decision

When `has_tracing_enabled` is `False` (TwP mode) and an incoming trace has `parent_sampled=True`, `_make_sampling_decision` now returns `True` instead of the previous `False`, so `start_streamed_span` creates a real `StreamedSpan` (not `NoOpStreamedSpan`). That span calls `self._scope._capture_span(self)` on end, which has no guard for `has_tracing_enabled`, so the span is queued and sent to Sentry. Have you confirmed this is the intended behaviour for TwP + incoming-sampled trace?
return propagation_context.parent_sampled, None, None, None
Comment thread
sentry-warden[bot] marked this conversation as resolved.

sample_rand = None
if propagation_context.baggage is not None:
sample_rand = propagation_context.baggage._sample_rand()
Expand All @@ -1572,7 +1580,10 @@
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"]

Expand All @@ -1582,15 +1593,15 @@
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"
else:
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
Expand Down
40 changes: 28 additions & 12 deletions tests/integrations/stdlib/test_httplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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;"
),
}

Expand All @@ -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)
Expand All @@ -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


Expand Down
Loading
Loading