Skip to content
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

Fix spans for streaming responses in WSGI based frameworks #3798

Merged
merged 54 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
1fe8679
Keep transaction open until all streaming responses are finished.
antonpirker Nov 18, 2024
ef90c73
Fix current scope in wsgi integration
antonpirker Nov 19, 2024
b00723f
Merge branch 'master' into antonpirker/fix-streaming-response
antonpirker Nov 19, 2024
8f2463b
Revert "Keep transaction open until all streaming responses are finis…
antonpirker Nov 19, 2024
f15d596
Merge branch 'antonpirker/fix-streaming-response' of github.com:getse…
antonpirker Nov 19, 2024
61a5c5d
fix merge screw up
antonpirker Nov 19, 2024
17df886
use the right import
antonpirker Nov 19, 2024
0c82ddb
Keep transaction open until all streaming responses are finished.
antonpirker Nov 18, 2024
5bab9ec
fix
antonpirker Nov 19, 2024
beeee5f
Bottle never calls response.close()
antonpirker Nov 19, 2024
c32c202
Manually close transaction in flask
antonpirker Nov 19, 2024
660be15
Make sure we do not close already closed transactions
antonpirker Nov 19, 2024
93e9219
Make code a bit nicer
antonpirker Nov 19, 2024
54cda52
preserve behavior
antonpirker Nov 19, 2024
d904f38
Fix
antonpirker Nov 19, 2024
a614fc6
Closing the transaction always.
antonpirker Nov 19, 2024
24be3d3
.
antonpirker Nov 19, 2024
e762ef9
.
antonpirker Nov 19, 2024
198a621
Remove manual closing in bottle, not needed anymore
antonpirker Nov 19, 2024
26ec3d7
cleanup
antonpirker Nov 19, 2024
b8c7c1b
Adding exception info when closing transaction
antonpirker Nov 20, 2024
9523882
Cleanup
antonpirker Nov 20, 2024
da5ea3f
linting
antonpirker Nov 20, 2024
8757d2b
Cleanup
antonpirker Nov 20, 2024
08a606b
Merge branch 'master' into antonpirker/fix-streaming-response
antonpirker Nov 20, 2024
dbbe9b8
More cleanup
antonpirker Nov 20, 2024
4ebc14e
Merge branch 'antonpirker/fix-streaming-response' of github.com:getse…
antonpirker Nov 20, 2024
ae9440f
Dont close transaction here (that was the whole idea) only in error c…
antonpirker Nov 20, 2024
8ed0ac7
fix error capturing
antonpirker Nov 20, 2024
273e41e
remove debugging
antonpirker Nov 20, 2024
ee257a0
.
antonpirker Nov 20, 2024
5d3a044
Fixed some Flask tests
antonpirker Nov 20, 2024
6c276ad
Fixing Django tests
antonpirker Nov 20, 2024
0ed34bb
Consume response in strawberry tests
antonpirker Nov 20, 2024
17e0d3b
Ok
antonpirker Nov 20, 2024
cf03f72
Merge branch 'master' into antonpirker/fix-streaming-response
antonpirker Nov 21, 2024
c4eef6c
Explain why we need close
antonpirker Nov 21, 2024
71bebfd
Explain why we need close
antonpirker Nov 21, 2024
c62879b
Explain why we need close
antonpirker Nov 21, 2024
f306e48
Merge branch 'master' into antonpirker/fix-streaming-response
antonpirker Nov 21, 2024
2452820
Finish the transaction using same scopes as when creating them
antonpirker Nov 21, 2024
22d72f0
Make sure to always consume the response
antonpirker Nov 21, 2024
c612326
Cleanup
antonpirker Nov 21, 2024
d2c44c7
Finish long running transactions at some point.
antonpirker Nov 21, 2024
17b77be
Merge branch 'master' into antonpirker/fix-streaming-response
antonpirker Nov 21, 2024
308a179
Use Timer to finish transaction
antonpirker Nov 22, 2024
2feb299
Format
antonpirker Nov 22, 2024
2e69500
Cleanup
antonpirker Nov 22, 2024
30d18fa
Cancel timer when transaction already finished
antonpirker Nov 22, 2024
9105254
typing
antonpirker Nov 22, 2024
baa5498
Cleanup
antonpirker Nov 22, 2024
7090227
oops
antonpirker Nov 22, 2024
9583029
Improvements from code review
antonpirker Nov 25, 2024
8574c06
Cleanup
antonpirker Nov 25, 2024
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
140 changes: 99 additions & 41 deletions sentry_sdk/integrations/wsgi.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
from datetime import datetime, timezone
import sys
from functools import partial
from threading import Timer

import sentry_sdk
from sentry_sdk._werkzeug import get_host, _get_headers
from sentry_sdk.api import continue_trace
from sentry_sdk.consts import OP
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.scope import should_send_default_pii, use_isolation_scope, use_scope
from sentry_sdk.integrations._wsgi_common import (
DEFAULT_HTTP_METHODS_TO_CAPTURE,
_filter_headers,
nullcontext,
)
from sentry_sdk.sessions import track_session
from sentry_sdk.scope import use_isolation_scope
from sentry_sdk.tracing import Transaction, TRANSACTION_SOURCE_ROUTE
from sentry_sdk.tracing_utils import finish_running_transaction
from sentry_sdk.utils import (
ContextVar,
capture_internal_exceptions,
Expand Down Expand Up @@ -46,6 +47,9 @@ def __call__(self, status, response_headers, exc_info=None): # type: ignore
pass


MAX_TRANSACTION_DURATION_SECONDS = 5 * 60


_wsgi_middleware_applied = ContextVar("sentry_wsgi_middleware_applied")


Expand Down Expand Up @@ -98,6 +102,7 @@ def __call__(self, environ, start_response):
_wsgi_middleware_applied.set(True)
try:
with sentry_sdk.isolation_scope() as scope:
current_scope = sentry_sdk.get_current_scope()
with track_session(scope, session_mode="request"):
with capture_internal_exceptions():
scope.clear_breadcrumbs()
Expand All @@ -109,6 +114,7 @@ def __call__(self, environ, start_response):
)

method = environ.get("REQUEST_METHOD", "").upper()

transaction = None
if method in self.http_methods_to_capture:
transaction = continue_trace(
Expand All @@ -119,27 +125,43 @@ def __call__(self, environ, start_response):
origin=self.span_origin,
)

with (
timer = None
if transaction is not None:
sentry_sdk.start_transaction(
transaction,
custom_sampling_context={"wsgi_environ": environ},
).__enter__()
timer = Timer(
MAX_TRANSACTION_DURATION_SECONDS,
_finish_long_running_transaction,
args=(current_scope, scope),
)
if transaction is not None
else nullcontext()
):
try:
response = self.app(
environ,
partial(
_sentry_start_response, start_response, transaction
),
)
except BaseException:
reraise(*_capture_exception())
timer.start()

try:
response = self.app(
environ,
partial(
_sentry_start_response,
start_response,
transaction,
),
)
except BaseException:
exc_info = sys.exc_info()
_capture_exception(exc_info)
finish_running_transaction(current_scope, exc_info, timer)
reraise(*exc_info)

finally:
_wsgi_middleware_applied.set(False)

return _ScopedResponse(scope, response)
return _ScopedResponse(
response=response,
current_scope=current_scope,
isolation_scope=scope,
timer=timer,
)


def _sentry_start_response( # type: ignore
Expand Down Expand Up @@ -201,13 +223,13 @@ def get_client_ip(environ):
return environ.get("REMOTE_ADDR")


def _capture_exception():
# type: () -> ExcInfo
def _capture_exception(exc_info=None):
# type: (Optional[ExcInfo]) -> ExcInfo
"""
Captures the current exception and sends it to Sentry.
Returns the ExcInfo tuple to it can be reraised afterwards.
"""
exc_info = sys.exc_info()
exc_info = exc_info or sys.exc_info()
e = exc_info[1]

# SystemExit(0) is the only uncaught exception that is expected behavior
Expand All @@ -225,7 +247,7 @@ def _capture_exception():

class _ScopedResponse:
"""
Users a separate scope for each response chunk.
Use separate scopes for each response chunk.

This will make WSGI apps more tolerant against:
- WSGI servers streaming responses from a different thread/from
Expand All @@ -234,37 +256,54 @@ class _ScopedResponse:
- WSGI servers streaming responses interleaved from the same thread
"""

__slots__ = ("_response", "_scope")
__slots__ = ("_response", "_current_scope", "_isolation_scope", "_timer")

def __init__(self, scope, response):
# type: (sentry_sdk.scope.Scope, Iterator[bytes]) -> None
self._scope = scope
def __init__(
self,
response, # type: Iterator[bytes]
current_scope, # type: sentry_sdk.scope.Scope
isolation_scope, # type: sentry_sdk.scope.Scope
timer=None, # type: Optional[Timer]
):
# type: (...) -> None
self._response = response
self._current_scope = current_scope
self._isolation_scope = isolation_scope
self._timer = timer

def __iter__(self):
# type: () -> Iterator[bytes]
iterator = iter(self._response)

while True:
with use_isolation_scope(self._scope):
try:
chunk = next(iterator)
except StopIteration:
break
except BaseException:
reraise(*_capture_exception())
try:
while True:
with use_isolation_scope(self._isolation_scope):
with use_scope(self._current_scope):
try:
chunk = next(iterator)
except StopIteration:
break
except BaseException:
reraise(*_capture_exception())

yield chunk

yield chunk
finally:
with use_isolation_scope(self._isolation_scope):
with use_scope(self._current_scope):
finish_running_transaction(timer=self._timer)

def close(self):
# type: () -> None
with use_isolation_scope(self._scope):
try:
self._response.close() # type: ignore
except AttributeError:
pass
except BaseException:
reraise(*_capture_exception())
with use_isolation_scope(self._isolation_scope):
with use_scope(self._current_scope):
try:
finish_running_transaction(timer=self._timer)
self._response.close() # type: ignore
except AttributeError:
sentrivana marked this conversation as resolved.
Show resolved Hide resolved
pass
except BaseException:
reraise(*_capture_exception())


def _make_wsgi_event_processor(environ, use_x_forwarded_for):
Expand Down Expand Up @@ -308,3 +347,22 @@ def event_processor(event, hint):
return event

return event_processor


def _finish_long_running_transaction(current_scope, isolation_scope):
# type: (sentry_sdk.scope.Scope, sentry_sdk.scope.Scope) -> None
"""
Make sure we don't keep transactions open for too long.
Triggered after MAX_TRANSACTION_DURATION_SECONDS have passed.
"""
try:
transaction_duration = (
datetime.now(timezone.utc) - current_scope.transaction.start_timestamp
).total_seconds()
if transaction_duration > MAX_TRANSACTION_DURATION_SECONDS:
antonpirker marked this conversation as resolved.
Show resolved Hide resolved
with use_isolation_scope(isolation_scope):
with use_scope(current_scope):
finish_running_transaction()
except AttributeError:
# transaction is not there anymore
pass
18 changes: 18 additions & 0 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@

from types import FrameType

from sentry_sdk._types import ExcInfo
from threading import Timer


SENTRY_TRACE_REGEX = re.compile(
"^[ \t]*" # whitespace
Expand Down Expand Up @@ -739,3 +742,18 @@ def get_current_span(scope=None):

if TYPE_CHECKING:
from sentry_sdk.tracing import Span


def finish_running_transaction(scope=None, exc_info=None, timer=None):
# type: (Optional[sentry_sdk.Scope], Optional[ExcInfo], Optional[Timer]) -> None
current_scope = scope or sentry_sdk.get_current_scope()
if current_scope.transaction is not None and hasattr(
current_scope.transaction, "_context_manager_state"
):
if timer is not None:
timer.cancel()
antonpirker marked this conversation as resolved.
Show resolved Hide resolved

if exc_info is not None:
current_scope.transaction.__exit__(*exc_info)
else:
current_scope.transaction.__exit__(None, None, None)
Loading
Loading