diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index c764605c45..cb31d90785 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/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": diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index a971dd7d9e..1e11807dd6 100644 --- a/sentry_sdk/tracing_utils.py +++ b/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) diff --git a/tests/integrations/requests/test_requests.py b/tests/integrations/requests/test_requests.py index 17d5895918..d43ab6ea81 100644 --- a/tests/integrations/requests/test_requests.py +++ b/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"] diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 66d1e8db37..60485595f3 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -102,6 +102,45 @@ 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()], + trace_lifecycle="stream", + send_default_pii=send_default_pii, + ) + events = capture_events() + + url = "http://localhost:{}/some/random/url".format(PORT) + urlopen(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.HTTP_STATUS_CODE: 200, + SPANDATA.URL_FRAGMENT: "", + SPANDATA.URL_QUERY: "", + } + ) + else: + assert crumb["data"] == ApproxDict( + { + SPANDATA.HTTP_REQUEST_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: 200, + } + ) + + @pytest.mark.parametrize( "status_code,level", [ @@ -146,6 +185,65 @@ def test_crumb_capture_client_error(sentry_init, capture_events, status_code, le ) +@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()], + trace_lifecycle="stream", + send_default_pii=send_default_pii, + ) + events = capture_events() + + url = f"http://localhost:{PORT}/status/{status_code}" # noqa:E231 + try: + urlopen(url) + except HTTPError: + pass + + 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.HTTP_STATUS_CODE: status_code, + SPANDATA.URL_FRAGMENT: "", + SPANDATA.URL_QUERY: "", + } + ) + else: + assert crumb["data"] == ApproxDict( + { + SPANDATA.HTTP_REQUEST_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: status_code, + } + ) + + def test_crumb_capture_hint(sentry_init, capture_events): def before_breadcrumb(crumb, hint): crumb["data"]["extra"] = "foo" @@ -180,6 +278,53 @@ def before_breadcrumb(crumb, hint): ) +@pytest.mark.parametrize("send_default_pii", [True, False]) +def test_crumb_capture_hint_span_streaming( + sentry_init, capture_events, send_default_pii +): + def before_breadcrumb(crumb, hint): + crumb["data"]["extra"] = "foo" + return crumb + + sentry_init( + integrations=[StdlibIntegration()], + before_breadcrumb=before_breadcrumb, + send_default_pii=send_default_pii, + trace_lifecycle="stream", + ) + events = capture_events() + + url = "http://localhost:{}/some/random/url".format(PORT) + urlopen(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.HTTP_STATUS_CODE: 200, + "extra": "foo", + SPANDATA.URL_FRAGMENT: "", + SPANDATA.URL_QUERY: "", + } + ) + else: + assert crumb["data"] == ApproxDict( + { + SPANDATA.HTTP_REQUEST_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: 200, + "extra": "foo", + } + ) + + @pytest.mark.parametrize("span_streaming", [True, False]) def test_empty_realurl( sentry_init,