ref: Update transaction status to match final protocol

sentry_sdk/integrations/aiohttp.py

@@ -79,11 +79,15 @@ async def inner():
                     with hub.start_span(span):
                         try:
                             response = await old_handle(self, request)
-                        except HTTPException:
+                        except HTTPException as e:
+                            span.set_http_status(e.status_code)
                             raise
                         except Exception:
+                            # This will probably map to a 500 but seems like we
+                            # have no way to tell. Do not set span status.
                             reraise(*_capture_exception(hub))
 
+                        span.set_http_status(response.status)
                         return response
 
             # Explicitly wrap in task such that current contextvar context is

sentry_sdk/integrations/asgi.py

@@ -108,6 +108,9 @@ async def _run_app(self, scope, callback):
                 span.transaction = "generic ASGI request"
 
                 with hub.start_span(span) as span:
+                    # XXX: Would be cool to have correct span status, but we
+                    # would have to wrap send(). That is a bit hard to do with
+                    # the current abstraction over ASGI 2/3.
                     try:
                         return await callback()
                     except Exception as exc:

sentry_sdk/integrations/celery.py

@@ -126,6 +126,9 @@ def _inner(*args, **kwargs):
             span.op = "celery.task"
             span.transaction = "unknown celery task"
 
+            # Could possibly use a better hook than this one
+            span.set_status("ok")
+
             with capture_internal_exceptions():
                 # Celery task objects are not a thing to be trusted. Even
                 # something such as attribute access can fail.
@@ -194,7 +197,12 @@ def _capture_exception(task, exc_info):
     if hub.get_integration(CeleryIntegration) is None:
         return
     if isinstance(exc_info[1], CELERY_CONTROL_FLOW_EXCEPTIONS):
+        # ??? Doesn't map to anything
+        _set_status(hub, "aborted")
         return
+
+    _set_status(hub, "internal_error")
+
     if hasattr(task, "throws") and isinstance(exc_info[1], task.throws):
         return
 
@@ -209,10 +217,13 @@ def _capture_exception(task, exc_info):
 
     hub.capture_event(event, hint=hint)
 
+
+def _set_status(hub, status):
+    # type: (Hub, str) -> None
     with capture_internal_exceptions():
         with hub.configure_scope() as scope:
             if scope.span is not None:
-                scope.span.set_failure()
+                scope.span.set_status(status)
 
 
 def _patch_worker_exit():

sentry_sdk/integrations/sqlalchemy.py

@@ -68,4 +68,4 @@ def _dbapi_error(conn, *args):
     span = getattr(conn, "_sentry_sql_span", None)  # type: Optional[Span]
 
     if span is not None:
-        span.set_failure()
+        span.set_status("internal_error")

sentry_sdk/integrations/stdlib.py

@@ -6,7 +6,7 @@
 from sentry_sdk.hub import Hub
 from sentry_sdk.integrations import Integration
 from sentry_sdk.scope import add_global_event_processor
-from sentry_sdk.tracing import EnvironHeaders, record_http_request
+from sentry_sdk.tracing import EnvironHeaders
 from sentry_sdk.utils import capture_internal_exceptions, safe_repr
 
 from sentry_sdk._types import MYPY
@@ -78,48 +78,33 @@ def putrequest(self, method, url, *args, **kwargs):
                 url,
             )
 
-        recorder = record_http_request(hub, real_url, method)
-        data_dict = recorder.__enter__()
+        span = hub.start_span(op="http", description="%s %s" % (method, real_url))
 
-        try:
-            rv = real_putrequest(self, method, url, *args, **kwargs)
+        span.set_data("method", method)
+        span.set_data("url", real_url)
 
-            for key, value in hub.iter_trace_propagation_headers():
-                self.putheader(key, value)
-        except Exception:
-            recorder.__exit__(*sys.exc_info())
-            raise
+        rv = real_putrequest(self, method, url, *args, **kwargs)
 
-        self._sentrysdk_recorder = recorder
-        self._sentrysdk_data_dict = data_dict
+        for key, value in hub.iter_trace_propagation_headers():
+            self.putheader(key, value)
+
+        self._sentrysdk_span = span
 
         return rv
 
     def getresponse(self, *args, **kwargs):
         # type: (HTTPConnection, *Any, **Any) -> Any
-        recorder = getattr(self, "_sentrysdk_recorder", None)
+        span = getattr(self, "_sentrysdk_span", None)
 
-        if recorder is None:
+        if span is None:
             return real_getresponse(self, *args, **kwargs)
 
-        data_dict = getattr(self, "_sentrysdk_data_dict", None)
-
-        try:
-            rv = real_getresponse(self, *args, **kwargs)
-
-            if data_dict is not None:
-                data_dict["status_code"] = rv.status
-                data_dict["reason"] = rv.reason
-        except TypeError:
-            # python-requests provokes a typeerror to discover py3 vs py2 differences
-            #
-            # > TypeError("getresponse() got an unexpected keyword argument 'buffering'")
-            raise
-        except Exception:
-            recorder.__exit__(*sys.exc_info())
-            raise
-        else:
-            recorder.__exit__(None, None, None)
+        rv = real_getresponse(self, *args, **kwargs)
+
+        span.set_data("status_code", rv.status)
+        span.set_http_status(int(rv.status))
+        span.set_data("reason", rv.reason)
+        span.finish()
 
         return rv
 

sentry_sdk/integrations/wsgi.py

@@ -130,9 +130,7 @@ def _sentry_start_response(
     # type: (Callable[[str, U, Optional[E]], T], Span, str, U, Optional[E]) -> T
     with capture_internal_exceptions():
         status_int = int(status.split(" ", 1)[0])
-        span.set_tag("http.status_code", status_int)
-        if 500 <= status_int < 600:
-            span.set_failure()
+        span.set_http_status(status_int)
 
     return old_start_response(status, response_headers, exc_info)
 

sentry_sdk/tracing.py

@@ -173,7 +173,7 @@ def __enter__(self):
     def __exit__(self, ty, value, tb):
         # type: (Optional[Any], Optional[Any], Optional[Any]) -> None
         if value is not None:
-            self.set_failure()
+            self._tags.setdefault("status", "internal_error")
 
         hub, scope, old_span = self._context_manager_state
         del self._context_manager_state
@@ -259,17 +259,44 @@ def set_data(self, key, value):
         # type: (str, Any) -> None
         self._data[key] = value
 
-    def set_failure(self):
-        # type: () -> None
-        self.set_tag("status", "failure")
+    def set_status(self, value):
+        # type: (str) -> None
+        self.set_tag("status", value)
 
-    def set_success(self):
-        # type: () -> None
-        self.set_tag("status", "success")
+    def set_http_status(self, http_status):
+        # type: (int) -> None
+        self.set_tag("http.status_code", http_status)
+
+        if http_status < 400:
+            self.set_status("ok")
+        elif 400 <= http_status < 500:
+            if http_status == 403:
+                self.set_status("permission_denied")
+            elif http_status == 429:
+                self.set_status("resource_exhausted")
+            elif http_status == 413:
+                self.set_status("failed_precondition")
+            elif http_status == 401:
+                self.set_status("unauthenticated")
+            elif http_status == 409:
+                self.set_status("already_exists")
+            else:
+                self.set_status("invalid_argument")
+        elif 500 <= http_status < 600:
+            if http_status == 504:
+                self.set_status("deadline_exceeded")
+            elif http_status == 501:
+                self.set_status("unimplemented")
+            elif http_status == 503:
+                self.set_status("unavailable")
+            else:
+                self.set_status("internal_error")
+        else:
+            self.set_status("unknown_error")
 
     def is_success(self):
         # type: () -> bool
-        return self._tags.get("status") in (None, "success")
+        return self._tags.get("status") == "ok"
 
     def finish(self, hub=None):
         # type: (Optional[sentry_sdk.Hub]) -> Optional[str]
@@ -315,6 +342,7 @@ def finish(self, hub=None):
                 "type": "transaction",
                 "transaction": self.transaction,
                 "contexts": {"trace": self.get_trace_context()},
+                "tags": self._tags,
                 "timestamp": self.timestamp,
                 "start_timestamp": self.start_timestamp,
                 "spans": [
@@ -427,29 +455,13 @@ def record_sql_queries(
         yield span
 
 
-@contextlib.contextmanager
-def record_http_request(hub, url, method):
-    # type: (sentry_sdk.Hub, str, str) -> Generator[Dict[str, str], None, None]
-    data_dict = {"url": url, "method": method}
-
-    with hub.start_span(op="http", description="%s %s" % (method, url)) as span:
-        try:
-            yield data_dict
-        finally:
-            if span is not None:
-                if "status_code" in data_dict:
-                    span.set_tag("http.status_code", data_dict["status_code"])
-                for k, v in data_dict.items():
-                    span.set_data(k, v)
-
-
 def _maybe_create_breadcrumbs_from_span(hub, span):
     # type: (sentry_sdk.Hub, Span) -> None
     if span.op == "redis":
         hub.add_breadcrumb(
             message=span.description, type="redis", category="redis", data=span._tags
         )
-    elif span.op == "http" and span.is_success():
+    elif span.op == "http":
         hub.add_breadcrumb(type="http", category="httplib", data=span._data)
     elif span.op == "subprocess":
         hub.add_breadcrumb(

tests/integrations/celery/test_celery.py

@@ -124,9 +124,9 @@ def dummy_task(x, y):
     assert submission_event["contexts"]["trace"]["trace_id"] == span.trace_id
 
     if task_fails:
-        assert execution_event["contexts"]["trace"]["status"] == "failure"
+        assert execution_event["contexts"]["trace"]["status"] == "internal_error"
     else:
-        assert "status" not in execution_event["contexts"]["trace"]
+        assert execution_event["contexts"]["trace"]["status"] == "ok"
 
     assert execution_event["spans"] == []
     assert submission_event["spans"] == [

tests/integrations/flask/test_flask.py

@@ -570,7 +570,7 @@ def test_tracing_success(sentry_init, capture_events, app):
 
     assert transaction_event["type"] == "transaction"
     assert transaction_event["transaction"] == "hi"
-    assert "status" not in transaction_event["contexts"]["trace"]
+    assert transaction_event["contexts"]["trace"]["status"] == "ok"
 
     assert message_event["message"] == "hi"
     assert message_event["transaction"] == "hi"
@@ -594,7 +594,7 @@ def error():
 
     assert transaction_event["type"] == "transaction"
     assert transaction_event["transaction"] == "error"
-    assert transaction_event["contexts"]["trace"]["status"] == "failure"
+    assert transaction_event["contexts"]["trace"]["status"] == "internal_error"
 
     assert error_event["transaction"] == "error"
     exception, = error_event["exception"]["values"]

tests/test_tracing.py

@@ -25,7 +25,7 @@ def test_basic(sentry_init, capture_events, sample_rate):
 
         span1, span2 = event["spans"]
         parent_span = event
-        assert span1["tags"]["status"] == "failure"
+        assert span1["tags"]["status"] == "internal_error"
         assert span1["op"] == "foo"
         assert span1["description"] == "foodesc"
         assert "status" not in span2.get("tags", {})