Skip to content

Commit 0b79069

Browse files
artemziborevartemziborev
committed
fix(fastapi-instrumentation): properly remove app from instrumented list to avoid memory leaks
1 parent b93c547 commit 0b79069

File tree

4 files changed

+285
-0
lines changed

4 files changed

+285
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ 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 memory leak in `uninstrument_app()` method by properly removing apps from the tracking set
17+
([#XXXX](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/XXXX))
18+
1419
## Version 1.36.0/0.57b0 (2025-07-29)
1520

1621
### Fixed

‎instrumentation/opentelemetry-instrumentation-fastapi/demo_memory_leak_fix.py‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Demo script to demonstrate the memory leak fix in FastAPIInstrumentor.uninstrument_app()
4+
5+
This script shows the problem described in the issue:
6+
- Calling FastAPIInstrumentor.uninstrument_app() doesn't remove the app parameter
7+
from the _InstrumentedFastAPI._instrumented_fastapi_apps set
8+
- This can lead to memory leaks when instrumenting and uninstrumenting repeatedly
9+
10+
The fix adds code to remove the app from the set during uninstrument_app().
11+
"""
12+
13+
import sys
14+
15+
import fastapi
16+
17+
from opentelemetry.instrumentation.fastapi import (
18+
FastAPIInstrumentor,
19+
_InstrumentedFastAPI,
20+
)
21+
22+
23+
def demonstrate_problem():
24+
"""Demonstrate the memory leak problem"""
25+
print("=== Demonstrating Memory Leak Problem ===")
26+
27+
app = fastapi.FastAPI()
28+
print(f"Initial refcount: {sys.getrefcount(app)}")
29+
print(
30+
f"Initial set size: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
31+
)
32+
33+
# Instrument the app
34+
FastAPIInstrumentor.instrument_app(app)
35+
print(f"After instrument - refcount: {sys.getrefcount(app)}")
36+
print(
37+
f"After instrument - set size: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
38+
)
39+
print(
40+
f"App in set: {app in _InstrumentedFastAPI._instrumented_fastapi_apps}"
41+
)
42+
43+
# Uninstrument the app (before fix, this wouldn't remove from set)
44+
FastAPIInstrumentor.uninstrument_app(app)
45+
print(f"After uninstrument - refcount: {sys.getrefcount(app)}")
46+
print(
47+
f"After uninstrument - set size: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
48+
)
49+
print(
50+
f"App in set: {app in _InstrumentedFastAPI._instrumented_fastapi_apps}"
51+
)
52+
53+
# With the fix, the app should be removed from the set
54+
if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
55+
print("FIXED: App was properly removed from the set")
56+
else:
57+
print("BUG: App is still in the set (memory leak)")
58+
59+
60+
def demonstrate_multiple_cycles():
61+
"""Demonstrate multiple instrument/uninstrument cycles"""
62+
print("\n=== Multiple Instrument/Uninstrument Cycles ===")
63+
64+
app = fastapi.FastAPI()
65+
initial_refcount = sys.getrefcount(app)
66+
print(f"Initial refcount: {initial_refcount}")
67+
68+
# Perform multiple cycles
69+
for i in range(3):
70+
FastAPIInstrumentor.instrument_app(app)
71+
FastAPIInstrumentor.uninstrument_app(app)
72+
current_refcount = sys.getrefcount(app)
73+
set_size = len(_InstrumentedFastAPI._instrumented_fastapi_apps)
74+
print(f"Cycle {i+1}: refcount={current_refcount}, set_size={set_size}")
75+
76+
final_refcount = sys.getrefcount(app)
77+
final_set_size = len(_InstrumentedFastAPI._instrumented_fastapi_apps)
78+
79+
print(f"Final refcount: {final_refcount}")
80+
print(f"Final set size: {final_set_size}")
81+
82+
if final_set_size == 0:
83+
print("FIXED: No memory leak - set is empty")
84+
else:
85+
print("BUG: Memory leak - set still contains apps")
86+
87+
88+
def demonstrate_multiple_apps():
89+
"""Demonstrate multiple apps"""
90+
print("\n=== Multiple Apps Test ===")
91+
92+
apps = [fastapi.FastAPI() for _ in range(3)]
93+
94+
print("Instrumenting all apps...")
95+
for i, app in enumerate(apps):
96+
FastAPIInstrumentor.instrument_app(app)
97+
print(f"App {i}: refcount={sys.getrefcount(app)}")
98+
99+
print(
100+
f"Set size after instrumenting: {len(_InstrumentedFastAPI._instrumented_fastapi_apps)}"
101+
)
102+
103+
print("Uninstrumenting all apps...")
104+
for i, app in enumerate(apps):
105+
FastAPIInstrumentor.uninstrument_app(app)
106+
print(f"App {i}: refcount={sys.getrefcount(app)}")
107+
108+
final_set_size = len(_InstrumentedFastAPI._instrumented_fastapi_apps)
109+
print(f"Final set size: {final_set_size}")
110+
111+
if final_set_size == 0:
112+
print("FIXED: All apps properly removed from set")
113+
else:
114+
print("BUG: Some apps still in set")
115+
116+
117+
if __name__ == "__main__":
118+
print("FastAPIInstrumentor Memory Leak Fix Demo")
119+
print("=" * 50)
120+
121+
demonstrate_problem()
122+
demonstrate_multiple_cycles()
123+
demonstrate_multiple_apps()
124+
125+
print("\n" + "=" * 50)
126+
print("Summary:")
127+
print(
128+
"- The fix adds code to remove apps from _instrumented_fastapi_apps during uninstrument_app()"
129+
)
130+
print(
131+
"- This prevents memory leaks when instrumenting/uninstrumenting repeatedly"
132+
)
133+
print(
134+
"- The fix is backward compatible and doesn't break existing functionality"
135+
)

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,10 @@ def uninstrument_app(app: fastapi.FastAPI):
358358
app.middleware_stack = app.build_middleware_stack()
359359
app._is_instrumented_by_opentelemetry = False
360360

361+
# Remove the app from the set of instrumented apps to prevent memory leaks
362+
if app in _InstrumentedFastAPI._instrumented_fastapi_apps:
363+
_InstrumentedFastAPI._instrumented_fastapi_apps.remove(app)
364+
361365
def instrumentation_dependencies(self) -> Collection[str]:
362366
return _instruments
363367

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

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
# Copyright The OpenTelemetry Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
import sys
16+
import unittest
17+
18+
import fastapi
19+
20+
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor
21+
22+
23+
class TestFastAPIMemoryLeak(unittest.TestCase):
24+
"""Test for memory leak in FastAPIInstrumentor.uninstrument_app()"""
25+
26+
def test_refcount_after_uninstrument(self):
27+
"""Test that refcount is restored after uninstrument_app()"""
28+
app = fastapi.FastAPI()
29+
30+
# Instrument the app
31+
FastAPIInstrumentor.instrument_app(app)
32+
refcount_after_instrument = sys.getrefcount(app)
33+
34+
# Uninstrument the app
35+
FastAPIInstrumentor.uninstrument_app(app)
36+
refcount_after_uninstrument = sys.getrefcount(app)
37+
38+
# The refcount should be reduced after uninstrument (may not be exactly initial due to Python internals)
39+
self.assertLess(
40+
refcount_after_uninstrument,
41+
refcount_after_instrument,
42+
"Refcount should be reduced after uninstrument_app()",
43+
)
44+
45+
# Verify that the app was removed from the set
46+
from opentelemetry.instrumentation.fastapi import _InstrumentedFastAPI
47+
48+
self.assertNotIn(
49+
app,
50+
_InstrumentedFastAPI._instrumented_fastapi_apps,
51+
"App should be removed from _instrumented_fastapi_apps after uninstrument_app()",
52+
)
53+
54+
def test_multiple_instrument_uninstrument_cycles(self):
55+
"""Test that multiple instrument/uninstrument cycles don't leak memory"""
56+
app = fastapi.FastAPI()
57+
58+
initial_refcount = sys.getrefcount(app)
59+
60+
# Perform multiple instrument/uninstrument cycles
61+
for i in range(5):
62+
FastAPIInstrumentor.instrument_app(app)
63+
FastAPIInstrumentor.uninstrument_app(app)
64+
65+
final_refcount = sys.getrefcount(app)
66+
67+
# The refcount should not grow significantly after multiple cycles
68+
# (may not be exactly initial due to Python internals)
69+
self.assertLessEqual(
70+
final_refcount,
71+
initial_refcount
72+
+ 2, # Allow small increase due to Python internals
73+
f"Refcount after {i+1} instrument/uninstrument cycles should not grow significantly",
74+
)
75+
76+
# Verify that the app is not in the set
77+
from opentelemetry.instrumentation.fastapi import _InstrumentedFastAPI
78+
79+
self.assertNotIn(
80+
app,
81+
_InstrumentedFastAPI._instrumented_fastapi_apps,
82+
"App should not be in _instrumented_fastapi_apps after uninstrument_app()",
83+
)
84+
85+
def test_multiple_apps_instrument_uninstrument(self):
86+
"""Test that multiple apps can be instrumented and uninstrumented without leaks"""
87+
apps = [fastapi.FastAPI() for _ in range(3)]
88+
initial_refcounts = [sys.getrefcount(app) for app in apps]
89+
90+
# Instrument all apps
91+
for app in apps:
92+
FastAPIInstrumentor.instrument_app(app)
93+
94+
# Uninstrument all apps
95+
for app in apps:
96+
FastAPIInstrumentor.uninstrument_app(app)
97+
98+
# Check that refcounts are not significantly increased
99+
for i, app in enumerate(apps):
100+
final_refcount = sys.getrefcount(app)
101+
self.assertLessEqual(
102+
final_refcount,
103+
initial_refcounts[i]
104+
+ 2, # Allow small increase due to Python internals
105+
f"App {i} refcount should not grow significantly",
106+
)
107+
108+
# Verify that no apps are in the set
109+
from opentelemetry.instrumentation.fastapi import _InstrumentedFastAPI
110+
111+
for app in apps:
112+
self.assertNotIn(
113+
app,
114+
_InstrumentedFastAPI._instrumented_fastapi_apps,
115+
"All apps should be removed from _instrumented_fastapi_apps",
116+
)
117+
118+
def test_demonstrate_fix(self):
119+
"""Demonstrate the fix for the memory leak issue"""
120+
app = fastapi.FastAPI()
121+
122+
# Before the fix: app would remain in _instrumented_fastapi_apps after uninstrument_app()
123+
# After the fix: app should be removed from _instrumented_fastapi_apps
124+
125+
FastAPIInstrumentor.instrument_app(app)
126+
from opentelemetry.instrumentation.fastapi import _InstrumentedFastAPI
127+
128+
# Verify app is in the set after instrumentation
129+
self.assertIn(app, _InstrumentedFastAPI._instrumented_fastapi_apps)
130+
131+
FastAPIInstrumentor.uninstrument_app(app)
132+
133+
# Verify app is removed from the set after uninstrumentation
134+
self.assertNotIn(app, _InstrumentedFastAPI._instrumented_fastapi_apps)
135+
self.assertEqual(
136+
len(_InstrumentedFastAPI._instrumented_fastapi_apps), 0
137+
)
138+
139+
140+
if __name__ == "__main__":
141+
unittest.main()

0 commit comments

Comments
 (0)