Skip to content

Commit

Permalink
Remove references to disposed SQLAlchemy engines from instrumentation…
Browse files Browse the repository at this point in the history
… singleton (#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 <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
  • Loading branch information
3 people authored Dec 3, 2024
1 parent 668cb75 commit b6541f0
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -432,3 +436,4 @@ def make_shortlived_engine():

gc.collect()
assert callback.call_count == 5
assert len(EngineTracer._remove_event_listener_params) == 0

0 comments on commit b6541f0

Please sign in to comment.