From 5e1e57abc0466c8de307db61de576f7fc0896191 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 18 Nov 2024 16:44:21 -0800 Subject: [PATCH 01/20] DB-API integration: abstract classes BaseTracedConnection|CursorProxy --- .../instrumentation/dbapi/__init__.py | 106 ++++++++++-------- 1 file changed, 61 insertions(+), 45 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 fc3911f744..79e69a1439 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -41,6 +41,7 @@ import logging import re import typing +from abc import ABC, abstractmethod import wrapt @@ -390,35 +391,41 @@ def get_connection_attributes(self, connection): self.span_attributes[SpanAttributes.NET_PEER_PORT] = port +class BaseTracedConnectionProxy(ABC, wrapt.ObjectProxy): + # pylint: disable=unused-argument + def __init__(self, connection, *args, **kwargs): + wrapt.ObjectProxy.__init__(self, connection) + + def __getattribute__(self, name): + if object.__getattribute__(self, name): + return object.__getattribute__(self, name) + + return object.__getattribute__( + object.__getattribute__(self, "_connection"), name + ) + + @abstractmethod + def cursor(self, *args, **kwargs): + """Returns instrumented database query cursor""" + + def __enter__(self): + self.__wrapped__.__enter__() + return self + + def __exit__(self, *args, **kwargs): + self.__wrapped__.__exit__(*args, **kwargs) + + def get_traced_connection_proxy( connection, db_api_integration, *args, **kwargs ): # pylint: disable=abstract-method - class TracedConnectionProxy(wrapt.ObjectProxy): - # pylint: disable=unused-argument - def __init__(self, connection, *args, **kwargs): - wrapt.ObjectProxy.__init__(self, connection) - - def __getattribute__(self, name): - if object.__getattribute__(self, name): - return object.__getattribute__(self, name) - - return object.__getattribute__( - object.__getattribute__(self, "_connection"), name - ) - + class TracedConnectionProxy(BaseTracedConnectionProxy): def cursor(self, *args, **kwargs): return get_traced_cursor_proxy( self.__wrapped__.cursor(*args, **kwargs), db_api_integration ) - def __enter__(self): - self.__wrapped__.__enter__() - return self - - def __exit__(self, *args, **kwargs): - self.__wrapped__.__exit__(*args, **kwargs) - return TracedConnectionProxy(connection, *args, **kwargs) @@ -538,35 +545,44 @@ def traced_execution( return query_method(*args, **kwargs) -def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs): - _cursor_tracer = CursorTracer(db_api_integration) +# pylint: disable=abstract-method +class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy): + # pylint: disable=unused-argument + @abstractmethod + def __init__(self, cursor, *args, **kwargs): + """Wrap db client cursor for tracing""" + wrapt.ObjectProxy.__init__(self, cursor) + self._cursor_tracer = None + + def callproc(self, *args, **kwargs): + return self._cursor_tracer.traced_execution( + self.__wrapped__, self.__wrapped__.callproc, *args, **kwargs + ) - # pylint: disable=abstract-method - class TracedCursorProxy(wrapt.ObjectProxy): - # pylint: disable=unused-argument - def __init__(self, cursor, *args, **kwargs): - wrapt.ObjectProxy.__init__(self, cursor) - - def execute(self, *args, **kwargs): - return _cursor_tracer.traced_execution( - self.__wrapped__, self.__wrapped__.execute, *args, **kwargs - ) + def execute(self, *args, **kwargs): + return self._cursor_tracer.traced_execution( + self.__wrapped__, self.__wrapped__.execute, *args, **kwargs + ) - def executemany(self, *args, **kwargs): - return _cursor_tracer.traced_execution( - self.__wrapped__, self.__wrapped__.executemany, *args, **kwargs - ) + def executemany(self, *args, **kwargs): + return self._cursor_tracer.traced_execution( + self.__wrapped__, self.__wrapped__.executemany, *args, **kwargs + ) - def callproc(self, *args, **kwargs): - return _cursor_tracer.traced_execution( - self.__wrapped__, self.__wrapped__.callproc, *args, **kwargs - ) + def __enter__(self): + self.__wrapped__.__enter__() + return self - def __enter__(self): - self.__wrapped__.__enter__() - return self + def __exit__(self, *args, **kwargs): + self.__wrapped__.__exit__(*args, **kwargs) - def __exit__(self, *args, **kwargs): - self.__wrapped__.__exit__(*args, **kwargs) + +def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs): + class TracedCursorProxy(BaseTracedCursorProxy): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._cursor_tracer = CursorTracer( + db_api_integration, + ) return TracedCursorProxy(cursor, *args, **kwargs) From d2424547fc87b47b8711d585249526049d72871c Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 18 Nov 2024 16:44:40 -0800 Subject: [PATCH 02/20] Add mysql-connector sqlcomment support --- .../instrumentation/mysql/__init__.py | 221 +++++++++++++++++- 1 file changed, 215 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index d88dffde2b..b3d6d22220 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -34,19 +34,94 @@ cursor.close() cnx.close() +SQLCOMMENTER +***************************************** +You can optionally configure mysql-connector instrumentation to enable sqlcommenter which enriches +the query with contextual information. + +Usage +----- + +.. code:: python + + import mysql.connector + from opentelemetry.instrumentation.mysql import MySQLInstrumentor + + MySQLInstrumentor().instrument(enable_commenter=True, commenter_options={}) + + cnx = mysql.connector.connect(database="MySQL_Database") + cursor = cnx.cursor() + cursor.execute("INSERT INTO test (testField) VALUES (123)") + 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 mysql.connector and its version, e.g. /*mysql.connector%%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 --- """ -from typing import Collection +import logging +import typing +from importlib import import_module import mysql.connector +import wrapt +from opentelemetry import trace as trace_api from opentelemetry.instrumentation import dbapi from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.mysql.package import _instruments from opentelemetry.instrumentation.mysql.version import __version__ +_logger = logging.getLogger(__name__) + class MySQLInstrumentor(BaseInstrumentor): _CONNECTION_ATTRIBUTES = { @@ -58,7 +133,7 @@ class MySQLInstrumentor(BaseInstrumentor): _DATABASE_SYSTEM = "mysql" - def instrumentation_dependencies(self) -> Collection[str]: + def instrumentation_dependencies(self) -> typing.Collection[str]: return _instruments def _instrument(self, **kwargs): @@ -66,6 +141,8 @@ def _instrument(self, **kwargs): https://dev.mysql.com/doc/connector-python/en/ """ tracer_provider = kwargs.get("tracer_provider") + enable_sqlcommenter = kwargs.get("enable_commenter", False) + commenter_options = kwargs.get("commenter_options", {}) dbapi.wrap_connect( __name__, @@ -75,6 +152,9 @@ def _instrument(self, **kwargs): self._CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, + db_api_integration_factory=DatabaseApiIntegration, + enable_commenter=enable_sqlcommenter, + commenter_options=commenter_options, ) def _uninstrument(self, **kwargs): @@ -82,27 +162,46 @@ def _uninstrument(self, **kwargs): dbapi.unwrap_connect(mysql.connector, "connect") # pylint:disable=no-self-use - def instrument_connection(self, connection, tracer_provider=None): + def instrument_connection( + self, + connection, + tracer_provider: typing.Optional[trace_api.TracerProvider] = None, + enable_commenter: bool = False, + commenter_options: dict = None, + ): """Enable instrumentation in a MySQL connection. Args: connection: The connection to instrument. tracer_provider: The optional tracer provider to use. If omitted the current globally configured one is used. + enable_commenter: Flag to enable/disable sqlcommenter. + commenter_options: Configurations for tags to be appended at the sql query. Returns: An instrumented connection. """ - return dbapi.instrument_connection( + if isinstance(connection, wrapt.ObjectProxy): + _logger.warning("Connection already instrumented") + return connection + + db_integration = DatabaseApiIntegration( __name__, - connection, self._DATABASE_SYSTEM, self._CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, + enable_commenter=enable_commenter, + commenter_options=commenter_options, + connect_module=import_module("mysql.connector"), ) + db_integration.get_connection_attributes(connection) + return get_traced_connection_proxy(connection, db_integration) - def uninstrument_connection(self, connection): + def uninstrument_connection( + self, + connection, + ): """Disable instrumentation in a MySQL connection. Args: @@ -112,3 +211,113 @@ def uninstrument_connection(self, connection): An uninstrumented connection. """ return dbapi.uninstrument_connection(connection) + + +class DatabaseApiIntegration(dbapi.DatabaseApiIntegration): + def wrapped_connection( + self, + connect_method: typing.Callable[..., typing.Any], + args: typing.Tuple[typing.Any, typing.Any], + kwargs: typing.Dict[typing.Any, typing.Any], + ): + """Add object proxy to connection object that checks cursor type.""" + connection = connect_method(*args, **kwargs) + self.get_connection_attributes(connection) + return get_traced_connection_proxy(connection, self) + + +def get_traced_connection_proxy( + connection, db_api_integration, *args, **kwargs +): + # pylint: disable=abstract-method + class TracedConnectionProxy(dbapi.BaseTracedConnectionProxy): + def cursor(self, *args, **kwargs): + wrapped_cursor = self.__wrapped__.cursor(*args, **kwargs) + + # It's common to have multiple db client cursors per app, + # so enable_commenter is calculated for the cursor level for + # traced query execution. + enable_commenter_cursor = db_api_integration.enable_commenter + + # If a mysql-connector cursor was created with prepared=True, + # then MySQL statements will be prepared and executed natively. + # 1:1 sqlcomment and span correlation in instrumentation would + # break, so sqlcomment is not supported for this use case. + # This is here because wrapped cursor is created when application + # side creates cursor. After that, the instrumentor knows what + # kind of cursor was initialized. + if enable_commenter_cursor: + is_prepared = False + if ( + db_api_integration.database_system == "mysql" + and db_api_integration.connect_module.__name__ + == "mysql.connector" + ): + is_prepared = self.is_mysql_connector_cursor_prepared( + wrapped_cursor + ) + if is_prepared: + _logger.warning( + "sqlcomment is not supported for query statements executed by cursors with native prepared statement support. Disabling sqlcommenting for instrumentation of %s.", + db_api_integration.connect_module.__name__, + ) + enable_commenter_cursor = False + return get_traced_cursor_proxy( + wrapped_cursor, + db_api_integration, + enable_commenter_cursor, + ) + + def is_mysql_connector_cursor_prepared(self, cursor): # pylint: disable=no-self-use + try: + from mysql.connector.cursor_cext import ( # pylint: disable=import-outside-toplevel + CMySQLCursorPrepared, + CMySQLCursorPreparedDict, + CMySQLCursorPreparedNamedTuple, + CMySQLCursorPreparedRaw, + ) + + if type(cursor) in [ + CMySQLCursorPrepared, + CMySQLCursorPreparedDict, + CMySQLCursorPreparedNamedTuple, + CMySQLCursorPreparedRaw, + ]: + return True + + except ImportError as exc: + _logger.warning( + "Could not verify mysql.connector cursor, skipping prepared cursor check: %s", + exc, + ) + + return False + + return TracedConnectionProxy(connection, *args, **kwargs) + + +class CursorTracer(dbapi.CursorTracer): + def __init__( + self, + db_api_integration: DatabaseApiIntegration, + enable_commenter: bool = False, + ) -> None: + super().__init__(db_api_integration) + # It's common to have multiple db client cursors per app, + # so enable_commenter is set at the cursor level and used + # during CursorTracer.traced_execution for mysql-connector + self._commenter_enabled = enable_commenter + + +def get_traced_cursor_proxy( + cursor, db_api_integration, enable_commenter_cursor, *args, **kwargs +): + class TracedCursorProxy(dbapi.BaseTracedCursorProxy): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._cursor_tracer = CursorTracer( + db_api_integration, + enable_commenter_cursor, + ) + + return TracedCursorProxy(cursor, *args, **kwargs) From 089c90ab21f58e73662d89fbb9f9ae58fba25237 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 18 Nov 2024 16:58:55 -0800 Subject: [PATCH 03/20] Add tests --- .../tests/test_mysql_integration.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py index 3614febffd..21efdcecd2 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py @@ -102,6 +102,42 @@ def test_instrument_connection_no_op_tracer_provider(self, mock_connect): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 0) + @mock.patch("opentelemetry.instrumentation.mysql.DatabaseApiIntegration") + @mock.patch("mysql.connector.connect") + # pylint: disable=unused-argument + def test_instrument_connection_enable_commenter( + self, + mock_connect, + mock_mysql_dbapi, + ): + cnx, query = connect_and_execute_query() + cnx = MySQLInstrumentor().instrument_connection( + cnx, + enable_commenter=True, + commenter_options={"foo": True}, + ) + cursor = cnx.cursor() + cursor.execute(query) + kwargs = mock_mysql_dbapi.call_args[1] + self.assertEqual(kwargs["enable_commenter"], True) + self.assertEqual(kwargs["commenter_options"], {"foo": True}) + + @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") + @mock.patch("mysql.connector.connect") + # pylint: disable=unused-argument + def test__instrument_enable_commenter( + self, + mock_connect, + mock_wrap_connect, + ): + MySQLInstrumentor()._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}) + @mock.patch("mysql.connector.connect") # pylint: disable=unused-argument def test_uninstrument_connection(self, mock_connect): From bb1f010334c7fed243057e5661b4170e055e4118 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 18 Nov 2024 17:32:46 -0800 Subject: [PATCH 04/20] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 772384bdde..c136481511 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2976](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2976)) - Add `opentelemetry-instrumentation-openai-v2` to `opentelemetry-bootstrap` ([#2996](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2996)) +- `opentelemetry-instrumentation-mysql` Add sqlcommenter support + ([#2943](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2943)) ### Fixed From 7fabfcf8caac9b5ff1b9b1ffc9301bdee808f134 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 19 Nov 2024 11:17:17 -0800 Subject: [PATCH 05/20] Mv dbapi base class to proxy.py --- .../instrumentation/dbapi/__init__.py | 62 +-------------- .../instrumentation/dbapi/proxy.py | 75 +++++++++++++++++++ 2 files changed, 79 insertions(+), 58 deletions(-) create mode 100644 instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py 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 da99167444..cbe128f2fc 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -41,11 +41,14 @@ import logging import re import typing -from abc import ABC, abstractmethod import wrapt from opentelemetry import trace as trace_api +from opentelemetry.instrumentation.dbapi.proxy import ( + BaseTracedConnectionProxy, + BaseTracedCursorProxy, +) from opentelemetry.instrumentation.dbapi.version import __version__ from opentelemetry.instrumentation.sqlcommenter_utils import _add_sql_comment from opentelemetry.instrumentation.utils import ( @@ -391,31 +394,6 @@ def get_connection_attributes(self, connection): self.span_attributes[SpanAttributes.NET_PEER_PORT] = port -class BaseTracedConnectionProxy(ABC, wrapt.ObjectProxy): - # pylint: disable=unused-argument - def __init__(self, connection, *args, **kwargs): - wrapt.ObjectProxy.__init__(self, connection) - - def __getattribute__(self, name): - if object.__getattribute__(self, name): - return object.__getattribute__(self, name) - - return object.__getattribute__( - object.__getattribute__(self, "_connection"), name - ) - - @abstractmethod - def cursor(self, *args, **kwargs): - """Returns instrumented database query cursor""" - - def __enter__(self): - self.__wrapped__.__enter__() - return self - - def __exit__(self, *args, **kwargs): - self.__wrapped__.__exit__(*args, **kwargs) - - def get_traced_connection_proxy( connection, db_api_integration, *args, **kwargs ): @@ -550,38 +528,6 @@ def traced_execution( return query_method(*args, **kwargs) -# pylint: disable=abstract-method -class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy): - # pylint: disable=unused-argument - @abstractmethod - def __init__(self, cursor, *args, **kwargs): - """Wrap db client cursor for tracing""" - wrapt.ObjectProxy.__init__(self, cursor) - self._cursor_tracer = None - - def callproc(self, *args, **kwargs): - return self._cursor_tracer.traced_execution( - self.__wrapped__, self.__wrapped__.callproc, *args, **kwargs - ) - - def execute(self, *args, **kwargs): - return self._cursor_tracer.traced_execution( - self.__wrapped__, self.__wrapped__.execute, *args, **kwargs - ) - - def executemany(self, *args, **kwargs): - return self._cursor_tracer.traced_execution( - self.__wrapped__, self.__wrapped__.executemany, *args, **kwargs - ) - - def __enter__(self): - self.__wrapped__.__enter__() - return self - - def __exit__(self, *args, **kwargs): - self.__wrapped__.__exit__(*args, **kwargs) - - def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs): class TracedCursorProxy(BaseTracedCursorProxy): def __init__(self, *args, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py new file mode 100644 index 0000000000..a839750ab2 --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py @@ -0,0 +1,75 @@ +# Copyright The 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. + + +from abc import ABC, abstractmethod + +import wrapt + + +class BaseTracedConnectionProxy(ABC, wrapt.ObjectProxy): + # pylint: disable=unused-argument + def __init__(self, connection, *args, **kwargs): + wrapt.ObjectProxy.__init__(self, connection) + + def __getattribute__(self, name): + if object.__getattribute__(self, name): + return object.__getattribute__(self, name) + + return object.__getattribute__( + object.__getattribute__(self, "_connection"), name + ) + + @abstractmethod + def cursor(self, *args, **kwargs): + """Returns instrumented database query cursor""" + + def __enter__(self): + self.__wrapped__.__enter__() + return self + + def __exit__(self, *args, **kwargs): + self.__wrapped__.__exit__(*args, **kwargs) + + +# pylint: disable=abstract-method +class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy): + # pylint: disable=unused-argument + @abstractmethod + def __init__(self, cursor, *args, **kwargs): + """Wrap db client cursor for tracing""" + wrapt.ObjectProxy.__init__(self, cursor) + self._cursor_tracer = None + + def callproc(self, *args, **kwargs): + return self._cursor_tracer.traced_execution( + self.__wrapped__, self.__wrapped__.callproc, *args, **kwargs + ) + + def execute(self, *args, **kwargs): + return self._cursor_tracer.traced_execution( + self.__wrapped__, self.__wrapped__.execute, *args, **kwargs + ) + + def executemany(self, *args, **kwargs): + return self._cursor_tracer.traced_execution( + self.__wrapped__, self.__wrapped__.executemany, *args, **kwargs + ) + + def __enter__(self): + self.__wrapped__.__enter__() + return self + + def __exit__(self, *args, **kwargs): + self.__wrapped__.__exit__(*args, **kwargs) From 83f8e67247774d97f453546e4db7e84596340fc6 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 19 Nov 2024 11:42:24 -0800 Subject: [PATCH 06/20] Add docstring --- .../instrumentation/dbapi/proxy.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py index a839750ab2..c1c3d5a133 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py @@ -19,6 +19,16 @@ class BaseTracedConnectionProxy(ABC, wrapt.ObjectProxy): + """ABC for traced database client connection proxy. + + Child classes are used to wrap Connection objects and + generate telemetry based on provided integration settings. + + Child classes must implement cursor(), which should return + a traced database client cursor proxy depending on database + driver needs. + """ + # pylint: disable=unused-argument def __init__(self, connection, *args, **kwargs): wrapt.ObjectProxy.__init__(self, connection) @@ -45,6 +55,15 @@ def __exit__(self, *args, **kwargs): # pylint: disable=abstract-method class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy): + """ABC for traced database client cursor proxy. + + Child classes are used to wrap Cursor objects and + generate telemetry based on provided integration settings. + + Child classes must implement __init__(), which should set + a CursorTracer depending on database driver needs. + """ + # pylint: disable=unused-argument @abstractmethod def __init__(self, cursor, *args, **kwargs): From 569e81159afd69ab5d30513e2aee8aff651ae679 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 19 Nov 2024 14:10:01 -0800 Subject: [PATCH 07/20] Use class attr in Base to fix sqlite3 integration --- .../src/opentelemetry/instrumentation/dbapi/proxy.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py index c1c3d5a133..5e42dfc2f3 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py @@ -61,15 +61,17 @@ class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy): generate telemetry based on provided integration settings. Child classes must implement __init__(), which should set - a CursorTracer depending on database driver needs. + class variable _cursor_tracer as a CursorTracer depending + on database driver needs. """ + _cursor_tracer = None + # pylint: disable=unused-argument @abstractmethod def __init__(self, cursor, *args, **kwargs): """Wrap db client cursor for tracing""" wrapt.ObjectProxy.__init__(self, cursor) - self._cursor_tracer = None def callproc(self, *args, **kwargs): return self._cursor_tracer.traced_execution( From 5acb2d01d77a8470b0d2dc2b42ee929019e2ae21 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 19 Nov 2024 14:31:25 -0800 Subject: [PATCH 08/20] Add BaseMeta helper for pypy --- .../src/opentelemetry/instrumentation/dbapi/proxy.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py index 5e42dfc2f3..5b6a4872af 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/proxy.py @@ -13,12 +13,16 @@ # limitations under the License. -from abc import ABC, abstractmethod +from abc import ABC, ABCMeta, abstractmethod import wrapt -class BaseTracedConnectionProxy(ABC, wrapt.ObjectProxy): +class BaseMeta(ABCMeta, type(wrapt.ObjectProxy)): + """Metaclass compatibility helper for PyPy for derived classes""" + + +class BaseTracedConnectionProxy(ABC, wrapt.ObjectProxy, metaclass=BaseMeta): """ABC for traced database client connection proxy. Child classes are used to wrap Connection objects and @@ -54,7 +58,7 @@ def __exit__(self, *args, **kwargs): # pylint: disable=abstract-method -class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy): +class BaseTracedCursorProxy(ABC, wrapt.ObjectProxy, metaclass=BaseMeta): """ABC for traced database client cursor proxy. Child classes are used to wrap Cursor objects and From 880c0f70cd03897a857082509cc2e53dda80a763 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 21 Nov 2024 15:21:14 -0800 Subject: [PATCH 09/20] mysql instrument_connection passes connect_module --- .../src/opentelemetry/instrumentation/mysql/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index b3d6d22220..98b688d3be 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -109,7 +109,6 @@ import logging import typing -from importlib import import_module import mysql.connector import wrapt @@ -193,7 +192,7 @@ def instrument_connection( tracer_provider=tracer_provider, enable_commenter=enable_commenter, commenter_options=commenter_options, - connect_module=import_module("mysql.connector"), + connect_module=mysql.connector, ) db_integration.get_connection_attributes(connection) return get_traced_connection_proxy(connection, db_integration) From 94bf04ae1b9a8b4f715e8fb061ba298c35c96ee3 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 26 Nov 2024 09:12:52 -0800 Subject: [PATCH 10/20] Update comment --- .../src/opentelemetry/instrumentation/mysql/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index 98b688d3be..b1167b9bed 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -240,8 +240,11 @@ def cursor(self, *args, **kwargs): # If a mysql-connector cursor was created with prepared=True, # then MySQL statements will be prepared and executed natively. - # 1:1 sqlcomment and span correlation in instrumentation would - # break, so sqlcomment is not supported for this use case. + # Adding sqlcommenting would introduce a severe performance + # penalty by repeating `Prepare` of statements by mysql-connector + # that are made unique by traceparent in sqlcomment. Therefore, + # sqlcomment is not supported for this use case. It is not an + # issue if prepared=False. # This is here because wrapped cursor is created when application # side creates cursor. After that, the instrumentor knows what # kind of cursor was initialized. From 19bfd7cff91980d257ff691fb9f54819a78905ea Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 26 Nov 2024 10:48:51 -0800 Subject: [PATCH 11/20] dbapi.instrument_connection optional kwargs db_api_integration_factory, get_cnx_proxy --- .../instrumentation/dbapi/__init__.py | 16 ++++++++++++++-- 1 file changed, 14 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 b80112c204..c2b63e59fe 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -195,6 +195,8 @@ def instrument_connection( enable_commenter: bool = False, commenter_options: dict = None, connect_module: typing.Callable[..., typing.Any] = None, + db_api_integration_factory: typing.ClassVar = None, + get_cnx_proxy: typing.Callable[..., typing.Any] = None, ): """Enable instrumentation in a database connection. @@ -210,6 +212,10 @@ def instrument_connection( enable_commenter: Flag to enable/disable sqlcommenter. commenter_options: Configurations for tags to be appended at the sql query. connect_module: Module name where connect method is available. + db_api_integration_factory: The `DatabaseApiIntegration` to use. If none is passed the + default one is used. + get_cnx_proxy: Method to get traced connextion proxy. If none is passed the + default one is used. Returns: An instrumented connection. @@ -218,7 +224,11 @@ def instrument_connection( _logger.warning("Connection already instrumented") return connection - db_integration = DatabaseApiIntegration( + db_api_integration_factory = ( + db_api_integration_factory or DatabaseApiIntegration + ) + + db_integration = db_api_integration_factory( name, database_system, connection_attributes=connection_attributes, @@ -230,7 +240,9 @@ def instrument_connection( connect_module=connect_module, ) db_integration.get_connection_attributes(connection) - return get_traced_connection_proxy(connection, db_integration) + + get_cnx_proxy = get_cnx_proxy or get_traced_connection_proxy + return get_cnx_proxy(connection, db_integration) def uninstrument_connection(connection): From ee907e22f172a46da04961137f08c00f5175d2cd Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 26 Nov 2024 10:50:12 -0800 Subject: [PATCH 12/20] mysql uses updated dbapi.instrument_connection --- .../opentelemetry/instrumentation/mysql/__init__.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index b1167b9bed..64a1b69147 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -111,7 +111,6 @@ import typing import mysql.connector -import wrapt from opentelemetry import trace as trace_api from opentelemetry.instrumentation import dbapi @@ -180,12 +179,9 @@ def instrument_connection( Returns: An instrumented connection. """ - if isinstance(connection, wrapt.ObjectProxy): - _logger.warning("Connection already instrumented") - return connection - - db_integration = DatabaseApiIntegration( + return dbapi.instrument_connection( __name__, + connection, self._DATABASE_SYSTEM, self._CONNECTION_ATTRIBUTES, version=__version__, @@ -193,9 +189,9 @@ def instrument_connection( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=mysql.connector, + db_api_integration_factory=DatabaseApiIntegration, + get_cnx_proxy=get_traced_connection_proxy, ) - db_integration.get_connection_attributes(connection) - return get_traced_connection_proxy(connection, db_integration) def uninstrument_connection( self, From 4e1b77808d4518c1959eac865de7ad999da9db32 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 26 Nov 2024 11:55:04 -0800 Subject: [PATCH 13/20] Simplify get_traced_connection_proxy and update comment --- .../instrumentation/mysql/__init__.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index 64a1b69147..1519c89c3a 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -231,7 +231,7 @@ def cursor(self, *args, **kwargs): # It's common to have multiple db client cursors per app, # so enable_commenter is calculated for the cursor level for - # traced query execution. + # traced query execution, based on original configuration. enable_commenter_cursor = db_api_integration.enable_commenter # If a mysql-connector cursor was created with prepared=True, @@ -245,15 +245,9 @@ def cursor(self, *args, **kwargs): # side creates cursor. After that, the instrumentor knows what # kind of cursor was initialized. if enable_commenter_cursor: - is_prepared = False - if ( - db_api_integration.database_system == "mysql" - and db_api_integration.connect_module.__name__ - == "mysql.connector" - ): - is_prepared = self.is_mysql_connector_cursor_prepared( - wrapped_cursor - ) + is_prepared = self.is_mysql_connector_cursor_prepared( + wrapped_cursor + ) if is_prepared: _logger.warning( "sqlcomment is not supported for query statements executed by cursors with native prepared statement support. Disabling sqlcommenting for instrumentation of %s.", From a0d37362740b3ee06d409376feb313b70ddb8cea Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 26 Nov 2024 12:03:06 -0800 Subject: [PATCH 14/20] Rm is_prepared helper, add quick kwargs check --- .../instrumentation/mysql/__init__.py | 29 +------------------ 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index 1519c89c3a..7c154f6a72 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -245,9 +245,7 @@ def cursor(self, *args, **kwargs): # side creates cursor. After that, the instrumentor knows what # kind of cursor was initialized. if enable_commenter_cursor: - is_prepared = self.is_mysql_connector_cursor_prepared( - wrapped_cursor - ) + is_prepared = kwargs.get("prepared", False) if is_prepared: _logger.warning( "sqlcomment is not supported for query statements executed by cursors with native prepared statement support. Disabling sqlcommenting for instrumentation of %s.", @@ -260,31 +258,6 @@ def cursor(self, *args, **kwargs): enable_commenter_cursor, ) - def is_mysql_connector_cursor_prepared(self, cursor): # pylint: disable=no-self-use - try: - from mysql.connector.cursor_cext import ( # pylint: disable=import-outside-toplevel - CMySQLCursorPrepared, - CMySQLCursorPreparedDict, - CMySQLCursorPreparedNamedTuple, - CMySQLCursorPreparedRaw, - ) - - if type(cursor) in [ - CMySQLCursorPrepared, - CMySQLCursorPreparedDict, - CMySQLCursorPreparedNamedTuple, - CMySQLCursorPreparedRaw, - ]: - return True - - except ImportError as exc: - _logger.warning( - "Could not verify mysql.connector cursor, skipping prepared cursor check: %s", - exc, - ) - - return False - return TracedConnectionProxy(connection, *args, **kwargs) From 957636accea7aca37199aef43e6e5e2e547dfc4f Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 27 Nov 2024 17:01:06 -0800 Subject: [PATCH 15/20] Add dbapi proxy tests --- .../tests/test_dbapi_baseclass.py | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_baseclass.py diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_baseclass.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_baseclass.py new file mode 100644 index 0000000000..90965edef5 --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_baseclass.py @@ -0,0 +1,144 @@ +# Copyright The 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. + +from unittest import mock + +from opentelemetry.instrumentation.dbapi import ( + BaseTracedConnectionProxy, + BaseTracedCursorProxy, +) +from opentelemetry.test.test_base import TestBase + + +class TestDBApiBaseClasses(TestBase): + class TestConnectionProxy(BaseTracedConnectionProxy): + def cursor(self, *args, **kwargs): + return "foo-cursor" + + def test_base_traced_connection_proxy_init(self): + mock_connection = mock.Mock() + proxy = self.TestCursorProxy(mock_connection) + self.assertIs(proxy.__wrapped__, mock_connection) + + def test_base_traced_connection_proxy_getattr_direct_attr(self): + mock_connection = mock.Mock() + proxy = self.TestConnectionProxy(mock_connection) + proxy.foo_attribute = "bar" # pylint: disable=attribute-defined-outside-init + self.assertEqual(proxy.foo_attribute, "bar") + + def test_base_traced_connection_proxy_getattr_cnx_attr(self): + mock_connection = mock.Mock() + mock_connection.foo_attribute = "bar" + proxy = self.TestConnectionProxy(mock_connection) + self.assertEqual(proxy.foo_attribute, "bar") + + def test_base_traced_connection_proxy_cursor(self): + assert self.TestConnectionProxy("foo-cnx").cursor() == "foo-cursor" + + def test_base_traced_connection_proxy_enter(self): + mock_connection = mock.Mock() + mock_connection.__enter__ = mock.Mock() + mock_connection.__exit__ = mock.Mock() + proxy = self.TestConnectionProxy(mock_connection) + with proxy: + mock_connection.__enter__.assert_called_once() + + def test_base_traced_connection_proxy_exit(self): + mock_connection = mock.Mock() + mock_connection.__enter__ = mock.Mock() + mock_connection.__exit__ = mock.Mock() + proxy = self.TestConnectionProxy(mock_connection) + with proxy: + pass + mock_connection.__exit__.assert_called_once() + + class TestCursorProxy(BaseTracedCursorProxy): + def __init__(self, cursor, *args, **kwargs): + super().__init__(cursor, *args, **kwargs) + self._cursor_tracer = kwargs.get("test_cursor_tracer", "foo") + + def test_base_traced_cursor_proxy_init(self): + mock_cursor = mock.Mock() + mock_cursor_tracer = mock.Mock() + proxy = self.TestCursorProxy( + mock_cursor, + test_cursor_tracer=mock_cursor_tracer, + ) + self.assertIs(proxy.__wrapped__, mock_cursor) + assert proxy._cursor_tracer == mock_cursor_tracer + + def test_base_traced_cursor_proxy_callproc(self): + mock_cursor = mock.Mock() + mock_cursor.callproc = mock.Mock() + mock_cursor_tracer = mock.Mock() + mock_cursor_tracer.traced_execution = mock.Mock() + proxy = self.TestCursorProxy( + mock_cursor, + test_cursor_tracer=mock_cursor_tracer, + ) + args = ("foo_name",) + kwargs = {"foo": "bar"} + proxy.callproc(*args, **kwargs) + mock_cursor_tracer.traced_execution.assert_called_once_with( + mock_cursor, mock_cursor.callproc, *args, **kwargs + ) + + def test_base_traced_cursor_proxy_execute(self): + mock_cursor = mock.Mock() + mock_cursor.execute = mock.Mock() + mock_cursor_tracer = mock.Mock() + mock_cursor_tracer.traced_execution = mock.Mock() + proxy = self.TestCursorProxy( + mock_cursor, + test_cursor_tracer=mock_cursor_tracer, + ) + args = ("foo_name",) + kwargs = {"foo": "bar"} + proxy.execute(*args, **kwargs) + mock_cursor_tracer.traced_execution.assert_called_once_with( + mock_cursor, mock_cursor.execute, *args, **kwargs + ) + + def test_base_traced_cursor_proxy_executemany(self): + mock_cursor = mock.Mock() + mock_cursor.executemany = mock.Mock() + mock_cursor_tracer = mock.Mock() + mock_cursor_tracer.traced_execution = mock.Mock() + proxy = self.TestCursorProxy( + mock_cursor, + test_cursor_tracer=mock_cursor_tracer, + ) + args = ("foo_name",) + kwargs = {"foo": "bar"} + proxy.executemany(*args, **kwargs) + mock_cursor_tracer.traced_execution.assert_called_once_with( + mock_cursor, mock_cursor.executemany, *args, **kwargs + ) + + def test_base_traced_cursor_proxy_enter(self): + mock_cursor = mock.Mock() + mock_cursor.__enter__ = mock.Mock() + mock_cursor.__exit__ = mock.Mock() + proxy = self.TestCursorProxy(mock_cursor) + with proxy: + mock_cursor.__enter__.assert_called_once() + + def test_base_traced_cursor_proxy_exit(self): + mock_cursor = mock.Mock() + mock_cursor.__enter__ = mock.Mock() + mock_cursor.__exit__ = mock.Mock() + proxy = self.TestCursorProxy(mock_cursor) + with proxy: + pass + mock_cursor.__exit__.assert_called_once() From 11d4f07e37fc5d908567f6e0ed085d5f7065e78f Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 27 Nov 2024 17:20:16 -0800 Subject: [PATCH 16/20] Add dbapi instr_cnx optional params to tests --- .../tests/test_dbapi_integration.py | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 2ffa2f3d5b..04312cc9cf 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -592,9 +592,15 @@ def test_instrument_connection(self): connection2 = dbapi.instrument_connection(self.tracer, connection, "-") self.assertIs(connection2.__wrapped__, connection) + @mock.patch( + "opentelemetry.instrumentation.dbapi.get_traced_connection_proxy" + ) @mock.patch("opentelemetry.instrumentation.dbapi.DatabaseApiIntegration") - def test_instrument_connection_kwargs_defaults(self, mock_dbapiint): - dbapi.instrument_connection(self.tracer, mock.Mock(), "foo") + def test_instrument_connection_kwargs_defaults( + self, mock_dbapiint, mock_get_cnx_proxy + ): + mock_get_cnx_proxy.return_value = "foo_cnx" + cnx = dbapi.instrument_connection(self.tracer, mock.Mock(), "foo") kwargs = mock_dbapiint.call_args[1] self.assertEqual(kwargs["connection_attributes"], None) self.assertEqual(kwargs["version"], "") @@ -603,11 +609,19 @@ def test_instrument_connection_kwargs_defaults(self, mock_dbapiint): self.assertEqual(kwargs["enable_commenter"], False) self.assertEqual(kwargs["commenter_options"], None) self.assertEqual(kwargs["connect_module"], None) + assert cnx == "foo_cnx" + @mock.patch( + "opentelemetry.instrumentation.dbapi.get_traced_connection_proxy" + ) @mock.patch("opentelemetry.instrumentation.dbapi.DatabaseApiIntegration") - def test_instrument_connection_kwargs_provided(self, mock_dbapiint): + def test_instrument_connection_kwargs_provided( + self, mock_dbapiint, mock_get_cnx_proxy + ): mock_tracer_provider = mock.MagicMock() mock_connect_module = mock.MagicMock() + mock_custom_dbapiint = mock.MagicMock() + mock_custom_get_cnx_proxy = mock.MagicMock() dbapi.instrument_connection( self.tracer, mock.Mock(), @@ -619,8 +633,11 @@ def test_instrument_connection_kwargs_provided(self, mock_dbapiint): enable_commenter=True, commenter_options={"foo": "bar"}, connect_module=mock_connect_module, + db_api_integration_factory=mock_custom_dbapiint, + get_cnx_proxy=mock_custom_get_cnx_proxy, ) - kwargs = mock_dbapiint.call_args[1] + mock_dbapiint.assert_not_called() + kwargs = mock_custom_dbapiint.call_args[1] self.assertEqual(kwargs["connection_attributes"], {"foo": "bar"}) self.assertEqual(kwargs["version"], "test") self.assertIs(kwargs["tracer_provider"], mock_tracer_provider) @@ -628,6 +645,8 @@ def test_instrument_connection_kwargs_provided(self, mock_dbapiint): self.assertEqual(kwargs["enable_commenter"], True) self.assertEqual(kwargs["commenter_options"], {"foo": "bar"}) self.assertIs(kwargs["connect_module"], mock_connect_module) + mock_get_cnx_proxy.assert_not_called() + mock_custom_get_cnx_proxy.assert_called_once() def test_uninstrument_connection(self): connection = mock.Mock() From 1cf2238a96808f06d928ffb11e4a97fa4a7d7058 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 28 Nov 2024 08:48:15 -0800 Subject: [PATCH 17/20] Mv test --- .../tests/test_mysql_integration.py | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py index 21efdcecd2..deb01d458f 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py @@ -62,6 +62,21 @@ def test_instrumentor(self, mock_connect): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") + @mock.patch("mysql.connector.connect") + def test_instrumentor_sqlcomment_enabled( + self, + mock_connect, + mock_wrap_connect, + ): + MySQLInstrumentor().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}) + @mock.patch("mysql.connector.connect") # pylint: disable=unused-argument def test_custom_tracer_provider(self, mock_connect): @@ -122,22 +137,6 @@ def test_instrument_connection_enable_commenter( self.assertEqual(kwargs["enable_commenter"], True) self.assertEqual(kwargs["commenter_options"], {"foo": True}) - @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") - @mock.patch("mysql.connector.connect") - # pylint: disable=unused-argument - def test__instrument_enable_commenter( - self, - mock_connect, - mock_wrap_connect, - ): - MySQLInstrumentor()._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}) - @mock.patch("mysql.connector.connect") # pylint: disable=unused-argument def test_uninstrument_connection(self, mock_connect): From cb9a475321b344fa34c179db59a3e8757dc8979a Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 28 Nov 2024 09:32:42 -0800 Subject: [PATCH 18/20] Add mysql sqlcomment integration tests --- .../tests/test_mysql_integration.py | 224 +++++++++++++++++- 1 file changed, 222 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py index deb01d458f..8cc434bb3d 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py @@ -64,7 +64,7 @@ def test_instrumentor(self, mock_connect): @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") @mock.patch("mysql.connector.connect") - def test_instrumentor_sqlcomment_enabled( + def test_instrumentor_sqlcomment_enabled_dbapi_kwargs( self, mock_connect, mock_wrap_connect, @@ -77,6 +77,118 @@ def test_instrumentor_sqlcomment_enabled( 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__="mysql.connector", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_cursor = mock_connect_module.connect().cursor() + mock_cursor._cnx._cmysql.get_client_info.return_value = "foobaz" + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysql.mysql.connector", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + MySQLInstrumentor()._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='mysql.connector%%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__="mysql.connector", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_cursor = mock_connect_module.connect().cursor() + mock_cursor._cnx._cmysql.get_client_info.return_value = "foobaz" + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysql.mysql.connector", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + MySQLInstrumentor()._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='mysql.connector%%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__="mysql.connector", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_cursor = mock_connect_module.connect().cursor() + mock_cursor._cnx._cmysql.get_client_info.return_value = "foobaz" + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysql.mysql.connector", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + MySQLInstrumentor()._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("mysql.connector.connect") # pylint: disable=unused-argument def test_custom_tracer_provider(self, mock_connect): @@ -120,7 +232,7 @@ def test_instrument_connection_no_op_tracer_provider(self, mock_connect): @mock.patch("opentelemetry.instrumentation.mysql.DatabaseApiIntegration") @mock.patch("mysql.connector.connect") # pylint: disable=unused-argument - def test_instrument_connection_enable_commenter( + def test_instrument_connection_enable_commenter_dbapi_kwargs( self, mock_connect, mock_mysql_dbapi, @@ -137,6 +249,114 @@ def test_instrument_connection_enable_commenter( 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__="mysql.connector", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_cursor = mock_connect_module.connect().cursor() + mock_cursor._cnx._cmysql.get_client_info.return_value = "foobaz" + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysql.mysql.connector", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + cnx_proxy = MySQLInstrumentor().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='mysql.connector%%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__="mysql.connector", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_cursor = mock_connect_module.connect().cursor() + mock_cursor._cnx._cmysql.get_client_info.return_value = "foobaz" + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysql.mysql.connector", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + cnx_proxy = MySQLInstrumentor().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='mysql.connector%%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__="mysql.connector", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_cursor = mock_connect_module.connect().cursor() + mock_cursor._cnx._cmysql.get_client_info.return_value = "foobaz" + mock_connection = mock.MagicMock() + mock_connection.cursor.return_value = mock_cursor + + with mock.patch( + "opentelemetry.instrumentation.mysql.mysql.connector", + mock_connect_module, + ), mock.patch( + "opentelemetry.instrumentation.dbapi.util_version", + return_value="foobar", + ): + cnx_proxy = MySQLInstrumentor().instrument_connection( + mock_connection, + ) + cnx_proxy.cursor().execute("Select 1;") + self.assertEqual( + mock_cursor.execute.call_args[0][0], + "Select 1;", + ) + @mock.patch("mysql.connector.connect") # pylint: disable=unused-argument def test_uninstrument_connection(self, mock_connect): From b9a52e9f653a30c95cbc50ff22135316b35a7224 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 28 Nov 2024 10:40:45 -0800 Subject: [PATCH 19/20] Add get_traced_cnx or cur_proxy tests --- .../tests/test_mysql_integration.py | 95 ++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py index 8cc434bb3d..91ea8878c1 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/tests/test_mysql_integration.py @@ -15,10 +15,15 @@ from unittest import mock import mysql.connector +import pytest import opentelemetry.instrumentation.mysql from opentelemetry import trace as trace_api -from opentelemetry.instrumentation.mysql import MySQLInstrumentor +from opentelemetry.instrumentation.mysql import ( + MySQLInstrumentor, + get_traced_connection_proxy, + get_traced_cursor_proxy, +) from opentelemetry.sdk import resources from opentelemetry.test.test_base import TestBase @@ -33,6 +38,10 @@ def connect_and_execute_query(): class TestMysqlIntegration(TestBase): + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self.caplog = caplog # pylint: disable=attribute-defined-outside-init + def tearDown(self): super().tearDown() with self.disable_logging(): @@ -372,3 +381,87 @@ def test_uninstrument_connection(self, mock_connect): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + + def test_get_traced_cursor_proxy_with_enable_commenter_cursor(self): + mock_cursor = mock.Mock() + mock_dbapiint = mock.Mock() + mock_enable = mock.Mock() + proxy = get_traced_cursor_proxy( + mock_cursor, + mock_dbapiint, + mock_enable, + ) + self.assertIs(proxy.__wrapped__, mock_cursor) + self.assertIs(proxy._cursor_tracer._db_api_integration, mock_dbapiint) + self.assertIs(proxy._cursor_tracer._commenter_enabled, mock_enable) + + def test_get_traced_connection_proxy_not_dbapi_enable_commenter(self): + mock_connection = mock.Mock() + mock_dbapiint = mock.Mock() + mock_dbapiint.enable_commenter = False + cnx_proxy = get_traced_connection_proxy( + mock_connection, + mock_dbapiint, + ) + self.assertIs(cnx_proxy.__wrapped__, mock_connection) + cur_proxy = cnx_proxy.cursor() + self.assertIs( + cur_proxy._cursor_tracer._db_api_integration, mock_dbapiint + ) + self.assertFalse(cur_proxy._cursor_tracer._commenter_enabled) + + def test_get_traced_connection_proxy_dbapi_enable_commenter_none_prepared_cursor( + self, + ): + mock_connection = mock.Mock() + mock_dbapiint = mock.Mock() + mock_dbapiint.enable_commenter = True + cnx_proxy = get_traced_connection_proxy( + mock_connection, + mock_dbapiint, + ) + self.assertIs(cnx_proxy.__wrapped__, mock_connection) + cur_proxy = cnx_proxy.cursor() + self.assertIs( + cur_proxy._cursor_tracer._db_api_integration, mock_dbapiint + ) + self.assertTrue(cur_proxy._cursor_tracer._commenter_enabled) + + def test_get_traced_connection_proxy_dbapi_enable_commenter_not_prepared_cursor( + self, + ): + mock_connection = mock.Mock() + mock_dbapiint = mock.Mock() + mock_dbapiint.enable_commenter = True + cnx_proxy = get_traced_connection_proxy( + mock_connection, + mock_dbapiint, + ) + self.assertIs(cnx_proxy.__wrapped__, mock_connection) + cur_proxy = cnx_proxy.cursor(prepared=False) + self.assertIs( + cur_proxy._cursor_tracer._db_api_integration, mock_dbapiint + ) + self.assertTrue(cur_proxy._cursor_tracer._commenter_enabled) + + def test_get_traced_connection_proxy_dbapi_enable_commenter_prepared_cursor( + self, + ): + mock_connection = mock.Mock() + mock_dbapiint = mock.Mock() + mock_dbapiint.enable_commenter = True + mock_dbapiint.connect_module.__name__ = "foo-module" + cnx_proxy = get_traced_connection_proxy( + mock_connection, + mock_dbapiint, + ) + self.assertIs(cnx_proxy.__wrapped__, mock_connection) + cur_proxy = cnx_proxy.cursor(prepared=True) + self.assertIs( + cur_proxy._cursor_tracer._db_api_integration, mock_dbapiint + ) + self.assertFalse(cur_proxy._cursor_tracer._commenter_enabled) + assert ( + self.caplog.records[0].getMessage() + == "sqlcomment is not supported for query statements executed by cursors with native prepared statement support. Disabling sqlcommenting for instrumentation of foo-module." + ) From da13a718393200b539888d3c97c342c4dd4eabe7 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 28 Nov 2024 12:21:01 -0800 Subject: [PATCH 20/20] Update doc --- .../src/opentelemetry/instrumentation/mysql/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py index 7c154f6a72..208a399cc9 100644 --- a/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysql/src/opentelemetry/instrumentation/mysql/__init__.py @@ -63,6 +63,9 @@ the query will get appended with some configurable tags like "INSERT INTO test (testField) VALUES (123) /*tag=value*/;" +Note that sqlcommenter for mysql-connector is not supported if cursor is initialized with `prepared=True`, which would natively prepare and execute MySQL statements. Adding sqlcommenting would introduce a severe performance penalty by repeating `Prepare` of statements by mysql-connector that are made unique by traceparent in sqlcomment. Therefore, sqlcommenter is not supported for this use case. + + SQLCommenter Configurations *************************** We can configure the tags to be appended to the sqlquery log by adding configuration inside commenter_options(default:{}) keyword