Convert FastAPI arguments log to span, don't set to debug by default (#164)

Author: alexmojaki

logfire/_internal/integrations/fastapi.py

@@ -3,7 +3,7 @@
 import inspect
 from contextlib import contextmanager
 from functools import lru_cache
-from typing import Any, Callable, ContextManager, Iterable, Literal, cast
+from typing import Any, Awaitable, Callable, ContextManager, Iterable, cast
 from weakref import WeakKeyDictionary
 
 import fastapi.routing
@@ -88,11 +88,11 @@ def patch_fastapi():
     registry: WeakKeyDictionary[FastAPI, FastAPIInstrumentation] = WeakKeyDictionary()
 
     async def patched_solve_dependencies(*, request: Request | WebSocket, **kwargs: Any):
-        result = await original_solve_dependencies(request=request, **kwargs)
+        original = original_solve_dependencies(request=request, **kwargs)
         if instrumentation := registry.get(request.app):
-            return await instrumentation.solve_dependencies(request, result)
+            return await instrumentation.solve_dependencies(request, original)
         else:
-            return result  # pragma: no cover
+            return await original  # pragma: no cover
 
     # `solve_dependencies` is actually defined in `fastapi.dependencies.utils`,
     # but it's imported into `fastapi.routing`, which is where we need to patch it.
@@ -139,60 +139,73 @@ def __init__(
             self.excluded_urls_list = parse_excluded_urls(excluded_urls)  # pragma: no cover
 
     async def solve_dependencies(
-        self, request: Request | WebSocket, result: tuple[dict[str, Any], list[Any], Any, Any, Any]
+        self, request: Request | WebSocket, original: Awaitable[tuple[dict[str, Any], list[Any], Any, Any, Any]]
     ):
         try:
             url = cast(str, get_host_port_url_tuple(request.scope)[2])
-            if self.excluded_urls_list.url_disabled(url):
-                return result  # pragma: no cover
-
-            attributes: dict[str, Any] | None = {
-                # Shallow copy these so that the user can safely modify them, but we don't tell them that.
-                # We do explicitly tell them that the contents should not be modified.
-                # Making a deep copy could be very expensive and maybe even impossible.
-                'values': {
-                    k: v
-                    for k, v in result[0].items()
-                    if not isinstance(v, (Request, WebSocket, BackgroundTasks, SecurityScopes, Response))
-                },
-                'errors': result[1].copy(),
-            }
-
-            # Set the current app on `values` so that `patched_run_endpoint_function` can check it.
-            if isinstance(request, Request):  # pragma: no branch
-                instrumented_values = _InstrumentedValues(result[0])
-                instrumented_values.request = request
-                result = (instrumented_values, *result[1:])
-
-            attributes = self.request_attributes_mapper(request, attributes)
-            if not attributes:
-                # The user can return None to indicate that they don't want to log anything.
-                # We don't document it, but returning `False`, `{}` etc. seems like it should also work.
-                return result
-
-            # request_attributes_mapper may have removed the errors, so we need .get() here.
-            level: Literal['error', 'debug'] = 'error' if attributes.get('errors') else 'debug'
-
-            # Add a few basic attributes about the request, particularly so that the user can group logs by endpoint.
-            # Usually this will all be inside a span added by FastAPIInstrumentor with more detailed attributes.
-            # We only add these attributes after the request_attributes_mapper so that the user
-            # doesn't rely on what we add here - they can use `request` instead.
-            if isinstance(request, Request):  # pragma: no branch
-                attributes[SpanAttributes.HTTP_METHOD] = request.method
-            route: APIRoute | APIWebSocketRoute | None = request.scope.get('route')
-            if route:  # pragma: no branch
-                attributes.update(
-                    {
-                        SpanAttributes.HTTP_ROUTE: route.path,
-                        'fastapi.route.name': route.name,
-                    }
-                )
-                if isinstance(route, APIRoute):  # pragma: no branch
-                    attributes['fastapi.route.operation_id'] = route.operation_id
-
-            self.logfire_instance.log(level, 'FastAPI arguments', attributes=attributes)
+            excluded = self.excluded_urls_list.url_disabled(url)
         except Exception:  # pragma: no cover
-            self.logfire_instance.exception('Error logging FastAPI arguments')
+            excluded = False
+            self.logfire_instance.exception('Error checking if URL is excluded from instrumentation')
+
+        if excluded:
+            return await original  # pragma: no cover
+
+        with self.logfire_instance.span('FastAPI arguments') as span:
+            result = await original
+
+            try:
+                attributes: dict[str, Any] | None = {
+                    # Shallow copy these so that the user can safely modify them, but we don't tell them that.
+                    # We do explicitly tell them that the contents should not be modified.
+                    # Making a deep copy could be very expensive and maybe even impossible.
+                    'values': {
+                        k: v
+                        for k, v in result[0].items()
+                        if not isinstance(v, (Request, WebSocket, BackgroundTasks, SecurityScopes, Response))
+                    },
+                    'errors': result[1].copy(),
+                }
+
+                # Set the current app on `values` so that `patched_run_endpoint_function` can check it.
+                if isinstance(request, Request):  # pragma: no branch
+                    instrumented_values = _InstrumentedValues(result[0])
+                    instrumented_values.request = request
+                    result = (instrumented_values, *result[1:])
+
+                attributes = self.request_attributes_mapper(request, attributes)
+                if not attributes:
+                    # The user can return None to indicate that they don't want to log anything.
+                    # We don't document it, but returning `False`, `{}` etc. seems like it should also work.
+                    # We can't drop the span since it's already been created,
+                    # but we can set the level to debug so that it's hidden by default.
+                    span.set_level('debug')
+                    return result
+
+                # request_attributes_mapper may have removed the errors, so we need .get() here.
+                if attributes.get('errors'):
+                    span.set_level('error')
+
+                # Add a few basic attributes about the request, particularly so that the user can group logs by endpoint.
+                # Usually this will all be inside a span added by FastAPIInstrumentor with more detailed attributes.
+                # We only add these attributes after the request_attributes_mapper so that the user
+                # doesn't rely on what we add here - they can use `request` instead.
+                if isinstance(request, Request):  # pragma: no branch
+                    attributes[SpanAttributes.HTTP_METHOD] = request.method
+                route: APIRoute | APIWebSocketRoute | None = request.scope.get('route')
+                if route:  # pragma: no branch
+                    attributes.update(
+                        {
+                            SpanAttributes.HTTP_ROUTE: route.path,
+                            'fastapi.route.name': route.name,
+                        }
+                    )
+                    if isinstance(route, APIRoute):  # pragma: no branch
+                        attributes['fastapi.route.operation_id'] = route.operation_id
+
+                span.set_attributes(attributes)
+            except Exception as e:  # pragma: no cover
+                span.record_exception(e)
 
         return result