diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 858bf273f2..e142d3a131 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -36,6 +36,7 @@ TransactionSource, ) from sentry_sdk.tracing_utils import ( + add_http_breadcrumb, add_http_request_source, has_span_streaming_enabled, should_propagate_trace, @@ -388,57 +389,64 @@ async def on_request_start( with capture_internal_exceptions(): parsed_url = parse_url(str(params.url), sanitize=False) + breadcrumb = {} + span_name = "%s %s" % ( method, parsed_url.url if parsed_url else SENSITIVE_DATA_SUBSTITUTE, ) - span: "Union[Span, StreamedSpan, None]" + span: "Union[Span, StreamedSpan, None]" = None if has_span_streaming_enabled(client.options): - if sentry_sdk.traces.get_current_span() is None: - span = None - else: - attributes: "Attributes" = { - "sentry.op": OP.HTTP_CLIENT, - "sentry.origin": AioHttpIntegration.origin, - "http.request.method": method, - } - if parsed_url is not None: - if has_data_collection_enabled(client.options): - url_full = parsed_url.url - attributes["url.path"] = params.url.path - - if parsed_url.query: - filtered_query = ( - _apply_data_collection_filtering_to_query_string( - query_string=parsed_url.query, - behaviour=client.options["data_collection"][ - "url_query_params" - ], - ) + attributes: "Attributes" = { + "sentry.op": OP.HTTP_CLIENT, + "sentry.origin": AioHttpIntegration.origin, + "http.request.method": method, + } + if parsed_url is not None: + if has_data_collection_enabled(client.options): + url_full = parsed_url.url + attributes["url.path"] = params.url.path + + if parsed_url.query: + filtered_query = ( + _apply_data_collection_filtering_to_query_string( + query_string=parsed_url.query, + behaviour=client.options["data_collection"][ + "url_query_params" + ], ) - if filtered_query: - attributes["url.query"] = filtered_query - url_full += "?" + filtered_query - - if parsed_url.fragment: - attributes["url.fragment"] = parsed_url.fragment - url_full += "#" + parsed_url.fragment - - attributes["url.full"] = url_full - elif should_send_default_pii(): - url_full = parsed_url.url - attributes["url.path"] = params.url.path - - if parsed_url.query: - url_full += "?" + parsed_url.query - attributes["url.query"] = parsed_url.query - if parsed_url.fragment: - url_full += "#" + parsed_url.fragment - attributes["url.fragment"] = parsed_url.fragment - - attributes["url.full"] = url_full - + ) + if filtered_query: + attributes["url.query"] = filtered_query + url_full += "?" + filtered_query + breadcrumb[SPANDATA.HTTP_QUERY] = filtered_query + + if parsed_url.fragment: + attributes["url.fragment"] = parsed_url.fragment + url_full += "#" + parsed_url.fragment + breadcrumb[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment + + attributes["url.full"] = url_full + breadcrumb["url"] = url_full + + elif should_send_default_pii(): + url_full = parsed_url.url + attributes["url.path"] = params.url.path + + if parsed_url.query: + url_full += "?" + parsed_url.query + attributes["url.query"] = parsed_url.query + breadcrumb[SPANDATA.HTTP_QUERY] = parsed_url.query + if parsed_url.fragment: + url_full += "#" + parsed_url.fragment + attributes["url.fragment"] = parsed_url.fragment + breadcrumb[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment + + attributes["url.full"] = url_full + breadcrumb["url"] = url_full + + if sentry_sdk.traces.get_current_span() is not None: span = sentry_sdk.traces.start_span( name=span_name, attributes=attributes ) @@ -453,6 +461,13 @@ async def on_request_start( legacy_span.set_data("url", parsed_url.url) legacy_span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query) legacy_span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment) + breadcrumb.update( + { + SPANDATA.HTTP_QUERY: parsed_url.query, + SPANDATA.HTTP_FRAGMENT: parsed_url.fragment, + "url": parsed_url.url, + } + ) span = legacy_span if should_propagate_trace(client, str(params.url)): @@ -475,18 +490,35 @@ async def on_request_start( else: params.headers[key] = value - trace_config_ctx.span = span + trace_config_ctx._sentry_span = span + trace_config_ctx._sentry_breadcrumb = breadcrumb async def on_request_end( session: "ClientSession", trace_config_ctx: "SimpleNamespace", params: "TraceRequestEndParams", ) -> None: - if trace_config_ctx.span is None: + status = int(params.response.status) + + breadcrumb = trace_config_ctx._sentry_breadcrumb + if breadcrumb is not None: + breadcrumb.update( + { + SPANDATA.HTTP_METHOD: params.method.upper(), + SPANDATA.HTTP_STATUS_CODE: status, + "reason": params.response.reason, + } + ) + + add_http_breadcrumb( + status, + breadcrumb, + ) + + if trace_config_ctx._sentry_span is None: return - span = trace_config_ctx.span - status = int(params.response.status) + span = trace_config_ctx._sentry_span if isinstance(span, StreamedSpan): span.set_attribute("http.response.status_code", status) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index b3658f6f91..8a8824c554 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -210,10 +210,26 @@ def record_sql_queries( yield span +def add_http_breadcrumb(status_code, data): + # type: (Optional[int], dict[str, Any]) -> None + level = None + if status_code: + if 500 <= status_code <= 599: + level = "error" + elif 400 <= status_code <= 499: + level = "warning" + + kwargs: "dict[str, Any]" = {"type": "http", "category": "httplib", "data": data} + if level: + kwargs["level"] = level + + sentry_sdk.add_breadcrumb(**kwargs) + + def maybe_create_breadcrumbs_from_span( scope: "sentry_sdk.Scope", span: "sentry_sdk.tracing.Span" ) -> None: - if span.op == OP.HTTP_CLIENT: + if span.op == OP.HTTP_CLIENT and span.origin not in ("auto.http.aiohttp",): level = None status_code = span._data.get(SPANDATA.HTTP_STATUS_CODE) if status_code: diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index f70964e6dd..c8b15d4f17 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -484,14 +484,18 @@ async def hello(request): @pytest.mark.asyncio async def test_crumb_capture( - sentry_init, aiohttp_raw_server, aiohttp_client, capture_events + sentry_init, + aiohttp_raw_server, + aiohttp_client, + capture_events, ): def before_breadcrumb(crumb, hint): crumb["data"]["extra"] = "foo" return crumb sentry_init( - integrations=[AioHttpIntegration()], before_breadcrumb=before_breadcrumb + integrations=[AioHttpIntegration()], + before_breadcrumb=before_breadcrumb, ) async def handler(request): @@ -525,6 +529,94 @@ async def handler(request): ) +@pytest.mark.asyncio +@pytest.mark.parametrize( + "pii_options,url_expected,query_expected", + [ + ({}, False, False), + ({"send_default_pii": True}, True, True), + ({"send_default_pii": False}, False, False), + ( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "denylist", "terms": []} + } + } + }, + True, + True, + ), + ( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "allowlist", "terms": []} + } + } + }, + True, + False, + ), + ], +) +async def test_crumb_capture_span_streaming( + sentry_init, + aiohttp_raw_server, + aiohttp_client, + capture_events, + pii_options, + url_expected, + query_expected, +): + def before_breadcrumb(crumb, hint): + crumb["data"]["extra"] = "foo" + return crumb + + sentry_init( + integrations=[AioHttpIntegration()], + before_breadcrumb=before_breadcrumb, + trace_lifecycle="stream", + **pii_options, + ) + + async def handler(request): + return web.Response(text="OK") + + raw_server = await aiohttp_raw_server(handler) + + events = capture_events() + + client = await aiohttp_client(raw_server) + resp = await client.get("/?query=value") + assert resp.status == 200 + capture_message("Testing!") + + (event,) = events + + crumb = event["breadcrumbs"]["values"][0] + assert crumb["type"] == "http" + assert crumb["category"] == "httplib" + + expected = { + "http.method": "GET", + "http.response.status_code": 200, + "reason": "OK", + } + + if url_expected: + if query_expected: + expected["url"] = f"http://127.0.0.1:{raw_server.port}/?query=value" + expected["http.query"] = "query=value" + else: + expected["url"] = ( + f"http://127.0.0.1:{raw_server.port}/?query=%5BFiltered%5D" + ) + expected["http.query"] = "query=%5BFiltered%5D" + + assert crumb["data"] == ApproxDict(expected) + + @pytest.mark.parametrize( "status_code,level", [ @@ -579,6 +671,104 @@ async def handler(request): ) +@pytest.mark.parametrize( + "status_code,level,reason", + [ + (200, None, "OK"), + (301, None, "Moved Permanently"), + (403, "warning", "Forbidden"), + (405, "warning", "Method Not Allowed"), + (500, "error", "Internal Server Error"), + ], +) +@pytest.mark.parametrize( + "pii_options,url_expected,query_expected", + [ + ({}, False, False), + ({"send_default_pii": True}, True, True), + ({"send_default_pii": False}, False, False), + ( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "denylist", "terms": []} + } + } + }, + True, + True, + ), + ( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "allowlist", "terms": []} + } + } + }, + True, + False, + ), + ], +) +@pytest.mark.asyncio +async def test_crumb_capture_client_error_span_streaming( + sentry_init, + aiohttp_raw_server, + aiohttp_client, + capture_events, + status_code, + level, + reason, + pii_options, + url_expected, + query_expected, +): + sentry_init( + integrations=[AioHttpIntegration()], trace_lifecycle="stream", **pii_options + ) + + async def handler(request): + return web.Response(status=status_code) + + raw_server = await aiohttp_raw_server(handler) + + events = capture_events() + + client = await aiohttp_client(raw_server) + resp = await client.get("/?query=value") + assert resp.status == status_code + capture_message("Testing!") + + (event,) = events + + crumb = event["breadcrumbs"]["values"][0] + assert crumb["type"] == "http" + if level is None: + assert "level" not in crumb + else: + assert crumb["level"] == level + assert crumb["category"] == "httplib" + + expected = { + "http.method": "GET", + "http.response.status_code": status_code, + "reason": reason, + } + + if url_expected: + if query_expected: + expected["url"] = f"http://127.0.0.1:{raw_server.port}/?query=value" + expected["http.query"] = "query=value" + else: + expected["url"] = ( + f"http://127.0.0.1:{raw_server.port}/?query=%5BFiltered%5D" + ) + expected["http.query"] = "query=%5BFiltered%5D" + + assert crumb["data"] == ApproxDict(expected) + + @pytest.mark.asyncio async def test_outgoing_trace_headers(sentry_init, aiohttp_raw_server, aiohttp_client): sentry_init( @@ -885,7 +1075,7 @@ def fake_create_trace_context(*args, **kwargs): trace_context = create_trace_config() async def overwrite_timestamps(session, trace_config_ctx, params): - span = trace_config_ctx.span + span = trace_config_ctx._sentry_span span.start_timestamp = datetime.datetime(2024, 1, 1, microsecond=0) span.timestamp = datetime.datetime(2024, 1, 1, microsecond=99999) @@ -944,7 +1134,7 @@ def fake_create_trace_context(*args, **kwargs): trace_context = create_trace_config() async def overwrite_timestamps(session, trace_config_ctx, params): - span = trace_config_ctx.span + span = trace_config_ctx._sentry_span span.start_timestamp = datetime.datetime(2024, 1, 1, microsecond=0) span.timestamp = datetime.datetime(2024, 1, 1, microsecond=100001)