fix(transport): Use correct data category for transaction events (#826)

Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Co-authored-by: sentry-bot <markus+ghbot@sentry.io>

Author: 3 people

sentry_sdk/envelope.py

@@ -20,13 +20,6 @@
     from sentry_sdk._types import Event, EventDataCategory
 
 
-def get_event_data_category(event):
-    # type: (Event) -> EventDataCategory
-    if event.get("type") == "transaction":
-        return "transaction"
-    return "error"
-
-
 class Envelope(object):
     def __init__(
         self,
@@ -230,22 +223,27 @@ def __repr__(self):
     @property
     def data_category(self):
         # type: (...) -> EventDataCategory
-        rv = "default"  # type: Any
-        event = self.get_event()
-        if event is not None:
-            rv = get_event_data_category(event)
+        ty = self.headers.get("type")
+        if ty == "session":
+            return "session"
+        elif ty == "attachment":
+            return "attachment"
+        elif ty == "transaction":
+            return "transaction"
+        elif ty == "event":
+            return "error"
         else:
-            ty = self.headers.get("type")
-            if ty in ("session", "attachment"):
-                rv = ty
-        return rv
+            return "default"
 
     def get_bytes(self):
         # type: (...) -> bytes
         return self.payload.get_bytes()
 
     def get_event(self):
         # type: (...) -> Optional[Event]
+        """
+        Returns an error event if there is one.
+        """
         if self.headers.get("type") == "event" and self.payload.json is not None:
             return self.payload.json
         return None

sentry_sdk/transport.py

@@ -9,7 +9,7 @@
 
 from sentry_sdk.utils import Dsn, logger, capture_internal_exceptions, json_dumps
 from sentry_sdk.worker import BackgroundWorker
-from sentry_sdk.envelope import Envelope, get_event_data_category
+from sentry_sdk.envelope import Envelope
 
 from sentry_sdk._types import MYPY
 
@@ -58,7 +58,8 @@ def capture_event(
         self, event  # type: Event
     ):
         # type: (...) -> None
-        """This gets invoked with the event dictionary when an event should
+        """
+        This gets invoked with the event dictionary when an event should
         be sent to sentry.
         """
         raise NotImplementedError()
@@ -67,14 +68,15 @@ def capture_envelope(
         self, envelope  # type: Envelope
     ):
         # type: (...) -> None
-        """This gets invoked with an envelope when an event should
-        be sent to sentry.  The default implementation invokes `capture_event`
-        if the envelope contains an event and ignores all other envelopes.
         """
-        event = envelope.get_event()
-        if event is not None:
-            self.capture_event(event)
-        return None
+        Send an envelope to Sentry.
+
+        Envelopes are a data container format that can hold any type of data
+        submitted to Sentry. We use it for transactions and sessions, but
+        regular "error" events should go through `capture_event` for backwards
+        compat.
+        """
+        raise NotImplementedError()
 
     def flush(
         self,
@@ -208,7 +210,8 @@ def _send_event(
         self, event  # type: Event
     ):
         # type: (...) -> None
-        if self._check_disabled(get_event_data_category(event)):
+
+        if self._check_disabled("error"):
             return None
 
         body = io.BytesIO()

tests/conftest.py

@@ -132,8 +132,16 @@ def check_string_keys(map):
             check_string_keys(event)
             validate_event_schema(event)
 
+    def check_envelope(envelope):
+        with capture_internal_exceptions():
+            # Assert error events are sent without envelope to server, for compat.
+            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)
+
     def inner(client):
-        monkeypatch.setattr(client, "transport", TestTransport(check_event))
+        monkeypatch.setattr(
+            client, "transport", TestTransport(check_event, check_envelope)
+        )
 
     return inner
 
@@ -167,9 +175,10 @@ def inner(*a, **kw):
 
 
 class TestTransport(Transport):
-    def __init__(self, capture_event_callback):
+    def __init__(self, capture_event_callback, capture_envelope_callback):
         Transport.__init__(self)
         self.capture_event = capture_event_callback
+        self.capture_envelope = capture_envelope_callback
         self._queue = None
 
 

tests/test_client.py

@@ -14,6 +14,7 @@
     capture_message,
     capture_exception,
     capture_event,
+    start_transaction,
 )
 from sentry_sdk.integrations.executing import ExecutingIntegration
 from sentry_sdk.transport import Transport
@@ -726,3 +727,44 @@ def test_init_string_types(dsn, sentry_init):
         Hub.current.client.dsn
         == "http://894b7d594095440f8dfea9b300e6f572@localhost:8000/2"
     )
+
+
+def test_envelope_types():
+    """
+    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(traces_sample_rate=1.0, transport=CustomTransport())):
+        event_id = capture_message("hello")
+
+        # Assert error events get passed in via capture_event
+        assert not envelopes
+        event = events.pop()
+
+        assert event["event_id"] == event_id
+        assert "type" not in event
+
+        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

tests/test_transport.py

@@ -117,6 +117,7 @@ def test_simple_rate_limits(httpserver, capsys, caplog, make_client):
     client.flush()
 
     assert len(httpserver.requests) == 1
+    assert httpserver.requests[0].url.endswith("/api/132/envelope/")
     del httpserver.requests[:]
 
     assert set(client.transport._disabled_until) == set([None])
@@ -141,12 +142,13 @@ def test_data_category_limits(httpserver, capsys, caplog, response_code, make_cl
     client.flush()
 
     assert len(httpserver.requests) == 1
+    assert httpserver.requests[0].url.endswith("/api/132/envelope/")
     del httpserver.requests[:]
 
     assert set(client.transport._disabled_until) == set(["transaction"])
 
-    client.transport.capture_event({"type": "transaction"})
-    client.transport.capture_event({"type": "transaction"})
+    client.capture_event({"type": "transaction"})
+    client.capture_event({"type": "transaction"})
     client.flush()
 
     assert not httpserver.requests
@@ -172,12 +174,13 @@ def test_complex_limits_without_data_category(
     client.flush()
 
     assert len(httpserver.requests) == 1
+    assert httpserver.requests[0].url.endswith("/api/132/envelope/")
     del httpserver.requests[:]
 
     assert set(client.transport._disabled_until) == set([None])
 
-    client.transport.capture_event({"type": "transaction"})
-    client.transport.capture_event({"type": "transaction"})
+    client.capture_event({"type": "transaction"})
+    client.capture_event({"type": "transaction"})
     client.capture_event({"type": "event"})
     client.flush()