Fix spans for streaming responses in WSGI based frameworks #3798
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #3798 +/- ##
==========================================
+ Coverage 79.89% 79.94% +0.04%
==========================================
Files 137 137
Lines 15385 15424 +39
Branches 2611 2615 +4
==========================================
+ Hits 12292 12330 +38
- Misses 2222 2223 +1
Partials 871 871
|
…hed." This reverts commit 1fe8679.
…ntry/sentry-python into antonpirker/fix-streaming-response
antonpirker
changed the title
…ntry/sentry-python into antonpirker/fix-streaming-response
antonpirker
force-pushed
the
antonpirker/fix-streaming-response
branch
from
32c40ed
to
c612326
Compare
antonpirker
force-pushed
the
antonpirker/fix-streaming-response
branch
from
15eb914
to
30d18fa
Compare
| transaction_duration = ( | ||
| datetime.now(timezone.utc) - current_scope.transaction.start_timestamp | ||
| ).total_seconds() | ||
| if transaction_duration > MAX_TRANSACTION_DURATION_SECONDS: |
There was a problem hiding this comment.
Do we need to check this here? Since this function can only get called when the timer fires this should always be the case
| if timer is not None: | ||
| timer.cancel() |
There was a problem hiding this comment.
Should we maybe always cancel the timer as the first step in this function without the conditional checks on lines 749-752?
What I'm worried might happen is that if we for some reason don't cancel the timer (because there is no transaction anymore for instance), it'll still fire the _finish_long_running_transaction event at a later point and might potentially end up cancelling a different active transaction. IDK if that can realistically happen due to how we pass the scopes around, but probably doesn't hurt to be more cautious here.
Fixes spans in streaming responses in WSGI based frameworks. Only close the transaction once the response was consumed. This way all the spans created during creation of the response will be recorded with the transaction.
This PR makes sure that:
Fixes #3736