diff --git a/CHANGELOG.md b/CHANGELOG.md index 90475876b8..b250af62c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#4081](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4081)) - `opentelemetry-instrumentation-system-metrics`: Use proper numeric `cpython.gc.generation` attribute in CPython metrics, out of spec `generation` attribute is deprecated and will be removed in the future ([#4092](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4092)) +- `opentelemetry-instrumentation-dbapi`: Fix sqlcomment calculation of mysql_client_version field if connection reassignment, with "unknown" fallback + ([#3729](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3729)) - `opentelemetry-instrumentation-confluent-kafka`: Fix incorrect number of argument to `_inner_wrap_close` ([#3922](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3922)) 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 f9837d585f..cd154616a6 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -632,8 +632,22 @@ def _capture_mysql_version(self, cursor) -> None: "mysql_client_version" ] ): + try: + # Autoinstrumentation and some programmatic calls + client_version = cursor._cnx._cmysql.get_client_info() + except AttributeError: + # Other programmatic instrumentation with reassigned wrapped connection + try: + client_version = ( + cursor._connection._cmysql.get_client_info() + ) + except AttributeError as exc: + _logger.debug( + "Could not set mysql_client_version: %s", exc + ) + client_version = "unknown" self._db_api_integration.commenter_data["mysql_client_version"] = ( - cursor._cnx._cmysql.get_client_info() + client_version ) def _get_commenter_data(self) -> dict: diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 0db3eea0e8..f9fb9c952b 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -1180,6 +1180,89 @@ def test_non_string_sql_conversion(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + def test_capture_mysql_version_primary_success(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "mysql.connector" + connect_module.__version__ = "2.2.9" + db_integration = dbapi.DatabaseApiIntegration( + "instrumenting_module_test_name", + "mysql", + enable_commenter=True, + connect_module=connect_module, + ) + mock_cursor = mock.MagicMock() + mock_cursor._cnx._cmysql.get_client_info.return_value = "8.0.32" + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor._cnx = mock_cursor._cnx + cursor.execute("SELECT 1;") + mock_cursor._cnx._cmysql.get_client_info.assert_called_once() + self.assertEqual( + db_integration.commenter_data["mysql_client_version"], "8.0.32" + ) + + def test_capture_mysql_version_fallback_success(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "mysql.connector" + connect_module.__version__ = "2.2.9" + db_integration = dbapi.DatabaseApiIntegration( + "instrumenting_module_test_name", + "mysql", + enable_commenter=True, + connect_module=connect_module, + ) + mock_cursor = mock.MagicMock() + mock_cursor._cnx._cmysql.get_client_info.side_effect = AttributeError( + "Primary method failed" + ) + mock_cursor._connection._cmysql.get_client_info.return_value = "8.0.33" + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor._cnx = mock_cursor._cnx + cursor._connection = mock_cursor._connection + cursor.execute("SELECT 1;") + mock_cursor._cnx._cmysql.get_client_info.assert_called_once() + mock_cursor._connection._cmysql.get_client_info.assert_called_once() + self.assertEqual( + db_integration.commenter_data["mysql_client_version"], "8.0.33" + ) + + @mock.patch("opentelemetry.instrumentation.dbapi._logger") + def test_capture_mysql_version_fallback(self, mock_logger): + connect_module = mock.MagicMock() + connect_module.__name__ = "mysql.connector" + connect_module.__version__ = "2.2.9" + db_integration = dbapi.DatabaseApiIntegration( + "instrumenting_module_test_name", + "mysql", + enable_commenter=True, + connect_module=connect_module, + ) + mock_cursor = mock.MagicMock() + mock_cursor._cnx._cmysql.get_client_info.side_effect = AttributeError( + "Primary method failed" + ) + mock_cursor._connection._cmysql.get_client_info.side_effect = ( + AttributeError("Fallback method failed") + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor._cnx = mock_cursor._cnx + cursor._connection = mock_cursor._connection + cursor.execute("SELECT 1;") + mock_cursor._cnx._cmysql.get_client_info.assert_called_once() + mock_cursor._connection._cmysql.get_client_info.assert_called_once() + mock_logger.debug.assert_called_once() + self.assertEqual( + db_integration.commenter_data["mysql_client_version"], "unknown" + ) + # pylint: disable=unused-argument def mock_connect(*args, **kwargs): diff --git a/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_sqlcommenter.py b/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_sqlcommenter.py new file mode 100644 index 0000000000..d1e16993c9 --- /dev/null +++ b/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_sqlcommenter.py @@ -0,0 +1,104 @@ +# Copyright 2025, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +import mysql.connector + +from opentelemetry.instrumentation.mysql import MySQLInstrumentor +from opentelemetry.test.test_base import TestBase + +MYSQL_USER = os.getenv("MYSQL_USER", "testuser") +MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD", "testpassword") +MYSQL_HOST = os.getenv("MYSQL_HOST", "localhost") +MYSQL_PORT = int(os.getenv("MYSQL_PORT", "3306")) +MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME", "opentelemetry-tests") + + +class TestFunctionalMySqlCommenter(TestBase): + def test_commenter_enabled_direct_reference(self): + MySQLInstrumentor().instrument(enable_commenter=True) + cnx = mysql.connector.connect( + user=MYSQL_USER, + password=MYSQL_PASSWORD, + host=MYSQL_HOST, + port=MYSQL_PORT, + database=MYSQL_DB_NAME, + ) + cursor = cnx.cursor() + + cursor.execute("SELECT 1;") + cursor.fetchall() + self.assertRegex( + cursor.statement, + r"SELECT 1 /\*db_driver='mysql\.connector[^']*',dbapi_level='\d\.\d',dbapi_threadsafety=\d,driver_paramstyle='[^']*',mysql_client_version='[^']*',traceparent='[^']*'\*/;", + ) + self.assertRegex( + cursor.statement, r"mysql_client_version='(?!unknown)[^']+" + ) + + cursor.close() + cnx.close() + MySQLInstrumentor().uninstrument() + + def test_commenter_enabled_connection_proxy(self): + cnx = mysql.connector.connect( + user=MYSQL_USER, + password=MYSQL_PASSWORD, + host=MYSQL_HOST, + port=MYSQL_PORT, + database=MYSQL_DB_NAME, + ) + instrumented_cnx = MySQLInstrumentor().instrument_connection( + connection=cnx, + enable_commenter=True, + ) + cursor = instrumented_cnx.cursor() + + cursor.execute("SELECT 1;") + cursor.fetchall() + self.assertRegex( + cursor.statement, + r"SELECT 1 /\*db_driver='mysql\.connector[^']*',dbapi_level='\d\.\d',dbapi_threadsafety=\d,driver_paramstyle='[^']*',mysql_client_version='[^']*',traceparent='[^']*'\*/;", + ) + self.assertRegex( + cursor.statement, r"mysql_client_version='(?!unknown)[^']+" + ) + + cursor.close() + MySQLInstrumentor().uninstrument_connection(instrumented_cnx) + cnx.close() + + def test_commenter_mysql_client_version_fallback_to_unknown(self): + """Test that mysql_client_version falls back to 'unknown' with pure Python implementation""" + MySQLInstrumentor().instrument(enable_commenter=True) + cnx = mysql.connector.connect( + user=MYSQL_USER, + password=MYSQL_PASSWORD, + host=MYSQL_HOST, + port=MYSQL_PORT, + database=MYSQL_DB_NAME, + use_pure=True, # Use pure Python - no _cmysql module + ) + cursor = cnx.cursor() + + cursor.execute("SELECT 1;") + cursor.fetchall() + + # With use_pure=True, _cmysql is not available, should fall back to 'unknown' + self.assertRegex(cursor.statement, r"mysql_client_version='unknown'") + + cursor.close() + cnx.close() + MySQLInstrumentor().uninstrument()