From 224a771ef05d7118d55ec8bb87b99b6a05a7d08f Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 8 Oct 2024 18:02:31 -0700 Subject: [PATCH 01/17] WIP --- .../instrumentation/dbapi/__init__.py | 46 +++++++++++++++---- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 0857d2989b..70cd6caa6e 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -427,21 +427,47 @@ def traced_execution( if args and self._commenter_enabled: try: args_list = list(args) - if hasattr(self._connect_module, "__libpq_version__"): - libpq_version = self._connect_module.__libpq_version__ - else: - libpq_version = ( - self._connect_module.pq.__build_version__ - ) - + db_driver = self._db_api_integration.connect_module.__name__ commenter_data = { - # Psycopg2/framework information - "db_driver": f"psycopg2:{self._connect_module.__version__.split(' ')[0]}", + # TODO MySQLdb attribute + "db_driver": f"{db_driver}:{self._connect_module.__version__.split(' ')[0]}", "dbapi_threadsafety": self._connect_module.threadsafety, "dbapi_level": self._connect_module.apilevel, - "libpq_version": libpq_version, "driver_paramstyle": self._connect_module.paramstyle, } + + if self._db_api_integration.database_system == "postgresql": + if hasattr(self._connect_module, "__libpq_version__"): + libpq_version = self._connect_module.__libpq_version__ + else: + libpq_version = ( + self._connect_module.pq.__build_version__ + ) + commenter_data.update( + { + "libpq_version": libpq_version, + } + ) + + elif self._db_api_integration.database_system == "mysql": + db_driver = self._db_api_integration.connect_module.__name__ + + mysqlc_version = "" + if db_driver == "mysql.connector": + # TODO Version of mysql.connector not same as mysql C API + mysqlc_version = self._db_api_integration.connect_module.__version__ + elif db_driver == "MySQLdb": + # TODO + mysqlc_version = "TODO" + + commenter_data.update( + { + "mysqlc_version": mysqlc_version, + } + ) + + _logger.debug("Using commenter_data: %s", commenter_data) + if self._commenter_options.get( "opentelemetry_values", True ): From 3c436043960933a56d024f99b0a9b53e9f58cfaf Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 11:59:45 -0700 Subject: [PATCH 02/17] Add _DB_DRIVER_ALIASES --- .../instrumentation/dbapi/__init__.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 70cd6caa6e..2f0148cae1 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -53,6 +53,11 @@ ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, TracerProvider, get_tracer +from opentelemetry.util._importlib_metadata import version + +_DB_DRIVER_ALIASES = { + "MySQLdb": "mysqlclient", +} _logger = logging.getLogger(__name__) @@ -428,9 +433,14 @@ def traced_execution( try: args_list = list(args) db_driver = self._db_api_integration.connect_module.__name__ + db_version = "" + if db_driver in _DB_DRIVER_ALIASES.keys(): + db_version = version(_DB_DRIVER_ALIASES[db_driver]) + else: + db_version = self._db_api_integration.connect_module.__version__ + commenter_data = { - # TODO MySQLdb attribute - "db_driver": f"{db_driver}:{self._connect_module.__version__.split(' ')[0]}", + "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", "dbapi_threadsafety": self._connect_module.threadsafety, "dbapi_level": self._connect_module.apilevel, "driver_paramstyle": self._connect_module.paramstyle, From cec3a86420129153fa05ca5653c850386cbe8553 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 14:42:24 -0700 Subject: [PATCH 03/17] Add mysql_client_version to sqlcomment --- .../instrumentation/dbapi/__init__.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 2f0148cae1..5f053938e8 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -458,21 +458,16 @@ def traced_execution( "libpq_version": libpq_version, } ) - - elif self._db_api_integration.database_system == "mysql": - db_driver = self._db_api_integration.connect_module.__name__ - + elif self._db_api_integration.database_system == "mysql": mysqlc_version = "" if db_driver == "mysql.connector": - # TODO Version of mysql.connector not same as mysql C API - mysqlc_version = self._db_api_integration.connect_module.__version__ - elif db_driver == "MySQLdb": - # TODO - mysqlc_version = "TODO" + mysqlc_version = cursor._cnx._cmysql.get_client_info() + if db_driver == "MySQLdb": + mysqlc_version = self._db_api_integration.connect_module._mysql.get_client_info() commenter_data.update( { - "mysqlc_version": mysqlc_version, + "mysql_client_version": mysqlc_version, } ) From 47cdaa95e85782b9dffbd457e27436d51004573a Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 15:13:31 -0700 Subject: [PATCH 04/17] lint --- .../instrumentation/dbapi/__init__.py | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 5f053938e8..0c256bc69d 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -53,7 +53,7 @@ ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, TracerProvider, get_tracer -from opentelemetry.util._importlib_metadata import version +from opentelemetry.util._importlib_metadata import version as util_version _DB_DRIVER_ALIASES = { "MySQLdb": "mysqlclient", @@ -432,12 +432,18 @@ def traced_execution( if args and self._commenter_enabled: try: args_list = list(args) - db_driver = self._db_api_integration.connect_module.__name__ + db_driver = ( + self._db_api_integration.connect_module.__name__ + ) db_version = "" - if db_driver in _DB_DRIVER_ALIASES.keys(): - db_version = version(_DB_DRIVER_ALIASES[db_driver]) + if db_driver in _DB_DRIVER_ALIASES: + db_version = util_version( + _DB_DRIVER_ALIASES[db_driver] + ) else: - db_version = self._db_api_integration.connect_module.__version__ + db_version = ( + self._db_api_integration.connect_module.__version__ + ) commenter_data = { "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", @@ -446,9 +452,14 @@ def traced_execution( "driver_paramstyle": self._connect_module.paramstyle, } - if self._db_api_integration.database_system == "postgresql": + if ( + self._db_api_integration.database_system + == "postgresql" + ): if hasattr(self._connect_module, "__libpq_version__"): - libpq_version = self._connect_module.__libpq_version__ + libpq_version = ( + self._connect_module.__libpq_version__ + ) else: libpq_version = ( self._connect_module.pq.__build_version__ @@ -458,12 +469,16 @@ def traced_execution( "libpq_version": libpq_version, } ) - elif self._db_api_integration.database_system == "mysql": + elif self._db_api_integration.database_system == "mysql": mysqlc_version = "" if db_driver == "mysql.connector": - mysqlc_version = cursor._cnx._cmysql.get_client_info() + mysqlc_version = ( + cursor._cnx._cmysql.get_client_info() + ) if db_driver == "MySQLdb": - mysqlc_version = self._db_api_integration.connect_module._mysql.get_client_info() + mysqlc_version = ( + self._db_api_integration.connect_module._mysql.get_client_info() + ) commenter_data.update( { @@ -471,8 +486,6 @@ def traced_execution( } ) - _logger.debug("Using commenter_data: %s", commenter_data) - if self._commenter_options.get( "opentelemetry_values", True ): From b9335f055c0ccf69aef09d13981c284fe353fa15 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 15:46:00 -0700 Subject: [PATCH 05/17] Fix existing tests --- .../tests/test_dbapi_integration.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index eb2d628a3a..3bcfb4421c 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -252,6 +252,7 @@ def test_executemany(self): def test_executemany_comment(self): connect_module = mock.MagicMock() + connect_module.__name__ = mock.MagicMock() connect_module.__version__ = mock.MagicMock() connect_module.__libpq_version__ = 123 connect_module.apilevel = 123 @@ -260,7 +261,7 @@ def test_executemany_comment(self): db_integration = dbapi.DatabaseApiIntegration( "testname", - "testcomponent", + "postgresql", enable_commenter=True, commenter_options={"db_driver": False, "dbapi_level": False}, connect_module=connect_module, @@ -277,6 +278,7 @@ def test_executemany_comment(self): def test_compatible_build_version_psycopg_psycopg2_libpq(self): connect_module = mock.MagicMock() + connect_module.__name__ = mock.MagicMock() connect_module.__version__ = mock.MagicMock() connect_module.pq = mock.MagicMock() connect_module.pq.__build_version__ = 123 @@ -286,7 +288,7 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): db_integration = dbapi.DatabaseApiIntegration( "testname", - "testcomponent", + "postgresql", enable_commenter=True, commenter_options={"db_driver": False, "dbapi_level": False}, connect_module=connect_module, @@ -303,6 +305,7 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): def test_executemany_flask_integration_comment(self): connect_module = mock.MagicMock() + connect_module.__name__ = mock.MagicMock() connect_module.__version__ = mock.MagicMock() connect_module.__libpq_version__ = 123 connect_module.apilevel = 123 @@ -311,7 +314,7 @@ def test_executemany_flask_integration_comment(self): db_integration = dbapi.DatabaseApiIntegration( "testname", - "testcomponent", + "postgresql", enable_commenter=True, commenter_options={"db_driver": False, "dbapi_level": False}, connect_module=connect_module, From 881f4af78ecf35f01d702f351c48a2e5e6a72544 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 15:52:22 -0700 Subject: [PATCH 06/17] lint test --- .../tests/test_dbapi_integration.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 3bcfb4421c..bc76eeb578 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -252,7 +252,7 @@ def test_executemany(self): def test_executemany_comment(self): connect_module = mock.MagicMock() - connect_module.__name__ = mock.MagicMock() + connect_module.__name__ = "test" connect_module.__version__ = mock.MagicMock() connect_module.__libpq_version__ = 123 connect_module.apilevel = 123 @@ -278,7 +278,7 @@ def test_executemany_comment(self): def test_compatible_build_version_psycopg_psycopg2_libpq(self): connect_module = mock.MagicMock() - connect_module.__name__ = mock.MagicMock() + connect_module.__name__ = "test" connect_module.__version__ = mock.MagicMock() connect_module.pq = mock.MagicMock() connect_module.pq.__build_version__ = 123 @@ -305,7 +305,7 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): def test_executemany_flask_integration_comment(self): connect_module = mock.MagicMock() - connect_module.__name__ = mock.MagicMock() + connect_module.__name__ = "test" connect_module.__version__ = mock.MagicMock() connect_module.__libpq_version__ = 123 connect_module.apilevel = 123 From b0a9efd4acc6170cec7ce1f9396ba71218b40e86 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 16:22:10 -0700 Subject: [PATCH 07/17] Add PyMySQL dbapi commenter case --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 0c256bc69d..4f72a03a85 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -410,7 +410,7 @@ def get_statement(self, cursor, args): # pylint: disable=no-self-use return statement.decode("utf8", "replace") return statement - def traced_execution( + def traced_execution( # pylint: disable=too-many-branches self, cursor, query_method: typing.Callable[..., typing.Any], @@ -479,6 +479,10 @@ def traced_execution( mysqlc_version = ( self._db_api_integration.connect_module._mysql.get_client_info() ) + if db_driver == "pymysql": + mysqlc_version = ( + self._db_api_integration.connect_module.get_client_info() + ) commenter_data.update( { From 4dd9f7320e040e8d7b6759d1ef62e88f46093135 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 17:50:44 -0700 Subject: [PATCH 08/17] Add test --- .../tests/test_dbapi_integration.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index bc76eeb578..e657449989 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -303,6 +303,32 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + def test_executemany_mysqlconnector_integration_comment(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "mysql.connector" + connect_module.__version__ = "1.2.3" + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "mysql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + ) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='mysql.connector%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='1.2.3',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + def test_executemany_flask_integration_comment(self): connect_module = mock.MagicMock() connect_module.__name__ = "test" @@ -335,6 +361,11 @@ def test_executemany_flask_integration_comment(self): r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',flask=1,libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + clear_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {}, current_context + ) + context.attach(clear_context) + def test_callproc(self): db_integration = dbapi.DatabaseApiIntegration( "testname", "testcomponent" @@ -418,6 +449,12 @@ class MockCursor: def __init__(self) -> None: self.query = "" self.params = None + # Mock mysql.connector modules and method + self._cnx = mock.MagicMock() + self._cnx._cmysql = mock.MagicMock() + self._cnx._cmysql.get_client_info = mock.MagicMock( + return_value="1.2.3" + ) # pylint: disable=unused-argument, no-self-use def execute(self, query, params=None, throw_exception=False): From d56eb215bfc266530ec3dc9c375e100c86352c46 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 18:23:30 -0700 Subject: [PATCH 09/17] Add test --- .../tests/test_dbapi_integration.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index e657449989..9e1c70d9a0 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -329,6 +329,41 @@ def test_executemany_mysqlconnector_integration_comment(self): r"Select 1 /\*db_driver='mysql.connector%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='1.2.3',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + @mock.patch("opentelemetry.instrumentation.dbapi.util_version") + def test_executemany_mysqlclient_integration_comment( + self, + mock_dbapi_util_version, + ): + mock_dbapi_util_version.return_value = "1.2.3" + connect_module = mock.MagicMock() + connect_module.__name__ = "MySQLdb" + connect_module.__version__ = "1.2.3" + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + connect_module._mysql = mock.MagicMock() + connect_module._mysql.get_client_info = mock.MagicMock( + return_value="123" + ) + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "mysql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + ) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='MySQLdb%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + def test_executemany_flask_integration_comment(self): connect_module = mock.MagicMock() connect_module.__name__ = "test" From 385ea6c2b1ec445f1c5254b4625e0e8c0ac1cc68 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 18:38:50 -0700 Subject: [PATCH 10/17] Add test --- .../tests/test_dbapi_integration.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 9e1c70d9a0..b5b296074c 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -364,6 +364,33 @@ def test_executemany_mysqlclient_integration_comment( r"Select 1 /\*db_driver='MySQLdb%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + def test_executemany_pymysql_integration_comment(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "pymysql" + connect_module.__version__ = "1.2.3" + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + connect_module.get_client_info = mock.MagicMock(return_value="123") + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "mysql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + ) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='pymysql%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + def test_executemany_flask_integration_comment(self): connect_module = mock.MagicMock() connect_module.__name__ = "test" From 52430b5dc9c5479de04524a4590f0fc9d03a01dd Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 9 Oct 2024 18:46:11 -0700 Subject: [PATCH 11/17] Add tests --- .../tests/test_dbapi_integration.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index b5b296074c..d44de49ca1 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -24,6 +24,7 @@ from opentelemetry.test.test_base import TestBase +# pylint: disable=too-many-public-methods class TestDBApiIntegration(TestBase): def setUp(self): super().setUp() @@ -303,6 +304,59 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + def test_executemany_psycopg2_integration_comment(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "psycopg2" + connect_module.__version__ = "1.2.3" + connect_module.__libpq_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='psycopg2%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + + def test_executemany_psycopg_integration_comment(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "psycopg" + connect_module.__version__ = "1.2.3" + connect_module.pq = mock.MagicMock() + connect_module.pq.__build_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='psycopg%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + def test_executemany_mysqlconnector_integration_comment(self): connect_module = mock.MagicMock() connect_module.__name__ = "mysql.connector" From 98ee0190a08193d4e0a2895c4c223fa5ac8a8649 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Fri, 11 Oct 2024 09:59:42 -0700 Subject: [PATCH 12/17] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4dcc10c7b..9cfe372459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-aiokafka` Wrap `AIOKafkaConsumer.getone()` instead of `AIOKafkaConsumer.__anext__` ([#2874](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2874)) +- `opentelemetry-instrumentation-dbapi` sqlcommenter key values created from PostgreSQL, MySQL systems ([#2897](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2897)) ## Version 1.27.0/0.48b0 (2024-08-28) From 8e4cb08eedfd5a52147c31f9767820be7250789f Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Fri, 11 Oct 2024 13:56:59 -0700 Subject: [PATCH 13/17] calculate_commenter_data at init of DatabaseApiIntegration --- .../instrumentation/dbapi/__init__.py | 115 +++++++++--------- 1 file changed, 60 insertions(+), 55 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 4f72a03a85..cf8a599f78 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -280,6 +280,52 @@ def __init__( self.name = "" self.database = "" self.connect_module = connect_module + self.commenter_data = self.calculate_commenter_data() + + def calculate_commenter_data( + self, + ): + commenter_data = {} + if not self.enable_commenter: + return commenter_data + + db_driver = self.connect_module.__name__ + db_version = "" + if db_driver in _DB_DRIVER_ALIASES: + db_version = util_version(_DB_DRIVER_ALIASES[db_driver]) + else: + db_version = self.connect_module.__version__ + commenter_data = { + "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", + "dbapi_threadsafety": self.connect_module.threadsafety, + "dbapi_level": self.connect_module.apilevel, + "driver_paramstyle": self.connect_module.paramstyle, + } + + if self.database_system == "postgresql": + if hasattr(self.connect_module, "__libpq_version__"): + libpq_version = self.connect_module.__libpq_version__ + else: + libpq_version = self.connect_module.pq.__build_version__ + commenter_data.update( + { + "libpq_version": libpq_version, + } + ) + elif self.database_system == "mysql": + mysqlc_version = "" + if db_driver == "MySQLdb": + mysqlc_version = self.connect_module._mysql.get_client_info() + elif db_driver == "pymysql": + mysqlc_version = self.connect_module.get_client_info() + + commenter_data.update( + { + "mysql_client_version": mysqlc_version, + } + ) + + return commenter_data def wrapped_connection( self, @@ -410,7 +456,7 @@ def get_statement(self, cursor, args): # pylint: disable=no-self-use return statement.decode("utf8", "replace") return statement - def traced_execution( # pylint: disable=too-many-branches + def traced_execution( self, cursor, query_method: typing.Callable[..., typing.Any], @@ -432,64 +478,23 @@ def traced_execution( # pylint: disable=too-many-branches if args and self._commenter_enabled: try: args_list = list(args) - db_driver = ( - self._db_api_integration.connect_module.__name__ - ) - db_version = "" - if db_driver in _DB_DRIVER_ALIASES: - db_version = util_version( - _DB_DRIVER_ALIASES[db_driver] - ) - else: - db_version = ( - self._db_api_integration.connect_module.__version__ - ) - - commenter_data = { - "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", - "dbapi_threadsafety": self._connect_module.threadsafety, - "dbapi_level": self._connect_module.apilevel, - "driver_paramstyle": self._connect_module.paramstyle, - } + # lazy capture of mysql-connector client version using cursor if ( - self._db_api_integration.database_system - == "postgresql" + self._db_api_integration.database_system == "mysql" + and self._db_api_integration.connect_module.__name__ + == "mysql.connector" + and not self._db_api_integration.commenter_data[ + "mysql_client_version" + ] ): - if hasattr(self._connect_module, "__libpq_version__"): - libpq_version = ( - self._connect_module.__libpq_version__ - ) - else: - libpq_version = ( - self._connect_module.pq.__build_version__ - ) - commenter_data.update( - { - "libpq_version": libpq_version, - } - ) - elif self._db_api_integration.database_system == "mysql": - mysqlc_version = "" - if db_driver == "mysql.connector": - mysqlc_version = ( - cursor._cnx._cmysql.get_client_info() - ) - if db_driver == "MySQLdb": - mysqlc_version = ( - self._db_api_integration.connect_module._mysql.get_client_info() - ) - if db_driver == "pymysql": - mysqlc_version = ( - self._db_api_integration.connect_module.get_client_info() - ) - - commenter_data.update( - { - "mysql_client_version": mysqlc_version, - } - ) + self._db_api_integration.commenter_data[ + "mysql_client_version" + ] = cursor._cnx._cmysql.get_client_info() + commenter_data = dict( + self._db_api_integration.commenter_data + ) if self._commenter_options.get( "opentelemetry_values", True ): From 4e6ab3f7fd82b1bde3ac863d989d77f5ca959856 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 16 Oct 2024 11:52:02 -0700 Subject: [PATCH 14/17] try-except if NoneType module --- .../opentelemetry/instrumentation/dbapi/__init__.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index cf8a599f78..c0ba62d770 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -289,12 +289,20 @@ def calculate_commenter_data( if not self.enable_commenter: return commenter_data - db_driver = self.connect_module.__name__ + try: + db_driver = self.connect_module.__name__ + except AttributeError: + db_driver = "unknown" + db_version = "" if db_driver in _DB_DRIVER_ALIASES: db_version = util_version(_DB_DRIVER_ALIASES[db_driver]) else: - db_version = self.connect_module.__version__ + try: + db_version = self.connect_module.__version__ + except AttributeError: + db_driver = "unknown" + commenter_data = { "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", "dbapi_threadsafety": self.connect_module.threadsafety, From 9047363a9c932f5e1edc8f20c11b701e5ca4f038 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 28 Oct 2024 13:44:46 -0700 Subject: [PATCH 15/17] Fix var assn --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index c0ba62d770..98319765bc 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -301,7 +301,7 @@ def calculate_commenter_data( try: db_version = self.connect_module.__version__ except AttributeError: - db_driver = "unknown" + db_version = "unknown" commenter_data = { "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", From c461544d226437e552a1c6ffdabf8bf406fb9f38 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 29 Oct 2024 16:24:36 -0700 Subject: [PATCH 16/17] Fix style --- .../instrumentation/dbapi/__init__.py | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 98319765bc..f3269eac68 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -282,6 +282,19 @@ def __init__( self.connect_module = connect_module self.commenter_data = self.calculate_commenter_data() + def _get_db_version( + self, + db_driver, + ): + if db_driver in _DB_DRIVER_ALIASES: + return util_version(_DB_DRIVER_ALIASES[db_driver]) + db_version = "" + try: + db_version = self.connect_module.__version__ + except AttributeError: + db_version = "unknown" + return db_version + def calculate_commenter_data( self, ): @@ -289,19 +302,8 @@ def calculate_commenter_data( if not self.enable_commenter: return commenter_data - try: - db_driver = self.connect_module.__name__ - except AttributeError: - db_driver = "unknown" - - db_version = "" - if db_driver in _DB_DRIVER_ALIASES: - db_version = util_version(_DB_DRIVER_ALIASES[db_driver]) - else: - try: - db_version = self.connect_module.__version__ - except AttributeError: - db_version = "unknown" + db_driver = getattr(self.connect_module, "__name__", "unknown") + db_version = self._get_db_version(db_driver) commenter_data = { "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", From daf3a739aa545f39af6d34523b06f64c96e3002a Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 29 Oct 2024 17:11:01 -0700 Subject: [PATCH 17/17] Add fallbacks in case driver module not PEP 249-compliant --- .../instrumentation/dbapi/__init__.py | 14 +++++++-- .../tests/test_dbapi_integration.py | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index f3269eac68..fc3911f744 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -307,9 +307,17 @@ def calculate_commenter_data( commenter_data = { "db_driver": f"{db_driver}:{db_version.split(' ')[0]}", - "dbapi_threadsafety": self.connect_module.threadsafety, - "dbapi_level": self.connect_module.apilevel, - "driver_paramstyle": self.connect_module.paramstyle, + # PEP 249-compliant drivers should have the following attributes. + # We can assume apilevel "1.0" if not given. + # We use "unknown" for others to prevent uncaught AttributeError. + # https://peps.python.org/pep-0249/#globals + "dbapi_threadsafety": getattr( + self.connect_module, "threadsafety", "unknown" + ), + "dbapi_level": getattr(self.connect_module, "apilevel", "1.0"), + "driver_paramstyle": getattr( + self.connect_module, "paramstyle", "unknown" + ), } if self.database_system == "postgresql": diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index d44de49ca1..e29a8ad380 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -277,6 +277,35 @@ def test_executemany_comment(self): r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + def test_executemany_comment_non_pep_249_compliant(self): + class MockConnectModule: + def __getattr__(self, name): + if name == "__name__": + return "test" + if name == "__version__": + return mock.MagicMock() + if name == "__libpq_version__": + return 123 + raise AttributeError("attribute missing") + + connect_module = MockConnectModule() + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + connect_module=connect_module, + commenter_options={"db_driver": False}, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*dbapi_level='1.0',dbapi_threadsafety='unknown',driver_paramstyle='unknown',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + def test_compatible_build_version_psycopg_psycopg2_libpq(self): connect_module = mock.MagicMock() connect_module.__name__ = "test"