ref(stdlib): Move crumbs to integration (#7161)

Migrate `http.client` breadcrumbs from
`maybe_create_breadcrumbs_from_span` directly to the
`StdlibIntegration`.

As the flow is scattered (one point where a request starts, but multiple
points where it might end), we save the breadcrumb data alongside the
span and emit a breadcrumb whenever we detect a response is finished.

Also:
- Add tests to both the standard library test suite and requests.

#### Issues
Part of #7067

Author: sentrivana

sentry_sdk/integrations/stdlib.py

@@ -13,6 +13,7 @@
 from sentry_sdk.tracing import Span
 from sentry_sdk.tracing_utils import (
     EnvironHeaders,
+    add_http_breadcrumb,
     add_http_request_source,
     has_span_streaming_enabled,
     should_propagate_trace,
@@ -112,11 +113,21 @@ def putrequest(
             parsed_url = parse_url(real_url, sanitize=False)
 
         span_streaming = has_span_streaming_enabled(client.options)
-        span: "Union[Span, StreamedSpan, None]"
+        span: "Union[Span, StreamedSpan, None]" = None
+        breadcrumb: "dict[str, Any]" = {}
+
         if span_streaming:
-            if sentry_sdk.traces.get_current_span() is None:
-                span = None
-            else:
+            breadcrumb[SPANDATA.HTTP_REQUEST_METHOD] = method
+            if parsed_url is not None and should_send_default_pii():
+                breadcrumb.update(
+                    {
+                        SPANDATA.URL_FRAGMENT: parsed_url.fragment,
+                        SPANDATA.URL_FULL: parsed_url.url,
+                        SPANDATA.URL_QUERY: parsed_url.query,
+                    }
+                )
+
+            if sentry_sdk.traces.get_current_span() is not None:
                 span = sentry_sdk.traces.start_span(
                     name="%s %s"
                     % (
@@ -136,6 +147,7 @@ def putrequest(
                     span.set_attribute(SPANDATA.URL_QUERY, parsed_url.query)
 
                 set_on_span = span.set_attribute
+
         else:
             span = sentry_sdk.start_span(
                 op=OP.HTTP_CLIENT,
@@ -145,17 +157,35 @@ def putrequest(
             )
 
             span.set_data(SPANDATA.HTTP_METHOD, method)
+            breadcrumb[SPANDATA.HTTP_METHOD] = method
+
             if parsed_url is not None:
                 span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
                 span.set_data("url", parsed_url.url)
                 span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
 
+                breadcrumb.update(
+                    {
+                        SPANDATA.HTTP_FRAGMENT: parsed_url.fragment,
+                        "url": parsed_url.url,
+                        SPANDATA.HTTP_QUERY: parsed_url.query,
+                    }
+                )
+
             set_on_span = span.set_data
 
         # for proxies, these point to the proxy host/port
-        if span and tunnel_host:
-            set_on_span(SPANDATA.NETWORK_PEER_ADDRESS, self.host)
-            set_on_span(SPANDATA.NETWORK_PEER_PORT, self.port)
+        if tunnel_host:
+            if span:
+                set_on_span(SPANDATA.NETWORK_PEER_ADDRESS, self.host)
+                set_on_span(SPANDATA.NETWORK_PEER_PORT, self.port)
+
+            breadcrumb.update(
+                {
+                    SPANDATA.NETWORK_PEER_ADDRESS: self.host,
+                    SPANDATA.NETWORK_PEER_PORT: self.port,
+                }
+            )
 
         rv = real_putrequest(self, method, url, *args, **kwargs)
 
@@ -174,28 +204,46 @@ def putrequest(
                 self.putheader(key, value)
 
         self._sentrysdk_span = span  # type: ignore[attr-defined]
+        self._sentrysdk_breadcrumb = breadcrumb  # type: ignore[attr-defined]
 
         return rv
 
     def getresponse(self: "HTTPConnection", *args: "Any", **kwargs: "Any") -> "Any":
         span = getattr(self, "_sentrysdk_span", None)
-
-        if span is None:
-            return real_getresponse(self, *args, **kwargs)
+        breadcrumb = getattr(self, "_sentrysdk_breadcrumb", None)
 
         try:
             rv = real_getresponse(self, *args, **kwargs)
-        except BaseException:
-            _complete_span(span)
+        except BaseException as ex:
+            if span:
+                _complete_span(span)
+            if (
+                breadcrumb
+                and "getresponse() got an unexpected keyword argument 'buffering'"
+                not in str(ex)
+            ):
+                # the exception msg check is needed for Python 3.6/requests compat
+                add_http_breadcrumb(None, breadcrumb)
             raise
 
+        status_code = int(rv.status)
+
+        if breadcrumb:
+            breadcrumb[SPANDATA.HTTP_STATUS_CODE] = status_code
+
+        if span is None:
+            if breadcrumb:
+                add_http_breadcrumb(status_code, breadcrumb)
+            return rv
+
         if isinstance(span, StreamedSpan):
-            status_code = int(rv.status)
             span.status = "error" if status_code >= 400 else "ok"
-            span.set_attribute("http.response.status_code", status_code)
-        else:
-            span.set_http_status(int(rv.status))
+            span.set_attribute(SPANDATA.HTTP_STATUS_CODE, status_code)
+        elif isinstance(span, Span):
+            span.set_http_status(status_code)
             span.set_data("reason", rv.reason)
+            if breadcrumb:
+                breadcrumb["reason"] = rv.reason
 
         # getresponse doesn't include actually reading the response body. This
         # is done in read(). So if the metadata/headers suggest there's a body to
@@ -206,6 +254,11 @@ def getresponse(self: "HTTPConnection", *args: "Any", **kwargs: "Any") -> "Any":
         else:
             _complete_span(span)
 
+        if breadcrumb:
+            # Regardless of whether the response itself has been fully read or not,
+            # the breadcrumb can now be emitted since we now have the status code.
+            add_http_breadcrumb(status_code, breadcrumb)
+
         return rv
 
     def read(self: "HTTPResponse", *args: "Any", **kwargs: "Any") -> "Any":

sentry_sdk/tracing_utils.py

@@ -233,6 +233,7 @@ def maybe_create_breadcrumbs_from_span(
         "auto.http.pyreqwest",
         "auto.http.httpx",
         "auto.http.httpx2",
+        "auto.http.stdlib.httplib",
     ):
         level = None
         status_code = span._data.get(SPANDATA.HTTP_STATUS_CODE)

tests/integrations/requests/test_requests.py

@@ -36,6 +36,43 @@ def test_crumb_capture(sentry_init, capture_events):
     )
 
 
+@pytest.mark.parametrize("send_default_pii", [True, False])
+def test_crumb_capture_span_streaming(sentry_init, capture_events, send_default_pii):
+    sentry_init(
+        integrations=[StdlibIntegration()],
+        send_default_pii=send_default_pii,
+        trace_lifecycle="stream",
+    )
+    events = capture_events()
+
+    url = f"http://localhost:{PORT}/hello-world"  # noqa:E231
+    response = requests.get(url)
+    capture_message("Testing!")
+
+    (event,) = events
+    (crumb,) = event["breadcrumbs"]["values"]
+    assert crumb["type"] == "http"
+    assert crumb["category"] == "httplib"
+
+    if send_default_pii:
+        assert crumb["data"] == ApproxDict(
+            {
+                SPANDATA.URL_FULL: url,
+                SPANDATA.HTTP_REQUEST_METHOD: "GET",
+                SPANDATA.URL_FRAGMENT: "",
+                SPANDATA.URL_QUERY: "",
+                SPANDATA.HTTP_STATUS_CODE: response.status_code,
+            }
+        )
+    else:
+        assert crumb["data"] == ApproxDict(
+            {
+                SPANDATA.HTTP_REQUEST_METHOD: "GET",
+                SPANDATA.HTTP_STATUS_CODE: response.status_code,
+            }
+        )
+
+
 @pytest.mark.skipif(
     sys.version_info < (3, 7),
     reason="The response status is not set on the span early enough in 3.6",
@@ -84,6 +121,68 @@ def test_crumb_capture_client_error(sentry_init, capture_events, status_code, le
     )
 
 
+@pytest.mark.skipif(
+    sys.version_info < (3, 7),
+    reason="The response status is not set on the span early enough in 3.6",
+)
+@pytest.mark.parametrize(
+    "status_code,level",
+    [
+        (200, None),
+        (301, None),
+        (403, "warning"),
+        (405, "warning"),
+        (500, "error"),
+    ],
+)
+@pytest.mark.parametrize("send_default_pii", [True, False])
+def test_crumb_capture_client_error_span_streaming(
+    sentry_init, capture_events, status_code, level, send_default_pii
+):
+    sentry_init(
+        integrations=[StdlibIntegration()],
+        send_default_pii=send_default_pii,
+        trace_lifecycle="stream",
+    )
+
+    events = capture_events()
+
+    url = f"http://localhost:{PORT}/status/{status_code}"  # noqa:E231
+    response = requests.get(url)
+
+    assert response.status_code == status_code
+
+    capture_message("Testing!")
+
+    (event,) = events
+    (crumb,) = event["breadcrumbs"]["values"]
+    assert crumb["type"] == "http"
+    assert crumb["category"] == "httplib"
+
+    if level is None:
+        assert "level" not in crumb
+    else:
+        assert crumb["level"] == level
+
+    if send_default_pii:
+        assert crumb["data"] == ApproxDict(
+            {
+                SPANDATA.URL_FULL: url,
+                SPANDATA.HTTP_REQUEST_METHOD: "GET",
+                SPANDATA.URL_FRAGMENT: "",
+                SPANDATA.URL_QUERY: "",
+                SPANDATA.HTTP_STATUS_CODE: response.status_code,
+            }
+        )
+    else:
+        assert crumb["data"] == ApproxDict(
+            {
+                SPANDATA.HTTP_REQUEST_METHOD: "GET",
+                SPANDATA.HTTP_STATUS_CODE: response.status_code,
+            }
+        )
+
+
 @pytest.mark.tests_internal_exceptions
 def test_omit_url_data_if_parsing_fails(sentry_init, capture_events):
     sentry_init(integrations=[StdlibIntegration()])
@@ -112,3 +211,36 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events):
     assert "url" not in event["breadcrumbs"]["values"][0]["data"]
     assert SPANDATA.HTTP_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"]
     assert SPANDATA.HTTP_QUERY not in event["breadcrumbs"]["values"][0]["data"]
+
+
+@pytest.mark.tests_internal_exceptions
+def test_omit_url_data_if_parsing_fails_span_streaming(sentry_init, capture_events):
+    sentry_init(
+        integrations=[StdlibIntegration()],
+        trace_lifecycle="stream",
+        send_default_pii=True,
+    )
+
+    events = capture_events()
+
+    url = f"http://localhost:{PORT}/ok"  # noqa:E231
+
+    with mock.patch(
+        "sentry_sdk.integrations.stdlib.parse_url",
+        side_effect=ValueError,
+    ):
+        response = requests.get(url)
+
+    capture_message("Testing!")
+
+    (event,) = events
+    assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict(
+        {
+            SPANDATA.HTTP_REQUEST_METHOD: "GET",
+            SPANDATA.HTTP_STATUS_CODE: response.status_code,
+            # no url related data
+        }
+    )
+    assert SPANDATA.URL_FULL not in event["breadcrumbs"]["values"][0]["data"]
+    assert SPANDATA.URL_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"]
+    assert SPANDATA.URL_QUERY not in event["breadcrumbs"]["values"][0]["data"]