From 9f40f5d18d550ce7929783520fdaa12167dfc30d Mon Sep 17 00:00:00 2001 From: Pete Hodgson Date: Thu, 22 Aug 2024 17:41:16 -0700 Subject: [PATCH 1/8] 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. --- .../instrumentation/sqlalchemy/__init__.py | 8 ++++++++ .../tests/test_sqlalchemy.py | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 2107bc3e23..99bd23c65e 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -180,6 +180,13 @@ def _instrument(self, **kwargs): tracer, connections_usage, enable_commenter, commenter_options ), ) + _w( + "sqlalchemy.engine.create", + "create_engine", + _wrap_create_engine( + tracer, connections_usage, enable_commenter, commenter_options + ), + ) _w( "sqlalchemy.engine.base", "Engine.connect", @@ -223,6 +230,7 @@ def _instrument(self, **kwargs): def _uninstrument(self, **kwargs): unwrap(sqlalchemy, "create_engine") unwrap(sqlalchemy.engine, "create_engine") + unwrap(sqlalchemy.engine.create, "create_engine") unwrap(Engine, "connect") if parse_version(sqlalchemy.__version__).release >= (1, 4): unwrap(sqlalchemy.ext.asyncio, "create_async_engine") diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index f729fa6d80..ca7c38d7d4 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -177,6 +177,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("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( From 1a1627704bb87da7d75083e0cba64d12fa402469 Mon Sep 17 00:00:00 2001 From: Pete Hodgson Date: Thu, 22 Aug 2024 17:51:03 -0700 Subject: [PATCH 2/8] add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5abca8227a..28295ef390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2792](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2792)) - `opentelemetry-instrumentation-tornado` Handle http client exception and record exception info into span ([#2563](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2563)) +- `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)) ## Version 1.26.0/0.47b0 (2024-07-23) From e0500866d5995accdf8925446122e125aff9a0b3 Mon Sep 17 00:00:00 2001 From: Pete Hodgson Date: Fri, 23 Aug 2024 07:42:25 -0700 Subject: [PATCH 3/8] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28295ef390..2d34f8c83a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2792](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2792)) - `opentelemetry-instrumentation-tornado` Handle http client exception and record exception info into span ([#2563](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2563)) -- `opentelemetry-instrumentation-sqlalchemy` Fixes engines from sqlalchemy.engine_from_config(...) not being fully instrumented +- `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)) ## Version 1.26.0/0.47b0 (2024-07-23) From cbb04476ad958f14e29967c92c0005ff5bab1177 Mon Sep 17 00:00:00 2001 From: Pete Hodgson Date: Fri, 8 Nov 2024 15:55:00 -0800 Subject: [PATCH 4/8] Update instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> --- .../tests/test_sqlalchemy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index a5aa62e371..458ab8a623 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -188,7 +188,7 @@ def test_instrument_engine_from_config(self): engine = engine_from_config({"sqlalchemy.url": "sqlite:///:memory:"}) cnx = engine.connect() - cnx.execute("SELECT 1 + 1;").fetchall() + cnx.execute(text("SELECT 1 + 1;")).fetchall() spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) From ab6c5e489fdfb05fc6174a61dc2922f2244b94e6 Mon Sep 17 00:00:00 2001 From: Pete Hodgson Date: Thu, 21 Nov 2024 12:16:39 -0800 Subject: [PATCH 5/8] make some monkey-patching conditional on the version of SQLAlchemy --- .../instrumentation/sqlalchemy/__init__.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 0b5e14b929..e5aaef4782 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -181,13 +181,15 @@ def _instrument(self, **kwargs): tracer, connections_usage, enable_commenter, commenter_options ), ) - _w( - "sqlalchemy.engine.create", - "create_engine", - _wrap_create_engine( - 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", @@ -231,7 +233,8 @@ def _instrument(self, **kwargs): def _uninstrument(self, **kwargs): unwrap(sqlalchemy, "create_engine") unwrap(sqlalchemy.engine, "create_engine") - unwrap(sqlalchemy.engine.create, "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") From 67db7b9be586299f31efd0fb31e872bdecbdd883 Mon Sep 17 00:00:00 2001 From: Pete Hodgson Date: Thu, 21 Nov 2024 17:02:19 -0800 Subject: [PATCH 6/8] lint --- .../src/opentelemetry/instrumentation/sqlalchemy/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index e5aaef4782..4182c0034e 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -187,7 +187,10 @@ def _instrument(self, **kwargs): "sqlalchemy.engine.create", "create_engine", _wrap_create_engine( - tracer, connections_usage, enable_commenter, commenter_options + tracer, + connections_usage, + enable_commenter, + commenter_options, ), ) _w( From 0b7e6bdb298010f154e24d597d5883daf27c1d3f Mon Sep 17 00:00:00 2001 From: emdneto <9735060+emdneto@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:13:50 -0300 Subject: [PATCH 7/8] fix changelog Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e53dbd9980..d6d13c48dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,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)) ### Breaking changes @@ -129,8 +131,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2792](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2792)) - `opentelemetry-instrumentation-tornado` Handle http client exception and record exception info into span ([#2563](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2563)) -- `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` fix `http.host` new http semantic convention mapping to depend on `kind` of span ([#2814](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2814)) - `opentelemetry-instrumentation` Fix the description of `http.server.duration` and `http.server.request.duration` From 2eef6f10bad1a6cf5d1136c6ad1a97055dc4de7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:15:36 -0300 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6d13c48dd..cc39664729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ 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 +- `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)) ### Breaking changes