Skip to content

Commit

Permalink
ref(sessions): Deprecate hub-based sessions.py logic (getsentry#3419)
Browse files Browse the repository at this point in the history
Make several changes to prepare for fully removing Hubs in the next major:
  - Deprecate the Hub-based `auto_session_tracking` function, replacing it with a new Scope-based function called `track_session`
  - Deprecate the scope-based `auto_session_tracking_scope` in favor of the new `track_session` function
  - Change usages of `auto_session_tracking_scope` to `track_sessions`. There are no usages of `auto_session_tracking` outside of tests.
  - Run all tests that were previously run against `auto_session_tracking` also against the new `track_session`. Previously, `auto_session_tracking_scope` was completely untested.

Fixes getsentry#3417
  • Loading branch information
szokeasaurusrex authored and arjenzorgdoc committed Sep 30, 2024
1 parent f8f05a4 commit 68d4fe8
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 15 deletions.
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from sentry_sdk.consts import OP, SPANSTATUS, SPANDATA
from sentry_sdk.integrations import Integration, DidNotEnable
from sentry_sdk.integrations.logging import ignore_logger
from sentry_sdk.sessions import auto_session_tracking_scope
from sentry_sdk.sessions import track_session
from sentry_sdk.integrations._wsgi_common import (
_filter_headers,
request_body_within_bounds,
Expand Down Expand Up @@ -105,7 +105,7 @@ async def sentry_app_handle(self, request, *args, **kwargs):
weak_request = weakref.ref(request)

with sentry_sdk.isolation_scope() as scope:
with auto_session_tracking_scope(scope, session_mode="request"):
with track_session(scope, session_mode="request"):
# Scope data will not leak between requests because aiohttp
# create a task to wrap each request.
scope.generate_propagation_context()
Expand Down
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
_get_request_data,
_get_url,
)
from sentry_sdk.sessions import auto_session_tracking_scope
from sentry_sdk.sessions import track_session
from sentry_sdk.tracing import (
SOURCE_FOR_STYLE,
TRANSACTION_SOURCE_ROUTE,
Expand Down Expand Up @@ -169,7 +169,7 @@ async def _run_app(self, scope, receive, send, asgi_version):
_asgi_middleware_applied.set(True)
try:
with sentry_sdk.isolation_scope() as sentry_scope:
with auto_session_tracking_scope(sentry_scope, session_mode="request"):
with track_session(sentry_scope, session_mode="request"):
sentry_scope.clear_breadcrumbs()
sentry_scope._name = "asgi"
processor = partial(self.event_processor, asgi_scope=scope)
Expand Down
6 changes: 2 additions & 4 deletions sentry_sdk/integrations/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
from sentry_sdk.consts import OP
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.integrations._wsgi_common import _filter_headers
from sentry_sdk.sessions import (
auto_session_tracking_scope as auto_session_tracking,
) # When the Hub is removed, this should be renamed (see comment in sentry_sdk/sessions.py)
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 (
Expand Down Expand Up @@ -83,7 +81,7 @@ def __call__(self, environ, start_response):
_wsgi_middleware_applied.set(True)
try:
with sentry_sdk.isolation_scope() as scope:
with auto_session_tracking(scope, session_mode="request"):
with track_session(scope, session_mode="request"):
with capture_internal_exceptions():
scope.clear_breadcrumbs()
scope._name = "wsgi"
Expand Down
34 changes: 28 additions & 6 deletions sentry_sdk/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,15 @@ def is_auto_session_tracking_enabled(hub=None):
@contextmanager
def auto_session_tracking(hub=None, session_mode="application"):
# type: (Optional[sentry_sdk.Hub], str) -> Generator[None, None, None]
"""Starts and stops a session automatically around a block."""
# TODO: add deprecation warning
"""DEPRECATED: Use track_session instead
Starts and stops a session automatically around a block.
"""
warnings.warn(
"This function is deprecated and will be removed in the next major release. "
"Use track_session instead.",
DeprecationWarning,
stacklevel=2,
)

if hub is None:
hub = sentry_sdk.Hub.current
Expand Down Expand Up @@ -98,13 +105,28 @@ def _is_auto_session_tracking_enabled(scope):
@contextmanager
def auto_session_tracking_scope(scope, session_mode="application"):
# type: (sentry_sdk.Scope, str) -> Generator[None, None, None]
"""
"""DEPRECATED: This function is a deprecated alias for track_session.
Starts and stops a session automatically around a block.
"""

TODO: This uses the new scopes. When the Hub is removed, the function
auto_session_tracking should be removed and this function
should be renamed to auto_session_tracking.
warnings.warn(
"This function is a deprecated alias for track_session and will be removed in the next major release.",
DeprecationWarning,
stacklevel=2,
)

with track_session(scope, session_mode=session_mode):
yield


@contextmanager
def track_session(scope, session_mode="application"):
# type: (sentry_sdk.Scope, str) -> Generator[None, None, None]
"""
Start a new session in the provided scope, assuming session tracking is enabled.
This is a no-op context manager if session tracking is not enabled.
"""

should_track = _is_auto_session_tracking_enabled(scope)
if should_track:
scope.start_session(session_mode=session_mode)
Expand Down
106 changes: 105 additions & 1 deletion tests/test_sessions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from unittest import mock

import sentry_sdk
from sentry_sdk.sessions import auto_session_tracking
from sentry_sdk.sessions import auto_session_tracking, track_session


def sorted_aggregates(item):
Expand Down Expand Up @@ -50,6 +50,48 @@ def test_aggregates(sentry_init, capture_envelopes):
)
envelopes = capture_envelopes()

with sentry_sdk.isolation_scope() as scope:
with track_session(scope, session_mode="request"):
try:
scope.set_user({"id": "42"})
raise Exception("all is wrong")
except Exception:
sentry_sdk.capture_exception()

with sentry_sdk.isolation_scope() as scope:
with track_session(scope, session_mode="request"):
pass

sentry_sdk.get_isolation_scope().start_session(session_mode="request")
sentry_sdk.get_isolation_scope().end_session()
sentry_sdk.flush()

assert len(envelopes) == 2
assert envelopes[0].get_event() is not None

sess = envelopes[1]
assert len(sess.items) == 1
sess_event = sess.items[0].payload.json
assert sess_event["attrs"] == {
"release": "fun-release",
"environment": "not-fun-env",
}

aggregates = sorted_aggregates(sess_event)
assert len(aggregates) == 1
assert aggregates[0]["exited"] == 2
assert aggregates[0]["errored"] == 1


def test_aggregates_deprecated(
sentry_init, capture_envelopes, suppress_deprecation_warnings
):
sentry_init(
release="fun-release",
environment="not-fun-env",
)
envelopes = capture_envelopes()

with auto_session_tracking(session_mode="request"):
with sentry_sdk.new_scope() as scope:
try:
Expand Down Expand Up @@ -90,6 +132,39 @@ def test_aggregates_explicitly_disabled_session_tracking_request_mode(
)
envelopes = capture_envelopes()

with sentry_sdk.isolation_scope() as scope:
with track_session(scope, session_mode="request"):
try:
raise Exception("all is wrong")
except Exception:
sentry_sdk.capture_exception()

with sentry_sdk.isolation_scope() as scope:
with track_session(scope, session_mode="request"):
pass

sentry_sdk.get_isolation_scope().start_session(session_mode="request")
sentry_sdk.get_isolation_scope().end_session()
sentry_sdk.flush()

sess = envelopes[1]
assert len(sess.items) == 1
sess_event = sess.items[0].payload.json

aggregates = sorted_aggregates(sess_event)
assert len(aggregates) == 1
assert aggregates[0]["exited"] == 1
assert "errored" not in aggregates[0]


def test_aggregates_explicitly_disabled_session_tracking_request_mode_deprecated(
sentry_init, capture_envelopes, suppress_deprecation_warnings
):
sentry_init(
release="fun-release", environment="not-fun-env", auto_session_tracking=False
)
envelopes = capture_envelopes()

with auto_session_tracking(session_mode="request"):
with sentry_sdk.new_scope():
try:
Expand Down Expand Up @@ -120,6 +195,35 @@ def test_no_thread_on_shutdown_no_errors(sentry_init):
environment="not-fun-env",
)

# make it seem like the interpreter is shutting down
with mock.patch(
"threading.Thread.start",
side_effect=RuntimeError("can't create new thread at interpreter shutdown"),
):
with sentry_sdk.isolation_scope() as scope:
with track_session(scope, session_mode="request"):
try:
raise Exception("all is wrong")
except Exception:
sentry_sdk.capture_exception()

with sentry_sdk.isolation_scope() as scope:
with track_session(scope, session_mode="request"):
pass

sentry_sdk.get_isolation_scope().start_session(session_mode="request")
sentry_sdk.get_isolation_scope().end_session()
sentry_sdk.flush()


def test_no_thread_on_shutdown_no_errors_deprecated(
sentry_init, suppress_deprecation_warnings
):
sentry_init(
release="fun-release",
environment="not-fun-env",
)

# make it seem like the interpreter is shutting down
with mock.patch(
"threading.Thread.start",
Expand Down

0 comments on commit 68d4fe8

Please sign in to comment.