Skip to content

Commit

Permalink
Fix instrumentation of SQLAlchemy when using sqlalchemy.engine_from_c…
Browse files Browse the repository at this point in the history
…onfig (open-telemetry#2816)

* wrap sqlalchemy.engine.create.create_engine

sqlalchemy.engine_from_config directly calls create_engine imported from that path; if we don't
wrap that copy of the `create_engine` call then engines created via engine_from_config are not
instrumented.

* add changelog entry

* Update CHANGELOG.md

Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>

* Update instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py

Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com>

* make some monkey-patching conditional on the version of SQLAlchemy

* lint

* fix changelog

Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>

* Update CHANGELOG.md

---------

Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
Co-authored-by: Shalev Roda <65566801+shalevr@users.noreply.github.com>
Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
  • Loading branch information
6 people authored Dec 4, 2024
1 parent 9e83e25 commit 7617031
Show file tree
Hide file tree
Showing 3 changed files with 27 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` Fixes engines from `sqlalchemy.engine_from_config` not being fully instrumented
([#2816](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2816))
- `opentelemetry-instrumentation-sqlalchemy`: Fix a remaining memory leak in EngineTracer
([#3053](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3053))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,18 @@ def _instrument(self, **kwargs):
tracer, connections_usage, enable_commenter, commenter_options
),
)
# sqlalchemy.engine.create is not present in earlier versions of sqlalchemy (which we support)
if parse_version(sqlalchemy.__version__).release >= (1, 4):
_w(
"sqlalchemy.engine.create",
"create_engine",
_wrap_create_engine(
tracer,
connections_usage,
enable_commenter,
commenter_options,
),
)
_w(
"sqlalchemy.engine.base",
"Engine.connect",
Expand Down Expand Up @@ -224,6 +236,8 @@ def _instrument(self, **kwargs):
def _uninstrument(self, **kwargs):
unwrap(sqlalchemy, "create_engine")
unwrap(sqlalchemy.engine, "create_engine")
if parse_version(sqlalchemy.__version__).release >= (1, 4):
unwrap(sqlalchemy.engine.create, "create_engine")
unwrap(Engine, "connect")
if parse_version(sqlalchemy.__version__).release >= (1, 4):
unwrap(sqlalchemy.ext.asyncio, "create_async_engine")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,17 @@ def test_create_engine_wrapper(self):
"opentelemetry.instrumentation.sqlalchemy",
)

def test_instrument_engine_from_config(self):
SQLAlchemyInstrumentor().instrument()
from sqlalchemy import engine_from_config # pylint: disable-all

engine = engine_from_config({"sqlalchemy.url": "sqlite:///:memory:"})
cnx = engine.connect()
cnx.execute(text("SELECT 1 + 1;")).fetchall()
spans = self.memory_exporter.get_finished_spans()

self.assertEqual(len(spans), 2)

def test_create_engine_wrapper_enable_commenter(self):
logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)
SQLAlchemyInstrumentor().instrument(
Expand Down

0 comments on commit 7617031

Please sign in to comment.