From 50120cee62cf2acd91c3700377c1cb15da226398 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 09:43:08 +0200 Subject: [PATCH 01/36] rewrite FastAPIInstrumentor:build_middleware_stack to become failsafe --- .../instrumentation/fastapi/__init__.py | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 8ba83985c6..fa32c2415f 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -289,22 +289,33 @@ def instrument_app( schema_url=_get_schema_url(sem_conv_opt_in_mode), ) - # Instead of using `app.add_middleware` we monkey patch `build_middleware_stack` to insert our middleware - # as the outermost middleware. - # Otherwise `OpenTelemetryMiddleware` would have unhandled exceptions tearing through it and would not be able - # to faithfully record what is returned to the client since it technically cannot know what `ServerErrorMiddleware` is going to do. - + # In order to make traces available at any stage of the request + # processing - including exception handling - we wrap ourselves as + # the new, outermost middleware. However in order to prevent + # 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(*args, **kwargs): + try: + return func(*args, **kwargs) + except Exception: + pass + + return wrapper + inner_server_error_middleware: ASGIApp = ( # type: ignore self._original_build_middleware_stack() # type: ignore ) - otel_middleware = OpenTelemetryMiddleware( + + return OpenTelemetryMiddleware( inner_server_error_middleware, excluded_urls=excluded_urls, default_span_details=_get_default_span_details, - server_request_hook=server_request_hook, - client_request_hook=client_request_hook, - client_response_hook=client_response_hook, + server_request_hook=failsafe(server_request_hook), + client_request_hook=failsafe(client_request_hook), + client_response_hook=failsafe(client_response_hook), # Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation tracer=tracer, meter=meter, @@ -313,23 +324,6 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: http_capture_headers_sanitize_fields=http_capture_headers_sanitize_fields, 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( From 8a1a39ecf3f2ba52c0af21e3601728f1c411785a Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 12:31:25 +0200 Subject: [PATCH 02/36] add test cases for FastAPI failsafe handling --- .../tests/test_fastapi_instrumentation.py | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 523c165f85..f53e05b621 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1877,3 +1877,107 @@ def test_custom_header_not_present_in_non_recording_span(self): self.assertEqual(200, resp.status_code) span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 0) + + +class TestTraceableExceptionHandling(TestBase): + """Tests to ensure FastAPI exception handlers are only executed once and with a valid context""" + + def setUp(self): + super().setUp() + + self.app = fastapi.FastAPI() + + otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + self.client = TestClient(self.app) + self.tracer = self.tracer_provider.get_tracer(__name__) + self.executed = 0 + self.request_trace_id = None + self.error_trace_id = None + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + def test_error_handler_context(self): + """OTEL tracing contexts must be available during error handler execution""" + + @self.app.exception_handler(Exception) + async def _(*_): + self.error_trace_id = ( + trace.get_current_span().get_span_context().trace_id + ) + + @self.app.get("/foobar") + async def _(): + self.request_trace_id = ( + trace.get_current_span().get_span_context().trace_id + ) + raise Exception("Test Exception") + + try: + self.client.get( + "/foobar", + ) + except Exception: + pass + + self.assertIsNotNone(self.request_trace_id) + self.assertEqual(self.request_trace_id, self.error_trace_id) + + def test_error_handler_side_effects(self): + """FastAPI default exception handlers (aka error handlers) must be executed exactly once per exception""" + + @self.app.exception_handler(Exception) + async def _(*_): + self.executed += 1 + + @self.app.get("/foobar") + async def _(): + raise Exception("Test Exception") + + try: + self.client.get( + "/foobar", + ) + except Exception: + pass + + self.assertEqual(self.executed, 1) + + +class TestFailsafeHooks(TestBase): + """Tests to ensure FastAPI instrumentation hooks don't tear through""" + + def setUp(self): + super().setUp() + + self.app = fastapi.FastAPI() + + @self.app.get("/foobar") + async def _(): + return {"message": "Hello World"} + + def failing_hook(*_): + raise Exception("Hook Exception") + + otel_fastapi.FastAPIInstrumentor().instrument_app( + self.app, + server_request_hook=failing_hook, + client_request_hook=failing_hook, + client_response_hook=failing_hook, + ) + self.client = TestClient(self.app) + + def tearDown(self) -> None: + super().tearDown() + with self.disable_logging(): + otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) + + def test_failsafe_hooks(self): + """Crashing hooks must not tear through""" + resp = self.client.get( + "/foobar", + ) + + self.assertEqual(200, resp.status_code) From 2150e5c0ca4b62210454a4015a6ef050504227e3 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 12:59:56 +0200 Subject: [PATCH 03/36] add CHANGELOG entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f252e8290..51c963bd9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- `opentelemetry-instrumentation-fastapi`: Implemented failsafe middleware stack. + ([#3664](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3664)) + ## Version 1.36.0/0.57b0 (2025-07-29) ### Fixed From 2faf00534e009d8ab165564646457d0022b480c3 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 13:02:09 +0200 Subject: [PATCH 04/36] remove unused import --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index fa32c2415f..7ed6b36cf3 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -189,7 +189,6 @@ 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 From 5665a0ada680dfea0c9bcf5241f1390bdacf0088 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 13:06:11 +0200 Subject: [PATCH 05/36] [lint] don't return from failsafe wrapper --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 7ed6b36cf3..9551f8b2a9 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -298,7 +298,7 @@ def failsafe(func): @functools.wraps(func) def wrapper(*args, **kwargs): try: - return func(*args, **kwargs) + func(*args, **kwargs) except Exception: pass From 425a7f3e9c029b5075eca7712069e910dc29b9e8 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 13:13:41 +0200 Subject: [PATCH 06/36] [lint] allow broad exceptions --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 9551f8b2a9..00ed7846f0 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -299,7 +299,7 @@ def failsafe(func): def wrapper(*args, **kwargs): try: func(*args, **kwargs) - except Exception: + except Exception: # pylint: disable=W0718 pass return wrapper From 6c487038f003727109c2bd3a61242ffec752152d Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 30 Jul 2025 13:29:45 +0200 Subject: [PATCH 07/36] [lint] more allowing --- .../tests/test_fastapi_instrumentation.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index f53e05b621..3bc638d264 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1913,13 +1913,13 @@ async def _(): self.request_trace_id = ( trace.get_current_span().get_span_context().trace_id ) - raise Exception("Test Exception") + raise UnhandledException("Test Exception") try: self.client.get( "/foobar", ) - except Exception: + except Exception: # pylint: disable=W0718 pass self.assertIsNotNone(self.request_trace_id) @@ -1934,13 +1934,13 @@ async def _(*_): @self.app.get("/foobar") async def _(): - raise Exception("Test Exception") + raise UnhandledException("Test Exception") try: self.client.get( "/foobar", ) - except Exception: + except Exception: # pylint: disable=W0718 pass self.assertEqual(self.executed, 1) @@ -1959,7 +1959,7 @@ async def _(): return {"message": "Hello World"} def failing_hook(*_): - raise Exception("Hook Exception") + raise UnhandledException("Hook Exception") otel_fastapi.FastAPIInstrumentor().instrument_app( self.app, From 55e9c438329716f4ef32a50439d51770c946ca31 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 14 Aug 2025 13:06:15 +0200 Subject: [PATCH 08/36] record FastAPI hook exceptions in active span --- .../instrumentation/fastapi/__init__.py | 17 +++++++++----- .../tests/test_fastapi_instrumentation.py | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 00ed7846f0..7afcd40226 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -209,7 +209,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.metrics import MeterProvider, get_meter from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE -from opentelemetry.trace import TracerProvider, get_tracer +from opentelemetry.trace import Span, TracerProvider, get_tracer from opentelemetry.util.http import ( get_excluded_urls, parse_excluded_urls, @@ -296,11 +296,18 @@ def instrument_app( def build_middleware_stack(self: Starlette) -> ASGIApp: def failsafe(func): @functools.wraps(func) - def wrapper(*args, **kwargs): + def wrapper(span: Span, *args, **kwargs): try: - func(*args, **kwargs) - except Exception: # pylint: disable=W0718 - pass + func(span, *args, **kwargs) + except Exception as exc: # pylint: disable=W0718 + span.record_exception(exc) + # span.set_status(Status(StatusCode.ERROR, str(exc))) + # if span.is_recording(): + # span.set_attribute( + # ErrorAttributes.ERROR_TYPE, type(exc).__qualname__ + # ) + # if span.record_exception + # span.end() return wrapper diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 3bc638d264..aee5a4b58b 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -59,6 +59,9 @@ from opentelemetry.semconv._incubating.attributes.net_attributes import ( NET_HOST_PORT, ) +from opentelemetry.semconv.attributes.exception_attributes import ( + EXCEPTION_TYPE, +) from opentelemetry.semconv.attributes.http_attributes import ( HTTP_REQUEST_METHOD, HTTP_RESPONSE_STATUS_CODE, @@ -1981,3 +1984,22 @@ def test_failsafe_hooks(self): ) self.assertEqual(200, resp.status_code) + + def test_failsafe_error_recording(self): + """Failing hooks must record the exception on the span""" + self.client.get( + "/foobar", + ) + + spans = self.memory_exporter.get_finished_spans() + + self.assertEqual(len(spans), 3) + span = spans[0] + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + assert event.attributes is not None + self.assertEqual( + event.attributes.get(EXCEPTION_TYPE), + f"{__name__}.UnhandledException", + ) From a7a4949f2f119bacad5dd4658ad4dd9a8764f817 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 14 Aug 2025 13:07:30 +0200 Subject: [PATCH 09/36] remove comment --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 7afcd40226..45396a8cb8 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -301,13 +301,6 @@ def wrapper(span: Span, *args, **kwargs): func(span, *args, **kwargs) except Exception as exc: # pylint: disable=W0718 span.record_exception(exc) - # span.set_status(Status(StatusCode.ERROR, str(exc))) - # if span.is_recording(): - # span.set_attribute( - # ErrorAttributes.ERROR_TYPE, type(exc).__qualname__ - # ) - # if span.record_exception - # span.end() return wrapper From 5632d1b84d9f589fec7bdc054c34070c9bf95f94 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 14 Aug 2025 14:35:14 +0200 Subject: [PATCH 10/36] properly deal with hooks not being set --- .../opentelemetry/instrumentation/fastapi/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 45396a8cb8..0039ab5b0d 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -297,10 +297,11 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: def failsafe(func): @functools.wraps(func) def wrapper(span: Span, *args, **kwargs): - try: - func(span, *args, **kwargs) - except Exception as exc: # pylint: disable=W0718 - span.record_exception(exc) + if func is not None: + try: + func(span, *args, **kwargs) + except Exception as exc: # pylint: disable=W0718 + span.record_exception(exc) return wrapper From 0203e4be1fc59c1aa20f15de557ae623ef18ba42 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 14 Aug 2025 15:42:37 +0200 Subject: [PATCH 11/36] add custom FastAPI exception recording --- .../instrumentation/fastapi/__init__.py | 34 +++++++++++++++++-- .../tests/test_fastapi_instrumentation.py | 29 ++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 0039ab5b0d..3eb9d804b7 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -190,7 +190,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A import fastapi from starlette.applications import Starlette from starlette.routing import Match -from starlette.types import ASGIApp +from starlette.types import ASGIApp, Receive, Scope, Send from opentelemetry.instrumentation._semconv import ( _get_schema_url, @@ -209,7 +209,13 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.metrics import MeterProvider, get_meter from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE -from opentelemetry.trace import Span, TracerProvider, get_tracer +from opentelemetry.trace import ( + Span, + TracerProvider, + get_current_span, + get_tracer, +) +from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( get_excluded_urls, parse_excluded_urls, @@ -300,7 +306,7 @@ def wrapper(span: Span, *args, **kwargs): if func is not None: try: func(span, *args, **kwargs) - except Exception as exc: # pylint: disable=W0718 + except Exception as exc: # pylint: disable=broad-exception-caught span.record_exception(exc) return wrapper @@ -333,6 +339,28 @@ def wrapper(span: Span, *args, **kwargs): app, ) + class ExceptionHandlerMiddleware: + def __init__(self, app): + self.app = app + + async def __call__( + self, scope: Scope, receive: Receive, send: Send + ) -> None: + try: + await self.app(scope, receive, send) + except Exception as exc: # pylint: disable=broad-exception-caught + span = get_current_span() + span.record_exception(exc) + span.set_status( + Status( + status_code=StatusCode.ERROR, + description=f"{type(exc).__name__}: {exc}", + ) + ) + raise + + app.add_middleware(ExceptionHandlerMiddleware) + app._is_instrumented_by_opentelemetry = True if app not in _InstrumentedFastAPI._instrumented_fastapi_apps: _InstrumentedFastAPI._instrumented_fastapi_apps.add(app) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index aee5a4b58b..0dec9c61fe 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -73,6 +73,7 @@ from opentelemetry.semconv.attributes.url_attributes import URL_SCHEME from opentelemetry.test.globals_test import reset_trace_globals from opentelemetry.test.test_base import TestBase +from opentelemetry.trace.status import StatusCode from opentelemetry.util._importlib_metadata import entry_points from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, @@ -1948,6 +1949,34 @@ async def _(): self.assertEqual(self.executed, 1) + def test_exception_span_recording(self): + """Exception are always recorded in the active span""" + + @self.app.get("/foobar") + async def _(): + raise UnhandledException("Test Exception") + + try: + self.client.get( + "/foobar", + ) + except Exception: # pylint: disable=W0718 + pass + + spans = self.memory_exporter.get_finished_spans() + + self.assertEqual(len(spans), 3) + span = spans[2] + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + assert event.attributes is not None + self.assertEqual( + event.attributes.get(EXCEPTION_TYPE), + f"{__name__}.UnhandledException", + ) + class TestFailsafeHooks(TestBase): """Tests to ensure FastAPI instrumentation hooks don't tear through""" From 4c17f00451341a17ca664d9ceead21790a4b4a2a Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 14 Aug 2025 15:56:04 +0200 Subject: [PATCH 12/36] move failsafe hook handling down to OpenTelemetryMiddleware --- .../instrumentation/asgi/__init__.py | 20 ++++++++-- .../instrumentation/fastapi/__init__.py | 39 +++++++++++-------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 7e72dbf11f..e6eb9c8dbb 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/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 diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 3eb9d804b7..bc0604c596 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/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)( From 56139a3196633b6da678a53879a7f22a40da0f22 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 14 Aug 2025 16:01:27 +0200 Subject: [PATCH 13/36] shut up pylint --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index bc0604c596..9ebe2b27c2 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -210,11 +210,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.metrics import MeterProvider, get_meter from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE -from opentelemetry.trace import ( - TracerProvider, - get_current_span, - get_tracer, -) +from opentelemetry.trace import TracerProvider, get_current_span, get_tracer from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( get_excluded_urls, @@ -247,7 +243,7 @@ def instrument_app( http_capture_headers_server_response: list[str] | None = None, http_capture_headers_sanitize_fields: list[str] | None = None, exclude_spans: list[Literal["receive", "send"]] | None = None, - ): + ): # pylint: disable=too-many-locals """Instrument an uninstrumented FastAPI application. Args: From 843d8c7564e3d952acc8b78785c693655ffe345c Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 10:53:09 +0200 Subject: [PATCH 14/36] optimize failsafe to check for `None` only once --- .../opentelemetry/instrumentation/asgi/__init__.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index e6eb9c8dbb..a6be59b86f 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -648,13 +648,15 @@ def __init__( ) def failsafe(func): + if func is None: + return None + @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) + try: + func(span, *args, **kwargs) + except Exception as exc: # pylint: disable=broad-exception-caught + span.record_exception(exc) return wrapper From c2c326e2c7699e88cd4338165ce908b0c0a6d4e9 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 11:03:31 +0200 Subject: [PATCH 15/36] remove confusing comment and simplify wrapper logic --- .../instrumentation/fastapi/__init__.py | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 9ebe2b27c2..c239bc07cb 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -290,11 +290,6 @@ def instrument_app( schema_url=_get_schema_url(sem_conv_opt_in_mode), ) - # In order to make traces available at any stage of the request - # processing - including exception handling - we wrap ourselves as - # the new, outermost middleware. However in order to prevent - # exceptions from user-provided hooks of tearing through, we wrap - # them to return without failure unconditionally. def build_middleware_stack(self: Starlette) -> ASGIApp: inner_server_error_middleware: ASGIApp = ( # type: ignore self._original_build_middleware_stack() # type: ignore @@ -320,19 +315,9 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: # 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 + return ServerErrorMiddleware( + app=otel_middleware, + ) app._original_build_middleware_stack = app.build_middleware_stack app.build_middleware_stack = types.MethodType( From 3a19941047ad878d840dfc3708446a5a587dbdbd Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 11:45:24 +0200 Subject: [PATCH 16/36] add clarifying comment --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index c239bc07cb..da6bf88881 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -327,6 +327,12 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: app, ) + # Define an additional middleware for exception handling that gets + # added as a regular user middleware. + # Normally, `opentelemetry.trace.use_span` covers the recording of + # exceptions into the active span, but because of order of + # middleware execution, that doesn't work so we have to do it in an + # inner middleware. class ExceptionHandlerMiddleware: def __init__(self, app): self.app = app From 4e29a7693a6b97f613316b0fa739a8144720c7bc Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 11:46:25 +0200 Subject: [PATCH 17/36] test proper exception / status code recording --- .../tests/test_fastapi_instrumentation.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 0dec9c61fe..0eb7af43af 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1929,6 +1929,22 @@ async def _(): self.assertIsNotNone(self.request_trace_id) self.assertEqual(self.request_trace_id, self.error_trace_id) + spans = self.memory_exporter.get_finished_spans() + + self.assertEqual(len(spans), 1) + span = spans[0] + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual( + len(span.events), 2 + ) # The other span is a `TypeError` from the exception handler not returning a proper response, that's expected + event = span.events[0] + self.assertEqual(event.name, "exception") + assert event.attributes is not None + self.assertEqual( + event.attributes.get(EXCEPTION_TYPE), + f"{__name__}.UnhandledException", + ) + def test_error_handler_side_effects(self): """FastAPI default exception handlers (aka error handlers) must be executed exactly once per exception""" From 19025487f1ce48fe25680cd4c6eba7d147832373 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 13:17:30 +0200 Subject: [PATCH 18/36] add HTTP status code check --- .../tests/test_fastapi_instrumentation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 0eb7af43af..177ef2c6fb 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1982,6 +1982,9 @@ async def _(): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) + span = spans[0] + self.assertEqual(span.name, "GET /foobar http send") + self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) span = spans[2] self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual(len(span.events), 1) From fce516475649fd92e2c0551d15df40177c0458ac Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 13:19:01 +0200 Subject: [PATCH 19/36] test HTTP status on the exception recording span --- .../tests/test_fastapi_instrumentation.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 177ef2c6fb..09b5b15616 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1982,10 +1982,9 @@ async def _(): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) - span = spans[0] - self.assertEqual(span.name, "GET /foobar http send") - self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) span = spans[2] + self.assertEqual(span.name, "GET /foobar") + self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual(len(span.events), 1) event = span.events[0] From 3cc31d03547e360b9086ef3b94ac163746fdd719 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 13:23:13 +0200 Subject: [PATCH 20/36] improve test by removing TypeError --- .../tests/test_fastapi_instrumentation.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 09b5b15616..04bbb47ae0 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -21,7 +21,7 @@ import fastapi from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware -from fastapi.responses import JSONResponse +from fastapi.responses import JSONResponse, PlainTextResponse from fastapi.testclient import TestClient import opentelemetry.instrumentation.fastapi as otel_fastapi @@ -38,9 +38,7 @@ from opentelemetry.instrumentation.auto_instrumentation._load import ( _load_instrumentors, ) -from opentelemetry.instrumentation.dependencies import ( - DependencyConflict, -) +from opentelemetry.instrumentation.dependencies import DependencyConflict from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, NumberDataPoint, @@ -1911,6 +1909,7 @@ async def _(*_): self.error_trace_id = ( trace.get_current_span().get_span_context().trace_id ) + return PlainTextResponse("", status_code=500) @self.app.get("/foobar") async def _(): @@ -1931,12 +1930,12 @@ async def _(): spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 1) - span = spans[0] + self.assertEqual(len(spans), 3) + span = spans[2] + self.assertEqual(span.name, "GET /foobar") + self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) self.assertEqual(span.status.status_code, StatusCode.ERROR) - self.assertEqual( - len(span.events), 2 - ) # The other span is a `TypeError` from the exception handler not returning a proper response, that's expected + self.assertEqual(len(span.events), 1) event = span.events[0] self.assertEqual(event.name, "exception") assert event.attributes is not None From 217763b3c33c0a63067ab03e40672749bfb63727 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 13:27:24 +0200 Subject: [PATCH 21/36] rectify comment/explanation on inner middleware for exception handling --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index da6bf88881..53a86ce8c7 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -330,9 +330,8 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: # Define an additional middleware for exception handling that gets # added as a regular user middleware. # Normally, `opentelemetry.trace.use_span` covers the recording of - # exceptions into the active span, but because of order of - # middleware execution, that doesn't work so we have to do it in an - # inner middleware. + # exceptions into the active span, but `OpenTelemetryMiddleware` + # ends the span too early before the excepetion can be recorded. class ExceptionHandlerMiddleware: def __init__(self, app): self.app = app From b4cdc0ea74bf2dd7dbdd946598adea2f4f75e5c8 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 13:29:34 +0200 Subject: [PATCH 22/36] minor typo --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 53a86ce8c7..37e2be7795 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -331,7 +331,7 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: # added as a regular user middleware. # Normally, `opentelemetry.trace.use_span` covers the recording of # exceptions into the active span, but `OpenTelemetryMiddleware` - # ends the span too early before the excepetion can be recorded. + # ends the span too early before the exception can be recorded. class ExceptionHandlerMiddleware: def __init__(self, app): self.app = app From 2739fbbc221a89314e78dc3465e2dac446b92fd2 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Mon, 18 Aug 2025 17:27:34 +0200 Subject: [PATCH 23/36] move ExceptionHandlingMiddleware as the outermost inner middleware Also improve code documentation and add another test. --- .../instrumentation/fastapi/__init__.py | 108 ++++++++++++------ .../tests/test_fastapi_instrumentation.py | 36 +++++- 2 files changed, 110 insertions(+), 34 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 37e2be7795..ae7edb930b 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -291,12 +291,77 @@ def instrument_app( ) def build_middleware_stack(self: Starlette) -> ASGIApp: - inner_server_error_middleware: ASGIApp = ( # type: ignore + # Define an additional middleware for exception handling that gets + # added as a regular user middleware. + # Normally, `opentelemetry.trace.use_span` covers the recording of + # exceptions into the active span, but `OpenTelemetryMiddleware` + # ends the span too early before the exception can be recorded. + class ExceptionHandlerMiddleware: + def __init__(self, app): + self.app = app + + async def __call__( + self, scope: Scope, receive: Receive, send: Send + ) -> None: + try: + await self.app(scope, receive, send) + except Exception as exc: # pylint: disable=broad-exception-caught + span = get_current_span() + span.record_exception(exc) + span.set_status( + Status( + status_code=StatusCode.ERROR, + description=f"{type(exc).__name__}: {exc}", + ) + ) + raise + + # For every possible use case of error handling, exception + # handling, trace availability in exception handlers and + # automatic exception recording to work, we need to make a + # series of wrapping and re-wrapping middlewares. + + # First, grab the original middleware stack from Starlette. It + # comprises a stack of + # `ServerErrorMiddleware` -> [user defined middlewares] -> `ExceptionMiddleware` + inner_server_error_middleware: ServerErrorMiddleware = ( # type: ignore self._original_build_middleware_stack() # type: ignore ) + if not isinstance( + inner_server_error_middleware, ServerErrorMiddleware + ): + # Oops, something changed about how Starlette creates middleware stacks + _logger.error( + "Cannot instrument FastAPI as the expected middleware stack has changed" + ) + return inner_server_error_middleware + + # We take [user defined middlewares] -> `ExceptionHandlerMiddleware` + # out of the outermost `ServerErrorMiddleware` and instead pass + # it to our own `ExceptionHandlerMiddleware` + exception_middleware = ExceptionHandlerMiddleware( + inner_server_error_middleware.app + ) + + # Now, we create a new `ServerErrorMiddleware` that wraps + # `ExceptionHandlerMiddleware` but otherwise uses the same + # original `handler` and debug setting. The end result is a + # middleware stack that's identical to the original stack except + # all user middlewares are covered by our + # `ExceptionHandlerMiddleware`. + error_middleware = ServerErrorMiddleware( + app=exception_middleware, + handler=inner_server_error_middleware.handler, + debug=inner_server_error_middleware.debug, + ) + + # Finally, we wrap the stack above in our actual OTEL + # middleware. As a result, an active tracing context exists for + # every use case of user-defined error and exception handlers as + # well as automatic recording of exceptions in active spans. otel_middleware = OpenTelemetryMiddleware( - inner_server_error_middleware, + error_middleware, excluded_urls=excluded_urls, default_span_details=_get_default_span_details, server_request_hook=server_request_hook, @@ -311,10 +376,14 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: 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. + # Ultimately, wrap everything in another default + # `ServerErrorMiddleware` (w/o user handlers) 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. return ServerErrorMiddleware( app=otel_middleware, ) @@ -327,33 +396,6 @@ def build_middleware_stack(self: Starlette) -> ASGIApp: app, ) - # Define an additional middleware for exception handling that gets - # added as a regular user middleware. - # Normally, `opentelemetry.trace.use_span` covers the recording of - # exceptions into the active span, but `OpenTelemetryMiddleware` - # ends the span too early before the exception can be recorded. - class ExceptionHandlerMiddleware: - def __init__(self, app): - self.app = app - - async def __call__( - self, scope: Scope, receive: Receive, send: Send - ) -> None: - try: - await self.app(scope, receive, send) - except Exception as exc: # pylint: disable=broad-exception-caught - span = get_current_span() - span.record_exception(exc) - span.set_status( - Status( - status_code=StatusCode.ERROR, - description=f"{type(exc).__name__}: {exc}", - ) - ) - raise - - app.add_middleware(ExceptionHandlerMiddleware) - app._is_instrumented_by_opentelemetry = True if app not in _InstrumentedFastAPI._instrumented_fastapi_apps: _InstrumentedFastAPI._instrumented_fastapi_apps.add(app) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 04bbb47ae0..a8340d4b3d 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1965,7 +1965,7 @@ async def _(): self.assertEqual(self.executed, 1) def test_exception_span_recording(self): - """Exception are always recorded in the active span""" + """Exceptions are always recorded in the active span""" @self.app.get("/foobar") async def _(): @@ -1994,6 +1994,40 @@ async def _(): f"{__name__}.UnhandledException", ) + def test_middleware_exceptions(self): + """Exceptions from user middlewares are recorded in the active span""" + + @self.app.get("/foobar") + async def _(): + return PlainTextResponse("Hello World") + + @self.app.middleware("http") + async def _(*_): + raise UnhandledException("Test Exception") + + try: + self.client.get( + "/foobar", + ) + except Exception: # pylint: disable=W0718 + pass + + spans = self.memory_exporter.get_finished_spans() + + self.assertEqual(len(spans), 3) + span = spans[2] + self.assertEqual(span.name, "GET /foobar") + self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + assert event.attributes is not None + self.assertEqual( + event.attributes.get(EXCEPTION_TYPE), + f"{__name__}.UnhandledException", + ) + class TestFailsafeHooks(TestBase): """Tests to ensure FastAPI instrumentation hooks don't tear through""" From 3007304717b4b40883037b7cf583b055f6588f8a Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 09:19:34 +0200 Subject: [PATCH 24/36] use distinct status code in test --- .../tests/test_fastapi_instrumentation.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index a8340d4b3d..a9d3975d7d 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1904,12 +1904,14 @@ def tearDown(self) -> None: def test_error_handler_context(self): """OTEL tracing contexts must be available during error handler execution""" + status_code = 501 + @self.app.exception_handler(Exception) async def _(*_): self.error_trace_id = ( trace.get_current_span().get_span_context().trace_id ) - return PlainTextResponse("", status_code=500) + return PlainTextResponse("", status_code) @self.app.get("/foobar") async def _(): @@ -1933,7 +1935,7 @@ async def _(): self.assertEqual(len(spans), 3) span = spans[2] self.assertEqual(span.name, "GET /foobar") - self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) + self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), status_code) self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual(len(span.events), 1) event = span.events[0] From 4493759ba73d72cfa3a02fff60ec3ad49222e2b9 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 09:21:01 +0200 Subject: [PATCH 25/36] improve comemnt Co-authored-by: Alex Hall --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index ae7edb930b..d697ac9b7e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -291,8 +291,7 @@ def instrument_app( ) def build_middleware_stack(self: Starlette) -> ASGIApp: - # Define an additional middleware for exception handling that gets - # added as a regular user middleware. + # Define an additional middleware for exception handling # Normally, `opentelemetry.trace.use_span` covers the recording of # exceptions into the active span, but `OpenTelemetryMiddleware` # ends the span too early before the exception can be recorded. From 2bc27d141a410561e33dd4bdb6b3e8cefb7fb1de Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 09:21:57 +0200 Subject: [PATCH 26/36] narrow down exception handling Co-authored-by: Alex Hall --- .../tests/test_fastapi_instrumentation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index a9d3975d7d..ba8614958f 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1924,7 +1924,7 @@ async def _(): self.client.get( "/foobar", ) - except Exception: # pylint: disable=W0718 + except UnhandledException: pass self.assertIsNotNone(self.request_trace_id) From b81343a5e53791259bc5511e79f97b2ee910202b Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 09:23:47 +0200 Subject: [PATCH 27/36] narrow down FastAPI exception tests to relevant spans --- .../tests/test_fastapi_instrumentation.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index ba8614958f..ed862a977c 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1889,7 +1889,9 @@ def setUp(self): self.app = fastapi.FastAPI() - otel_fastapi.FastAPIInstrumentor().instrument_app(self.app) + otel_fastapi.FastAPIInstrumentor().instrument_app( + self.app, exclude_spans=["receive", "send"] + ) self.client = TestClient(self.app) self.tracer = self.tracer_provider.get_tracer(__name__) self.executed = 0 @@ -1932,8 +1934,8 @@ async def _(): spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - span = spans[2] + self.assertEqual(len(spans), 1) + span = spans[0] self.assertEqual(span.name, "GET /foobar") self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), status_code) self.assertEqual(span.status.status_code, StatusCode.ERROR) @@ -1982,8 +1984,8 @@ async def _(): spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - span = spans[2] + self.assertEqual(len(spans), 1) + span = spans[0] self.assertEqual(span.name, "GET /foobar") self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) self.assertEqual(span.status.status_code, StatusCode.ERROR) @@ -2016,8 +2018,8 @@ async def _(*_): spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - span = spans[2] + self.assertEqual(len(spans), 1) + span = spans[0] self.assertEqual(span.name, "GET /foobar") self.assertEqual(span.attributes.get(HTTP_STATUS_CODE), 500) self.assertEqual(span.status.status_code, StatusCode.ERROR) From 3d4afe64268e41e843cbd4ca8cb6125884b33109 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 09:30:45 +0200 Subject: [PATCH 28/36] collapse tests, more narrow exceptions --- .../tests/test_fastapi_instrumentation.py | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index ed862a977c..4d083d3a9a 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1904,7 +1904,8 @@ def tearDown(self) -> None: otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) def test_error_handler_context(self): - """OTEL tracing contexts must be available during error handler execution""" + """OTEL tracing contexts must be available during error handler + execution, and handlers must only be executed once""" status_code = 501 @@ -1913,6 +1914,7 @@ async def _(*_): self.error_trace_id = ( trace.get_current_span().get_span_context().trace_id ) + self.executed += 1 return PlainTextResponse("", status_code) @self.app.get("/foobar") @@ -1947,25 +1949,6 @@ async def _(): event.attributes.get(EXCEPTION_TYPE), f"{__name__}.UnhandledException", ) - - def test_error_handler_side_effects(self): - """FastAPI default exception handlers (aka error handlers) must be executed exactly once per exception""" - - @self.app.exception_handler(Exception) - async def _(*_): - self.executed += 1 - - @self.app.get("/foobar") - async def _(): - raise UnhandledException("Test Exception") - - try: - self.client.get( - "/foobar", - ) - except Exception: # pylint: disable=W0718 - pass - self.assertEqual(self.executed, 1) def test_exception_span_recording(self): @@ -1979,7 +1962,7 @@ async def _(): self.client.get( "/foobar", ) - except Exception: # pylint: disable=W0718 + except UnhandledException: pass spans = self.memory_exporter.get_finished_spans() @@ -2013,7 +1996,7 @@ async def _(*_): self.client.get( "/foobar", ) - except Exception: # pylint: disable=W0718 + except UnhandledException: pass spans = self.memory_exporter.get_finished_spans() From 6dece9b865b8e2eff63703d32ea32149a09824e9 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 11:04:22 +0200 Subject: [PATCH 29/36] move failsafe hook tests to ASGI test suite --- .../tests/test_asgi_middleware.py | 51 +++++++++++++++++ .../tests/test_fastapi_instrumentation.py | 56 ------------------- 2 files changed, 51 insertions(+), 56 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index b8791cf730..0eeeea9e12 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -277,6 +277,17 @@ async def error_asgi(scope, receive, send): await send({"type": "http.response.body", "body": b"*"}) +class UnhandledException(Exception): + pass + + +def failing_hook(msg): + def hook(*_): + raise UnhandledException(msg) + + return hook + + # pylint: disable=too-many-public-methods class TestAsgiApplication(AsyncAsgiTestBase): def setUp(self): @@ -481,6 +492,12 @@ def validate_outputs( span.instrumentation_scope.name, "opentelemetry.instrumentation.asgi", ) + if "events" in expected: + self.assertEqual(len(span.events), len(expected["events"])) + for event, expected in zip(span.events, expected["events"]): + self.assertEqual(event.name, expected["name"]) + for k, v in expected["attributes"].items(): + self.assertEqual(event.attributes[k], v) async def test_basic_asgi_call(self): """Test that spans are emitted as expected.""" @@ -1206,6 +1223,40 @@ def update_expected_hook_results(expected): outputs, modifiers=[update_expected_hook_results] ) + async def test_hook_exceptions(self): + def exception_event(msg): + return { + "name": "exception", + "attributes": { + "exception.type": f"{__name__}.UnhandledException", + "exception.message": msg, + }, + } + + def update_expected_hook_results(expected): + for entry in expected: + if entry["kind"] == trace_api.SpanKind.SERVER: + entry["events"] = [exception_event("server request")] + elif entry["name"] == "GET / http receive": + entry["events"] = [exception_event("client request")] + elif entry["name"] == "GET / http send": + entry["events"] = [exception_event("client response")] + + return expected + + app = otel_asgi.OpenTelemetryMiddleware( + simple_asgi, + server_request_hook=failing_hook("server request"), + client_request_hook=failing_hook("client request"), + client_response_hook=failing_hook("client response"), + ) + self.seed_app(app) + await self.send_default_request() + outputs = await self.get_all_output() + self.validate_outputs( + outputs, modifiers=[update_expected_hook_results] + ) + async def test_asgi_metrics(self): app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 4d083d3a9a..33a6bec0ac 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -2014,59 +2014,3 @@ async def _(*_): event.attributes.get(EXCEPTION_TYPE), f"{__name__}.UnhandledException", ) - - -class TestFailsafeHooks(TestBase): - """Tests to ensure FastAPI instrumentation hooks don't tear through""" - - def setUp(self): - super().setUp() - - self.app = fastapi.FastAPI() - - @self.app.get("/foobar") - async def _(): - return {"message": "Hello World"} - - def failing_hook(*_): - raise UnhandledException("Hook Exception") - - otel_fastapi.FastAPIInstrumentor().instrument_app( - self.app, - server_request_hook=failing_hook, - client_request_hook=failing_hook, - client_response_hook=failing_hook, - ) - self.client = TestClient(self.app) - - def tearDown(self) -> None: - super().tearDown() - with self.disable_logging(): - otel_fastapi.FastAPIInstrumentor().uninstrument_app(self.app) - - def test_failsafe_hooks(self): - """Crashing hooks must not tear through""" - resp = self.client.get( - "/foobar", - ) - - self.assertEqual(200, resp.status_code) - - def test_failsafe_error_recording(self): - """Failing hooks must record the exception on the span""" - self.client.get( - "/foobar", - ) - - spans = self.memory_exporter.get_finished_spans() - - self.assertEqual(len(spans), 3) - span = spans[0] - self.assertEqual(len(span.events), 1) - event = span.events[0] - self.assertEqual(event.name, "exception") - assert event.attributes is not None - self.assertEqual( - event.attributes.get(EXCEPTION_TYPE), - f"{__name__}.UnhandledException", - ) From 762aa300b1cc8d38207c87722b85b7e7be6975f6 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 11:06:35 +0200 Subject: [PATCH 30/36] update CHANGELOG --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1799d8ca14..1f0c1676db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `opentelemetry-instrumentation-fastapi`: Implement failsafe middleware stack. +- `opentelemetry-instrumentation-fastapi`: Fix middleware ordering to cover all exception handling use cases. + ([#3664](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3664)) +- `opentelemetry-instrumentation-asgi`: Make all user hooks failsafe and record exceptions in hooks. ([#3664](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3664)) - `opentelemetry-instrumentation`: Avoid calls to `context.detach` with `None` token. ([#3673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3673)) From ff00dde73879c8abde54705944172e0c99596406 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Tue, 19 Aug 2025 11:10:19 +0200 Subject: [PATCH 31/36] satisfy linter --- .../tests/test_asgi_middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 0eeeea9e12..23a830bcb6 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -496,8 +496,8 @@ def validate_outputs( self.assertEqual(len(span.events), len(expected["events"])) for event, expected in zip(span.events, expected["events"]): self.assertEqual(event.name, expected["name"]) - for k, v in expected["attributes"].items(): - self.assertEqual(event.attributes[k], v) + for name, value in expected["attributes"].items(): + self.assertEqual(event.attributes[name], value) async def test_basic_asgi_call(self): """Test that spans are emitted as expected.""" From b9a2530d1030824c41fbe082dd8269b93ae64fb7 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 27 Aug 2025 16:18:53 +0200 Subject: [PATCH 32/36] don't record exception if span is not recording --- .../instrumentation/fastapi/__init__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index d697ac9b7e..f5f40599cf 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -306,13 +306,14 @@ async def __call__( await self.app(scope, receive, send) except Exception as exc: # pylint: disable=broad-exception-caught span = get_current_span() - span.record_exception(exc) - span.set_status( - Status( - status_code=StatusCode.ERROR, - description=f"{type(exc).__name__}: {exc}", + if span.is_recording(): + span.record_exception(exc) + span.set_status( + Status( + status_code=StatusCode.ERROR, + description=f"{type(exc).__name__}: {exc}", + ) ) - ) raise # For every possible use case of error handling, exception From 5946f9289bb8f5a02b51d5989d016b080ee0d06f Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 27 Aug 2025 17:03:31 +0200 Subject: [PATCH 33/36] add test for unhappy instrumentation codepath --- .../tests/test_fastapi_instrumentation.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 33a6bec0ac..21c2611fa5 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -14,12 +14,14 @@ # pylint: disable=too-many-lines +import logging import unittest from contextlib import ExitStack from timeit import default_timer from unittest.mock import Mock, call, patch import fastapi +import pytest from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware from fastapi.responses import JSONResponse, PlainTextResponse from fastapi.testclient import TestClient @@ -2014,3 +2016,38 @@ async def _(*_): event.attributes.get(EXCEPTION_TYPE), f"{__name__}.UnhandledException", ) + + +class TestFastAPIFallback(TestBaseFastAPI): + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self._caplog = caplog + + @staticmethod + def _create_fastapi_app(): + app = TestBaseFastAPI._create_fastapi_app() + + def build_middleware_stack(): + return app.router + + app.build_middleware_stack = build_middleware_stack + return app + + def setUp(self): + super().setUp() + self.client = TestClient(self._app) + + def test_no_instrumentation(self): + self.client.get( + "/foobar", + ) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) + + errors = [ + record + for record in self._caplog.get_records("call") + if record.levelno >= logging.ERROR + ] + self.assertEqual(len(errors), 1) From f6bcc9b3b1bd9a07516182b4fc4d067fe72dfae3 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 27 Aug 2025 17:17:22 +0200 Subject: [PATCH 34/36] make inject fixtures private --- .../tests/test_fastapi_instrumentation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 5eb8a14b3b..b3d5c5ec54 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -2032,7 +2032,7 @@ async def _(*_): class TestFastAPIFallback(TestBaseFastAPI): @pytest.fixture(autouse=True) - def inject_fixtures(self, caplog): + def _inject_fixtures(self, caplog): self._caplog = caplog @staticmethod From 33ad22f3d420b7cdd38ededfb203e733df5b695d Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Wed, 27 Aug 2025 17:43:18 +0200 Subject: [PATCH 35/36] give up and shut up pylint --- .../tests/test_fastapi_instrumentation.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index b3d5c5ec54..cf011db1bb 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -2030,10 +2030,11 @@ async def _(*_): ) +# pylint: disable=attribute-defined-outside-init class TestFastAPIFallback(TestBaseFastAPI): @pytest.fixture(autouse=True) - def _inject_fixtures(self, caplog): - self._caplog = caplog + def inject_fixtures(self, caplog): + self.caplog = caplog @staticmethod def _create_fastapi_app(): @@ -2059,7 +2060,7 @@ def test_no_instrumentation(self): errors = [ record - for record in self._caplog.get_records("call") + for record in self.caplog.get_records("call") if record.levelno >= logging.ERROR ] self.assertEqual(len(errors), 1) From fccb7c1cdce81d359b4b73df44abd36fd4a42ea9 Mon Sep 17 00:00:00 2001 From: Alexander Dorn Date: Thu, 28 Aug 2025 09:38:15 +0200 Subject: [PATCH 36/36] improve instrumentation failure error message and add test --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 4 +++- .../tests/test_fastapi_instrumentation.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index e5884eaaf3..c5c87f9878 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -334,7 +334,9 @@ async def __call__( ): # Oops, something changed about how Starlette creates middleware stacks _logger.error( - "Cannot instrument FastAPI as the expected middleware stack has changed" + "Skipping FastAPI instrumentation due to unexpected middleware stack: expected %s, got %s", + ServerErrorMiddleware.__name__, + type(inner_server_error_middleware), ) return inner_server_error_middleware diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index cf011db1bb..08a4668f5b 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -2064,3 +2064,7 @@ def test_no_instrumentation(self): if record.levelno >= logging.ERROR ] self.assertEqual(len(errors), 1) + self.assertEqual( + errors[0].getMessage(), + "Skipping FastAPI instrumentation due to unexpected middleware stack: expected ServerErrorMiddleware, got ", + )