fix: fastapi memory leak only (#3688)

* fix(fastapi-instrumentation): properly remove app from instrumented list to avoid memory leaks

* fix(fastapi-instrumentation): fix tests and finalize memory leak fix

* docs(changelog): add FastAPI uninstrument memory leak fix with PR references (#3683)

* test(fastapi): add GC-based app collection test; refactor tracking to WeakSet and safe discard

* docs(changelog): reference FastAPI memory leak fix PR (#3688)

* refactor(fastapi): drop __del__ as WeakSet handles cleanup

* chore(fastapi): formatting after ruff auto-fix

* chore(test-fastapi): ruff import order

* Update comment to clarify purpose of removing app from WeakSet

* fix: codereview comments

* Apply suggestions from code review

---------

Co-authored-by: Emídio Neto <[email protected]>
Co-authored-by: Riccardo Magliocchetti <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `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-tornado` Fix server (request) duration metric calculation
   ([#3679](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3679))
 - `opentelemetry-instrumentation`: Avoid calls to `context.detach` with `None` token.

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):
     """

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",
     {