-
Notifications
You must be signed in to change notification settings - Fork 636
fix(tracing): Fix unsampled/deferred trace propagation in span streaming #6757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
ffdd6ec
ac59d43
60c7f81
5a46d50
d6f1178
751cd69
d00c0f6
1a35a5e
259605b
c923e09
a4ddd47
23a9e78
cd73571
46b410a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ignored spans wrong trace propagationMedium Severity Segment-level ignored spans create an active Reviewed by Cursor Bugbot for commit 46b410a. Configure here. |
||
| unsampled_reason="ignored", | ||
| ) | ||
|
|
||
|
|
@@ -1314,9 +1319,15 @@ 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, | ||
| ) | ||
|
|
||
|
|
@@ -1338,11 +1349,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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -610,15 +610,37 @@ 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, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default When an ignored child span ( Evidence
Identified by Warden code-review · RXZ-PUR
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will verify but we might need to take the segment's |
||
| unsampled_reason: "Optional[str]" = None, | ||
| scope: "Optional[sentry_sdk.Scope]" = 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 = None | ||
| self._sample_rate = None | ||
|
|
||
| self._scope = scope # type: ignore[assignment] | ||
| self._unsampled_reason = unsampled_reason | ||
|
|
||
|
|
@@ -693,9 +715,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 +735,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 +747,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: | ||
|
|
||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.