move failsafe hook handling down to OpenTelemetryMiddleware

Author: outergod

instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

@@ -268,7 +268,7 @@ def client_response_hook(span: Span, scope: Scope, message: dict[str, Any]):
     HTTP_SERVER_REQUEST_DURATION,
 )
 from opentelemetry.semconv.trace import SpanAttributes
-from opentelemetry.trace import set_span_in_context
+from opentelemetry.trace import Span, set_span_in_context
 from opentelemetry.util.http import (
     OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS,
     OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST,
@@ -646,9 +646,21 @@ def __init__(
         self.default_span_details = (
             default_span_details or get_default_span_details
         )
-        self.server_request_hook = server_request_hook
-        self.client_request_hook = client_request_hook
-        self.client_response_hook = client_response_hook
+
+        def failsafe(func):
+            @wraps(func)
+            def wrapper(span: Span, *args, **kwargs):
+                if func is not None:
+                    try:
+                        func(span, *args, **kwargs)
+                    except Exception as exc:  # pylint: disable=broad-exception-caught
+                        span.record_exception(exc)
+
+            return wrapper
+
+        self.server_request_hook = failsafe(server_request_hook)
+        self.client_request_hook = failsafe(client_request_hook)
+        self.client_response_hook = failsafe(client_response_hook)
         self.content_length_header = None
         self._sem_conv_opt_in_mode = sem_conv_opt_in_mode
 

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py

@@ -189,6 +189,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 
 import fastapi
 from starlette.applications import Starlette
+from starlette.middleware.errors import ServerErrorMiddleware
 from starlette.routing import Match
 from starlette.types import ASGIApp, Receive, Scope, Send
 
@@ -210,7 +211,6 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 from opentelemetry.metrics import MeterProvider, get_meter
 from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE
 from opentelemetry.trace import (
-    Span,
     TracerProvider,
     get_current_span,
     get_tracer,
@@ -300,28 +300,17 @@ def instrument_app(
             # exceptions from user-provided hooks of tearing through, we wrap
             # them to return without failure unconditionally.
             def build_middleware_stack(self: Starlette) -> ASGIApp:
-                def failsafe(func):
-                    @functools.wraps(func)
-                    def wrapper(span: Span, *args, **kwargs):
-                        if func is not None:
-                            try:
-                                func(span, *args, **kwargs)
-                            except Exception as exc:  # pylint: disable=broad-exception-caught
-                                span.record_exception(exc)
-
-                    return wrapper
-
                 inner_server_error_middleware: ASGIApp = (  # type: ignore
                     self._original_build_middleware_stack()  # type: ignore
                 )
 
-                return OpenTelemetryMiddleware(
+                otel_middleware = OpenTelemetryMiddleware(
                     inner_server_error_middleware,
                     excluded_urls=excluded_urls,
                     default_span_details=_get_default_span_details,
-                    server_request_hook=failsafe(server_request_hook),
-                    client_request_hook=failsafe(client_request_hook),
-                    client_response_hook=failsafe(client_response_hook),
+                    server_request_hook=server_request_hook,
+                    client_request_hook=client_request_hook,
+                    client_response_hook=client_response_hook,
                     # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
                     tracer=tracer,
                     meter=meter,
@@ -331,6 +320,24 @@ def wrapper(span: Span, *args, **kwargs):
                     exclude_spans=exclude_spans,
                 )
 
+                # Wrap in an outer layer of ServerErrorMiddleware so that any exceptions raised in OpenTelemetryMiddleware
+                # are handled.
+                # This should not happen unless there is a bug in OpenTelemetryMiddleware, but if there is we don't want that
+                # to impact the user's application just because we wrapped the middlewares in this order.
+                if isinstance(
+                    inner_server_error_middleware, ServerErrorMiddleware
+                ):  # usually true
+                    outer_server_error_middleware = ServerErrorMiddleware(
+                        app=otel_middleware,
+                    )
+                else:
+                    # Something else seems to have patched things, or maybe Starlette changed.
+                    # Just create a default ServerErrorMiddleware.
+                    outer_server_error_middleware = ServerErrorMiddleware(
+                        app=otel_middleware
+                    )
+                return outer_server_error_middleware
+
             app._original_build_middleware_stack = app.build_middleware_stack
             app.build_middleware_stack = types.MethodType(
                 functools.wraps(app.build_middleware_stack)(