feat(release-health): Enable session tracking by default (getsentry#994)

ahmedetefy · sentry-bot · web-flow · commit 1279eeca6763 · 2021-02-22T15:40:46.000+01:00
* Auto enabled auto session tracking

* Moved auto_session_tracking outof expeirmental features and added it by default

* fix: Formatting

* Fixed type error

* Removed auto_session_tracking from from Experiment type

* Removed redundant default

* Auto detection of session mode when auto_session_tracking is enabled

* fix: Formatting

* Added test that ensures session mode is flips from applicatoin to request in WSGI handler

* New line at end of file

* Linting fixes

* Added default for session_mode in auto_session_tracking

* Added defaults to session_mode to Session class

* Fixed failing test due to changes in WSGI handler tracking requests:

* Reordered param to the end

* fix: Formatting

* Modified flask test to match request mode sessions

* Removed redundant typing Union

Co-authored-by: sentry-bot &lt;markus+ghbot@sentry.io&gt;
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
@@ -105,12 +105,8 @@ def _capture_envelope(envelope):
         try:
             _client_init_debug.set(self.options["debug"])
             self.transport = make_transport(self.options)
-            session_mode = self.options["_experiments"].get(
-                "session_mode", "application"
-            )
-            self.session_flusher = SessionFlusher(
-                capture_func=_capture_envelope, session_mode=session_mode
-            )
+
+            self.session_flusher = SessionFlusher(capture_func=_capture_envelope)
 
             request_bodies = ("always", "never", "small", "medium")
             if self.options["request_bodies"] not in request_bodies:
diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py
@@ -31,7 +31,6 @@
         {
             "max_spans": Optional[int],
             "record_sql_params": Optional[bool],
-            "auto_session_tracking": Optional[bool],
             "smart_transaction_trimming": Optional[bool],
         },
         total=False,
@@ -75,6 +74,7 @@ def __init__(
         traces_sample_rate=None,  # type: Optional[float]
         traces_sampler=None,  # type: Optional[TracesSampler]
         auto_enabling_integrations=True,  # type: bool
+        auto_session_tracking=True,  # type: bool
         _experiments={},  # type: Experiments  # noqa: B006
     ):
         # type: (...) -> None
diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py
@@ -623,7 +623,9 @@ def inner():
 
         return inner()
 
-    def start_session(self):
+    def start_session(
+        self, session_mode="application"  # type: str
+    ):
         # type: (...) -> None
         """Starts a new session."""
         self.end_session()
@@ -632,6 +634,7 @@ def start_session(self):
             release=client.options["release"] if client else None,
             environment=client.options["environment"] if client else None,
             user=scope._user,
+            session_mode=session_mode,
         )
 
     def end_session(self):
diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py
@@ -103,7 +103,7 @@ def __call__(self, environ, start_response):
         _wsgi_middleware_applied.set(True)
         try:
             hub = Hub(Hub.current)
-            with auto_session_tracking(hub):
+            with auto_session_tracking(hub, session_mode="request"):
                 with hub:
                     with capture_internal_exceptions():
                         with hub.configure_scope() as scope:
diff --git a/sentry_sdk/session.py b/sentry_sdk/session.py
@@ -42,6 +42,7 @@ def __init__(
         ip_address=None,  # type: Optional[str]
         errors=None,  # type: Optional[int]
         user=None,  # type: Optional[Any]
+        session_mode="application",  # type: str
     ):
         # type: (...) -> None
         if sid is None:
@@ -58,6 +59,7 @@ def __init__(
         self.duration = None  # type: Optional[float]
         self.user_agent = None  # type: Optional[str]
         self.ip_address = None  # type: Optional[str]
+        self.session_mode = session_mode  # type: str
         self.errors = 0
 
         self.update(
diff --git a/sentry_sdk/sessions.py b/sentry_sdk/sessions.py
@@ -25,20 +25,20 @@ def is_auto_session_tracking_enabled(hub=None):
         hub = sentry_sdk.Hub.current
     should_track = hub.scope._force_auto_session_tracking
     if should_track is None:
-        exp = hub.client.options["_experiments"] if hub.client else {}
-        should_track = exp.get("auto_session_tracking")
+        client_options = hub.client.options if hub.client else {}
+        should_track = client_options["auto_session_tracking"]
     return should_track
 
 
 @contextmanager
-def auto_session_tracking(hub=None):
-    # type: (Optional[sentry_sdk.Hub]) -> Generator[None, None, None]
+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."""
     if hub is None:
         hub = sentry_sdk.Hub.current
     should_track = is_auto_session_tracking_enabled(hub)
     if should_track:
-        hub.start_session()
+        hub.start_session(session_mode=session_mode)
     try:
         yield
     finally:
@@ -59,12 +59,10 @@ class SessionFlusher(object):
     def __init__(
         self,
         capture_func,  # type: Callable[[Envelope], None]
-        session_mode,  # type: str
         flush_interval=60,  # type: int
     ):
         # type: (...) -> None
         self.capture_func = capture_func
-        self.session_mode = session_mode
         self.flush_interval = flush_interval
         self.pending_sessions = []  # type: List[Any]
         self.pending_aggregates = {}  # type: Dict[Any, Any]
@@ -158,7 +156,7 @@ def add_session(
         self, session  # type: Session
     ):
         # type: (...) -> None
-        if self.session_mode == "request":
+        if session.session_mode == "request":
             self.add_aggregate_session(session)
         else:
             self.pending_sessions.append(session.to_json())
diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py
@@ -247,9 +247,6 @@ def test_flask_session_tracking(sentry_init, capture_envelopes, app):
     sentry_init(
         integrations=[flask_sentry.FlaskIntegration()],
         release="demo-release",
-        _experiments=dict(
-            auto_session_tracking=True,
-        ),
     )
 
     @app.route("/")
@@ -276,16 +273,15 @@ def index():
     first_event = first_event.get_event()
     error_event = error_event.get_event()
     session = session.items[0].payload.json
+    aggregates = session["aggregates"]
 
     assert first_event["exception"]["values"][0]["type"] == "ValueError"
     assert error_event["exception"]["values"][0]["type"] == "ZeroDivisionError"
-    assert session["status"] == "crashed"
-    assert session["did"] == "42"
-    assert session["errors"] == 2
-    assert session["init"]
+
+    assert len(aggregates) == 1
+    assert aggregates[0]["crashed"] == 1
+    assert aggregates[0]["started"]
     assert session["attrs"]["release"] == "demo-release"
-    assert session["attrs"]["ip_address"] == "1.2.3.4"
-    assert session["attrs"]["user_agent"] == "blafasel/1.0"
 
 
 @pytest.mark.parametrize("data", [{}, []], ids=["empty-dict", "empty-list"])
diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py
@@ -1,6 +1,7 @@
 from werkzeug.test import Client
 import pytest
 
+import sentry_sdk
 from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware
 
 try:
@@ -201,3 +202,37 @@ def app(environ, start_response):
             }
         )
     )
+
+
+def test_session_mode_defaults_to_request_mode_in_wsgi_handler(
+    capture_envelopes, sentry_init
+):
+    """
+    Test that ensures that even though the default `session_mode` for
+    auto_session_tracking is `application`, that flips to `request` when we are
+    in the WSGI handler
+    """
+
+    def app(environ, start_response):
+        start_response("200 OK", [])
+        return ["Go get the ball! Good dog!"]
+
+    traces_sampler = mock.Mock(return_value=True)
+    sentry_init(send_default_pii=True, traces_sampler=traces_sampler)
+
+    app = SentryWsgiMiddleware(app)
+    envelopes = capture_envelopes()
+
+    client = Client(app)
+
+    client.get("/dogs/are/great/")
+
+    sentry_sdk.flush()
+
+    sess = envelopes[1]
+    assert len(sess.items) == 1
+    sess_event = sess.items[0].payload.json
+
+    aggregates = sess_event["aggregates"]
+    assert len(aggregates) == 1
+    assert aggregates[0]["exited"] == 1
diff --git a/tests/test_sessions.py b/tests/test_sessions.py
@@ -47,13 +47,12 @@ def test_aggregates(sentry_init, capture_envelopes):
     sentry_init(
         release="fun-release",
         environment="not-fun-env",
-        _experiments={"auto_session_tracking": True, "session_mode": "request"},
     )
     envelopes = capture_envelopes()
 
     hub = Hub.current
 
-    with auto_session_tracking():
+    with auto_session_tracking(session_mode="request"):
         with sentry_sdk.push_scope():
             try:
                 with sentry_sdk.configure_scope() as scope:
@@ -62,10 +61,10 @@ def test_aggregates(sentry_init, capture_envelopes):
             except Exception:
                 sentry_sdk.capture_exception()
 
-    with auto_session_tracking():
+    with auto_session_tracking(session_mode="request"):
         pass
 
-    hub.start_session()
+    hub.start_session(session_mode="request")
     hub.end_session()
 
     sentry_sdk.flush()
@@ -85,3 +84,38 @@ def test_aggregates(sentry_init, capture_envelopes):
     assert len(aggregates) == 1
     assert aggregates[0]["exited"] == 2
     assert aggregates[0]["errored"] == 1
+
+
+def test_aggregates_explicitly_disabled_session_tracking_request_mode(
+    sentry_init, capture_envelopes
+):
+    sentry_init(
+        release="fun-release", environment="not-fun-env", auto_session_tracking=False
+    )
+    envelopes = capture_envelopes()
+
+    hub = Hub.current
+
+    with auto_session_tracking(session_mode="request"):
+        with sentry_sdk.push_scope():
+            try:
+                raise Exception("all is wrong")
+            except Exception:
+                sentry_sdk.capture_exception()
+
+    with auto_session_tracking(session_mode="request"):
+        pass
+
+    hub.start_session(session_mode="request")
+    hub.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]
