From b1ae34fea3c73867a401d4a0e25ba694fef13a1e Mon Sep 17 00:00:00 2001 From: avzis Date: Sun, 13 Nov 2022 17:29:53 +0200 Subject: [PATCH 1/8] fix enable_commenter functionality --- .../instrumentation/sqlalchemy/__init__.py | 7 ++++--- .../opentelemetry/instrumentation/sqlalchemy/engine.py | 10 +++++----- 2 files changed, 9 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 e56485ca77..6b338c9008 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -136,11 +136,12 @@ def _instrument(self, **kwargs): An instrumented engine if passed in as an argument or list of instrumented engines, None otherwise. """ tracer_provider = kwargs.get("tracer_provider") - _w("sqlalchemy", "create_engine", _wrap_create_engine(tracer_provider)) + enable_commenter = kwargs.get("enable_commenter", False) + _w("sqlalchemy", "create_engine", _wrap_create_engine(tracer_provider, enable_commenter)) _w( "sqlalchemy.engine", "create_engine", - _wrap_create_engine(tracer_provider), + _wrap_create_engine(tracer_provider, enable_commenter), ) _w( "sqlalchemy.engine.base", @@ -151,7 +152,7 @@ def _instrument(self, **kwargs): _w( "sqlalchemy.ext.asyncio", "create_async_engine", - _wrap_create_async_engine(tracer_provider), + _wrap_create_async_engine(tracer_provider, enable_commenter), ) if kwargs.get("engine") is not None: return EngineTracer( diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py index 73941200c2..524bb1822a 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -48,27 +48,27 @@ def _get_tracer(tracer_provider=None): ) -def _wrap_create_async_engine(tracer_provider=None): +def _wrap_create_async_engine(tracer_provider=None, enable_commenter=False): # pylint: disable=unused-argument def _wrap_create_async_engine_internal(func, module, args, kwargs): """Trace the SQLAlchemy engine, creating an `EngineTracer` object that will listen to SQLAlchemy events. """ engine = func(*args, **kwargs) - EngineTracer(_get_tracer(tracer_provider), engine.sync_engine) + EngineTracer(_get_tracer(tracer_provider), engine.sync_engine, enable_commenter) return engine return _wrap_create_async_engine_internal -def _wrap_create_engine(tracer_provider=None): +def _wrap_create_engine(tracer_provider=None, enable_commenter=False): # pylint: disable=unused-argument def _wrap_create_engine_internal(func, module, args, kwargs): """Trace the SQLAlchemy engine, creating an `EngineTracer` object that will listen to SQLAlchemy events. """ engine = func(*args, **kwargs) - EngineTracer(_get_tracer(tracer_provider), engine) + EngineTracer(_get_tracer(tracer_provider), engine, enable_commenter) return engine return _wrap_create_engine_internal @@ -93,7 +93,7 @@ def _wrap_connect_internal(func, module, args, kwargs): class EngineTracer: def __init__( - self, tracer, engine, enable_commenter=True, commenter_options=None + self, tracer, engine, enable_commenter=False, commenter_options=None ): self.tracer = tracer self.engine = engine From 789c0b6909fdde2d63a8ffe6f4580a90eee8e127 Mon Sep 17 00:00:00 2001 From: avzis Date: Sun, 13 Nov 2022 17:33:33 +0200 Subject: [PATCH 2/8] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc71d38d2c..f3d8f8f692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1424](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1424)) - Remove db.name attribute from Redis instrumentation ([#1427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1427)) +- Fix bug in SQLAlchemy instrumentation - support disabling enable_commenter variable + ([#1427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1427)) ## Version 1.14.0/0.35b0 (2022-11-03) From 7efbea3748ba8c42c41bac472ab41aa725d99b97 Mon Sep 17 00:00:00 2001 From: avzis Date: Sun, 13 Nov 2022 17:41:51 +0200 Subject: [PATCH 3/8] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3d8f8f692..a3e5f96d27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Remove db.name attribute from Redis instrumentation ([#1427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1427)) - Fix bug in SQLAlchemy instrumentation - support disabling enable_commenter variable - ([#1427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1427)) + ([#1440](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1440)) ## Version 1.14.0/0.35b0 (2022-11-03) From 797eee4f8207bd66c91cfc7c81a432cf8e1774f6 Mon Sep 17 00:00:00 2001 From: avzis Date: Mon, 14 Nov 2022 13:38:35 +0200 Subject: [PATCH 4/8] lint --- .../src/opentelemetry/instrumentation/sqlalchemy/engine.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py index 524bb1822a..8eddc06e6d 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -55,7 +55,9 @@ def _wrap_create_async_engine_internal(func, module, args, kwargs): object that will listen to SQLAlchemy events. """ engine = func(*args, **kwargs) - EngineTracer(_get_tracer(tracer_provider), engine.sync_engine, enable_commenter) + EngineTracer( + _get_tracer(tracer_provider), engine.sync_engine, enable_commenter + ) return engine return _wrap_create_async_engine_internal From 4629273804ab21a8cdcfc2ad61cb22ee42cdac4e Mon Sep 17 00:00:00 2001 From: avzis Date: Tue, 15 Nov 2022 10:32:47 +0200 Subject: [PATCH 5/8] lint --- .../opentelemetry/instrumentation/sqlalchemy/__init__.py | 6 +++++- 1 file changed, 5 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 6b338c9008..6c91ae16e0 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -137,7 +137,11 @@ def _instrument(self, **kwargs): """ tracer_provider = kwargs.get("tracer_provider") enable_commenter = kwargs.get("enable_commenter", False) - _w("sqlalchemy", "create_engine", _wrap_create_engine(tracer_provider, enable_commenter)) + _w( + "sqlalchemy", + "create_engine", + _wrap_create_engine(tracer_provider, enable_commenter), + ) _w( "sqlalchemy.engine", "create_engine", From 9b42b61e4ca71ce96cd15d5f6b1c6a83dd715a0e Mon Sep 17 00:00:00 2001 From: avzis Date: Sun, 4 Dec 2022 16:10:25 +0200 Subject: [PATCH 6/8] add tests --- .../tests/test_sqlcommenter.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index 6cdc5b2f67..5492987ed2 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -77,3 +77,32 @@ def test_sqlcommenter_flask_integration(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)',flask=1,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + + def test_sqlcommenter_enabled_create_engine_after_instrumentation(self): + SQLAlchemyInstrumentor().instrument( + tracer_provider=self.tracer_provider, + enable_commenter=True, + ) + + from sqlalchemy import create_engine # pylint: disable-all + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + + cnx.execute("SELECT 1;").fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + + def test_sqlcommenter_disabled_create_engine_after_instrumentation(self): + SQLAlchemyInstrumentor().instrument( + tracer_provider=self.tracer_provider, + enable_commenter=False, + ) + + from sqlalchemy import create_engine # pylint: disable-all + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + + cnx.execute("SELECT 1;").fetchall() + self.assertEqual(self.caplog.records[-2].getMessage(), "SELECT 1;") From 9535dcecba2e2d3610b5aa34c92e251ae7445db8 Mon Sep 17 00:00:00 2001 From: avzis Date: Sun, 4 Dec 2022 16:46:32 +0200 Subject: [PATCH 7/8] reverse --- .../tests/test_mysql_integration.py | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py index e2a0f2057c..8274851ff1 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py @@ -17,6 +17,7 @@ import mysql.connector import opentelemetry.instrumentation.mysql +from opentelemetry import trace as trace_api from opentelemetry.instrumentation.mysql import MySQLInstrumentor from opentelemetry.sdk import resources from opentelemetry.test.test_base import TestBase @@ -31,6 +32,15 @@ def cursor(self): return MockConnection() +def connect_and_execute_query(): + cnx = mysql.connector.connect(database="test") + cursor = cnx.cursor() + query = "SELECT * FROM test" + cursor.execute(query) + + return cnx, query + + class TestMysqlIntegration(TestBase): def tearDown(self): super().tearDown() @@ -42,10 +52,7 @@ def tearDown(self): def test_instrumentor(self): MySQLInstrumentor().instrument() - cnx = mysql.connector.connect(database="test") - cursor = cnx.cursor() - query = "SELECT * FROM test" - cursor.execute(query) + connect_and_execute_query() spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) @@ -59,10 +66,7 @@ def test_instrumentor(self): # check that no spans are generated after uninstrumen MySQLInstrumentor().uninstrument() - cnx = mysql.connector.connect(database="test") - cursor = cnx.cursor() - query = "SELECT * FROM test" - cursor.execute(query) + connect_and_execute_query() spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) @@ -74,10 +78,7 @@ def test_custom_tracer_provider(self): tracer_provider, exporter = result MySQLInstrumentor().instrument(tracer_provider=tracer_provider) - cnx = mysql.connector.connect(database="test") - cursor = cnx.cursor() - query = "SELECT * FROM test" - cursor.execute(query) + connect_and_execute_query() span_list = exporter.get_finished_spans() self.assertEqual(len(span_list), 1) @@ -88,10 +89,7 @@ def test_custom_tracer_provider(self): @patch("mysql.connector.connect", new=mock_connect) # pylint: disable=unused-argument def test_instrument_connection(self): - cnx = mysql.connector.connect(database="test") - query = "SELECT * FROM test" - cursor = cnx.cursor() - cursor.execute(query) + cnx, query = connect_and_execute_query() spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 0) @@ -103,14 +101,20 @@ def test_instrument_connection(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + @patch("mysql.connector.connect", new=mock_connect) + def test_instrument_connection_no_op_tracer_provider(self): + tracer_provider = trace_api.NoOpTracerProvider() + MySQLInstrumentor().instrument(tracer_provider=tracer_provider) + connect_and_execute_query() + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 0) + @patch("mysql.connector.connect", new=mock_connect) # pylint: disable=unused-argument def test_uninstrument_connection(self): MySQLInstrumentor().instrument() - cnx = mysql.connector.connect(database="test") - query = "SELECT * FROM test" - cursor = cnx.cursor() - cursor.execute(query) + cnx, query = connect_and_execute_query() spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) From f20028934a2fc1cdc75941bcee35ebf272bf522f Mon Sep 17 00:00:00 2001 From: avzis Date: Tue, 6 Dec 2022 14:26:35 +0200 Subject: [PATCH 8/8] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1992c9e35a..b72b9d09bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1461](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1461)) - Add grpc.aio instrumentation to package entry points ([#1442](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1442)) -- Fix bug in SQLAlchemy instrumentation - support disabling enable_commenter variable +- Fix a bug in SQLAlchemy instrumentation - support disabling enable_commenter variable ([#1440](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1440)) ## Version 1.14.0/0.35b0 (2022-11-03)