diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f252e8290..39624e81ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-instrumentation-fastapi`: Fix memory leak in `uninstrument_app()` by properly removing apps from the tracking set + ([#3688](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3688) + ## Version 1.36.0/0.57b0 (2025-07-29) ### Fixed @@ -1760,8 +1763,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#195](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/195)) - `opentelemetry-instrumentation-dbapi` Stop capturing query parameters by default ([#156](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/156)) -- `opentelemetry-instrumentation-asyncpg` Update asyncpg instrumentation to follow semantic conventions - ([#188](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/188)) +- `opentelemetry-instrumentation-fastapi`: Fix memory leak in `uninstrument_app()` by properly removing apps from the tracking set + ([#3688](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3688)) - `opentelemetry-instrumentation-grpc` Update protobuf versions ([#1356](https://github.com/open-telemetry/opentelemetry-python/pull/1356)) 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..3e39a74a49 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -186,6 +186,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A import logging import types from typing import Collection, Literal +from weakref import WeakSet as _WeakSet import fastapi from starlette.applications import Starlette @@ -358,6 +359,11 @@ def uninstrument_app(app: fastapi.FastAPI): app.middleware_stack = app.build_middleware_stack() app._is_instrumented_by_opentelemetry = False + # Remove the app from the set of instrumented apps to avoid calling uninstrument twice + # if the instrumentation is later disabled or such + # Use discard to avoid KeyError if already GC'ed + _InstrumentedFastAPI._instrumented_fastapi_apps.discard(app) + def instrumentation_dependencies(self) -> Collection[str]: return _instruments @@ -388,7 +394,11 @@ def _instrument(self, **kwargs): fastapi.FastAPI = _InstrumentedFastAPI def _uninstrument(self, **kwargs): - for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: + # Create a copy of the set to avoid RuntimeError during iteration + instances_to_uninstrument = list( + _InstrumentedFastAPI._instrumented_fastapi_apps + ) + for instance in instances_to_uninstrument: self.uninstrument_app(instance) _InstrumentedFastAPI._instrumented_fastapi_apps.clear() fastapi.FastAPI = self._original_fastapi @@ -406,7 +416,8 @@ class _InstrumentedFastAPI(fastapi.FastAPI): _http_capture_headers_sanitize_fields: list[str] | None = None _exclude_spans: list[Literal["receive", "send"]] | None = None - _instrumented_fastapi_apps = set() + # Track instrumented app instances using weak references to avoid GC leaks + _instrumented_fastapi_apps = _WeakSet() _sem_conv_opt_in_mode = _StabilityMode.DEFAULT def __init__(self, *args, **kwargs): @@ -426,10 +437,6 @@ def __init__(self, *args, **kwargs): ) _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) - def __del__(self): - if self in _InstrumentedFastAPI._instrumented_fastapi_apps: - _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) - def _get_route_details(scope): """ diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 523c165f85..0a4fd6c037 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -14,7 +14,9 @@ # pylint: disable=too-many-lines +import gc as _gc import unittest +import weakref as _weakref from contextlib import ExitStack from timeit import default_timer from unittest.mock import Mock, call, patch @@ -1400,6 +1402,16 @@ def test_mark_span_internal_in_presence_of_span_from_other_framework(self): ) +class TestFastAPIGarbageCollection(unittest.TestCase): + def test_fastapi_app_is_collected_after_instrument(self): + app = fastapi.FastAPI() + otel_fastapi.FastAPIInstrumentor().instrument_app(app) + app_ref = _weakref.ref(app) + del app + _gc.collect() + self.assertIsNone(app_ref()) + + @patch.dict( "os.environ", {