Extract span attrs from AIOHTTP request (#3782)

Author: sentrivana

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)         |
   | -------------- | ------------------------------- |

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

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)

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,17 +577,16 @@ 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",
                 "sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42",
                 "sentry-transaction=/interactions/other-dogs/new-dog",
                 "sentry-sample_rate=1.0",
                 "sentry-sampled=true",
-            ])
+            ]
         )