fastapi: Fix uninstrument memory leak

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`: Fix memory leak in `uninstrument_app()` method by properly removing apps from the tracking set
+  ([#XXXX](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/XXXX))
+
 ## Version 1.36.0/0.57b0 (2025-07-29)
 
 ### Fixed

instrumentation/opentelemetry-instrumentation-fastapi/demo_memory_leak_fix.py

@@ -0,0 +1,137 @@
+#!/usr/bin/env python3
+"""
+Demo script to demonstrate the memory leak fix in FastAPIInstrumentor.uninstrument_app()
+
+This script shows the problem described in the issue:
+- Calling FastAPIInstrumentor.uninstrument_app() doesn't remove the app parameter
+  from the _InstrumentedFastAPI._instrumented_fastapi_apps set
+- This can lead to memory leaks when instrumenting and uninstrumenting repeatedly
+
+The fix adds code to remove the app from the set during uninstrument_app().
+"""
+
+import sys
+
+import fastapi
+
+from opentelemetry.instrumentation.fastapi import (
+    FastAPIInstrumentor,
+    _InstrumentedFastAPI,
+)
+
+
+def demonstrate_problem():
+    """Demonstrate the memory leak problem"""
+    print("=== Demonstrating Memory Leak Problem ===")
+
+    app = fastapi.FastAPI()
+    print(f"Initial refcount: {sys.getrefcount(app)}")
+    print(
+        f"Initial set size: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
+    )
+
+    # Instrument the app
+    FastAPIInstrumentor.instrument_app(app)
+    print(f"After instrument - refcount: {sys.getrefcount(app)}")
+    print(
+        f"After instrument - set size: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
+    )
+    print(
+        f"App in set: {app in _InstrumentedFastAPI._instrumented_fastapi_apps}"
+    )
+
+    # Uninstrument the app (before fix, this wouldn't remove from set)
+    FastAPIInstrumentor.uninstrument_app(app)
+    print(f"After uninstrument - refcount: {sys.getrefcount(app)}")
+    print(
+        f"After uninstrument - set size: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
+    )
+    print(
+        f"App in set: {app in _InstrumentedFastAPI._instrumented_fastapi_apps}"
+    )
+
+    # With the fix, the app should be removed from the set
+    if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
+        print("FIXED: App was properly removed from the set")
+    else:
+        print("BUG: App is still in the set (memory leak)")
+
+
+def demonstrate_multiple_cycles():
+    """Demonstrate multiple instrument/uninstrument cycles"""
+    print("\n=== Multiple Instrument/Uninstrument Cycles ===")
+
+    app = fastapi.FastAPI()
+    initial_refcount = sys.getrefcount(app)
+    print(f"Initial refcount: {initial_refcount}")
+
+    # Perform multiple cycles
+    for cycle_num in range(3):
+        FastAPIInstrumentor.instrument_app(app)
+        FastAPIInstrumentor.uninstrument_app(app)
+        current_refcount = sys.getrefcount(app)
+        set_size = len(_InstrumentedFastAPI._instrumented_fastapi_apps)
+        print(
+            f"Cycle {cycle_num+1}: refcount={current_refcount}, set_size={set_size}"
+        )
+
+    final_refcount = sys.getrefcount(app)
+    final_set_size = len(_InstrumentedFastAPI._instrumented_fastapi_apps)
+
+    print(f"Final refcount: {final_refcount}")
+    print(f"Final set size: {final_set_size}")
+
+    if final_set_size == 0:
+        print("FIXED: No memory leak - set is empty")
+    else:
+        print("BUG: Memory leak - set still contains apps")
+
+
+def demonstrate_multiple_apps():
+    """Demonstrate multiple apps"""
+    print("\n=== Multiple Apps Test ===")
+
+    apps = [fastapi.FastAPI() for _ in range(3)]
+
+    print("Instrumenting all apps...")
+    for app_idx, app in enumerate(apps):
+        FastAPIInstrumentor.instrument_app(app)
+        print(f"App {app_idx}: refcount={sys.getrefcount(app)}")
+
+    print(
+        f"Set size after instrumenting: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
+    )
+
+    print("Uninstrumenting all apps...")
+    for app_idx, app in enumerate(apps):
+        FastAPIInstrumentor.uninstrument_app(app)
+        print(f"App {app_idx}: refcount={sys.getrefcount(app)}")
+
+    final_set_size = len(_InstrumentedFastAPI._instrumented_fastapi_apps)
+    print(f"Final set size: {final_set_size}")
+
+    if final_set_size == 0:
+        print("FIXED: All apps properly removed from set")
+    else:
+        print("BUG: Some apps still in set")
+
+
+if __name__ == "__main__":
+    print("FastAPIInstrumentor Memory Leak Fix Demo")
+    print("=" * 50)
+
+    demonstrate_problem()
+    demonstrate_multiple_cycles()
+    demonstrate_multiple_apps()
+
+    print("\n" + "=" * 50)
+    print("Summary:")
+    print(
+        "- The fix adds code to remove apps from _instrumented_fastapi_apps during uninstrument_app()"
+    )
+    print(
+        "- This prevents memory leaks when instrumenting/uninstrumenting repeatedly"
+    )
+    print(
+        "- The fix is backward compatible and doesn't break existing functionality"
+    )

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

@@ -358,6 +358,10 @@ 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 prevent memory leaks
+        if app in _InstrumentedFastAPI._instrumented_fastapi_apps:
+            _InstrumentedFastAPI._instrumented_fastapi_apps.remove(app)
+
     def instrumentation_dependencies(self) -> Collection[str]:
         return _instruments
 
@@ -388,7 +392,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

instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_memory_leak.py

@@ -0,0 +1,155 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import sys
+import unittest
+
+import fastapi
+
+from opentelemetry.instrumentation.fastapi import (
+    FastAPIInstrumentor,
+    _InstrumentedFastAPI,
+)
+
+# Check if sys.getrefcount is available (not available in PyPy)
+HAS_GETREFCOUNT = hasattr(sys, "getrefcount")
+
+
+class TestFastAPIMemoryLeak(unittest.TestCase):
+    """Test for memory leak in FastAPIInstrumentor.uninstrument_app()"""
+
+    def test_refcount_after_uninstrument(self):
+        """Test that refcount is restored after uninstrument_app()"""
+        if not HAS_GETREFCOUNT:
+            self.skipTest(
+                "sys.getrefcount not available in this Python implementation"
+            )
+
+        app = fastapi.FastAPI()
+
+        # Instrument the app
+        FastAPIInstrumentor.instrument_app(app)
+        refcount_after_instrument = sys.getrefcount(app)
+
+        # Uninstrument the app
+        FastAPIInstrumentor.uninstrument_app(app)
+        refcount_after_uninstrument = sys.getrefcount(app)
+
+        # The refcount should be reduced after uninstrument (may not be exactly initial due to Python internals)
+        self.assertLess(
+            refcount_after_uninstrument,
+            refcount_after_instrument,
+            "Refcount should be reduced after uninstrument_app()",
+        )
+
+        # Verify that the app was removed from the set
+        self.assertNotIn(
+            app,
+            _InstrumentedFastAPI._instrumented_fastapi_apps,
+            "App should be removed from _instrumented_fastapi_apps after uninstrument_app()",
+        )
+
+    def test_multiple_instrument_uninstrument_cycles(self):
+        """Test that multiple instrument/uninstrument cycles don't leak memory"""
+        if not HAS_GETREFCOUNT:
+            self.skipTest(
+                "sys.getrefcount not available in this Python implementation"
+            )
+
+        app = fastapi.FastAPI()
+
+        initial_refcount = sys.getrefcount(app)
+
+        # Perform multiple instrument/uninstrument cycles
+        for cycle_num in range(5):
+            FastAPIInstrumentor.instrument_app(app)
+            FastAPIInstrumentor.uninstrument_app(app)
+
+        final_refcount = sys.getrefcount(app)
+
+        # The refcount should not grow significantly after multiple cycles
+        # (may not be exactly initial due to Python internals)
+        self.assertLessEqual(
+            final_refcount,
+            initial_refcount
+            + 2,  # Allow small increase due to Python internals
+            f"Refcount after {cycle_num+1} instrument/uninstrument cycles should not grow significantly",
+        )
+
+        # Verify that the app is not in the set
+        self.assertNotIn(
+            app,
+            _InstrumentedFastAPI._instrumented_fastapi_apps,
+            "App should not be in _instrumented_fastapi_apps after uninstrument_app()",
+        )
+
+    def test_multiple_apps_instrument_uninstrument(self):
+        """Test that multiple apps can be instrumented and uninstrumented without leaks"""
+        if not HAS_GETREFCOUNT:
+            self.skipTest(
+                "sys.getrefcount not available in this Python implementation"
+            )
+
+        apps = [fastapi.FastAPI() for _ in range(3)]
+        initial_refcounts = [sys.getrefcount(app) for app in apps]
+
+        # Instrument all apps
+        for app in apps:
+            FastAPIInstrumentor.instrument_app(app)
+
+        # Uninstrument all apps
+        for app in apps:
+            FastAPIInstrumentor.uninstrument_app(app)
+
+        # Check that refcounts are not significantly increased
+        for app_idx, app in enumerate(apps):
+            final_refcount = sys.getrefcount(app)
+            self.assertLessEqual(
+                final_refcount,
+                initial_refcounts[app_idx]
+                + 2,  # Allow small increase due to Python internals
+                f"App {app_idx} refcount should not grow significantly",
+            )
+
+        # Verify that no apps are in the set
+        for app in apps:
+            self.assertNotIn(
+                app,
+                _InstrumentedFastAPI._instrumented_fastapi_apps,
+                "All apps should be removed from _instrumented_fastapi_apps",
+            )
+
+    def test_demonstrate_fix(self):
+        """Demonstrate the fix for the memory leak issue"""
+        app = fastapi.FastAPI()
+
+        # Before the fix: app would remain in _instrumented_fastapi_apps after uninstrument_app()
+        # After the fix: app should be removed from _instrumented_fastapi_apps
+
+        FastAPIInstrumentor.instrument_app(app)
+
+        # Verify app is in the set after instrumentation
+        self.assertIn(app, _InstrumentedFastAPI._instrumented_fastapi_apps)
+
+        FastAPIInstrumentor.uninstrument_app(app)
+
+        # Verify app is removed from the set after uninstrumentation
+        self.assertNotIn(app, _InstrumentedFastAPI._instrumented_fastapi_apps)
+        self.assertEqual(
+            len(_InstrumentedFastAPI._instrumented_fastapi_apps), 0
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

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

@@ -273,12 +273,14 @@ def response_hook(span: Span, status: str, response_headers: List):
 )
 from opentelemetry.instrumentation.utils import _start_internal_or_server_span
 from opentelemetry.metrics import get_meter
-from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE
+from opentelemetry.semconv._incubating.attributes.http_attributes import (
+    HTTP_ROUTE,
+    HTTP_TARGET,
+)
 from opentelemetry.semconv.metrics import MetricInstruments
 from opentelemetry.semconv.metrics.http_metrics import (
     HTTP_SERVER_REQUEST_DURATION,
 )
-from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.util._importlib_metadata import version
 from opentelemetry.util.http import (
     get_excluded_urls,
@@ -404,9 +406,7 @@ def _start_response(status, response_headers, *args, **kwargs):
 
             if request_route:
                 # http.target to be included in old semantic conventions
-                duration_attrs_old[SpanAttributes.HTTP_TARGET] = str(
-                    request_route
-                )
+                duration_attrs_old[HTTP_TARGET] = str(request_route)
 
             duration_histogram_old.record(
                 max(round(duration_s * 1000), 0), duration_attrs_old
@@ -449,7 +449,7 @@ def _before_request():
         if flask.request.url_rule:
             # For 404 that result from no route found, etc, we
             # don't have a url_rule.
-            attributes[SpanAttributes.HTTP_ROUTE] = flask.request.url_rule.rule
+            attributes[HTTP_ROUTE] = flask.request.url_rule.rule
         span, token = _start_internal_or_server_span(
             tracer=tracer,
             span_name=span_name,