From 1dc4c28b23917d608223d3b948545d3a6ea57151 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Fri, 15 Nov 2024 12:48:21 +0100 Subject: [PATCH] Extract span attrs from AIOHTTP request (#3782) --- MIGRATION_GUIDE.md | 15 +++++- sentry_sdk/integrations/aiohttp.py | 46 ++++++++++++++++--- .../integrations/opentelemetry/sampler.py | 1 - tests/integrations/aiohttp/test_aiohttp.py | 40 ++++++++-------- 4 files changed, 74 insertions(+), 28 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index da84dc1758..fce361a9ec 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -21,7 +21,18 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - clickhouse-driver integration: The query is now available under the `db.query.text` span attribute (only if `send_default_pii` is `True`). - `sentry_sdk.init` now returns `None` instead of a context manager. - The `sampling_context` argument of `traces_sampler` now additionally contains all span attributes known at span start. -- The `sampling_context` argument of `traces_sampler` doesn't contain the `wsgi_environ` object anymore for WSGI frameworks. Instead, the individual properties of the environment are accessible, if available, as follows: +- If you're using the AIOHTTP integration, the `sampling_context` argument of `traces_sampler` doesn't contain the `aiohttp_request` object anymore. Instead, some of the individual properties of the request are accessible, if available, as follows: + + | Request property | Sampling context key(s) | + | ---------------- | ------------------------------- | + | `path` | `url.path` | + | `query_string` | `url.query` | + | `method` | `http.request.method` | + | `host` | `server.address`, `server.port` | + | `scheme` | `url.scheme` | + | full URL | `url.full` | + +- If you're using the generic WSGI integration, the `sampling_context` argument of `traces_sampler` doesn't contain the `wsgi_environ` object anymore. Instead, the individual properties of the environment are accessible, if available, as follows: | Env property | Sampling context key(s) | | ----------------- | ------------------------------------------------- | @@ -34,7 +45,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh | `wsgi.url_scheme` | `url.scheme` | | full URL | `url.full` | -- The `sampling_context` argument of `traces_sampler` doesn't contain the `asgi_scope` object anymore for ASGI frameworks. Instead, the individual properties of the scope, if available, are accessible as follows: +- If you're using the generic ASGI integration, the `sampling_context` argument of `traces_sampler` doesn't contain the `asgi_scope` object anymore. Instead, the individual properties of the scope, if available, are accessible as follows: | Scope property | Sampling context key(s) | | -------------- | ------------------------------- | diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 0928c14c8b..9257eca49a 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -65,6 +65,13 @@ TRANSACTION_STYLE_VALUES = ("handler_name", "method_and_path_pattern") +REQUEST_PROPERTY_TO_ATTRIBUTE = { + "query_string": "url.query", + "method": "http.request.method", + "scheme": "url.scheme", + "path": "url.path", +} + class AioHttpIntegration(Integration): identifier = "aiohttp" @@ -127,19 +134,19 @@ async def sentry_app_handle(self, request, *args, **kwargs): headers = dict(request.headers) with sentry_sdk.continue_trace(headers): - with sentry_sdk.start_transaction( + with sentry_sdk.start_span( op=OP.HTTP_SERVER, # If this transaction name makes it to the UI, AIOHTTP's # URL resolver did not find a route or died trying. name="generic AIOHTTP request", source=TRANSACTION_SOURCE_ROUTE, origin=AioHttpIntegration.origin, - custom_sampling_context={"aiohttp_request": request}, - ) as transaction: + attributes=_prepopulate_attributes(request), + ) as span: try: response = await old_handle(self, request) except HTTPException as e: - transaction.set_http_status(e.status_code) + span.set_http_status(e.status_code) if ( e.status_code @@ -149,14 +156,14 @@ async def sentry_app_handle(self, request, *args, **kwargs): raise except (asyncio.CancelledError, ConnectionResetError): - transaction.set_status(SPANSTATUS.CANCELLED) + span.set_status(SPANSTATUS.CANCELLED) 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()) - transaction.set_http_status(response.status) + span.set_http_status(response.status) return response Application._handle = sentry_app_handle @@ -363,3 +370,30 @@ def get_aiohttp_request_data(request): # request has no body return None + + +def _prepopulate_attributes(request): + # type: (Request) -> dict[str, Any] + """Construct initial span attributes that can be used in traces sampler.""" + attributes = {} + + for prop, attr in REQUEST_PROPERTY_TO_ATTRIBUTE.items(): + if getattr(request, prop, None) is not None: + attributes[attr] = getattr(request, prop) + + if getattr(request, "host", None) is not None: + try: + host, port = request.host.split(":") + attributes["server.address"] = host + attributes["server.port"] = port + except ValueError: + attributes["server.address"] = request.host + + try: + url = f"{request.scheme}://{request.host}{request.path}" + if request.query_string: + attributes["url.full"] = f"{url}?{request.query_string}" + except Exception: + pass + + return attributes diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 302b66aaaa..0997048532 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -153,7 +153,6 @@ def should_sample( } sampling_context.update(attributes) sample_rate = client.options["traces_sampler"](sampling_context) - else: # Check if there is a parent with a sampling decision parent_sampled = get_parent_sampled(parent_span_context, trace_id) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 432427b08e..8327832acc 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -1,12 +1,12 @@ import asyncio import json +import re from contextlib import suppress from unittest import mock import pytest from aiohttp import web, ClientSession from aiohttp.client import ServerDisconnectedError -from aiohttp.web_request import Request from aiohttp.web_exceptions import ( HTTPInternalServerError, HTTPNetworkAuthenticationRequired, @@ -291,13 +291,12 @@ async def hello(request): @pytest.mark.asyncio -async def test_traces_sampler_gets_request_object_in_sampling_context( +async def test_traces_sampler_gets_attributes_in_sampling_context( sentry_init, aiohttp_client, - DictionaryContaining, # noqa: N803 - ObjectDescribedBy, # noqa: N803 ): - traces_sampler = mock.Mock() + traces_sampler = mock.Mock(return_value=True) + sentry_init( integrations=[AioHttpIntegration()], traces_sampler=traces_sampler, @@ -310,17 +309,21 @@ async def kangaroo_handler(request): app.router.add_get("/tricks/kangaroo", kangaroo_handler) client = await aiohttp_client(app) - await client.get("/tricks/kangaroo") - - traces_sampler.assert_any_call( - DictionaryContaining( - { - "aiohttp_request": ObjectDescribedBy( - type=Request, attrs={"method": "GET", "path": "/tricks/kangaroo"} - ) - } - ) + await client.get("/tricks/kangaroo?jump=high") + + assert traces_sampler.call_count == 1 + sampling_context = traces_sampler.call_args_list[0][0][0] + assert isinstance(sampling_context, dict) + assert re.match( + r"http:\/\/127\.0\.0\.1:[0-9]{4,5}\/tricks\/kangaroo\?jump=high", + sampling_context["url.full"], ) + assert sampling_context["url.path"] == "/tricks/kangaroo" + assert sampling_context["url.query"] == "jump=high" + assert sampling_context["url.scheme"] == "http" + assert sampling_context["http.request.method"] == "GET" + assert sampling_context["server.address"] == "127.0.0.1" + assert sampling_context["server.port"].isnumeric() @pytest.mark.asyncio @@ -574,9 +577,8 @@ async def handler(request): client = await aiohttp_client(raw_server) resp = await client.get("/", headers={"bagGage": "custom=value"}) - assert ( - sorted(resp.request_info.headers["baggage"].split(",")) - == sorted([ + assert sorted(resp.request_info.headers["baggage"].split(",")) == sorted( + [ "custom=value", f"sentry-trace_id={transaction.trace_id}", "sentry-environment=production", @@ -584,7 +586,7 @@ async def handler(request): "sentry-transaction=/interactions/other-dogs/new-dog", "sentry-sample_rate=1.0", "sentry-sampled=true", - ]) + ] )