Skip to content

Commit 4d6893e

Browse files
adriangbxrmxoutergodemdneto
authored
fastapi: fix wrapping of middlewares (open-telemetry#3012)
* fastapi: fix wrapping of middlewares * fix import, super * add test * changelog * lint * lint * fix * ci * fix wip * fix * fix * lint * lint * Exit? * Update test_fastapi_instrumentation.py Co-authored-by: Riccardo Magliocchetti <[email protected]> * remove break * fix * remove dunders * add test * lint * add endpoint to class * fmt * pr feedback * move type ignores * fix sphinx? * Update CHANGELOG.md * update fastapi versions * fix? * generate * stop passing on user-supplied error handler This prevents potential side effects, such as logging, to be executed more than once per request handler exception. * fix ci Signed-off-by: emdneto <[email protected]> * fix ruff Signed-off-by: emdneto <[email protected]> * remove unused funcs Co-authored-by: Emídio Neto <[email protected]> * fix lint,ruff Signed-off-by: emdneto <[email protected]> * fix changelog Signed-off-by: emdneto <[email protected]> * add changelog note Signed-off-by: emdneto <[email protected]> * fix conflicts with main Signed-off-by: emdneto <[email protected]> --------- Signed-off-by: emdneto <[email protected]> Co-authored-by: Riccardo Magliocchetti <[email protected]> Co-authored-by: Alexander Dorn <[email protected]> Co-authored-by: Emídio Neto <[email protected]>
1 parent dbdff31 commit 4d6893e

File tree

11 files changed

+181
-109
lines changed

11 files changed

+181
-109
lines changed

‎CHANGELOG.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111
1212
## Unreleased
1313

14+
### Fixed
15+
16+
- `opentelemetry-instrumentation-fastapi`: fix wrapping of middlewares
17+
([#3012](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3012))
18+
1419
### Breaking changes
1520

1621
- `opentelemetry-instrumentation-botocore` Use `cloud.region` instead of `aws.region` span attribute as per semantic conventions.
1722
([#3474](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3474))
18-
23+
- `opentelemetry-instrumentation-fastapi`: Drop support for FastAPI versions earlier than `0.92`
24+
([#3012](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3012))
1925

2026
## Version 1.33.0/0.54b0 (2025-05-09)
2127

‎docs/nitpick-exceptions.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ py-class=
4444
psycopg.Connection
4545
psycopg.AsyncConnection
4646
ObjectProxy
47+
fastapi.applications.FastAPI
4748

4849
any=
4950
; API

‎instrumentation/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
| [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | development
2323
| [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 6.0 | No | development
2424
| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 5.0.0 | Yes | migration
25-
| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | migration
25+
| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.92 | Yes | migration
2626
| [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration
2727
| [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio >= 1.42.0 | No | development
2828
| [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No | migration

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -665,9 +665,9 @@ def __init__(
665665
# pylint: disable=too-many-statements
666666
async def __call__(
667667
self,
668-
scope: dict[str, Any],
669-
receive: Callable[[], Awaitable[dict[str, Any]]],
670-
send: Callable[[dict[str, Any]], Awaitable[None]],
668+
scope: typing.MutableMapping[str, Any],
669+
receive: Callable[[], Awaitable[typing.MutableMapping[str, Any]]],
670+
send: Callable[[typing.MutableMapping[str, Any]], Awaitable[None]],
671671
) -> None:
672672
"""The ASGI application
673673

‎instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ dependencies = [
3535

3636
[project.optional-dependencies]
3737
instruments = [
38-
"fastapi ~= 0.58",
38+
"fastapi ~= 0.92",
3939
]
4040

4141
[project.entry-points.opentelemetry_instrumentor]

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

Lines changed: 84 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,16 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
182182

183183
from __future__ import annotations
184184

185+
import functools
185186
import logging
187+
import types
186188
from typing import Collection, Literal
187189

188190
import fastapi
191+
from starlette.applications import Starlette
192+
from starlette.middleware.errors import ServerErrorMiddleware
189193
from starlette.routing import Match
194+
from starlette.types import ASGIApp
190195

191196
from opentelemetry.instrumentation._semconv import (
192197
_get_schema_url,
@@ -203,9 +208,9 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
203208
from opentelemetry.instrumentation.fastapi.package import _instruments
204209
from opentelemetry.instrumentation.fastapi.version import __version__
205210
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
206-
from opentelemetry.metrics import get_meter
211+
from opentelemetry.metrics import MeterProvider, get_meter
207212
from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE
208-
from opentelemetry.trace import get_tracer
213+
from opentelemetry.trace import TracerProvider, get_tracer
209214
from opentelemetry.util.http import (
210215
get_excluded_urls,
211216
parse_excluded_urls,
@@ -226,13 +231,13 @@ class FastAPIInstrumentor(BaseInstrumentor):
226231

227232
@staticmethod
228233
def instrument_app(
229-
app,
234+
app: fastapi.FastAPI,
230235
server_request_hook: ServerRequestHook = None,
231236
client_request_hook: ClientRequestHook = None,
232237
client_response_hook: ClientResponseHook = None,
233-
tracer_provider=None,
234-
meter_provider=None,
235-
excluded_urls=None,
238+
tracer_provider: TracerProvider | None = None,
239+
meter_provider: MeterProvider | None = None,
240+
excluded_urls: str | None = None,
236241
http_capture_headers_server_request: list[str] | None = None,
237242
http_capture_headers_server_response: list[str] | None = None,
238243
http_capture_headers_sanitize_fields: list[str] | None = None,
@@ -284,21 +289,56 @@ def instrument_app(
284289
schema_url=_get_schema_url(sem_conv_opt_in_mode),
285290
)
286291

287-
app.add_middleware(
288-
OpenTelemetryMiddleware,
289-
excluded_urls=excluded_urls,
290-
default_span_details=_get_default_span_details,
291-
server_request_hook=server_request_hook,
292-
client_request_hook=client_request_hook,
293-
client_response_hook=client_response_hook,
294-
# Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
295-
tracer=tracer,
296-
meter=meter,
297-
http_capture_headers_server_request=http_capture_headers_server_request,
298-
http_capture_headers_server_response=http_capture_headers_server_response,
299-
http_capture_headers_sanitize_fields=http_capture_headers_sanitize_fields,
300-
exclude_spans=exclude_spans,
292+
# Instead of using `app.add_middleware` we monkey patch `build_middleware_stack` to insert our middleware
293+
# as the outermost middleware.
294+
# Otherwise `OpenTelemetryMiddleware` would have unhandled exceptions tearing through it and would not be able
295+
# to faithfully record what is returned to the client since it technically cannot know what `ServerErrorMiddleware` is going to do.
296+
297+
def build_middleware_stack(self: Starlette) -> ASGIApp:
298+
inner_server_error_middleware: ASGIApp = ( # type: ignore
299+
self._original_build_middleware_stack() # type: ignore
300+
)
301+
otel_middleware = OpenTelemetryMiddleware(
302+
inner_server_error_middleware,
303+
excluded_urls=excluded_urls,
304+
default_span_details=_get_default_span_details,
305+
server_request_hook=server_request_hook,
306+
client_request_hook=client_request_hook,
307+
client_response_hook=client_response_hook,
308+
# Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
309+
tracer=tracer,
310+
meter=meter,
311+
http_capture_headers_server_request=http_capture_headers_server_request,
312+
http_capture_headers_server_response=http_capture_headers_server_response,
313+
http_capture_headers_sanitize_fields=http_capture_headers_sanitize_fields,
314+
exclude_spans=exclude_spans,
315+
)
316+
# Wrap in an outer layer of ServerErrorMiddleware so that any exceptions raised in OpenTelemetryMiddleware
317+
# are handled.
318+
# This should not happen unless there is a bug in OpenTelemetryMiddleware, but if there is we don't want that
319+
# to impact the user's application just because we wrapped the middlewares in this order.
320+
if isinstance(
321+
inner_server_error_middleware, ServerErrorMiddleware
322+
): # usually true
323+
outer_server_error_middleware = ServerErrorMiddleware(
324+
app=otel_middleware,
325+
)
326+
else:
327+
# Something else seems to have patched things, or maybe Starlette changed.
328+
# Just create a default ServerErrorMiddleware.
329+
outer_server_error_middleware = ServerErrorMiddleware(
330+
app=otel_middleware
331+
)
332+
return outer_server_error_middleware
333+
334+
app._original_build_middleware_stack = app.build_middleware_stack
335+
app.build_middleware_stack = types.MethodType(
336+
functools.wraps(app.build_middleware_stack)(
337+
build_middleware_stack
338+
),
339+
app,
301340
)
341+
302342
app._is_instrumented_by_opentelemetry = True
303343
if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
304344
_InstrumentedFastAPI._instrumented_fastapi_apps.add(app)
@@ -309,11 +349,12 @@ def instrument_app(
309349

310350
@staticmethod
311351
def uninstrument_app(app: fastapi.FastAPI):
312-
app.user_middleware = [
313-
x
314-
for x in app.user_middleware
315-
if x.cls is not OpenTelemetryMiddleware
316-
]
352+
original_build_middleware_stack = getattr(
353+
app, "_original_build_middleware_stack", None
354+
)
355+
if original_build_middleware_stack:
356+
app.build_middleware_stack = original_build_middleware_stack
357+
del app._original_build_middleware_stack
317358
app.middleware_stack = app.build_middleware_stack()
318359
app._is_instrumented_by_opentelemetry = False
319360

@@ -341,12 +382,7 @@ def _instrument(self, **kwargs):
341382
_InstrumentedFastAPI._http_capture_headers_sanitize_fields = (
342383
kwargs.get("http_capture_headers_sanitize_fields")
343384
)
344-
_excluded_urls = kwargs.get("excluded_urls")
345-
_InstrumentedFastAPI._excluded_urls = (
346-
_excluded_urls_from_env
347-
if _excluded_urls is None
348-
else parse_excluded_urls(_excluded_urls)
349-
)
385+
_InstrumentedFastAPI._excluded_urls = kwargs.get("excluded_urls")
350386
_InstrumentedFastAPI._meter_provider = kwargs.get("meter_provider")
351387
_InstrumentedFastAPI._exclude_spans = kwargs.get("exclude_spans")
352388
fastapi.FastAPI = _InstrumentedFastAPI
@@ -365,43 +401,29 @@ class _InstrumentedFastAPI(fastapi.FastAPI):
365401
_server_request_hook: ServerRequestHook = None
366402
_client_request_hook: ClientRequestHook = None
367403
_client_response_hook: ClientResponseHook = None
404+
_http_capture_headers_server_request: list[str] | None = None
405+
_http_capture_headers_server_response: list[str] | None = None
406+
_http_capture_headers_sanitize_fields: list[str] | None = None
407+
_exclude_spans: list[Literal["receive", "send"]] | None = None
408+
368409
_instrumented_fastapi_apps = set()
369410
_sem_conv_opt_in_mode = _StabilityMode.DEFAULT
370411

371412
def __init__(self, *args, **kwargs):
372413
super().__init__(*args, **kwargs)
373-
tracer = get_tracer(
374-
__name__,
375-
__version__,
376-
_InstrumentedFastAPI._tracer_provider,
377-
schema_url=_get_schema_url(
378-
_InstrumentedFastAPI._sem_conv_opt_in_mode
379-
),
380-
)
381-
meter = get_meter(
382-
__name__,
383-
__version__,
384-
_InstrumentedFastAPI._meter_provider,
385-
schema_url=_get_schema_url(
386-
_InstrumentedFastAPI._sem_conv_opt_in_mode
387-
),
388-
)
389-
self.add_middleware(
390-
OpenTelemetryMiddleware,
391-
excluded_urls=_InstrumentedFastAPI._excluded_urls,
392-
default_span_details=_get_default_span_details,
393-
server_request_hook=_InstrumentedFastAPI._server_request_hook,
394-
client_request_hook=_InstrumentedFastAPI._client_request_hook,
395-
client_response_hook=_InstrumentedFastAPI._client_response_hook,
396-
# Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
397-
tracer=tracer,
398-
meter=meter,
399-
http_capture_headers_server_request=_InstrumentedFastAPI._http_capture_headers_server_request,
400-
http_capture_headers_server_response=_InstrumentedFastAPI._http_capture_headers_server_response,
401-
http_capture_headers_sanitize_fields=_InstrumentedFastAPI._http_capture_headers_sanitize_fields,
402-
exclude_spans=_InstrumentedFastAPI._exclude_spans,
414+
FastAPIInstrumentor.instrument_app(
415+
self,
416+
server_request_hook=self._server_request_hook,
417+
client_request_hook=self._client_request_hook,
418+
client_response_hook=self._client_response_hook,
419+
tracer_provider=self._tracer_provider,
420+
meter_provider=self._meter_provider,
421+
excluded_urls=self._excluded_urls,
422+
http_capture_headers_server_request=self._http_capture_headers_server_request,
423+
http_capture_headers_server_response=self._http_capture_headers_server_response,
424+
http_capture_headers_sanitize_fields=self._http_capture_headers_sanitize_fields,
425+
exclude_spans=self._exclude_spans,
403426
)
404-
self._is_instrumented_by_opentelemetry = True
405427
_InstrumentedFastAPI._instrumented_fastapi_apps.add(self)
406428

407429
def __del__(self):

‎instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
# limitations under the License.
1414

1515

16-
_instruments = ("fastapi ~= 0.58",)
16+
_instruments = ("fastapi ~= 0.92",)
1717

1818
_supports_metrics = True
1919

‎instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
# pylint: disable=too-many-lines
1616

1717
import unittest
18+
from contextlib import ExitStack
1819
from timeit import default_timer
1920
from unittest.mock import Mock, call, patch
2021

@@ -183,9 +184,14 @@ def setUp(self):
183184
self._instrumentor = otel_fastapi.FastAPIInstrumentor()
184185
self._app = self._create_app()
185186
self._app.add_middleware(HTTPSRedirectMiddleware)
186-
self._client = TestClient(self._app)
187+
self._client = TestClient(self._app, base_url="https://testserver:443")
188+
# run the lifespan, initialize the middleware stack
189+
# this is more in-line with what happens in a real application when the server starts up
190+
self._exit_stack = ExitStack()
191+
self._exit_stack.enter_context(self._client)
187192

188193
def tearDown(self):
194+
self._exit_stack.close()
189195
super().tearDown()
190196
self.env_patch.stop()
191197
self.exclude_patch.stop()
@@ -218,11 +224,19 @@ async def _(param: str):
218224
async def _():
219225
return {"message": "ok"}
220226

227+
@app.get("/error")
228+
async def _():
229+
raise UnhandledException("This is an unhandled exception")
230+
221231
app.mount("/sub", app=sub_app)
222232

223233
return app
224234

225235

236+
class UnhandledException(Exception):
237+
pass
238+
239+
226240
class TestBaseManualFastAPI(TestBaseFastAPI):
227241
@classmethod
228242
def setUpClass(cls):
@@ -233,6 +247,27 @@ def setUpClass(cls):
233247

234248
super(TestBaseManualFastAPI, cls).setUpClass()
235249

250+
def test_fastapi_unhandled_exception(self):
251+
"""If the application has an unhandled error the instrumentation should capture that a 500 response is returned."""
252+
try:
253+
resp = self._client.get("/error")
254+
assert (
255+
resp.status_code == 500
256+
), resp.content # pragma: no cover, for debugging this test if an exception is _not_ raised
257+
except UnhandledException:
258+
pass
259+
else:
260+
self.fail("Expected UnhandledException")
261+
262+
spans = self.memory_exporter.get_finished_spans()
263+
self.assertEqual(len(spans), 3)
264+
span = spans[0]
265+
assert span.name == "GET /error http send"
266+
assert span.attributes[HTTP_STATUS_CODE] == 500
267+
span = spans[2]
268+
assert span.name == "GET /error"
269+
assert span.attributes[HTTP_TARGET] == "/error"
270+
236271
def test_sub_app_fastapi_call(self):
237272
"""
238273
This test is to ensure that a span in case of a sub app targeted contains the correct server url
@@ -975,6 +1010,10 @@ async def _(param: str):
9751010
async def _():
9761011
return {"message": "ok"}
9771012

1013+
@app.get("/error")
1014+
async def _():
1015+
raise UnhandledException("This is an unhandled exception")
1016+
9781017
app.mount("/sub", app=sub_app)
9791018

9801019
return app
@@ -1137,9 +1176,11 @@ def test_request(self):
11371176
def test_mulitple_way_instrumentation(self):
11381177
self._instrumentor.instrument_app(self._app)
11391178
count = 0
1140-
for middleware in self._app.user_middleware:
1141-
if middleware.cls is OpenTelemetryMiddleware:
1179+
app = self._app.middleware_stack
1180+
while app is not None:
1181+
if isinstance(app, OpenTelemetryMiddleware):
11421182
count += 1
1183+
app = getattr(app, "app", None)
11431184
self.assertEqual(count, 1)
11441185

11451186
def test_uninstrument_after_instrument(self):

‎opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@
101101
"instrumentation": "opentelemetry-instrumentation-falcon==0.55b0.dev",
102102
},
103103
{
104-
"library": "fastapi ~= 0.58",
104+
"library": "fastapi ~= 0.92",
105105
"instrumentation": "opentelemetry-instrumentation-fastapi==0.55b0.dev",
106106
},
107107
{

‎pyproject.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ dependencies = [
6666
"opentelemetry-instrumentation-openai-v2[instruments]",
6767
]
6868

69+
6970
# https://docs.astral.sh/uv/reference/settings/
7071
[tool.uv]
7172
package = false # https://docs.astral.sh/uv/reference/settings/#package
@@ -147,6 +148,7 @@ members = [
147148
# TODO: remove after python 3.8 is dropped
148149
exclude = [
149150
"instrumentation-genai/opentelemetry-instrumentation-google-genai",
151+
"instrumentation/opentelemetry-instrumentation-starlette",
150152
]
151153

152154
[tool.ruff]

0 commit comments

Comments
 (0)