ref: Support outgoing trace propagation in span first (18)#5638
ref: Support outgoing trace propagation in span first (18)#5638sentrivana wants to merge 121 commits intomasterfrom
Conversation
…first-6-add-continue-and-new-trace
…na/span-first-7-add-trace-decorator
…-first-8-bucket-based-limits-in-batcher
…-first-8-bucket-based-limits-in-batcher
…-17-missing-data-category
…pan-first-18-trace-propagation
- don't re-implement parent span/scope management - actually set span on scope
| if not self._segment: | ||
| return |
There was a problem hiding this comment.
Bug: Calling scope.iter_trace_propagation_headers() when the active span is a NoOpStreamedSpan raises an AttributeError because the _segment attribute is not initialized.
Severity: HIGH
Suggested Fix
In sentry_sdk/scope.py, add a type check within the iter_trace_propagation_headers method to ensure the span is not an instance of NoOpStreamedSpan before calling span._iter_headers(), similar to the existing guards in get_traceparent() and get_baggage().
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: sentry_sdk/traces.py#L505-L506
Potential issue: When a `NoOpStreamedSpan` is the active span on a scope, a call to
`scope.iter_trace_propagation_headers()` will raise an `AttributeError`. This occurs
because the method calls `span._iter_headers()` without a type guard to check if the
span is a `NoOpStreamedSpan`. The `NoOpStreamedSpan` inherits the `_iter_headers` method
from `StreamedSpan` but does not initialize the `_segment` attribute that the method
accesses, leading to the error. This can be triggered during an outgoing HTTP request
when tracing is enabled but the transaction is not sampled, causing an unexpected crash.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| if has_tracing_enabled(client.options) and span is not None: | ||
| for header in span.iter_headers(): | ||
| for header in span._iter_headers(): |
There was a problem hiding this comment.
Missing NoOpStreamedSpan guard in iter_trace_propagation_headers
High Severity
iter_trace_propagation_headers calls span._iter_headers() without guarding against NoOpStreamedSpan, unlike get_traceparent, get_baggage, and get_trace_context which all check not isinstance(self.span, NoOpStreamedSpan). When a NoOpStreamedSpan is the active span on the scope, _iter_headers() (inherited from StreamedSpan) accesses self._segment, which was never initialized in NoOpStreamedSpan.__init__, raising AttributeError. This affects integrations like aiohttp, celery, and grpc that call this method for outgoing trace propagation.
Additional Locations (1)
| and self._span is not None | ||
| and not isinstance(self.span, NoOpStreamedSpan) | ||
| ): | ||
| return self._span._get_trace_context() |
There was a problem hiding this comment.
Inconsistent self._span vs self.span in condition
Low Severity
In get_trace_context, the condition mixes self._span is not None with not isinstance(self.span, NoOpStreamedSpan). The sibling methods get_traceparent and get_baggage consistently use self.span (the property) for both checks. While functionally equivalent since the property just returns self._span, this inconsistency looks like a copy-paste oversight and is confusing to read.
| from collections.abc import Mapping, MutableMapping | ||
| from datetime import timedelta | ||
| from random import Random | ||
| from typing import Pattern |
There was a problem hiding this comment.
Redundant Pattern import breaks Python 3.13+
High Severity
The new unconditional from typing import Pattern at line 11 runs before the existing try/except block at lines 15-19 that gracefully handles typing.Pattern removal. Since typing.Pattern was removed in Python 3.13, this unconditional import causes an ImportError at module load time, effectively breaking the entire SDK on Python 3.13+. The existing try/except block was specifically designed to handle this by preferring re.Pattern.


Couple things going on in this PR. Bear with me, this is probably the most all over the place span first PR because the outgoing trace propagation changes make mypy complain about things elsewhere in the sdk.
1. Outgoing trace propagation
Support getting trace propagation information from the span with
_get_traceparent,_get_baggage,_iter_headers, etc. These mirror the oldSpanclass to make integratingStreamedSpans with the rest of the SDK easier (since they're used throughout), with one difference: they're explicitly private, while the correspondingSpanmethods were public. Added aliases to them so that we can use the private methods everywhere.There is definite clean up potential here once we get rid of the old spans and we no longer have to make the streaming span interface work with the existing helper scope methods.
2. Addressing cascading mypy issues
Now that we're officially allowing
StreamedSpans to be set on the scope, a LOT of type hints need updating all over the SDK. In many places, I've added explicit guards against functionality that doesn't exist in span first mode. This should prevent folks from using the wrong APIs in the wrong SDK mode (tracing vs. static) as well as make mypy happy.