Remove references to disposed SQLAlchemy engines from instrumentation…

… singleton (open-telemetry#3053) * Remove references to SQLAlchemy engines which are disposed of EngineTracer in SQLAlchemy keeps weak references to a traced engine forever which can cause a noticeable memory leak if engines are constantly getting creating. * Updated changelog --------- Co-authored-by: Emídio Neto <[email protected]> Co-authored-by: Riccardo Magliocchetti <[email protected]>
fidelity-contributions · Dec 3, 2024 · b6541f0 · b6541f0
1 parent 668cb75
commit b6541f0
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -46,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#3022](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3022))
 - Replace all instrumentor unit test `assertEqualSpanInstrumentationInfo` calls with `assertEqualSpanInstrumentationScope` calls
   ([#3037](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3037))
+- `opentelemetry-instrumentation-sqlalchemy`: Fix a remaining memory leak in EngineTracer
+  ([#3053](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3053))
 
 ### Breaking changes
 

diff --git a/...lemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/...lemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py
@@ -167,13 +167,26 @@ def _pool_checkout(
         self._add_idle_to_connection_usage(-1)
         self._add_used_to_connection_usage(1)
 
+    @classmethod
+    def _dispose_of_event_listener(cls, obj):
+        try:
+            cls._remove_event_listener_params.remove(obj)
+        except ValueError:
+            pass
+
     @classmethod
     def _register_event_listener(cls, target, identifier, func, *args, **kw):
         listen(target, identifier, func, *args, **kw)
         cls._remove_event_listener_params.append(
             (weakref.ref(target), identifier, func)
         )
 
+        weakref.finalize(
+            target,
+            cls._dispose_of_event_listener,
+            (weakref.ref(target), identifier, func),
+        )
+
     @classmethod
     def remove_all_event_listeners(cls):
         for (

diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py
@@ -417,6 +417,10 @@ def test_no_memory_leakage_if_engine_diposed(self):
 
         from sqlalchemy import create_engine
 
+        from opentelemetry.instrumentation.sqlalchemy.engine import (
+            EngineTracer,
+        )
+
         callback = mock.Mock()
 
         def make_shortlived_engine():
@@ -432,3 +436,4 @@ def make_shortlived_engine():
 
         gc.collect()
         assert callback.call_count == 5
+        assert len(EngineTracer._remove_event_listener_params) == 0