-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref(sessions): Deprecate hub-based sessions.py
logic
#3419
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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): | ||||||||||
|
@@ -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: | ||||||||||
|
@@ -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: | ||||||||||
|
@@ -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() | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you also add this comment to the next test? (can not suggest this change from the github ui) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accidentally merged before fixed, opened #3430 with this change |
||||||||||
|
||||||||||
|
||||||||||
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", | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are always also a kind of documentation on how to use the code.
Could you add comments to the three sessions to mark what aggregate status is expected for each one of them?
Please add those comments to all similar tests in this file. This will help us in the future (or others) to understand what is going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied these tests from the existing tests, and modified them to call the new API. To be completely honest, I don't really understand these tests or know what aggregate status is expected from them.
Since all I am doing is modifying tests to call the new API (and keeping around a copy to still test the old deprecated API), I think it is out of scope to add this documentation here. But, I think this is a good idea, so I will open a new issue.