Skip to content

Commit ca828c9

Browse files
untitakeruntitaker
authored
ref: Update transaction status to match final protocol (getsentry#563)
* ref: Update transaction status to match final protocol * fix: Formatting * fix: Add type annotations * fix: Fix tests * ref: Add tags to tx event * fix: Fix more tests * fix: Fix tests * fix: Linters * fix: Do not add span/breadcrumb for bad invocations of httplib * fix: Refactor httplib integration
1 parent 4ff2688 commit ca828c9

File tree

10 files changed

+81
-68
lines changed

10 files changed

+81
-68
lines changed

‎sentry_sdk/integrations/aiohttp.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,15 @@ async def inner():
7979
with hub.start_span(span):
8080
try:
8181
response = await old_handle(self, request)
82-
except HTTPException:
82+
except HTTPException as e:
83+
span.set_http_status(e.status_code)
8384
raise
8485
except Exception:
86+
# This will probably map to a 500 but seems like we
87+
# have no way to tell. Do not set span status.
8588
reraise(*_capture_exception(hub))
8689

90+
span.set_http_status(response.status)
8791
return response
8892

8993
# Explicitly wrap in task such that current contextvar context is

‎sentry_sdk/integrations/asgi.py

+3
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ async def _run_app(self, scope, callback):
108108
span.transaction = "generic ASGI request"
109109

110110
with hub.start_span(span) as span:
111+
# XXX: Would be cool to have correct span status, but we
112+
# would have to wrap send(). That is a bit hard to do with
113+
# the current abstraction over ASGI 2/3.
111114
try:
112115
return await callback()
113116
except Exception as exc:

‎sentry_sdk/integrations/celery.py

+12-1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ def _inner(*args, **kwargs):
126126
span.op = "celery.task"
127127
span.transaction = "unknown celery task"
128128

129+
# Could possibly use a better hook than this one
130+
span.set_status("ok")
131+
129132
with capture_internal_exceptions():
130133
# Celery task objects are not a thing to be trusted. Even
131134
# something such as attribute access can fail.
@@ -194,7 +197,12 @@ def _capture_exception(task, exc_info):
194197
if hub.get_integration(CeleryIntegration) is None:
195198
return
196199
if isinstance(exc_info[1], CELERY_CONTROL_FLOW_EXCEPTIONS):
200+
# ??? Doesn't map to anything
201+
_set_status(hub, "aborted")
197202
return
203+
204+
_set_status(hub, "internal_error")
205+
198206
if hasattr(task, "throws") and isinstance(exc_info[1], task.throws):
199207
return
200208

@@ -209,10 +217,13 @@ def _capture_exception(task, exc_info):
209217

210218
hub.capture_event(event, hint=hint)
211219

220+
221+
def _set_status(hub, status):
222+
# type: (Hub, str) -> None
212223
with capture_internal_exceptions():
213224
with hub.configure_scope() as scope:
214225
if scope.span is not None:
215-
scope.span.set_failure()
226+
scope.span.set_status(status)
216227

217228

218229
def _patch_worker_exit():

‎sentry_sdk/integrations/sqlalchemy.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,4 @@ def _dbapi_error(conn, *args):
6868
span = getattr(conn, "_sentry_sql_span", None) # type: Optional[Span]
6969

7070
if span is not None:
71-
span.set_failure()
71+
span.set_status("internal_error")

‎sentry_sdk/integrations/stdlib.py

+17-32
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from sentry_sdk.hub import Hub
77
from sentry_sdk.integrations import Integration
88
from sentry_sdk.scope import add_global_event_processor
9-
from sentry_sdk.tracing import EnvironHeaders, record_http_request
9+
from sentry_sdk.tracing import EnvironHeaders
1010
from sentry_sdk.utils import capture_internal_exceptions, safe_repr
1111

1212
from sentry_sdk._types import MYPY
@@ -78,48 +78,33 @@ def putrequest(self, method, url, *args, **kwargs):
7878
url,
7979
)
8080

81-
recorder = record_http_request(hub, real_url, method)
82-
data_dict = recorder.__enter__()
81+
span = hub.start_span(op="http", description="%s %s" % (method, real_url))
8382

84-
try:
85-
rv = real_putrequest(self, method, url, *args, **kwargs)
83+
span.set_data("method", method)
84+
span.set_data("url", real_url)
8685

87-
for key, value in hub.iter_trace_propagation_headers():
88-
self.putheader(key, value)
89-
except Exception:
90-
recorder.__exit__(*sys.exc_info())
91-
raise
86+
rv = real_putrequest(self, method, url, *args, **kwargs)
9287

93-
self._sentrysdk_recorder = recorder
94-
self._sentrysdk_data_dict = data_dict
88+
for key, value in hub.iter_trace_propagation_headers():
89+
self.putheader(key, value)
90+
91+
self._sentrysdk_span = span
9592

9693
return rv
9794

9895
def getresponse(self, *args, **kwargs):
9996
# type: (HTTPConnection, *Any, **Any) -> Any
100-
recorder = getattr(self, "_sentrysdk_recorder", None)
97+
span = getattr(self, "_sentrysdk_span", None)
10198

102-
if recorder is None:
99+
if span is None:
103100
return real_getresponse(self, *args, **kwargs)
104101

105-
data_dict = getattr(self, "_sentrysdk_data_dict", None)
106-
107-
try:
108-
rv = real_getresponse(self, *args, **kwargs)
109-
110-
if data_dict is not None:
111-
data_dict["status_code"] = rv.status
112-
data_dict["reason"] = rv.reason
113-
except TypeError:
114-
# python-requests provokes a typeerror to discover py3 vs py2 differences
115-
#
116-
# > TypeError("getresponse() got an unexpected keyword argument 'buffering'")
117-
raise
118-
except Exception:
119-
recorder.__exit__(*sys.exc_info())
120-
raise
121-
else:
122-
recorder.__exit__(None, None, None)
102+
rv = real_getresponse(self, *args, **kwargs)
103+
104+
span.set_data("status_code", rv.status)
105+
span.set_http_status(int(rv.status))
106+
span.set_data("reason", rv.reason)
107+
span.finish()
123108

124109
return rv
125110

‎sentry_sdk/integrations/wsgi.py

+1-3
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,7 @@ def _sentry_start_response(
130130
# type: (Callable[[str, U, Optional[E]], T], Span, str, U, Optional[E]) -> T
131131
with capture_internal_exceptions():
132132
status_int = int(status.split(" ", 1)[0])
133-
span.set_tag("http.status_code", status_int)
134-
if 500 <= status_int < 600:
135-
span.set_failure()
133+
span.set_http_status(status_int)
136134

137135
return old_start_response(status, response_headers, exc_info)
138136

‎sentry_sdk/tracing.py

+37-25
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def __enter__(self):
173173
def __exit__(self, ty, value, tb):
174174
# type: (Optional[Any], Optional[Any], Optional[Any]) -> None
175175
if value is not None:
176-
self.set_failure()
176+
self._tags.setdefault("status", "internal_error")
177177

178178
hub, scope, old_span = self._context_manager_state
179179
del self._context_manager_state
@@ -259,17 +259,44 @@ def set_data(self, key, value):
259259
# type: (str, Any) -> None
260260
self._data[key] = value
261261

262-
def set_failure(self):
263-
# type: () -> None
264-
self.set_tag("status", "failure")
262+
def set_status(self, value):
263+
# type: (str) -> None
264+
self.set_tag("status", value)
265265

266-
def set_success(self):
267-
# type: () -> None
268-
self.set_tag("status", "success")
266+
def set_http_status(self, http_status):
267+
# type: (int) -> None
268+
self.set_tag("http.status_code", http_status)
269+
270+
if http_status < 400:
271+
self.set_status("ok")
272+
elif 400 <= http_status < 500:
273+
if http_status == 403:
274+
self.set_status("permission_denied")
275+
elif http_status == 429:
276+
self.set_status("resource_exhausted")
277+
elif http_status == 413:
278+
self.set_status("failed_precondition")
279+
elif http_status == 401:
280+
self.set_status("unauthenticated")
281+
elif http_status == 409:
282+
self.set_status("already_exists")
283+
else:
284+
self.set_status("invalid_argument")
285+
elif 500 <= http_status < 600:
286+
if http_status == 504:
287+
self.set_status("deadline_exceeded")
288+
elif http_status == 501:
289+
self.set_status("unimplemented")
290+
elif http_status == 503:
291+
self.set_status("unavailable")
292+
else:
293+
self.set_status("internal_error")
294+
else:
295+
self.set_status("unknown_error")
269296

270297
def is_success(self):
271298
# type: () -> bool
272-
return self._tags.get("status") in (None, "success")
299+
return self._tags.get("status") == "ok"
273300

274301
def finish(self, hub=None):
275302
# type: (Optional[sentry_sdk.Hub]) -> Optional[str]
@@ -315,6 +342,7 @@ def finish(self, hub=None):
315342
"type": "transaction",
316343
"transaction": self.transaction,
317344
"contexts": {"trace": self.get_trace_context()},
345+
"tags": self._tags,
318346
"timestamp": self.timestamp,
319347
"start_timestamp": self.start_timestamp,
320348
"spans": [
@@ -427,29 +455,13 @@ def record_sql_queries(
427455
yield span
428456

429457

430-
@contextlib.contextmanager
431-
def record_http_request(hub, url, method):
432-
# type: (sentry_sdk.Hub, str, str) -> Generator[Dict[str, str], None, None]
433-
data_dict = {"url": url, "method": method}
434-
435-
with hub.start_span(op="http", description="%s %s" % (method, url)) as span:
436-
try:
437-
yield data_dict
438-
finally:
439-
if span is not None:
440-
if "status_code" in data_dict:
441-
span.set_tag("http.status_code", data_dict["status_code"])
442-
for k, v in data_dict.items():
443-
span.set_data(k, v)
444-
445-
446458
def _maybe_create_breadcrumbs_from_span(hub, span):
447459
# type: (sentry_sdk.Hub, Span) -> None
448460
if span.op == "redis":
449461
hub.add_breadcrumb(
450462
message=span.description, type="redis", category="redis", data=span._tags
451463
)
452-
elif span.op == "http" and span.is_success():
464+
elif span.op == "http":
453465
hub.add_breadcrumb(type="http", category="httplib", data=span._data)
454466
elif span.op == "subprocess":
455467
hub.add_breadcrumb(

‎tests/integrations/celery/test_celery.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ def dummy_task(x, y):
124124
assert submission_event["contexts"]["trace"]["trace_id"] == span.trace_id
125125

126126
if task_fails:
127-
assert execution_event["contexts"]["trace"]["status"] == "failure"
127+
assert execution_event["contexts"]["trace"]["status"] == "internal_error"
128128
else:
129-
assert "status" not in execution_event["contexts"]["trace"]
129+
assert execution_event["contexts"]["trace"]["status"] == "ok"
130130

131131
assert execution_event["spans"] == []
132132
assert submission_event["spans"] == [

‎tests/integrations/flask/test_flask.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ def test_tracing_success(sentry_init, capture_events, app):
570570

571571
assert transaction_event["type"] == "transaction"
572572
assert transaction_event["transaction"] == "hi"
573-
assert "status" not in transaction_event["contexts"]["trace"]
573+
assert transaction_event["contexts"]["trace"]["status"] == "ok"
574574

575575
assert message_event["message"] == "hi"
576576
assert message_event["transaction"] == "hi"
@@ -594,7 +594,7 @@ def error():
594594

595595
assert transaction_event["type"] == "transaction"
596596
assert transaction_event["transaction"] == "error"
597-
assert transaction_event["contexts"]["trace"]["status"] == "failure"
597+
assert transaction_event["contexts"]["trace"]["status"] == "internal_error"
598598

599599
assert error_event["transaction"] == "error"
600600
exception, = error_event["exception"]["values"]

‎tests/test_tracing.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def test_basic(sentry_init, capture_events, sample_rate):
2525

2626
span1, span2 = event["spans"]
2727
parent_span = event
28-
assert span1["tags"]["status"] == "failure"
28+
assert span1["tags"]["status"] == "internal_error"
2929
assert span1["op"] == "foo"
3030
assert span1["description"] == "foodesc"
3131
assert "status" not in span2.get("tags", {})

0 commit comments

Comments
 (0)