From 1fe867954f5e6f1ea057674279c2fa8aa4516ce5 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 18 Nov 2024 16:25:21 +0100 Subject: [PATCH 01/47] Keep transaction open until all streaming responses are finished. --- sentry_sdk/integrations/wsgi.py | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 50deae10c5..274fe30b6c 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -119,27 +119,26 @@ def __call__(self, environ, start_response): origin=self.span_origin, ) - with ( - sentry_sdk.start_transaction( + if transaction is not None: + transaction = sentry_sdk.start_transaction( transaction, custom_sampling_context={"wsgi_environ": environ}, ) - if transaction is not None - else nullcontext() - ): - try: - response = self.app( - environ, - partial( - _sentry_start_response, start_response, transaction - ), - ) - except BaseException: - reraise(*_capture_exception()) + transaction.__enter__() + + try: + response = self.app( + environ, + partial( + _sentry_start_response, start_response, transaction + ), + ) + except BaseException: + reraise(*_capture_exception()) finally: _wsgi_middleware_applied.set(False) - return _ScopedResponse(scope, response) + return _ScopedResponse(scope, response, transaction) def _sentry_start_response( # type: ignore @@ -234,12 +233,13 @@ class _ScopedResponse: - WSGI servers streaming responses interleaved from the same thread """ - __slots__ = ("_response", "_scope") + __slots__ = ("_response", "_scope", "_transaction") - def __init__(self, scope, response): - # type: (sentry_sdk.scope.Scope, Iterator[bytes]) -> None + def __init__(self, scope, response, transaction): + # type: (sentry_sdk.scope.Scope, Iterator[bytes], Optional[sentry_sdk.tracing.Transaction]) -> None self._scope = scope self._response = response + self._transaction = transaction def __iter__(self): # type: () -> Iterator[bytes] @@ -261,6 +261,12 @@ def close(self): with use_isolation_scope(self._scope): try: self._response.close() # type: ignore + # Close the Sentry transaction + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + # Of course this here works also for non streaming responses. + if self._transaction is not None: + self._transaction.__exit__(None, None, None) except AttributeError: pass except BaseException: From ef90c738db95c7e9f6d0bafc01901d4f43cf68b6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 10:50:22 +0100 Subject: [PATCH 02/47] Fix current scope in wsgi integration --- sentry_sdk/integrations/wsgi.py | 69 ++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 274fe30b6c..734ca133fd 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -12,7 +12,6 @@ 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.utils import ( ContextVar, @@ -98,6 +97,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() @@ -138,7 +138,12 @@ def __call__(self, environ, start_response): finally: _wsgi_middleware_applied.set(False) - return _ScopedResponse(scope, response, transaction) + return _ScopedResponse( + response=response, + current_scope=current_scope, + isolation_scope=scope, + transaction=transaction, + ) def _sentry_start_response( # type: ignore @@ -224,7 +229,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 @@ -233,12 +238,19 @@ class _ScopedResponse: - WSGI servers streaming responses interleaved from the same thread """ - __slots__ = ("_response", "_scope", "_transaction") + __slots__ = ("_response", "_current_scope", "_isolation_scope", "_transaction") - def __init__(self, scope, response, transaction): - # type: (sentry_sdk.scope.Scope, Iterator[bytes], Optional[sentry_sdk.tracing.Transaction]) -> 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 + transaction, # type: Optional[sentry_sdk.tracing.Transaction] + ): + # type: (...) -> None self._response = response + self._current_scope = current_scope + self._isolation_scope = isolation_scope self._transaction = transaction def __iter__(self): @@ -246,31 +258,34 @@ def __iter__(self): iterator = iter(self._response) while True: - with use_isolation_scope(self._scope): - try: - chunk = next(iterator) - except StopIteration: - break - except BaseException: - reraise(*_capture_exception()) + with sentry_sdk.use_isolation_scope(self._isolation_scope): + with sentry_sdk.use_scope(self._current_scope): + try: + chunk = next(iterator) + except StopIteration: + break + except BaseException: + reraise(*_capture_exception()) yield chunk def close(self): # type: () -> None - with use_isolation_scope(self._scope): - try: - self._response.close() # type: ignore - # Close the Sentry transaction - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - # Of course this here works also for non streaming responses. - if self._transaction is not None: - self._transaction.__exit__(None, None, None) - except AttributeError: - pass - except BaseException: - reraise(*_capture_exception()) + with sentry_sdk.use_isolation_scope(self._isolation_scope): + with sentry_sdk.use_scope(self._current_scope): + try: + self._response.close() # type: ignore + + # Close the Sentry transaction + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + # Of course this here works also for non streaming responses. + if self._transaction is not None: + self._transaction.__exit__(None, None, None) + except AttributeError: + pass + except BaseException: + reraise(*_capture_exception()) def _make_wsgi_event_processor(environ, use_x_forwarded_for): From 8f2463b453cfe7b9b0444906a2ff341174e24546 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 11:01:31 +0100 Subject: [PATCH 03/47] Revert "Keep transaction open until all streaming responses are finished." This reverts commit 1fe867954f5e6f1ea057674279c2fa8aa4516ce5. --- sentry_sdk/integrations/wsgi.py | 37 +++++++++++++-------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 734ca133fd..006c03bad3 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -119,22 +119,23 @@ def __call__(self, environ, start_response): origin=self.span_origin, ) - if transaction is not None: - transaction = sentry_sdk.start_transaction( + with ( + sentry_sdk.start_transaction( transaction, custom_sampling_context={"wsgi_environ": environ}, ) - transaction.__enter__() - - try: - response = self.app( - environ, - partial( - _sentry_start_response, start_response, transaction - ), - ) - except BaseException: - reraise(*_capture_exception()) + if transaction is not None + else nullcontext() + ): + try: + response = self.app( + environ, + partial( + _sentry_start_response, start_response, transaction + ), + ) + except BaseException: + reraise(*_capture_exception()) finally: _wsgi_middleware_applied.set(False) @@ -142,7 +143,6 @@ def __call__(self, environ, start_response): response=response, current_scope=current_scope, isolation_scope=scope, - transaction=transaction, ) @@ -238,7 +238,7 @@ class _ScopedResponse: - WSGI servers streaming responses interleaved from the same thread """ - __slots__ = ("_response", "_current_scope", "_isolation_scope", "_transaction") + __slots__ = ("_response", "_current_scope", "_isolation_scope") def __init__( self, @@ -275,13 +275,6 @@ def close(self): with sentry_sdk.use_scope(self._current_scope): try: self._response.close() # type: ignore - - # Close the Sentry transaction - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - # Of course this here works also for non streaming responses. - if self._transaction is not None: - self._transaction.__exit__(None, None, None) except AttributeError: pass except BaseException: From 61a5c5df89e18d7c6e0c431eb326fca12e19d1c2 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 11:02:43 +0100 Subject: [PATCH 04/47] fix merge screw up --- sentry_sdk/integrations/wsgi.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 006c03bad3..256b5fa740 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -245,13 +245,11 @@ def __init__( response, # type: Iterator[bytes] current_scope, # type: sentry_sdk.scope.Scope isolation_scope, # type: sentry_sdk.scope.Scope - transaction, # type: Optional[sentry_sdk.tracing.Transaction] ): # type: (...) -> None self._response = response self._current_scope = current_scope self._isolation_scope = isolation_scope - self._transaction = transaction def __iter__(self): # type: () -> Iterator[bytes] From 17df886befb80a5ce1c3b77cb4ab7a9f6771ec91 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 11:05:07 +0100 Subject: [PATCH 05/47] use the right import --- sentry_sdk/integrations/wsgi.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 256b5fa740..bc6c368c67 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -5,7 +5,7 @@ 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, @@ -256,8 +256,8 @@ def __iter__(self): iterator = iter(self._response) while True: - with sentry_sdk.use_isolation_scope(self._isolation_scope): - with sentry_sdk.use_scope(self._current_scope): + with use_isolation_scope(self._isolation_scope): + with use_scope(self._current_scope): try: chunk = next(iterator) except StopIteration: @@ -269,8 +269,8 @@ def __iter__(self): def close(self): # type: () -> None - with sentry_sdk.use_isolation_scope(self._isolation_scope): - with sentry_sdk.use_scope(self._current_scope): + with use_isolation_scope(self._isolation_scope): + with use_scope(self._current_scope): try: self._response.close() # type: ignore except AttributeError: From 0c82ddbc234d3916d2049773c5c63691cb093d5b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 18 Nov 2024 16:25:21 +0100 Subject: [PATCH 06/47] Keep transaction open until all streaming responses are finished. --- sentry_sdk/integrations/wsgi.py | 37 ++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index bc6c368c67..55bb1d8517 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -119,23 +119,22 @@ def __call__(self, environ, start_response): origin=self.span_origin, ) - with ( - sentry_sdk.start_transaction( + if transaction is not None: + transaction = sentry_sdk.start_transaction( transaction, custom_sampling_context={"wsgi_environ": environ}, ) - if transaction is not None - else nullcontext() - ): - try: - response = self.app( - environ, - partial( - _sentry_start_response, start_response, transaction - ), - ) - except BaseException: - reraise(*_capture_exception()) + transaction.__enter__() + + try: + response = self.app( + environ, + partial( + _sentry_start_response, start_response, transaction + ), + ) + except BaseException: + reraise(*_capture_exception()) finally: _wsgi_middleware_applied.set(False) @@ -143,6 +142,7 @@ def __call__(self, environ, start_response): response=response, current_scope=current_scope, isolation_scope=scope, + transaction=transaction, ) @@ -245,11 +245,13 @@ def __init__( response, # type: Iterator[bytes] current_scope, # type: sentry_sdk.scope.Scope isolation_scope, # type: sentry_sdk.scope.Scope + transaction, # type: Optional[sentry_sdk.tracing.Transaction] ): # type: (...) -> None self._response = response self._current_scope = current_scope self._isolation_scope = isolation_scope + self._transaction = transaction def __iter__(self): # type: () -> Iterator[bytes] @@ -273,6 +275,13 @@ def close(self): with use_scope(self._current_scope): try: self._response.close() # type: ignore + + # Close the Sentry transaction + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + # Of course this here works also for non streaming responses. + if self._transaction is not None: + self._transaction.__exit__(None, None, None) except AttributeError: pass except BaseException: From 5bab9ec21c68b763173a07d44418708f6fe10919 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 12:19:26 +0100 Subject: [PATCH 07/47] fix --- sentry_sdk/integrations/wsgi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 55bb1d8517..fb3cbfd226 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -238,7 +238,7 @@ class _ScopedResponse: - WSGI servers streaming responses interleaved from the same thread """ - __slots__ = ("_response", "_current_scope", "_isolation_scope") + __slots__ = ("_response", "_current_scope", "_isolation_scope", "_transaction") def __init__( self, From beeee5f97aca4838a9e7ee4b184f0f4d6e42b28e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 13:09:38 +0100 Subject: [PATCH 08/47] Bottle never calls response.close() --- sentry_sdk/integrations/bottle.py | 3 +++ sentry_sdk/integrations/wsgi.py | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/bottle.py b/sentry_sdk/integrations/bottle.py index a2d6b51033..dcb6480bb1 100644 --- a/sentry_sdk/integrations/bottle.py +++ b/sentry_sdk/integrations/bottle.py @@ -138,6 +138,9 @@ def wrapped_callback(*args, **kwargs): ): _capture_exception(res, handled=True) + # Manually close the transaction because Bottle does not call `close()` on the WSGI response + sentry_sdk.get_current_scope().transaction.__exit__(None, None, None) + return res return wrapped_callback diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index fb3cbfd226..14187aa065 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -269,6 +269,12 @@ def __iter__(self): yield chunk + # Close the Sentry transaction (it could be that response.close() is never called by the framework) + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + if self._transaction is not None: + self._transaction.__exit__(None, None, None) + def close(self): # type: () -> None with use_isolation_scope(self._isolation_scope): @@ -279,9 +285,8 @@ def close(self): # Close the Sentry transaction # This is done here to make sure the Transaction stays # open until all streaming responses are done. - # Of course this here works also for non streaming responses. if self._transaction is not None: - self._transaction.__exit__(None, None, None) + self._transaction.__exit__(None, None, None) except AttributeError: pass except BaseException: From c32c2020f195812296bc1533d9ca67aeeec6d1be Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 13:37:09 +0100 Subject: [PATCH 09/47] Manually close transaction in flask --- sentry_sdk/integrations/flask.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index 128301ddb4..f223679891 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -36,6 +36,7 @@ before_render_template, got_request_exception, request_started, + request_finished, ) from markupsafe import Markup except ImportError: @@ -82,6 +83,7 @@ def setup_once(): before_render_template.connect(_add_sentry_trace) request_started.connect(_request_started) + request_finished.connect(_request_finished) got_request_exception.connect(_capture_exception) old_app = Flask.__call__ @@ -152,6 +154,11 @@ def _request_started(app, **kwargs): scope.add_event_processor(evt_processor) +def _request_finished(sender, response, **kwargs): + # Manually close the transaction because Bottle does not call `close()` on the WSGI response + sentry_sdk.get_current_scope().transaction.__exit__(None, None, None) + + class FlaskRequestExtractor(RequestExtractor): def env(self): # type: () -> Dict[str, str] From 660be15dffea01b441f78d92cf30335990eb1c43 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 14:26:10 +0100 Subject: [PATCH 10/47] Make sure we do not close already closed transactions --- sentry_sdk/integrations/wsgi.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 14187aa065..51e2fd6659 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -119,12 +119,11 @@ def __call__(self, environ, start_response): origin=self.span_origin, ) - if transaction is not None: - transaction = sentry_sdk.start_transaction( - transaction, - custom_sampling_context={"wsgi_environ": environ}, - ) - transaction.__enter__() + transaction = sentry_sdk.start_transaction( + transaction, + custom_sampling_context={"wsgi_environ": environ}, + ) + transaction.__enter__() try: response = self.app( @@ -272,7 +271,7 @@ def __iter__(self): # Close the Sentry transaction (it could be that response.close() is never called by the framework) # This is done here to make sure the Transaction stays # open until all streaming responses are done. - if self._transaction is not None: + if self._transaction is not None and hasattr(self._transaction, "_context_manager_state"): self._transaction.__exit__(None, None, None) def close(self): @@ -285,7 +284,7 @@ def close(self): # Close the Sentry transaction # This is done here to make sure the Transaction stays # open until all streaming responses are done. - if self._transaction is not None: + if self._transaction is not None and hasattr(self._transaction, "_context_manager_state"): self._transaction.__exit__(None, None, None) except AttributeError: pass From 93e92197304b8e499a899256e25a8701c7a90cc3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 14:42:28 +0100 Subject: [PATCH 11/47] Make code a bit nicer --- sentry_sdk/integrations/bottle.py | 3 ++- sentry_sdk/integrations/flask.py | 3 ++- sentry_sdk/integrations/wsgi.py | 15 ++++++--------- sentry_sdk/tracing_utils.py | 7 +++++++ 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/integrations/bottle.py b/sentry_sdk/integrations/bottle.py index dcb6480bb1..c32a4c9e0c 100644 --- a/sentry_sdk/integrations/bottle.py +++ b/sentry_sdk/integrations/bottle.py @@ -2,6 +2,7 @@ import sentry_sdk from sentry_sdk.tracing import SOURCE_FOR_STYLE +from sentry_sdk.tracing_utils import finish_running_transaction from sentry_sdk.utils import ( capture_internal_exceptions, ensure_integration_enabled, @@ -139,7 +140,7 @@ def wrapped_callback(*args, **kwargs): _capture_exception(res, handled=True) # Manually close the transaction because Bottle does not call `close()` on the WSGI response - sentry_sdk.get_current_scope().transaction.__exit__(None, None, None) + finish_running_transaction() return res diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index f223679891..4d85d83189 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -7,6 +7,7 @@ from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware from sentry_sdk.scope import should_send_default_pii from sentry_sdk.tracing import SOURCE_FOR_STYLE +from sentry_sdk.tracing_utils import finish_running_transaction from sentry_sdk.utils import ( capture_internal_exceptions, ensure_integration_enabled, @@ -156,7 +157,7 @@ def _request_started(app, **kwargs): def _request_finished(sender, response, **kwargs): # Manually close the transaction because Bottle does not call `close()` on the WSGI response - sentry_sdk.get_current_scope().transaction.__exit__(None, None, None) + finish_running_transaction() class FlaskRequestExtractor(RequestExtractor): diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 51e2fd6659..dfe3cc09f9 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -9,10 +9,10 @@ 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.tracing import Transaction, TRANSACTION_SOURCE_ROUTE +from sentry_sdk.tracing_utils import finish_running_transaction from sentry_sdk.utils import ( ContextVar, capture_internal_exceptions, @@ -262,18 +262,16 @@ def __iter__(self): try: chunk = next(iterator) except StopIteration: + # Close the Sentry transaction (it could be that response.close() is never called by the framework) + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + finish_running_transaction() break except BaseException: reraise(*_capture_exception()) yield chunk - # Close the Sentry transaction (it could be that response.close() is never called by the framework) - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - if self._transaction is not None and hasattr(self._transaction, "_context_manager_state"): - self._transaction.__exit__(None, None, None) - def close(self): # type: () -> None with use_isolation_scope(self._isolation_scope): @@ -284,8 +282,7 @@ def close(self): # Close the Sentry transaction # This is done here to make sure the Transaction stays # open until all streaming responses are done. - if self._transaction is not None and hasattr(self._transaction, "_context_manager_state"): - self._transaction.__exit__(None, None, None) + finish_running_transaction() except AttributeError: pass except BaseException: diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 150e73661e..8ea540a8a7 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -724,3 +724,10 @@ def get_current_span(scope=None): if TYPE_CHECKING: from sentry_sdk.tracing import Span + + +def finish_running_transaction(): + # type: () -> None + current_scope = sentry_sdk.get_current_scope() + if current_scope._transaction is not None and hasattr(current_scope._transaction, "_context_manager_state"): + current_scope._transaction.__exit__(None, None, None) From 54cda5253c3c21214ce0b9447900b94cacdae938 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 14:47:00 +0100 Subject: [PATCH 12/47] preserve behavior --- sentry_sdk/integrations/wsgi.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index dfe3cc09f9..42700a0c02 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -119,11 +119,12 @@ def __call__(self, environ, start_response): origin=self.span_origin, ) - transaction = sentry_sdk.start_transaction( - transaction, - custom_sampling_context={"wsgi_environ": environ}, - ) - transaction.__enter__() + if transaction is not None: + transaction = sentry_sdk.start_transaction( + transaction, + custom_sampling_context={"wsgi_environ": environ}, + ) + transaction.__enter__() try: response = self.app( From d904f388f7688cca17220e0407cb730943cd7e0b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 14:56:27 +0100 Subject: [PATCH 13/47] Fix --- sentry_sdk/integrations/flask.py | 2 +- sentry_sdk/tracing_utils.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index 4d85d83189..975b776f3e 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -156,7 +156,7 @@ def _request_started(app, **kwargs): def _request_finished(sender, response, **kwargs): - # Manually close the transaction because Bottle does not call `close()` on the WSGI response + # Manually close the transaction because Flask does not call `close()` on the WSGI response finish_running_transaction() diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 8ea540a8a7..cd4e71528c 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -729,5 +729,7 @@ def get_current_span(scope=None): def finish_running_transaction(): # type: () -> None current_scope = sentry_sdk.get_current_scope() - if current_scope._transaction is not None and hasattr(current_scope._transaction, "_context_manager_state"): - current_scope._transaction.__exit__(None, None, None) + if current_scope.transaction is not None and hasattr( + current_scope.transaction, "_context_manager_state" + ): + current_scope.transaction.__exit__(None, None, None) From a614fc655b257ffab98a03193a7477cb649f1a82 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 16:17:33 +0100 Subject: [PATCH 14/47] Closing the transaction always. --- sentry_sdk/integrations/flask.py | 1 + sentry_sdk/integrations/wsgi.py | 31 +++++++++++++++++-------------- sentry_sdk/tracing_utils.py | 6 +++--- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index 975b776f3e..196f92bc2a 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -156,6 +156,7 @@ def _request_started(app, **kwargs): def _request_finished(sender, response, **kwargs): + # type: (Flask, Any, **Any) -> None # Manually close the transaction because Flask does not call `close()` on the WSGI response finish_running_transaction() diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 42700a0c02..780beeb69a 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -257,21 +257,24 @@ def __iter__(self): # type: () -> Iterator[bytes] iterator = iter(self._response) - while True: - with use_isolation_scope(self._isolation_scope): - with use_scope(self._current_scope): - try: - chunk = next(iterator) - except StopIteration: - # Close the Sentry transaction (it could be that response.close() is never called by the framework) - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - finish_running_transaction() - 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: + # Close the Sentry transaction (it could be that response.close() is never called by the framework) + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + finish_running_transaction(self._current_scope) def close(self): # type: () -> None diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index cd4e71528c..90656698f9 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -726,9 +726,9 @@ def get_current_span(scope=None): from sentry_sdk.tracing import Span -def finish_running_transaction(): - # type: () -> None - current_scope = sentry_sdk.get_current_scope() +def finish_running_transaction(scope): + # type: (Optional[sentry_sdk.Scope]) -> 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" ): From 24be3d3a8928229581a65af8fca94e2e3023e5a9 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 16:20:28 +0100 Subject: [PATCH 15/47] . --- sentry_sdk/integrations/wsgi.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 780beeb69a..ef06e76f52 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -264,6 +264,10 @@ def __iter__(self): try: chunk = next(iterator) except StopIteration: + # Close the Sentry transaction (it could be that response.close() is never called by the framework) + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + finish_running_transaction(self._current_scope) break except BaseException: reraise(*_capture_exception()) From e762ef943a93f43674f1ce5be26fe0f53b091f77 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 16:23:33 +0100 Subject: [PATCH 16/47] . --- sentry_sdk/tracing_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 90656698f9..38d631dfd1 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -726,7 +726,7 @@ def get_current_span(scope=None): from sentry_sdk.tracing import Span -def finish_running_transaction(scope): +def finish_running_transaction(scope=None): # type: (Optional[sentry_sdk.Scope]) -> None current_scope = scope or sentry_sdk.get_current_scope() if current_scope.transaction is not None and hasattr( From 198a621c0f18cdd6398bff519f0e1985df983fa9 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 16:41:12 +0100 Subject: [PATCH 17/47] Remove manual closing in bottle, not needed anymore --- sentry_sdk/integrations/bottle.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sentry_sdk/integrations/bottle.py b/sentry_sdk/integrations/bottle.py index c32a4c9e0c..5ae742122e 100644 --- a/sentry_sdk/integrations/bottle.py +++ b/sentry_sdk/integrations/bottle.py @@ -139,9 +139,6 @@ def wrapped_callback(*args, **kwargs): ): _capture_exception(res, handled=True) - # Manually close the transaction because Bottle does not call `close()` on the WSGI response - finish_running_transaction() - return res return wrapped_callback From 26ec3d7642057fe842d1ac63eed35005616c89d0 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 19 Nov 2024 16:43:21 +0100 Subject: [PATCH 18/47] cleanup --- sentry_sdk/integrations/bottle.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/integrations/bottle.py b/sentry_sdk/integrations/bottle.py index 5ae742122e..a2d6b51033 100644 --- a/sentry_sdk/integrations/bottle.py +++ b/sentry_sdk/integrations/bottle.py @@ -2,7 +2,6 @@ import sentry_sdk from sentry_sdk.tracing import SOURCE_FOR_STYLE -from sentry_sdk.tracing_utils import finish_running_transaction from sentry_sdk.utils import ( capture_internal_exceptions, ensure_integration_enabled, From b8c7c1b0689ef349554252567807401ad067a6bc Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 09:20:19 +0100 Subject: [PATCH 19/47] Adding exception info when closing transaction --- sentry_sdk/integrations/wsgi.py | 2 ++ sentry_sdk/tracing_utils.py | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index ef06e76f52..57d2059ee5 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -135,6 +135,8 @@ def __call__(self, environ, start_response): ) except BaseException: reraise(*_capture_exception()) + finally: + finish_running_transaction(current_scope, sys.exc_info()) finally: _wsgi_middleware_applied.set(False) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 38d631dfd1..96783f4c86 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -726,10 +726,13 @@ def get_current_span(scope=None): from sentry_sdk.tracing import Span -def finish_running_transaction(scope=None): - # type: (Optional[sentry_sdk.Scope]) -> None +def finish_running_transaction(scope=None, exc_info=None): + # type: (Optional[sentry_sdk.Scope], Optional[ExcInfo]) -> 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" ): - current_scope.transaction.__exit__(None, None, None) + if exc_info is not None: + current_scope.transaction.__exit__(*exc_info) + else: + current_scope.transaction.__exit__(None, None, None) From 9523882dfdcc34a15364e8ac925994ffa121d4f5 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 09:21:51 +0100 Subject: [PATCH 20/47] Cleanup --- sentry_sdk/integrations/flask.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index 196f92bc2a..128301ddb4 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -7,7 +7,6 @@ from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware from sentry_sdk.scope import should_send_default_pii from sentry_sdk.tracing import SOURCE_FOR_STYLE -from sentry_sdk.tracing_utils import finish_running_transaction from sentry_sdk.utils import ( capture_internal_exceptions, ensure_integration_enabled, @@ -37,7 +36,6 @@ before_render_template, got_request_exception, request_started, - request_finished, ) from markupsafe import Markup except ImportError: @@ -84,7 +82,6 @@ def setup_once(): before_render_template.connect(_add_sentry_trace) request_started.connect(_request_started) - request_finished.connect(_request_finished) got_request_exception.connect(_capture_exception) old_app = Flask.__call__ @@ -155,12 +152,6 @@ def _request_started(app, **kwargs): scope.add_event_processor(evt_processor) -def _request_finished(sender, response, **kwargs): - # type: (Flask, Any, **Any) -> None - # Manually close the transaction because Flask does not call `close()` on the WSGI response - finish_running_transaction() - - class FlaskRequestExtractor(RequestExtractor): def env(self): # type: () -> Dict[str, str] From da5ea3f35bff35c264b43b44b72950de21cfcc2c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 10:07:04 +0100 Subject: [PATCH 21/47] linting --- sentry_sdk/integrations/wsgi.py | 9 +++++---- sentry_sdk/tracing_utils.py | 2 ++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 57d2059ee5..b186e3c42b 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -120,17 +120,18 @@ def __call__(self, environ, start_response): ) if transaction is not None: - transaction = sentry_sdk.start_transaction( + sentry_sdk.start_transaction( transaction, custom_sampling_context={"wsgi_environ": environ}, - ) - transaction.__enter__() + ).__enter__() try: response = self.app( environ, partial( - _sentry_start_response, start_response, transaction + _sentry_start_response, + start_response, + transaction, ), ) except BaseException: diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 96783f4c86..fe21230a56 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -36,6 +36,8 @@ from types import FrameType + from sentry_sdk._types import ExcInfo + SENTRY_TRACE_REGEX = re.compile( "^[ \t]*" # whitespace From 8757d2bc3ee1360672f301cb12ba6389f24f288f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 10:07:53 +0100 Subject: [PATCH 22/47] Cleanup --- sentry_sdk/integrations/wsgi.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index b186e3c42b..b5ac133107 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -267,10 +267,6 @@ def __iter__(self): try: chunk = next(iterator) except StopIteration: - # Close the Sentry transaction (it could be that response.close() is never called by the framework) - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - finish_running_transaction(self._current_scope) break except BaseException: reraise(*_capture_exception()) From dbbe9b822d47a373f060106d79de1070c24a5eaa Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 10:12:23 +0100 Subject: [PATCH 23/47] More cleanup --- sentry_sdk/integrations/wsgi.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index b5ac133107..cddd795a33 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -145,7 +145,6 @@ def __call__(self, environ, start_response): response=response, current_scope=current_scope, isolation_scope=scope, - transaction=transaction, ) @@ -241,20 +240,18 @@ class _ScopedResponse: - WSGI servers streaming responses interleaved from the same thread """ - __slots__ = ("_response", "_current_scope", "_isolation_scope", "_transaction") + __slots__ = ("_response", "_current_scope", "_isolation_scope") def __init__( self, response, # type: Iterator[bytes] current_scope, # type: sentry_sdk.scope.Scope isolation_scope, # type: sentry_sdk.scope.Scope - transaction, # type: Optional[sentry_sdk.tracing.Transaction] ): # type: (...) -> None self._response = response self._current_scope = current_scope self._isolation_scope = isolation_scope - self._transaction = transaction def __iter__(self): # type: () -> Iterator[bytes] From ae9440ff8c862c167d82fa5479c8965386021816 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 11:31:57 +0100 Subject: [PATCH 24/47] Dont close transaction here (that was the whole idea) only in error case. --- sentry_sdk/integrations/wsgi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index cddd795a33..2a8e898c45 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -135,9 +135,9 @@ def __call__(self, environ, start_response): ), ) except BaseException: - reraise(*_capture_exception()) - finally: finish_running_transaction(current_scope, sys.exc_info()) + reraise(*_capture_exception()) + finally: _wsgi_middleware_applied.set(False) From 8ed0ac7426d02dd4924da40d3f926bb07c46ea36 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 14:44:41 +0100 Subject: [PATCH 25/47] fix error capturing --- sentry_sdk/integrations/wsgi.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 2a8e898c45..fee84e302c 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -135,8 +135,10 @@ def __call__(self, environ, start_response): ), ) except BaseException: - finish_running_transaction(current_scope, sys.exc_info()) - reraise(*_capture_exception()) + exc_info = sys.exc_info() + _capture_exception(exc_info) + finish_running_transaction(current_scope, exc_info) + reraise(*exc_info) finally: _wsgi_middleware_applied.set(False) @@ -207,13 +209,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 @@ -274,6 +276,7 @@ def __iter__(self): # Close the Sentry transaction (it could be that response.close() is never called by the framework) # This is done here to make sure the Transaction stays # open until all streaming responses are done. + import ipdb; ipdb.set_trace() finish_running_transaction(self._current_scope) def close(self): From 273e41ebba8e9307038013f82c013b1c9dd075ae Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 14:46:16 +0100 Subject: [PATCH 26/47] remove debugging --- sentry_sdk/integrations/wsgi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index fee84e302c..5beaf859b9 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -276,7 +276,6 @@ def __iter__(self): # Close the Sentry transaction (it could be that response.close() is never called by the framework) # This is done here to make sure the Transaction stays # open until all streaming responses are done. - import ipdb; ipdb.set_trace() finish_running_transaction(self._current_scope) def close(self): From ee257a064516c009dadbd2f22cc4feb4c5ed2acd Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 15:17:35 +0100 Subject: [PATCH 27/47] . --- sentry_sdk/integrations/wsgi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 5beaf859b9..834b65dd67 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -283,12 +283,12 @@ def close(self): with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): try: - self._response.close() # type: ignore - # Close the Sentry transaction # This is done here to make sure the Transaction stays # open until all streaming responses are done. finish_running_transaction() + + self._response.close() # type: ignore except AttributeError: pass except BaseException: From 5d3a04459d34714bddd7c398ac286389168907c3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 15:58:27 +0100 Subject: [PATCH 28/47] Fixed some Flask tests --- tests/integrations/flask/test_flask.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index 6febb12b8b..014cab66bb 100644 --- a/tests/integrations/flask/test_flask.py +++ b/tests/integrations/flask/test_flask.py @@ -394,6 +394,7 @@ def index(): client = app.test_client() response = client.post("/", data=data) assert response.status_code == 200 + response.close() event, transaction_event = events @@ -746,6 +747,7 @@ def hi_tx(): with app.test_client() as client: response = client.get("/message_tx") assert response.status_code == 200 + response.close() message_event, transaction_event = events @@ -938,7 +940,8 @@ def test_response_status_code_not_found_in_transaction_context( envelopes = capture_envelopes() client = app.test_client() - client.get("/not-existing-route") + response = client.get("/not-existing-route") + response.close() sentry_sdk.get_client().flush() @@ -983,14 +986,18 @@ def test_transaction_http_method_default( events = capture_events() client = app.test_client() + response = client.get("/nomessage") assert response.status_code == 200 + response.close() response = client.options("/nomessage") assert response.status_code == 200 + response.close() response = client.head("/nomessage") assert response.status_code == 200 + response.close() (event,) = events @@ -1020,14 +1027,18 @@ def test_transaction_http_method_custom( events = capture_events() client = app.test_client() + response = client.get("/nomessage") assert response.status_code == 200 + response.close() response = client.options("/nomessage") assert response.status_code == 200 + response.close() response = client.head("/nomessage") assert response.status_code == 200 + response.close() assert len(events) == 2 From 6c276ad87f1b41c1199db67ac6bf5c68e1393e57 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 16:56:54 +0100 Subject: [PATCH 29/47] Fixing Django tests --- tests/integrations/django/test_basic.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 0e3f700105..4fa023fc99 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -157,7 +157,8 @@ def test_has_trace_if_performance_enabled(sentry_init, client, capture_events): traces_sample_rate=1.0, ) events = capture_events() - client.head(reverse("view_exc_with_msg")) + response = client.head(reverse("view_exc_with_msg")) + response.close() (msg_event, error_event, transaction_event) = events @@ -213,9 +214,10 @@ def test_trace_from_headers_if_performance_enabled(sentry_init, client, capture_ trace_id = "582b43a4192642f0b136d5159a501701" sentry_trace_header = "{}-{}-{}".format(trace_id, "6e8f22c393e68f19", 1) - client.head( + response = client.head( reverse("view_exc_with_msg"), headers={"sentry-trace": sentry_trace_header} ) + response.close() (msg_event, error_event, transaction_event) = events @@ -1186,7 +1188,8 @@ def test_span_origin(sentry_init, client, capture_events): ) events = capture_events() - client.get(reverse("view_with_signal")) + response = client.get(reverse("view_with_signal")) + response.close() (transaction,) = events @@ -1211,9 +1214,9 @@ def test_transaction_http_method_default(sentry_init, client, capture_events): ) events = capture_events() - client.get("/nomessage") - client.options("/nomessage") - client.head("/nomessage") + client.get("/nomessage").close() + client.options("/nomessage").close() + client.head("/nomessage").close() (event,) = events @@ -1235,9 +1238,9 @@ def test_transaction_http_method_custom(sentry_init, client, capture_events): ) events = capture_events() - client.get("/nomessage") - client.options("/nomessage") - client.head("/nomessage") + client.get("/nomessage").close() + client.options("/nomessage").close() + client.head("/nomessage").close() assert len(events) == 2 From 0ed34bbb6e6b7a336503cb62ed20e2ad67743119 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 17:07:34 +0100 Subject: [PATCH 30/47] Consume response in strawberry tests --- .../strawberry/test_strawberry.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index 7b40b238d2..dcae1d8dc7 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -198,7 +198,7 @@ def test_capture_request_if_available_and_send_pii_is_on( client = client_factory(schema) query = "query ErrorQuery { error }" - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}) + client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).get_data() assert len(events) == 1 @@ -253,7 +253,7 @@ def test_do_not_capture_request_if_send_pii_is_off( client = client_factory(schema) query = "query ErrorQuery { error }" - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}) + client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).get_data() assert len(events) == 1 @@ -293,7 +293,7 @@ def test_breadcrumb_no_operation_name( client = client_factory(schema) query = "{ error }" - client.post("/graphql", json={"query": query}) + client.post("/graphql", json={"query": query}).get_data() assert len(events) == 1 @@ -332,7 +332,7 @@ def test_capture_transaction_on_error( client = client_factory(schema) query = "query ErrorQuery { error }" - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}) + client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).get_data() assert len(events) == 2 (_, transaction_event) = events @@ -409,7 +409,7 @@ def test_capture_transaction_on_success( client = client_factory(schema) query = "query GreetingQuery { hello }" - client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}) + client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).get_data() assert len(events) == 1 (transaction_event,) = events @@ -486,7 +486,7 @@ def test_transaction_no_operation_name( client = client_factory(schema) query = "{ hello }" - client.post("/graphql", json={"query": query}) + client.post("/graphql", json={"query": query}).get_data() assert len(events) == 1 (transaction_event,) = events @@ -566,7 +566,7 @@ def test_transaction_mutation( client = client_factory(schema) query = 'mutation Change { change(attribute: "something") }' - client.post("/graphql", json={"query": query}) + client.post("/graphql", json={"query": query}).get_data() assert len(events) == 1 (transaction_event,) = events @@ -641,7 +641,7 @@ def test_handle_none_query_gracefully( client_factory = request.getfixturevalue(client_factory) client = client_factory(schema) - client.post("/graphql", json={}) + client.post("/graphql", json={}).get_data() assert len(events) == 0, "expected no events to be sent to Sentry" @@ -673,7 +673,7 @@ def test_span_origin( client = client_factory(schema) query = 'mutation Change { change(attribute: "something") }' - client.post("/graphql", json={"query": query}) + client.post("/graphql", json={"query": query}).get_data() (event,) = events @@ -715,7 +715,7 @@ def test_span_origin2( client = client_factory(schema) query = "query GreetingQuery { hello }" - client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}) + client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).get_data() (event,) = events @@ -757,7 +757,7 @@ def test_span_origin3( client = client_factory(schema) query = "subscription { messageAdded { content } }" - client.post("/graphql", json={"query": query}) + client.post("/graphql", json={"query": query}).get_data() (event,) = events From 17e0d3b73313e0d3d91893f47ac9d4614eb7a261 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 20 Nov 2024 17:10:42 +0100 Subject: [PATCH 31/47] Ok --- .../strawberry/test_strawberry.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index dcae1d8dc7..6a61c1bca9 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -198,7 +198,7 @@ def test_capture_request_if_available_and_send_pii_is_on( client = client_factory(schema) query = "query ErrorQuery { error }" - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).get_data() + client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() assert len(events) == 1 @@ -253,7 +253,7 @@ def test_do_not_capture_request_if_send_pii_is_off( client = client_factory(schema) query = "query ErrorQuery { error }" - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).get_data() + client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() assert len(events) == 1 @@ -293,7 +293,7 @@ def test_breadcrumb_no_operation_name( client = client_factory(schema) query = "{ error }" - client.post("/graphql", json={"query": query}).get_data() + client.post("/graphql", json={"query": query}).close() assert len(events) == 1 @@ -332,7 +332,7 @@ def test_capture_transaction_on_error( client = client_factory(schema) query = "query ErrorQuery { error }" - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).get_data() + client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() assert len(events) == 2 (_, transaction_event) = events @@ -409,7 +409,7 @@ def test_capture_transaction_on_success( client = client_factory(schema) query = "query GreetingQuery { hello }" - client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).get_data() + client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).close() assert len(events) == 1 (transaction_event,) = events @@ -486,7 +486,7 @@ def test_transaction_no_operation_name( client = client_factory(schema) query = "{ hello }" - client.post("/graphql", json={"query": query}).get_data() + client.post("/graphql", json={"query": query}).close() assert len(events) == 1 (transaction_event,) = events @@ -566,7 +566,7 @@ def test_transaction_mutation( client = client_factory(schema) query = 'mutation Change { change(attribute: "something") }' - client.post("/graphql", json={"query": query}).get_data() + client.post("/graphql", json={"query": query}).close() assert len(events) == 1 (transaction_event,) = events @@ -641,7 +641,7 @@ def test_handle_none_query_gracefully( client_factory = request.getfixturevalue(client_factory) client = client_factory(schema) - client.post("/graphql", json={}).get_data() + client.post("/graphql", json={}).close() assert len(events) == 0, "expected no events to be sent to Sentry" @@ -673,7 +673,7 @@ def test_span_origin( client = client_factory(schema) query = 'mutation Change { change(attribute: "something") }' - client.post("/graphql", json={"query": query}).get_data() + client.post("/graphql", json={"query": query}).close() (event,) = events @@ -715,7 +715,7 @@ def test_span_origin2( client = client_factory(schema) query = "query GreetingQuery { hello }" - client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).get_data() + client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).close() (event,) = events @@ -757,7 +757,7 @@ def test_span_origin3( client = client_factory(schema) query = "subscription { messageAdded { content } }" - client.post("/graphql", json={"query": query}).get_data() + client.post("/graphql", json={"query": query}).close() (event,) = events From c4eef6ca27568bcc9bc787ec777c1fff0c430a52 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 13:16:23 +0100 Subject: [PATCH 32/47] Explain why we need close --- tests/integrations/flask/test_flask.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index 014cab66bb..4960e84704 100644 --- a/tests/integrations/flask/test_flask.py +++ b/tests/integrations/flask/test_flask.py @@ -394,6 +394,7 @@ def index(): client = app.test_client() response = client.post("/", data=data) assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() event, transaction_event = events @@ -747,6 +748,7 @@ def hi_tx(): with app.test_client() as client: response = client.get("/message_tx") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() message_event, transaction_event = events @@ -941,6 +943,7 @@ def test_response_status_code_not_found_in_transaction_context( client = app.test_client() response = client.get("/not-existing-route") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() sentry_sdk.get_client().flush() @@ -989,14 +992,17 @@ def test_transaction_http_method_default( response = client.get("/nomessage") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() response = client.options("/nomessage") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() response = client.head("/nomessage") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() (event,) = events @@ -1030,14 +1036,17 @@ def test_transaction_http_method_custom( response = client.get("/nomessage") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() response = client.options("/nomessage") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() response = client.head("/nomessage") assert response.status_code == 200 + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() assert len(events) == 2 From 71bebfd95af167a58a13dd2c30464485a7030c50 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 13:18:42 +0100 Subject: [PATCH 33/47] Explain why we need close --- tests/integrations/django/test_basic.py | 32 ++++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 4fa023fc99..9089769d77 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -158,6 +158,7 @@ def test_has_trace_if_performance_enabled(sentry_init, client, capture_events): ) events = capture_events() response = client.head(reverse("view_exc_with_msg")) + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() (msg_event, error_event, transaction_event) = events @@ -217,6 +218,7 @@ def test_trace_from_headers_if_performance_enabled(sentry_init, client, capture_ response = client.head( reverse("view_exc_with_msg"), headers={"sentry-trace": sentry_trace_header} ) + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() (msg_event, error_event, transaction_event) = events @@ -1189,6 +1191,7 @@ def test_span_origin(sentry_init, client, capture_events): events = capture_events() response = client.get(reverse("view_with_signal")) + # Close the response to ensure the WSGI cycle is complete and the transaction is finished response.close() (transaction,) = events @@ -1214,9 +1217,17 @@ def test_transaction_http_method_default(sentry_init, client, capture_events): ) events = capture_events() - client.get("/nomessage").close() - client.options("/nomessage").close() - client.head("/nomessage").close() + response = client.get("/nomessage") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished + response.close() + + response = client.options("/nomessage") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished + response.close() + + response = client.head("/nomessage") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished + response.close() (event,) = events @@ -1238,9 +1249,18 @@ def test_transaction_http_method_custom(sentry_init, client, capture_events): ) events = capture_events() - client.get("/nomessage").close() - client.options("/nomessage").close() - client.head("/nomessage").close() + response = client.get("/nomessage") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished + response.close() + + response = client.options("/nomessage") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished + response.close() + + response = client.head("/nomessage") + # Close the response to ensure the WSGI cycle is complete and the transaction is finished + response.close() + assert len(events) == 2 From c62879bc6cd9b50b47d60562cf10d55a250dc5b6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 13:22:11 +0100 Subject: [PATCH 34/47] Explain why we need close --- tests/integrations/strawberry/test_strawberry.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index 6a61c1bca9..45e7940686 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -198,6 +198,7 @@ def test_capture_request_if_available_and_send_pii_is_on( client = client_factory(schema) query = "query ErrorQuery { error }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() assert len(events) == 1 @@ -253,6 +254,7 @@ def test_do_not_capture_request_if_send_pii_is_off( client = client_factory(schema) query = "query ErrorQuery { error }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() assert len(events) == 1 @@ -293,6 +295,7 @@ def test_breadcrumb_no_operation_name( client = client_factory(schema) query = "{ error }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query}).close() assert len(events) == 1 @@ -332,6 +335,7 @@ def test_capture_transaction_on_error( client = client_factory(schema) query = "query ErrorQuery { error }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() assert len(events) == 2 @@ -409,6 +413,7 @@ def test_capture_transaction_on_success( client = client_factory(schema) query = "query GreetingQuery { hello }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).close() assert len(events) == 1 @@ -486,6 +491,7 @@ def test_transaction_no_operation_name( client = client_factory(schema) query = "{ hello }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query}).close() assert len(events) == 1 @@ -566,6 +572,7 @@ def test_transaction_mutation( client = client_factory(schema) query = 'mutation Change { change(attribute: "something") }' + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query}).close() assert len(events) == 1 @@ -641,6 +648,7 @@ def test_handle_none_query_gracefully( client_factory = request.getfixturevalue(client_factory) client = client_factory(schema) + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={}).close() assert len(events) == 0, "expected no events to be sent to Sentry" @@ -673,6 +681,7 @@ def test_span_origin( client = client_factory(schema) query = 'mutation Change { change(attribute: "something") }' + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query}).close() (event,) = events @@ -715,6 +724,7 @@ def test_span_origin2( client = client_factory(schema) query = "query GreetingQuery { hello }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).close() (event,) = events @@ -757,6 +767,7 @@ def test_span_origin3( client = client_factory(schema) query = "subscription { messageAdded { content } }" + # Close the response to ensure the WSGI cycle is complete and the transaction is finished client.post("/graphql", json={"query": query}).close() (event,) = events From 2452820bf5df7d22cb5cf18096e2c2f8103ba1d6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 14:09:39 +0100 Subject: [PATCH 35/47] Finish the transaction using same scopes as when creating them --- sentry_sdk/integrations/wsgi.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 834b65dd67..6e55c125f7 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -273,10 +273,12 @@ def __iter__(self): yield chunk finally: - # Close the Sentry transaction (it could be that response.close() is never called by the framework) - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - finish_running_transaction(self._current_scope) + with use_isolation_scope(self._isolation_scope): + with use_scope(self._current_scope): + # Close the Sentry transaction (it could be that response.close() is never called by the framework) + # This is done here to make sure the Transaction stays + # open until all streaming responses are done. + finish_running_transaction(self._current_scope) def close(self): # type: () -> None From 22d72f012eebfb797eaa48c6d323f32b91272073 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 14:25:51 +0100 Subject: [PATCH 36/47] Make sure to always consume the response --- tests/integrations/django/test_basic.py | 53 ++++++++----------------- 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 9089769d77..eabe3fdd98 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -51,7 +51,7 @@ def test_view_exceptions(sentry_init, client, capture_exceptions, capture_events sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() events = capture_events() - client.get(reverse("view_exc")) + unpack_werkzeug_response(client.get(reverse("view_exc"))) (error,) = exceptions assert isinstance(error, ZeroDivisionError) @@ -72,7 +72,7 @@ def test_ensures_x_forwarded_header_is_honored_in_sdk_when_enabled_in_django( sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() events = capture_events() - client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"}) + unpack_werkzeug_response(client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"})) (error,) = exceptions assert isinstance(error, ZeroDivisionError) @@ -91,7 +91,7 @@ def test_ensures_x_forwarded_header_is_not_honored_when_unenabled_in_django( sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() events = capture_events() - client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"}) + unpack_werkzeug_response(client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"})) (error,) = exceptions assert isinstance(error, ZeroDivisionError) @@ -103,7 +103,7 @@ def test_ensures_x_forwarded_header_is_not_honored_when_unenabled_in_django( def test_middleware_exceptions(sentry_init, client, capture_exceptions): sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() - client.get(reverse("middleware_exc")) + unpack_werkzeug_response(client.get(reverse("middleware_exc"))) (error,) = exceptions assert isinstance(error, ZeroDivisionError) @@ -932,7 +932,7 @@ def test_render_spans(sentry_init, client, capture_events, render_span_tree): for url, expected_line in views_tests: events = capture_events() - client.get(url) + unpack_werkzeug_response(client.get(url)) transaction = events[0] assert expected_line in render_span_tree(transaction) @@ -971,7 +971,7 @@ def test_middleware_spans(sentry_init, client, capture_events, render_span_tree) ) events = capture_events() - client.get(reverse("message")) + unpack_werkzeug_response(client.get(reverse("message"))) message, transaction = events @@ -988,7 +988,7 @@ def test_middleware_spans_disabled(sentry_init, client, capture_events): ) events = capture_events() - client.get(reverse("message")) + unpack_werkzeug_response(client.get(reverse("message"))) message, transaction = events @@ -1012,7 +1012,7 @@ def test_signals_spans(sentry_init, client, capture_events, render_span_tree): ) events = capture_events() - client.get(reverse("message")) + unpack_werkzeug_response(client.get(reverse("message"))) message, transaction = events @@ -1035,7 +1035,7 @@ def test_signals_spans_disabled(sentry_init, client, capture_events): ) events = capture_events() - client.get(reverse("message")) + unpack_werkzeug_response(client.get(reverse("message"))) message, transaction = events @@ -1065,7 +1065,7 @@ def test_signals_spans_filtering(sentry_init, client, capture_events, render_spa ) events = capture_events() - client.get(reverse("send_myapp_custom_signal")) + unpack_werkzeug_response(client.get(reverse("send_myapp_custom_signal"))) (transaction,) = events @@ -1190,9 +1190,7 @@ def test_span_origin(sentry_init, client, capture_events): ) events = capture_events() - response = client.get(reverse("view_with_signal")) - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() + unpack_werkzeug_response(client.get(reverse("view_with_signal"))) (transaction,) = events @@ -1217,17 +1215,9 @@ def test_transaction_http_method_default(sentry_init, client, capture_events): ) events = capture_events() - response = client.get("/nomessage") - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() - - response = client.options("/nomessage") - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() - - response = client.head("/nomessage") - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() + unpack_werkzeug_response(client.get("/nomessage")) + unpack_werkzeug_response(client.options("/nomessage")) + unpack_werkzeug_response(client.head("/nomessage")) (event,) = events @@ -1249,18 +1239,9 @@ def test_transaction_http_method_custom(sentry_init, client, capture_events): ) events = capture_events() - response = client.get("/nomessage") - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() - - response = client.options("/nomessage") - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() - - response = client.head("/nomessage") - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() - + unpack_werkzeug_response(client.get("/nomessage")) + unpack_werkzeug_response(client.options("/nomessage")) + unpack_werkzeug_response(client.head("/nomessage")) assert len(events) == 2 From c61232601d2f02383c104f15b1a2d06f631c32b4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 14:28:16 +0100 Subject: [PATCH 37/47] Cleanup --- tests/integrations/django/test_basic.py | 20 ++++++++++--------- tests/integrations/flask/test_flask.py | 4 ++-- .../strawberry/test_strawberry.py | 20 ++++++++++++++----- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index eabe3fdd98..243431fdf5 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -72,7 +72,9 @@ def test_ensures_x_forwarded_header_is_honored_in_sdk_when_enabled_in_django( sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() events = capture_events() - unpack_werkzeug_response(client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"})) + unpack_werkzeug_response( + client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"}) + ) (error,) = exceptions assert isinstance(error, ZeroDivisionError) @@ -91,7 +93,9 @@ def test_ensures_x_forwarded_header_is_not_honored_when_unenabled_in_django( sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() events = capture_events() - unpack_werkzeug_response(client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"})) + unpack_werkzeug_response( + client.get(reverse("view_exc"), headers={"X_FORWARDED_HOST": "example.com"}) + ) (error,) = exceptions assert isinstance(error, ZeroDivisionError) @@ -157,9 +161,7 @@ def test_has_trace_if_performance_enabled(sentry_init, client, capture_events): traces_sample_rate=1.0, ) events = capture_events() - response = client.head(reverse("view_exc_with_msg")) - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() + unpack_werkzeug_response(client.head(reverse("view_exc_with_msg"))) (msg_event, error_event, transaction_event) = events @@ -215,11 +217,11 @@ def test_trace_from_headers_if_performance_enabled(sentry_init, client, capture_ trace_id = "582b43a4192642f0b136d5159a501701" sentry_trace_header = "{}-{}-{}".format(trace_id, "6e8f22c393e68f19", 1) - response = client.head( - reverse("view_exc_with_msg"), headers={"sentry-trace": sentry_trace_header} + unpack_werkzeug_response( + client.head( + reverse("view_exc_with_msg"), headers={"sentry-trace": sentry_trace_header} + ) ) - # Close the response to ensure the WSGI cycle is complete and the transaction is finished - response.close() (msg_event, error_event, transaction_event) = events diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index 4960e84704..e2c37aa5f7 100644 --- a/tests/integrations/flask/test_flask.py +++ b/tests/integrations/flask/test_flask.py @@ -989,7 +989,7 @@ def test_transaction_http_method_default( events = capture_events() client = app.test_client() - + response = client.get("/nomessage") assert response.status_code == 200 # Close the response to ensure the WSGI cycle is complete and the transaction is finished @@ -1033,7 +1033,7 @@ def test_transaction_http_method_custom( events = capture_events() client = app.test_client() - + response = client.get("/nomessage") assert response.status_code == 200 # Close the response to ensure the WSGI cycle is complete and the transaction is finished diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index 45e7940686..0aab78f443 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -199,7 +199,9 @@ def test_capture_request_if_available_and_send_pii_is_on( query = "query ErrorQuery { error }" # Close the response to ensure the WSGI cycle is complete and the transaction is finished - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() + client.post( + "/graphql", json={"query": query, "operationName": "ErrorQuery"} + ).close() assert len(events) == 1 @@ -255,7 +257,9 @@ def test_do_not_capture_request_if_send_pii_is_off( query = "query ErrorQuery { error }" # Close the response to ensure the WSGI cycle is complete and the transaction is finished - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() + client.post( + "/graphql", json={"query": query, "operationName": "ErrorQuery"} + ).close() assert len(events) == 1 @@ -336,7 +340,9 @@ def test_capture_transaction_on_error( query = "query ErrorQuery { error }" # Close the response to ensure the WSGI cycle is complete and the transaction is finished - client.post("/graphql", json={"query": query, "operationName": "ErrorQuery"}).close() + client.post( + "/graphql", json={"query": query, "operationName": "ErrorQuery"} + ).close() assert len(events) == 2 (_, transaction_event) = events @@ -414,7 +420,9 @@ def test_capture_transaction_on_success( query = "query GreetingQuery { hello }" # Close the response to ensure the WSGI cycle is complete and the transaction is finished - client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).close() + client.post( + "/graphql", json={"query": query, "operationName": "GreetingQuery"} + ).close() assert len(events) == 1 (transaction_event,) = events @@ -725,7 +733,9 @@ def test_span_origin2( query = "query GreetingQuery { hello }" # Close the response to ensure the WSGI cycle is complete and the transaction is finished - client.post("/graphql", json={"query": query, "operationName": "GreetingQuery"}).close() + client.post( + "/graphql", json={"query": query, "operationName": "GreetingQuery"} + ).close() (event,) = events From d2c44c7d790c9be3fc895d98bb4c4812337ea9ce Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 21 Nov 2024 16:03:28 +0100 Subject: [PATCH 38/47] Finish long running transactions at some point. --- sentry_sdk/integrations/wsgi.py | 15 +++++++++++++ tests/integrations/wsgi/test_wsgi.py | 32 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 6e55c125f7..021fc041fe 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -1,3 +1,4 @@ +from datetime import datetime, timezone import sys from functools import partial @@ -45,6 +46,9 @@ def __call__(self, status, response_headers, exc_info=None): # type: ignore pass +MAX_TRANSACTION_DURATION_MINUTES = 5 + + _wsgi_middleware_applied = ContextVar("sentry_wsgi_middleware_applied") @@ -272,6 +276,17 @@ def __iter__(self): yield chunk + # Finish long running transactions at some point + try: + transaction_duration = (datetime.now(timezone.utc) - self._current_scope.transaction.start_timestamp).total_seconds() / 60 + if transaction_duration > MAX_TRANSACTION_DURATION_MINUTES: + with use_isolation_scope(self._isolation_scope): + with use_scope(self._current_scope): + finish_running_transaction(self._current_scope) + except AttributeError: + # transaction is not there anymore + pass + finally: with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index 656fc1757f..aa7dd471cf 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -1,7 +1,9 @@ +import time from collections import Counter from unittest import mock import pytest +from sentry_sdk.utils import datetime_from_isoformat from werkzeug.test import Client import sentry_sdk @@ -495,3 +497,33 @@ def dogpark(environ, start_response): (event,) = events assert event["contexts"]["trace"]["origin"] == "auto.dogpark.deluxe" + + +def test_long_running_transaction_finished(sentry_init, capture_events): + # we allow transactions to be 0.5 seconds as a maximum + new_max_duration = 0.5 / 60 + + with mock.patch.object(sentry_sdk.integrations.wsgi, "MAX_TRANSACTION_DURATION_MINUTES", new_max_duration): + def generate_content(): + # This response will take 1.5 seconds to generate + for _ in range(15): + time.sleep(0.1) + yield "ok" + + def long_running_app(environ, start_response): + start_response("200 OK", []) + return generate_content() + + sentry_init(send_default_pii=True, traces_sample_rate=1.0) + app = SentryWsgiMiddleware(long_running_app) + + events = capture_events() + + client = Client(app) + response = client.get("/") + _ = response.get_data() + + (transaction,) = events + + transaction_duration = (datetime_from_isoformat(transaction["timestamp"]) - datetime_from_isoformat(transaction["start_timestamp"])).total_seconds() / 60 + assert transaction_duration <= new_max_duration * 1.05 # we allow 2% error margin for processing the request From 308a17953acc3bc9bb957456c594d651fa0fdb68 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 11:23:19 +0100 Subject: [PATCH 39/47] Use Timer to finish transaction --- sentry_sdk/integrations/wsgi.py | 40 +++++++++++++++++++--------- tests/integrations/wsgi/test_wsgi.py | 8 +++--- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 021fc041fe..a4af733bc7 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -1,6 +1,7 @@ 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 @@ -46,7 +47,7 @@ def __call__(self, status, response_headers, exc_info=None): # type: ignore pass -MAX_TRANSACTION_DURATION_MINUTES = 5 +MAX_TRANSACTION_DURATION_SECONDS = 5 * 60 _wsgi_middleware_applied = ContextVar("sentry_wsgi_middleware_applied") @@ -128,7 +129,12 @@ def __call__(self, environ, start_response): transaction, custom_sampling_context={"wsgi_environ": environ}, ).__enter__() - + timer = Timer( + MAX_TRANSACTION_DURATION_SECONDS, + finish_long_running_transaction, + args=(current_scope, scope), + ) + timer.start() try: response = self.app( environ, @@ -235,6 +241,25 @@ def _capture_exception(exc_info=None): return exc_info +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: + with use_isolation_scope(isolation_scope): + with use_scope(current_scope): + finish_running_transaction() + except AttributeError: + # transaction is not there anymore + pass + + class _ScopedResponse: """ Use separate scopes for each response chunk. @@ -276,17 +301,6 @@ def __iter__(self): yield chunk - # Finish long running transactions at some point - try: - transaction_duration = (datetime.now(timezone.utc) - self._current_scope.transaction.start_timestamp).total_seconds() / 60 - if transaction_duration > MAX_TRANSACTION_DURATION_MINUTES: - with use_isolation_scope(self._isolation_scope): - with use_scope(self._current_scope): - finish_running_transaction(self._current_scope) - except AttributeError: - # transaction is not there anymore - pass - finally: with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index aa7dd471cf..bbdfc1cc83 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -501,9 +501,9 @@ def dogpark(environ, start_response): def test_long_running_transaction_finished(sentry_init, capture_events): # we allow transactions to be 0.5 seconds as a maximum - new_max_duration = 0.5 / 60 + new_max_duration = 0.5 - with mock.patch.object(sentry_sdk.integrations.wsgi, "MAX_TRANSACTION_DURATION_MINUTES", new_max_duration): + with mock.patch.object(sentry_sdk.integrations.wsgi, "MAX_TRANSACTION_DURATION_SECONDS", new_max_duration): def generate_content(): # This response will take 1.5 seconds to generate for _ in range(15): @@ -525,5 +525,5 @@ def long_running_app(environ, start_response): (transaction,) = events - transaction_duration = (datetime_from_isoformat(transaction["timestamp"]) - datetime_from_isoformat(transaction["start_timestamp"])).total_seconds() / 60 - assert transaction_duration <= new_max_duration * 1.05 # we allow 2% error margin for processing the request + transaction_duration = (datetime_from_isoformat(transaction["timestamp"]) - datetime_from_isoformat(transaction["start_timestamp"])).total_seconds() + assert transaction_duration <= new_max_duration * 1.2 # we allow 2% margin for processing the request From 2feb29920fc4896262e29b36eb682b3662e6249a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 11:32:15 +0100 Subject: [PATCH 40/47] Format --- tests/integrations/wsgi/test_wsgi.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index bbdfc1cc83..6c0ff44022 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -503,7 +503,12 @@ def test_long_running_transaction_finished(sentry_init, capture_events): # we allow transactions to be 0.5 seconds as a maximum new_max_duration = 0.5 - with mock.patch.object(sentry_sdk.integrations.wsgi, "MAX_TRANSACTION_DURATION_SECONDS", new_max_duration): + with mock.patch.object( + sentry_sdk.integrations.wsgi, + "MAX_TRANSACTION_DURATION_SECONDS", + new_max_duration, + ): + def generate_content(): # This response will take 1.5 seconds to generate for _ in range(15): @@ -525,5 +530,10 @@ def long_running_app(environ, start_response): (transaction,) = events - transaction_duration = (datetime_from_isoformat(transaction["timestamp"]) - datetime_from_isoformat(transaction["start_timestamp"])).total_seconds() - assert transaction_duration <= new_max_duration * 1.2 # we allow 2% margin for processing the request + transaction_duration = ( + datetime_from_isoformat(transaction["timestamp"]) + - datetime_from_isoformat(transaction["start_timestamp"]) + ).total_seconds() + assert ( + transaction_duration <= new_max_duration * 1.2 + ) # we allow 2% margin for processing the request From 2e695004be39bf58da7457551b8b83882627efe0 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 12:27:40 +0100 Subject: [PATCH 41/47] Cleanup --- sentry_sdk/integrations/wsgi.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index a4af733bc7..da59ba6d7c 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -304,21 +304,14 @@ def __iter__(self): finally: with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): - # Close the Sentry transaction (it could be that response.close() is never called by the framework) - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. - finish_running_transaction(self._current_scope) + finish_running_transaction() def close(self): # type: () -> None with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): try: - # Close the Sentry transaction - # This is done here to make sure the Transaction stays - # open until all streaming responses are done. finish_running_transaction() - self._response.close() # type: ignore except AttributeError: pass From 30d18fa91a8550936b08e860c8a4913453dcb41c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 12:39:15 +0100 Subject: [PATCH 42/47] Cancel timer when transaction already finished --- sentry_sdk/integrations/wsgi.py | 14 ++++++++--- sentry_sdk/tracing_utils.py | 8 ++++-- tests/integrations/wsgi/test_wsgi.py | 37 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index da59ba6d7c..81e03b10ad 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -114,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( @@ -124,6 +125,7 @@ def __call__(self, environ, start_response): origin=self.span_origin, ) + timer = None if transaction is not None: sentry_sdk.start_transaction( transaction, @@ -135,6 +137,7 @@ def __call__(self, environ, start_response): args=(current_scope, scope), ) timer.start() + try: response = self.app( environ, @@ -147,7 +150,7 @@ def __call__(self, environ, start_response): except BaseException: exc_info = sys.exc_info() _capture_exception(exc_info) - finish_running_transaction(current_scope, exc_info) + finish_running_transaction(current_scope, exc_info, timer) reraise(*exc_info) finally: @@ -157,6 +160,7 @@ def __call__(self, environ, start_response): response=response, current_scope=current_scope, isolation_scope=scope, + timer=timer, ) @@ -271,18 +275,20 @@ class _ScopedResponse: - WSGI servers streaming responses interleaved from the same thread """ - __slots__ = ("_response", "_current_scope", "_isolation_scope") + __slots__ = ("_response", "_current_scope", "_isolation_scope", "_timer") 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] @@ -304,14 +310,14 @@ def __iter__(self): finally: with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): - finish_running_transaction() + finish_running_transaction(timer=self._timer) def close(self): # type: () -> None with use_isolation_scope(self._isolation_scope): with use_scope(self._current_scope): try: - finish_running_transaction() + finish_running_transaction(timer=self._timer) self._response.close() # type: ignore except AttributeError: pass diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 71ef710259..63f490b0bc 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -37,6 +37,7 @@ from types import FrameType from sentry_sdk._types import ExcInfo + from threading import Timer SENTRY_TRACE_REGEX = re.compile( @@ -743,12 +744,15 @@ def get_current_span(scope=None): from sentry_sdk.tracing import Span -def finish_running_transaction(scope=None, exc_info=None): - # type: (Optional[sentry_sdk.Scope], Optional[ExcInfo]) -> None +def finish_running_transaction(scope=None, exc_info=None, timer=None): + # type: (Optional[sentry_sdk.Scope], Optional[ExcInfo], 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() + if exc_info is not None: current_scope.transaction.__exit__(*exc_info) else: diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index 6c0ff44022..b7e1ac379d 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -537,3 +537,40 @@ def long_running_app(environ, start_response): assert ( transaction_duration <= new_max_duration * 1.2 ) # we allow 2% margin for processing the request + + +def test_long_running_transaction_timer_canceled(sentry_init, capture_events): + # we allow transactions to be 0.5 seconds as a maximum + new_max_duration = 0.5 + + with mock.patch.object( + sentry_sdk.integrations.wsgi, + "MAX_TRANSACTION_DURATION_SECONDS", + new_max_duration, + ): + with mock.patch( + "sentry_sdk.integrations.wsgi.finish_long_running_transaction" + ) as mock_finish: + + def generate_content(): + # This response will take 0.3 seconds to generate + for _ in range(3): + time.sleep(0.1) + yield "ok" + + def long_running_app(environ, start_response): + start_response("200 OK", []) + return generate_content() + + sentry_init(send_default_pii=True, traces_sample_rate=1.0) + app = SentryWsgiMiddleware(long_running_app) + + events = capture_events() + + client = Client(app) + response = client.get("/") + _ = response.get_data() + + (transaction,) = events + + mock_finish.assert_not_called() From 9105254f517017b1bfd5920b10bcdc3d076e63be Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 12:45:52 +0100 Subject: [PATCH 43/47] typing --- sentry_sdk/tracing_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 63f490b0bc..a5d2e69c1f 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -745,7 +745,7 @@ def get_current_span(scope=None): def finish_running_transaction(scope=None, exc_info=None, timer=None): - # type: (Optional[sentry_sdk.Scope], Optional[ExcInfo], 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" From baa549810c50b37a56f0b8d15a119bfa1cc3297b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 13:18:49 +0100 Subject: [PATCH 44/47] Cleanup --- sentry_sdk/integrations/wsgi.py | 40 ++++++++++++++-------------- tests/integrations/wsgi/test_wsgi.py | 2 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 81e03b10ad..a4658a4287 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -133,7 +133,7 @@ def __call__(self, environ, start_response): ).__enter__() timer = Timer( MAX_TRANSACTION_DURATION_SECONDS, - finish_long_running_transaction, + _finish_long_running_transaction, args=(current_scope, scope), ) timer.start() @@ -245,25 +245,6 @@ def _capture_exception(exc_info=None): return exc_info -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: - with use_isolation_scope(isolation_scope): - with use_scope(current_scope): - finish_running_transaction() - except AttributeError: - # transaction is not there anymore - pass - - class _ScopedResponse: """ Use separate scopes for each response chunk. @@ -366,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: + with use_isolation_scope(isolation_scope): + with use_scope(current_scope): + finish_running_transaction() + except AttributeError: + # transaction is not there anymore + pass diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index b7e1ac379d..613eb1afa7 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -535,7 +535,7 @@ def long_running_app(environ, start_response): - datetime_from_isoformat(transaction["start_timestamp"]) ).total_seconds() assert ( - transaction_duration <= new_max_duration * 1.2 + transaction_duration <= new_max_duration * 1.02 ) # we allow 2% margin for processing the request From 709022728fb89751f445c25fac0613f36381c62d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 22 Nov 2024 13:32:04 +0100 Subject: [PATCH 45/47] oops --- tests/integrations/wsgi/test_wsgi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index 613eb1afa7..a4f5ca0623 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -549,7 +549,7 @@ def test_long_running_transaction_timer_canceled(sentry_init, capture_events): new_max_duration, ): with mock.patch( - "sentry_sdk.integrations.wsgi.finish_long_running_transaction" + "sentry_sdk.integrations.wsgi._finish_long_running_transaction" ) as mock_finish: def generate_content(): From 9583029004ca741150e6dfb3321ae00add8090d3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 25 Nov 2024 09:11:08 +0100 Subject: [PATCH 46/47] Improvements from code review --- sentry_sdk/integrations/wsgi.py | 10 +++------- sentry_sdk/tracing_utils.py | 6 +++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index a4658a4287..6d699707a7 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -356,13 +356,9 @@ def _finish_long_running_transaction(current_scope, isolation_scope): 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: - with use_isolation_scope(isolation_scope): - with use_scope(current_scope): - finish_running_transaction() + with use_isolation_scope(isolation_scope): + with use_scope(current_scope): + finish_running_transaction() except AttributeError: # transaction is not there anymore pass diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index a5d2e69c1f..969e0812e4 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -746,13 +746,13 @@ def get_current_span(scope=None): def finish_running_transaction(scope=None, exc_info=None, timer=None): # type: (Optional[sentry_sdk.Scope], Optional[ExcInfo], Optional[Timer]) -> None + if timer is not None: + timer.cancel() + 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() - if exc_info is not None: current_scope.transaction.__exit__(*exc_info) else: From 8574c0612853f825de3dc45dae8248ea14788f03 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 25 Nov 2024 09:27:35 +0100 Subject: [PATCH 47/47] Cleanup --- sentry_sdk/integrations/wsgi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 6d699707a7..751735f462 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -1,4 +1,3 @@ -from datetime import datetime, timezone import sys from functools import partial from threading import Timer