From beff723389288b6cfc890d8780cdc426f4f11720 Mon Sep 17 00:00:00 2001 From: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> Date: Thu, 21 Nov 2024 07:37:08 -0800 Subject: [PATCH] Add mysqlclient instrumentor support for sqlcommenting (#2941) * WIP * Add _DB_DRIVER_ALIASES * Add mysql_client_version to sqlcomment * lint * Fix existing tests * lint test * Add PyMySQL dbapi commenter case * Add test * Add test * Add test * Add tests * Changelog * calculate_commenter_data at init of DatabaseApiIntegration * Add mysqlclient sqlcomment support * Fix typo * try-except if NoneType module * Add unit test * CHangelog * mysqlclient instrument_connection with connect_module * add tests * more tests * more tests 2 * lint * Redesign tests * Rm unnecessary mocks --- CHANGELOG.md | 2 + .../instrumentation/mysqlclient/__init__.py | 86 +++++- .../tests/test_mysqlclient_integration.py | 251 ++++++++++++++++++ 3 files changed, 335 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25002bb657..c3ec7e3798 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2935](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2935)) - `opentelemetry-instrumentation-dbapi` instrument_connection accepts optional connect_module ([#3027](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3027)) +- `opentelemetry-instrumentation-mysqlclient` Add sqlcommenter support + ([#2941](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2941)) ### Fixed diff --git a/instrumentation/opentelemetry-instrumentation-mysqlclient/src/opentelemetry/instrumentation/mysqlclient/__init__.py b/instrumentation/opentelemetry-instrumentation-mysqlclient/src/opentelemetry/instrumentation/mysqlclient/__init__.py index 85083cff2e..5b08b0b50d 100644 --- a/instrumentation/opentelemetry-instrumentation-mysqlclient/src/opentelemetry/instrumentation/mysqlclient/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysqlclient/src/opentelemetry/instrumentation/mysqlclient/__init__.py @@ -36,6 +36,72 @@ cursor.close() cnx.close() +SQLCOMMENTER +***************************************** +You can optionally configure MySQLClient instrumentation to enable sqlcommenter which enriches +the query with contextual information. + +.. code:: python + + import MySQLdb + from opentelemetry.instrumentation.mysqlclient import MySQLClientInstrumentor + + + MySQLClientInstrumentor().instrument(enable_commenter=True, commenter_options={}) + + cnx = MySQLdb.connect(database="MySQL_Database") + cursor = cnx.cursor() + cursor.execute("INSERT INTO test (testField) VALUES (123)" + cnx.commit() + cursor.close() + cnx.close() + +For example, +:: + + Invoking cursor.execute("INSERT INTO test (testField) VALUES (123)") will lead to sql query "INSERT INTO test (testField) VALUES (123)" but when SQLCommenter is enabled + the query will get appended with some configurable tags like "INSERT INTO test (testField) VALUES (123) /*tag=value*/;" + +SQLCommenter Configurations +*************************** +We can configure the tags to be appended to the sqlquery log by adding configuration inside commenter_options(default:{}) keyword + +db_driver = True(Default) or False + +For example, +:: +Enabling this flag will add MySQLdb and its version, e.g. /*MySQLdb%%3A1.2.3*/ + +dbapi_threadsafety = True(Default) or False + +For example, +:: +Enabling this flag will add threadsafety /*dbapi_threadsafety=2*/ + +dbapi_level = True(Default) or False + +For example, +:: +Enabling this flag will add dbapi_level /*dbapi_level='2.0'*/ + +mysql_client_version = True(Default) or False + +For example, +:: +Enabling this flag will add mysql_client_version /*mysql_client_version='123'*/ + +driver_paramstyle = True(Default) or False + +For example, +:: +Enabling this flag will add driver_paramstyle /*driver_paramstyle='pyformat'*/ + +opentelemetry_values = True(Default) or False + +For example, +:: +Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ + API --- """ @@ -59,14 +125,16 @@ class MySQLClientInstrumentor(BaseInstrumentor): - def instrumentation_dependencies(self) -> Collection[str]: + def instrumentation_dependencies(self) -> Collection[str]: # pylint: disable=no-self-use return _instruments - def _instrument(self, **kwargs): + def _instrument(self, **kwargs): # pylint: disable=no-self-use """Integrate with the mysqlclient library. https://github.com/PyMySQL/mysqlclient/ """ tracer_provider = kwargs.get("tracer_provider") + enable_sqlcommenter = kwargs.get("enable_commenter", False) + commenter_options = kwargs.get("commenter_options", {}) dbapi.wrap_connect( __name__, @@ -76,14 +144,21 @@ def _instrument(self, **kwargs): _CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, + enable_commenter=enable_sqlcommenter, + commenter_options=commenter_options, ) - def _uninstrument(self, **kwargs): + def _uninstrument(self, **kwargs): # pylint: disable=no-self-use """ "Disable mysqlclient instrumentation""" dbapi.unwrap_connect(MySQLdb, "connect") @staticmethod - def instrument_connection(connection, tracer_provider=None): + def instrument_connection( + connection, + tracer_provider=None, + enable_commenter=None, + commenter_options=None, + ): """Enable instrumentation in a mysqlclient connection. Args: @@ -102,6 +177,9 @@ def instrument_connection(connection, tracer_provider=None): _CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, + enable_commenter=enable_commenter, + commenter_options=commenter_options, + connect_module=MySQLdb, ) @staticmethod diff --git a/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py b/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py index 35fdecc8e1..5c375ac4aa 100644 --- a/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py @@ -23,6 +23,7 @@ class TestMySQLClientIntegration(TestBase): + # pylint: disable=invalid-name def tearDown(self): super().tearDown() with self.disable_logging(): @@ -96,6 +97,256 @@ def test_instrument_connection(self, mock_connect): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + @mock.patch("opentelemetry.instrumentation.dbapi.instrument_connection") + @mock.patch("MySQLdb.connect") + # pylint: disable=unused-argument + def test_instrument_connection_enable_commenter_dbapi_kwargs( + self, + mock_connect, + mock_instrument_connection, + ): + cnx = MySQLdb.connect(database="test") + cnx = MySQLClientInstrumentor().instrument_connection( + cnx, + enable_commenter=True, + commenter_options={"foo": True}, + ) + cursor = cnx.cursor() + cursor.execute("Select 1;") + kwargs = mock_instrument_connection.call_args[1] + self.assertEqual(kwargs["enable_commenter"], True) + self.assertEqual(kwargs["commenter_options"], {"foo": True}) + + def test_instrument_connection_with_dbapi_sqlcomment_enabled(self): + mock_connect_module = mock.MagicMock( + __name__="MySQLdb", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module._mysql.get_client_info.return_value = "foobaz" + mock_cursor = mock_connect_module.connect().cursor() + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysqlclient.MySQLdb", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + cnx_proxy = MySQLClientInstrumentor().instrument_connection( + mock_connection, + enable_commenter=True, + ) + cnx_proxy.cursor().execute("Select 1;") + + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + span_id = format(span.get_span_context().span_id, "016x") + trace_id = format(span.get_span_context().trace_id, "032x") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + f"Select 1 /*db_driver='MySQLdb%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", + ) + + def test_instrument_connection_with_dbapi_sqlcomment_enabled_with_options( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="MySQLdb", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module._mysql.get_client_info.return_value = "foobaz" + mock_cursor = mock_connect_module.connect().cursor() + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysqlclient.MySQLdb", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + cnx_proxy = MySQLClientInstrumentor().instrument_connection( + mock_connection, + enable_commenter=True, + commenter_options={ + "dbapi_level": False, + "dbapi_threadsafety": True, + "driver_paramstyle": False, + }, + ) + cnx_proxy.cursor().execute("Select 1;") + + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + span_id = format(span.get_span_context().span_id, "016x") + trace_id = format(span.get_span_context().trace_id, "032x") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + f"Select 1 /*db_driver='MySQLdb%%3Afoobar',dbapi_threadsafety='123',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", + ) + + def test_instrument_connection_with_dbapi_sqlcomment_not_enabled_default( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="MySQLdb", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module._mysql.get_client_info.return_value = "foobaz" + mock_cursor = mock_connect_module.connect().cursor() + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysqlclient.MySQLdb", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + cnx_proxy = MySQLClientInstrumentor().instrument_connection( + mock_connection, + ) + cnx_proxy.cursor().execute("Select 1;") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + "Select 1;", + ) + + @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") + @mock.patch("MySQLdb.connect") + # pylint: disable=unused-argument + def test_instrument_enable_commenter_dbapi_kwargs( + self, + mock_connect, + mock_wrap_connect, + ): + MySQLClientInstrumentor()._instrument( + enable_commenter=True, + commenter_options={"foo": True}, + ) + kwargs = mock_wrap_connect.call_args[1] + self.assertEqual(kwargs["enable_commenter"], True) + self.assertEqual(kwargs["commenter_options"], {"foo": True}) + + def test_instrument_with_dbapi_sqlcomment_enabled( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="MySQLdb", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module._mysql.get_client_info.return_value = "foobaz" + mock_cursor = mock_connect_module.connect().cursor() + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysqlclient.MySQLdb", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + MySQLClientInstrumentor()._instrument( + enable_commenter=True, + ) + cnx = mock_connect_module.connect(database="test") + cursor = cnx.cursor() + cursor.execute("Select 1;") + + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + span_id = format(span.get_span_context().span_id, "016x") + trace_id = format(span.get_span_context().trace_id, "032x") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + f"Select 1 /*db_driver='MySQLdb%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", + ) + + def test_instrument_with_dbapi_sqlcomment_enabled_with_options( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="MySQLdb", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module._mysql.get_client_info.return_value = "foobaz" + mock_cursor = mock_connect_module.connect().cursor() + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysqlclient.MySQLdb", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + MySQLClientInstrumentor()._instrument( + enable_commenter=True, + commenter_options={ + "dbapi_level": False, + "dbapi_threadsafety": True, + "driver_paramstyle": False, + }, + ) + cnx = mock_connect_module.connect(database="test") + cursor = cnx.cursor() + cursor.execute("Select 1;") + + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + span_id = format(span.get_span_context().span_id, "016x") + trace_id = format(span.get_span_context().trace_id, "032x") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + f"Select 1 /*db_driver='MySQLdb%%3Afoobar',dbapi_threadsafety='123',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", + ) + + def test_instrument_with_dbapi_sqlcomment_not_enabled_default( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="MySQLdb", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module._mysql.get_client_info.return_value = "foobaz" + mock_cursor = mock_connect_module.connect().cursor() + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysqlclient.MySQLdb", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + MySQLClientInstrumentor()._instrument() + cnx = mock_connect_module.connect(database="test") + cursor = cnx.cursor() + cursor.execute("Select 1;") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + "Select 1;", + ) + @mock.patch("MySQLdb.connect") # pylint: disable=unused-argument def test_uninstrument_connection(self, mock_connect):