8000 fix(transport): Use correct data category for transaction events (#826) · adamchainz/sentry-python@93f6d33 · GitHub
[go: up one dir, main page]
Skip to content

Commit 93f6d33

Browse files
untitakerrhcarvalhosentry-bot
authored
fix(transport): Use correct data category for transaction events (getsentry#826)
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io> Co-authored-by: sentry-bot <markus+ghbot@sentry.io>
1 parent c95bda7 commit 93f6d33

File tree

5 files changed

+86
-31
lines changed

5 files changed

+86
-31
lines changed

‎sentry_sdk/envelope.py

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@
2020
from sentry_sdk._types import Event, EventDataCategory
2121

2222

23-
def get_event_data_category(event):
24-
# type: (Event) -> EventDataCategory
25-
if event.get("type") == "transaction":
26-
return "transaction"
27-
return "error"
28-
29-
3023
class Envelope(object):
3124
def __init__(
3225
self,
@@ -230,22 +223,27 @@ def __repr__(self):
230223
@property
231224
def data_category(self):
232225
# type: (...) -> EventDataCategory
233-
rv = "default" # type: Any
234-
event = self.get_event()
235-
if event is not None:
236-
rv = get_event_data_category(event)
226+
ty = self.headers.get("type")
227+
if ty == "session":
228+
return "session"
229+
elif ty == "attachment":
230+
return "attachment"
231+
elif ty == "transaction":
232+
return "transaction"
233+
elif ty == "event":
234+
return "error"
237235
else:
238-
ty = self.headers.get("type")
239-
if ty in ("session", "attachment"):
240-
rv = ty
241-
return rv
236+
return "default"
242237

243238
def get_bytes(self):
244239
# type: (...) -> bytes
245240
return self.payload.get_bytes()
246241

247242
def get_event(self):
248243
# type: (...) -> Optional[Event]
244+
"""
245+
Returns an error event if there is one.
246+
"""
249247
if self.headers.get("type") == "event" and self.payload.json is not None:
250248
return self.payload.json
251249
return None

‎sentry_sdk/transport.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
from sentry_sdk.utils import Dsn, logger, capture_internal_exceptions, json_dumps
1111
from sentry_sdk.worker import BackgroundWorker
12-
from sentry_sdk.envelope import Envelope, get_event_data_category
12+
from sentry_sdk.envelope import Envelope
1313

1414
from sentry_sdk._types import MYPY
1515

@@ -58,7 +58,8 @@ def capture_event(
5858
self, event # type: Event
5959
):
6060
# type: (...) -> None
61-
"""This gets invoked with the event dictionary when an event should
61+
"""
62+
This gets invoked with the event dictionary when an event should
6263
be sent to sentry.
6364
"""
6465
raise NotImplementedError()
@@ -67,14 +68,15 @@ def capture_envelope(
6768
self, envelope # type: Envelope
6869
):
6970
# type: (...) -> None
70-
"""This gets invoked with an envelope when an event should
71-
be sent to sentry. The default implementation invokes `capture_event`
72-
if the envelope contains an event and ignores all other envelopes.
7371
"""
74-
event = envelope.get_event()
75-
if event is not None:
76-
self.capture_event(event)
77-
return None
72+
Send an envelope to Sentry.
73+
74+
Envelopes are a data container format that can hold any type of data
75+
submitted to Sentry. We use it for transactions and sessions, but
76+
regular "error" events should go through `capture_event` for backwards
77+
compat.
78+
"""
79+
raise NotImplementedError()
7880

7981
def flush(
8082
self,
@@ -208,7 +210,8 @@ def _send_event(
208210
self, event # type: Event
209211
):
210212
# type: (...) -> None
211-
if self._check_disabled(get_event_data_category(event)):
213+
214+
if self._check_disabled("error"):
212215
return None
213216

214217
body = io.BytesIO()

‎tests/conftest.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,16 @@ def check_string_keys(map):
132132
check_string_keys(event)
133133
validate_event_schema(event)
134134

135+
def check_envelope(envelope):
136+
with capture_internal_exceptions():
137+
# Assert error events are sent without envelope to server, for compat.
138+
assert not any(item.data_category == "error" for item in envelope.items)
139+
assert not any(item.get_event() is not None for item in envelope.items)
140+
135141
def inner(client):
136-
monkeypatch.setattr(client, "transport", TestTransport(check_event))
142+
monkeypatch.setattr(
143+
client, "transport", TestTransport(check_event, check_envelope)
144+
)
137145

138146
return inner
139147

@@ -167,9 +175,10 @@ def inner(*a, **kw):
167175

168176

169177
class TestTransport(Transport):
170-
def __init__(self, capture_event_callback):
178+
def __init__(self, capture_event_callback, capture_envelope_callback):
171179
Transport.__init__(self)
172180
self.capture_event = capture_event_callback
181+
self.capture_envelope = capture_envelope_callback
173182
self._queue = None
174183

175184

‎tests/test_client.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
capture_message,
1515
capture_exception,
1616
capture_event,
17+
start_transaction,
1718
)
1819
from sentry_sdk.integrations.executing import ExecutingIntegration
1920
from sentry_sdk.transport import Transport
@@ -726,3 +727,44 @@ def test_init_string_types(dsn, sentry_init):
726727
Hub.current.client.dsn
727728
== "http://894b7d594095440f8dfea9b300e6f572@localhost:8000/2"
728729
)
730+
731+
732+
def test_envelope_types():
733+
"""
734+
Tests for calling the right transport method (capture_event vs
735+
capture_envelope) from the SDK client for different data types.
736+
"""
737+
738+
envelopes = []
739+
events = []
740+
741+
class CustomTransport(Transport):
742+
def capture_envelope(self, envelope):
743+
envelopes.append(envelope)
744+
745+
def capture_event(self, event):
746+
events.append(event)
747+
748+
with Hub(Client(traces_sample_rate=1.0, transport=CustomTransport())):
749+
event_id = capture_message("hello")
750+
751+
# Assert error events get passed in via capture_event
752+
assert not envelopes
753+
event = events.pop()
754+
755+
assert event["event_id"] == event_id
756+
assert "type" not in event
757+
758+
with start_transaction(name="foo"):
759+
pass
760+
761+
# Assert transactions get passed in via capture_envelope
762+
assert not events
763+
envelope = envelopes.pop()
764+
765+
(item,) = envelope.items
766+
assert item.data_category == "transaction"
767+
assert item.headers.get("type") == "transaction"
768+
769+
assert not envelopes
770+
assert not events

‎tests/test_transport.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ def test_simple_rate_limits(httpserver, capsys, caplog, make_client):
117117
client.flush()
118118

119119
assert len(httpserver.requests) == 1
120+
assert httpserver.requests[0].url.endswith("/api/132/envelope/")
120121
del httpserver.requests[:]
121122

122123
assert set(client.transport._disabled_until) == set([None])
@@ -141,12 +142,13 @@ def test_data_category_limits(httpserver, capsys, caplog, response_code, make_cl
141142
client.flush()
142143

143144
assert len(httpserver.requests) == 1
145+
assert httpserver.requests[0].url.endswith("/api/132/envelope/")
144146
del httpserver.requests[:]
145147

146148
assert set(client.transport._disabled_until) == set(["transaction"])
147149

148-
client.transport.capture_event({"type": "transaction"})
149-
client.transport.capture_event({"type": "transaction"})
150+
client.capture_event({"type": "transaction"})
151+
client.capture_event({"type": "transaction"})
150152
client.flush()
151153

152154
assert not httpserver.requests
@@ -172,12 +174,13 @@ def test_complex_limits_without_data_category(
172174
client.flush()
173175

174176
assert len(httpserver.requests) == 1
177+
assert httpserver.requests[0].url.endswith("/api/132/envelope/")
175178
del httpserver.requests[:]
176179

177180
assert set(client.transport._disabled_until) == set([None])
178181

179-
client.transport.capture_event({"type": "transaction"})
180-
client.transport.capture_event({"type": "transaction"})
182+
client.capture_event({"type": "transaction"})
183+
client.capture_event({"type": "transaction"})
181184
client.capture_event({"type": "event"})
182185
client.flush()
183186

0 commit comments

Comments
 (0)
0