Send all events to /envelope endpoint when tracing is enabled (#2009)

Author: antonpirker

sentry_sdk/client.py

@@ -18,7 +18,7 @@
     logger,
 )
 from sentry_sdk.serializer import serialize
-from sentry_sdk.tracing import trace
+from sentry_sdk.tracing import trace, has_tracing_enabled
 from sentry_sdk.transport import make_transport
 from sentry_sdk.consts import (
     DEFAULT_OPTIONS,
@@ -495,6 +495,8 @@ def capture_event(
         if not is_transaction and not self._should_sample_error(event):
             return None
 
+        tracing_enabled = has_tracing_enabled(self.options)
+        is_checkin = event_opt.get("type") == "check_in"
         attachments = hint.get("attachments")
 
         dynamic_sampling_context = (
@@ -503,12 +505,12 @@ def capture_event(
             .pop("dynamic_sampling_context", {})
         )
 
-        is_checkin = event_opt.get("type") == "check_in"
-
-        # Transactions, events with attachments, and checkins should go to the /envelope/
-        # endpoint.
-        if is_transaction or is_checkin or attachments:
-
+        # If tracing is enabled all events should go to /envelope endpoint.
+        # If no tracing is enabled only transactions, events with attachments, and checkins should go to the /envelope endpoint.
+        should_use_envelope_endpoint = (
+            tracing_enabled or is_transaction or is_checkin or bool(attachments)
+        )
+        if should_use_envelope_endpoint:
             headers = {
                 "event_id": event_opt["event_id"],
                 "sent_at": format_timestamp(datetime.utcnow()),
@@ -532,9 +534,11 @@ def capture_event(
                 envelope.add_item(attachment.to_envelope_item())
 
             self.transport.capture_envelope(envelope)
+
         else:
-            # All other events go to the /store/ endpoint.
+            # All other events go to the legacy /store/ endpoint (will be removed in the future).
             self.transport.capture_event(event_opt)
+
         return event_id
 
     def capture_session(

tests/conftest.py

@@ -157,11 +157,11 @@ def check_string_keys(map):
 
     def check_envelope(envelope):
         with capture_internal_exceptions():
-            # Assert error events are sent without envelope to server, for compat.
-            # This does not apply if any item in the envelope is an attachment.
-            if not any(x.type == "attachment" for x in envelope.items):
-                assert not any(item.data_category == "error" for item in envelope.items)
-                assert not any(item.get_event() is not None for item in envelope.items)
+            # There used to be a check here for errors are not sent in envelopes.
+            # We changed the behaviour to send errors in envelopes when tracing is enabled.
+            # This is checked in test_client.py::test_sending_events_with_tracing
+            # and test_client.py::test_sending_events_with_no_tracing
+            pass
 
     def inner(client):
         monkeypatch.setattr(

tests/integrations/gcp/test_gcp.py

@@ -94,8 +94,8 @@ def init_sdk(timeout_warning=False, **extra_init_args):
 def run_cloud_function():
     def inner(code, subprocess_kwargs=()):
 
-        event = []
-        envelope = []
+        events = []
+        envelopes = []
         return_value = None
 
         # STEP : Create a zip of cloud function
@@ -133,10 +133,10 @@ def inner(code, subprocess_kwargs=()):
                 print("GCP:", line)
                 if line.startswith("EVENT: "):
                     line = line[len("EVENT: ") :]
-                    event = json.loads(line)
+                    events.append(json.loads(line))
                 elif line.startswith("ENVELOPE: "):
                     line = line[len("ENVELOPE: ") :]
-                    envelope = json.loads(line)
+                    envelopes.append(json.loads(line))
                 elif line.startswith("RETURN VALUE: "):
                     line = line[len("RETURN VALUE: ") :]
                     return_value = json.loads(line)
@@ -145,13 +145,13 @@ def inner(code, subprocess_kwargs=()):
 
             stream.close()
 
-        return envelope, event, return_value
+        return envelopes, events, return_value
 
     return inner
 
 
 def test_handled_exception(run_cloud_function):
-    envelope, event, return_value = run_cloud_function(
+    _, events, return_value = run_cloud_function(
         dedent(
             """
         functionhandler = None
@@ -168,16 +168,16 @@ def cloud_function(functionhandler, event):
         """
         )
     )
-    assert event["level"] == "error"
-    (exception,) = event["exception"]["values"]
+    assert events[0]["level"] == "error"
+    (exception,) = events[0]["exception"]["values"]
 
     assert exception["type"] == "Exception"
     assert exception["value"] == "something went wrong"
     assert exception["mechanism"] == {"type": "gcp", "handled": False}
 
 
 def test_unhandled_exception(run_cloud_function):
-    envelope, event, return_value = run_cloud_function(
+    _, events, _ = run_cloud_function(
         dedent(
             """
         functionhandler = None
@@ -195,16 +195,16 @@ def cloud_function(functionhandler, event):
         """
         )
     )
-    assert event["level"] == "error"
-    (exception,) = event["exception"]["values"]
+    assert events[0]["level"] == "error"
+    (exception,) = events[0]["exception"]["values"]
 
     assert exception["type"] == "ZeroDivisionError"
     assert exception["value"] == "division by zero"
     assert exception["mechanism"] == {"type": "gcp", "handled": False}
 
 
 def test_timeout_error(run_cloud_function):
-    envelope, event, return_value = run_cloud_function(
+    _, events, _ = run_cloud_function(
         dedent(
             """
         functionhandler = None
@@ -222,8 +222,8 @@ def cloud_function(functionhandler, event):
         """
         )
     )
-    assert event["level"] == "error"
-    (exception,) = event["exception"]["values"]
+    assert events[0]["level"] == "error"
+    (exception,) = events[0]["exception"]["values"]
 
     assert exception["type"] == "ServerlessTimeoutWarning"
     assert (
@@ -234,7 +234,7 @@ def cloud_function(functionhandler, event):
 
 
 def test_performance_no_error(run_cloud_function):
-    envelope, event, return_value = run_cloud_function(
+    envelopes, _, _ = run_cloud_function(
         dedent(
             """
         functionhandler = None
@@ -252,15 +252,15 @@ def cloud_function(functionhandler, event):
         )
     )
 
-    assert envelope["type"] == "transaction"
-    assert envelope["contexts"]["trace"]["op"] == "function.gcp"
-    assert envelope["transaction"].startswith("Google Cloud function")
-    assert envelope["transaction_info"] == {"source": "component"}
-    assert envelope["transaction"] in envelope["request"]["url"]
+    assert envelopes[0]["type"] == "transaction"
+    assert envelopes[0]["contexts"]["trace"]["op"] == "function.gcp"
+    assert envelopes[0]["transaction"].startswith("Google Cloud function")
+    assert envelopes[0]["transaction_info"] == {"source": "component"}
+    assert envelopes[0]["transaction"] in envelopes[0]["request"]["url"]
 
 
 def test_performance_error(run_cloud_function):
-    envelope, event, return_value = run_cloud_function(
+    envelopes, events, _ = run_cloud_function(
         dedent(
             """
         functionhandler = None
@@ -278,17 +278,18 @@ def cloud_function(functionhandler, event):
         )
     )
 
-    assert envelope["type"] == "transaction"
-    assert envelope["contexts"]["trace"]["op"] == "function.gcp"
-    assert envelope["transaction"].startswith("Google Cloud function")
-    assert envelope["transaction"] in envelope["request"]["url"]
-    assert event["level"] == "error"
-    (exception,) = event["exception"]["values"]
+    assert envelopes[0]["level"] == "error"
+    (exception,) = envelopes[0]["exception"]["values"]
 
     assert exception["type"] == "Exception"
     assert exception["value"] == "something went wrong"
     assert exception["mechanism"] == {"type": "gcp", "handled": False}
 
+    assert envelopes[1]["type"] == "transaction"
+    assert envelopes[1]["contexts"]["trace"]["op"] == "function.gcp"
+    assert envelopes[1]["transaction"].startswith("Google Cloud function")
+    assert envelopes[1]["transaction"] in envelopes[0]["request"]["url"]
+
 
 def test_traces_sampler_gets_correct_values_in_sampling_context(
     run_cloud_function, DictionaryContaining  # noqa:N803

tests/test_client.py

@@ -886,7 +886,7 @@ def test_init_string_types(dsn, sentry_init):
     )
 
 
-def test_envelope_types():
+def test_sending_events_with_tracing():
     """
     Tests for calling the right transport method (capture_event vs
     capture_envelope) from the SDK client for different data types.
@@ -902,8 +902,56 @@ def capture_envelope(self, envelope):
         def capture_event(self, event):
             events.append(event)
 
-    with Hub(Client(traces_sample_rate=1.0, transport=CustomTransport())):
-        event_id = capture_message("hello")
+    with Hub(Client(enable_tracing=True, transport=CustomTransport())):
+        try:
+            1 / 0
+        except Exception:
+            event_id = capture_exception()
+
+        # Assert error events get passed in via capture_envelope
+        assert not events
+        envelope = envelopes.pop()
+        (item,) = envelope.items
+        assert item.data_category == "error"
+        assert item.headers.get("type") == "event"
+        assert item.get_event()["event_id"] == event_id
+
+        with start_transaction(name="foo"):
+            pass
+
+        # Assert transactions get passed in via capture_envelope
+        assert not events
+        envelope = envelopes.pop()
+
+        (item,) = envelope.items
+        assert item.data_category == "transaction"
+        assert item.headers.get("type") == "transaction"
+
+    assert not envelopes
+    assert not events
+
+
+def test_sending_events_with_no_tracing():
+    """
+    Tests for calling the right transport method (capture_event vs
+    capture_envelope) from the SDK client for different data types.
+    """
+
+    envelopes = []
+    events = []
+
+    class CustomTransport(Transport):
+        def capture_envelope(self, envelope):
+            envelopes.append(envelope)
+
+        def capture_event(self, event):
+            events.append(event)
+
+    with Hub(Client(enable_tracing=False, transport=CustomTransport())):
+        try:
+            1 / 0
+        except Exception:
+            event_id = capture_exception()
 
         # Assert error events get passed in via capture_event
         assert not envelopes
@@ -917,11 +965,7 @@ def capture_event(self, event):
 
         # Assert transactions get passed in via capture_envelope
         assert not events
-        envelope = envelopes.pop()
-
-        (item,) = envelope.items
-        assert item.data_category == "transaction"
-        assert item.headers.get("type") == "transaction"
+        assert not envelopes
 
     assert not envelopes
     assert not events