From b8b5a67102c584bd3c4195e7ce27eda528e5a478 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 15:32:14 -0800 Subject: [PATCH 1/6] (WIP) db-api opt-in for enable_attribute_commenter --- .../instrumentation/dbapi/__init__.py | 158 +++++-- .../tests/test_dbapi_integration.py | 405 +++++++++++++++++- 2 files changed, 519 insertions(+), 44 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 d8db967f47..6f9ca5d0dd 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -71,6 +71,7 @@ def trace_integration( capture_parameters: bool = False, enable_commenter: bool = False, db_api_integration_factory=None, + enable_attribute_commenter: bool = False, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -88,6 +89,7 @@ def trace_integration( enable_commenter: Flag to enable/disable sqlcommenter. db_api_integration_factory: The `DatabaseApiIntegration` to use. If none is passed the default one is used. + enable_attribute_commenter: Flag to enable/disable sqlcomment inclusion in `db.statement` span attribute. Only available if enable_commenter=True. """ wrap_connect( __name__, @@ -100,6 +102,7 @@ def trace_integration( capture_parameters=capture_parameters, enable_commenter=enable_commenter, db_api_integration_factory=db_api_integration_factory, + enable_attribute_commenter=enable_attribute_commenter, ) @@ -115,6 +118,7 @@ def wrap_connect( enable_commenter: bool = False, db_api_integration_factory=None, commenter_options: dict = None, + enable_attribute_commenter: bool = False, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -133,6 +137,7 @@ def wrap_connect( db_api_integration_factory: The `DatabaseApiIntegration` to use. If none is passed the default one is used. commenter_options: Configurations for tags to be appended at the sql query. + enable_attribute_commenter: Flag to enable/disable sqlcomment inclusion in `db.statement` span attribute. Only available if enable_commenter=True. """ db_api_integration_factory = ( @@ -156,6 +161,7 @@ def wrap_connect_( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=connect_module, + enable_attribute_commenter=enable_attribute_commenter, ) return db_integration.wrapped_connection(wrapped, args, kwargs) @@ -191,6 +197,7 @@ def instrument_connection( enable_commenter: bool = False, commenter_options: dict = None, connect_module: typing.Callable[..., typing.Any] = None, + enable_attribute_commenter: bool = False, ): """Enable instrumentation in a database connection. @@ -206,6 +213,7 @@ 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. + enable_attribute_commenter: Flag to enable/disable sqlcomment inclusion in `db.statement` span attribute. Only available if enable_commenter=True. Returns: An instrumented connection. @@ -224,6 +232,7 @@ def instrument_connection( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=connect_module, + enable_attribute_commenter=enable_attribute_commenter, ) db_integration.get_connection_attributes(connection) return get_traced_connection_proxy(connection, db_integration) @@ -257,6 +266,7 @@ def __init__( enable_commenter: bool = False, commenter_options: dict = None, connect_module: typing.Callable[..., typing.Any] = None, + enable_attribute_commenter: bool = False, ): self.connection_attributes = connection_attributes if self.connection_attributes is None: @@ -277,6 +287,7 @@ def __init__( self.capture_parameters = capture_parameters self.enable_commenter = enable_commenter self.commenter_options = commenter_options + self.enable_attribute_commenter = enable_attribute_commenter self.database_system = database_system self.connection_props = {} self.span_attributes = {} @@ -434,6 +445,9 @@ def __init__(self, db_api_integration: DatabaseApiIntegration) -> None: if self._db_api_integration.commenter_options else {} ) + self._enable_attribute_commenter = ( + self._db_api_integration.enable_attribute_commenter + ) self._connect_module = self._db_api_integration.connect_module self._leading_comment_remover = re.compile(r"^/\*.*?\*/") @@ -497,51 +511,109 @@ def traced_execution( ) as span: if span.is_recording(): if args and self._commenter_enabled: - try: - args_list = list(args) - - # lazy capture of mysql-connector client version using cursor - if ( - 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" - ] - ): - 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 - ): - commenter_data.update( - **_get_opentelemetry_values() + # sqlcomment in query and span attribute + if self._enable_attribute_commenter: + try: + args_list = list(args) + + # lazy capture of mysql-connector client version using cursor + if ( + 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" + ] + ): + 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 + ): + commenter_data.update( + **_get_opentelemetry_values() + ) + + # Filter down to just the requested attributes. + commenter_data = { + k: v + for k, v in commenter_data.items() + if self._commenter_options.get(k, True) + } + statement = _add_sql_comment( + args_list[0], **commenter_data + ) + + args_list[0] = statement + args = tuple(args_list) + + except Exception as exc: # pylint: disable=broad-except + _logger.exception( + "Exception while generating sql comment: %s", + exc, + ) + + self._populate_span(span, cursor, *args) + + # sqlcomment in query only + else: + self._populate_span(span, cursor, *args) + + try: + args_list = list(args) + + # lazy capture of mysql-connector client version using cursor + if ( + 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" + ] + ): + 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 + ): + commenter_data.update( + **_get_opentelemetry_values() + ) + + # Filter down to just the requested attributes. + commenter_data = { + k: v + for k, v in commenter_data.items() + if self._commenter_options.get(k, True) + } + statement = _add_sql_comment( + args_list[0], **commenter_data + ) + + args_list[0] = statement + args = tuple(args_list) + + except Exception as exc: # pylint: disable=broad-except + _logger.exception( + "Exception while generating sql comment: %s", + exc, ) - # Filter down to just the requested attributes. - commenter_data = { - k: v - for k, v in commenter_data.items() - if self._commenter_options.get(k, True) - } - statement = _add_sql_comment( - args_list[0], **commenter_data - ) - - args_list[0] = statement - args = tuple(args_list) - - except Exception as exc: # pylint: disable=broad-except - _logger.exception( - "Exception while generating sql comment: %s", exc - ) - - self._populate_span(span, cursor, *args) + # No sqlcommenting + else: + self._populate_span(span, cursor, *args) return query_method(*args, **kwargs) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 2ffa2f3d5b..794bcd6efb 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -277,6 +277,47 @@ def test_executemany_comment(self): cursor.query, 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = mock.MagicMock() + 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": False, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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: @@ -306,8 +347,54 @@ def __getattr__(self, name): 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) - def test_executemany_comment_matches_db_statement_attribute(self): + def test_executemany_comment_non_pep_249_compliant_stmt_enabled(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}, + enable_attribute_commenter=True, + ) + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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_executemany_comment_stmt_enabled_matches_db_statement_attribute( + self, + ): connect_module = mock.MagicMock() connect_module.__version__ = mock.MagicMock() connect_module.__libpq_version__ = 123 @@ -321,6 +408,7 @@ def test_executemany_comment_matches_db_statement_attribute(self): enable_commenter=True, commenter_options={"db_driver": False, "dbapi_level": False}, connect_module=connect_module, + enable_attribute_commenter=True, ) mock_connection = db_integration.wrapped_connection( mock_connect, {}, {} @@ -371,6 +459,50 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): cursor.query, 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_compatible_build_version_psycopg_psycopg2_libpq_stmt_enabled( + self, + ): + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = mock.MagicMock() + 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": False, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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() @@ -397,6 +529,47 @@ def test_executemany_psycopg2_integration_comment(self): 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_psycopg2_integration_comment_stmt_enabled(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, + enable_attribute_commenter=True, + ) + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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() @@ -424,6 +597,48 @@ def test_executemany_psycopg_integration_comment(self): 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_psycopg_integration_comment_stmt_enabled(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, + enable_attribute_commenter=True, + ) + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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() @@ -450,6 +665,47 @@ def test_executemany_mysqlconnector_integration_comment(self): 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_mysqlconnector_integration_comment_stmt_enabled(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, + enable_attribute_commenter=True, + ) + + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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( @@ -485,6 +741,56 @@ def test_executemany_mysqlclient_integration_comment( 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + @mock.patch("opentelemetry.instrumentation.dbapi.util_version") + def test_executemany_mysqlclient_integration_comment_stmt_enabled( + 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, + enable_attribute_commenter=True, + ) + + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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() @@ -512,6 +818,48 @@ def test_executemany_pymysql_integration_comment(self): 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_pymysql_integration_comment_stmt_enabled(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, + enable_attribute_commenter=True, + ) + + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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() @@ -544,6 +892,58 @@ def test_executemany_flask_integration_comment(self): cursor.query, 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}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + clear_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {}, current_context + ) + context.attach(clear_context) + + def test_executemany_flask_integration_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = mock.MagicMock() + 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": False, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + current_context = context.get_current() + sqlcommenter_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {"flask": 1}, current_context + ) + context.attach(sqlcommenter_context) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + 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}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + 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 @@ -603,6 +1003,7 @@ 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) + self.assertEqual(kwargs["enable_attribute_commenter"], False) @mock.patch("opentelemetry.instrumentation.dbapi.DatabaseApiIntegration") def test_instrument_connection_kwargs_provided(self, mock_dbapiint): @@ -619,6 +1020,7 @@ def test_instrument_connection_kwargs_provided(self, mock_dbapiint): enable_commenter=True, commenter_options={"foo": "bar"}, connect_module=mock_connect_module, + enable_attribute_commenter=True, ) kwargs = mock_dbapiint.call_args[1] self.assertEqual(kwargs["connection_attributes"], {"foo": "bar"}) @@ -628,6 +1030,7 @@ 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) + self.assertEqual(kwargs["enable_attribute_commenter"], True) def test_uninstrument_connection(self): connection = mock.Mock() From dfd5bbca813692c0e727934d7940f65460ca17ed Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 16:57:39 -0800 Subject: [PATCH 2/6] Refactor db-api traced_execution --- .../instrumentation/dbapi/__init__.py | 147 ++++++------------ .../tests/test_dbapi_integration.py | 1 + 2 files changed, 50 insertions(+), 98 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 6f9ca5d0dd..0c9b0ea63c 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -451,6 +451,46 @@ def __init__(self, db_api_integration: DatabaseApiIntegration) -> None: self._connect_module = self._db_api_integration.connect_module self._leading_comment_remover = re.compile(r"^/\*.*?\*/") + def _capture_mysql_version(self, cursor) -> None: + """Lazy capture of mysql-connector client version using cursor, if applicable""" + if ( + 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" + ] + ): + self._db_api_integration.commenter_data["mysql_client_version"] = ( + cursor._cnx._cmysql.get_client_info() + ) + + def _get_commenter_data(self) -> dict: + """Uses DB-API integration to return commenter data for sqlcomment""" + commenter_data = dict(self._db_api_integration.commenter_data) + if self._commenter_options.get("opentelemetry_values", True): + commenter_data.update(**_get_opentelemetry_values()) + return { + k: v + for k, v in commenter_data.items() + if self._commenter_options.get(k, True) + } + + def _update_args_with_added_sql_comment(self, args, cursor) -> tuple: + """Updates args with cursor info and adds sqlcomment to query statement""" + try: + args_list = list(args) + self._capture_mysql_version(cursor) + commenter_data = self._get_commenter_data() + statement = _add_sql_comment(args_list[0], **commenter_data) + args_list[0] = statement + args = tuple(args_list) + except Exception as exc: # pylint: disable=broad-except + _logger.exception( + "Exception while generating sql comment: %s", exc + ) + return args + def _populate_span( self, span: trace_api.Span, @@ -511,110 +551,21 @@ def traced_execution( ) as span: if span.is_recording(): if args and self._commenter_enabled: - # sqlcomment in query and span attribute if self._enable_attribute_commenter: - try: - args_list = list(args) - - # lazy capture of mysql-connector client version using cursor - if ( - 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" - ] - ): - 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 - ): - commenter_data.update( - **_get_opentelemetry_values() - ) - - # Filter down to just the requested attributes. - commenter_data = { - k: v - for k, v in commenter_data.items() - if self._commenter_options.get(k, True) - } - statement = _add_sql_comment( - args_list[0], **commenter_data - ) - - args_list[0] = statement - args = tuple(args_list) - - except Exception as exc: # pylint: disable=broad-except - _logger.exception( - "Exception while generating sql comment: %s", - exc, - ) - + # sqlcomment in query and span attribute + args = self._update_args_with_added_sql_comment( + args, cursor + ) self._populate_span(span, cursor, *args) - - # sqlcomment in query only else: + # sqlcomment in query only self._populate_span(span, cursor, *args) - - try: - args_list = list(args) - - # lazy capture of mysql-connector client version using cursor - if ( - 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" - ] - ): - 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 - ): - commenter_data.update( - **_get_opentelemetry_values() - ) - - # Filter down to just the requested attributes. - commenter_data = { - k: v - for k, v in commenter_data.items() - if self._commenter_options.get(k, True) - } - statement = _add_sql_comment( - args_list[0], **commenter_data - ) - - args_list[0] = statement - args = tuple(args_list) - - except Exception as exc: # pylint: disable=broad-except - _logger.exception( - "Exception while generating sql comment: %s", - exc, - ) - - # No sqlcommenting + args = self._update_args_with_added_sql_comment( + args, cursor + ) else: + # no sqlcomment self._populate_span(span, cursor, *args) - return query_method(*args, **kwargs) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 794bcd6efb..3d531fb791 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=too-many-lines import logging import re From 0829a79791b141c88ed3fa76f6dfe79f422947f2 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 17:02:35 -0800 Subject: [PATCH 3/6] Changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a243091b1d..6d5521aece 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3105](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3105)) +### Breaking changes + +- `opentelemetry-instrumentation-dbapi` including sqlcomment in `db.statement` span attribute value is now opt-in + ([#3115](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3115)) + + ## Version 1.29.0/0.50b0 (2024-12-11) ### Added From 1a515a1a812a153fd48bd1ed9f1a444c0e42f525 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 19 Dec 2024 12:21:22 -0800 Subject: [PATCH 4/6] Update comment --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 7 ++++--- 1 file changed, 4 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 0c9b0ea63c..9b709c584c 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -552,19 +552,20 @@ def traced_execution( if span.is_recording(): if args and self._commenter_enabled: if self._enable_attribute_commenter: - # sqlcomment in query and span attribute + # sqlcomment is added to executed query and db.statement span attribute args = self._update_args_with_added_sql_comment( args, cursor ) self._populate_span(span, cursor, *args) else: - # sqlcomment in query only + # sqlcomment is only added to executed query + # so db.statement is set before add_sql_comment self._populate_span(span, cursor, *args) args = self._update_args_with_added_sql_comment( args, cursor ) else: - # no sqlcomment + # no sqlcomment anywhere self._populate_span(span, cursor, *args) return query_method(*args, **kwargs) From 81640bbbcd270d8c7286b7c1c52f832962079e49 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 19 Dec 2024 13:22:46 -0800 Subject: [PATCH 5/6] psycopg(2), mysqlclient, pymysql support enable_attribute_commenter --- .../instrumentation/mysqlclient/__init__.py | 26 ++++ .../tests/test_mysqlclient_integration.py | 116 ++++++++++++++++++ .../instrumentation/psycopg/__init__.py | 26 ++++ .../instrumentation/psycopg2/__init__.py | 24 ++++ .../instrumentation/pymysql/__init__.py | 26 ++++ .../tests/test_pymysql_integration.py | 112 +++++++++++++++++ 6 files changed, 330 insertions(+) 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 5b08b0b50d..f34d67c9bb 100644 --- a/instrumentation/opentelemetry-instrumentation-mysqlclient/src/opentelemetry/instrumentation/mysqlclient/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-mysqlclient/src/opentelemetry/instrumentation/mysqlclient/__init__.py @@ -102,6 +102,26 @@ :: Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ +SQLComment in span attribute +**************************** +If sqlcommenter is enabled, you can optionally configure MySQLClient instrumentation to append sqlcomment to query span attribute for convenience of your platform. + +.. code:: python + + from opentelemetry.instrumentation.mysqlclient import MySQLClientInstrumentor + + MySQLClientInstrumentor().instrument( + enable_commenter=True, + enable_attribute_commenter=True, + ) + + +For example, +:: + + Invoking cursor.execute("select * from auth_users") will lead to sql query "select * from auth_users" but when SQLCommenter and attribute_commenter are enabled + the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. + API --- """ @@ -135,6 +155,9 @@ def _instrument(self, **kwargs): # pylint: disable=no-self-use tracer_provider = kwargs.get("tracer_provider") enable_sqlcommenter = kwargs.get("enable_commenter", False) commenter_options = kwargs.get("commenter_options", {}) + enable_attribute_commenter = kwargs.get( + "enable_attribute_commenter", False + ) dbapi.wrap_connect( __name__, @@ -146,6 +169,7 @@ def _instrument(self, **kwargs): # pylint: disable=no-self-use tracer_provider=tracer_provider, enable_commenter=enable_sqlcommenter, commenter_options=commenter_options, + enable_attribute_commenter=enable_attribute_commenter, ) def _uninstrument(self, **kwargs): # pylint: disable=no-self-use @@ -158,6 +182,7 @@ def instrument_connection( tracer_provider=None, enable_commenter=None, commenter_options=None, + enable_attribute_commenter=None, ): """Enable instrumentation in a mysqlclient connection. @@ -180,6 +205,7 @@ def instrument_connection( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=MySQLdb, + enable_attribute_commenter=enable_attribute_commenter, ) @staticmethod diff --git a/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py b/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py index ae221f68f4..e2b1e41aa3 100644 --- a/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py +++ b/instrumentation/opentelemetry-instrumentation-mysqlclient/tests/test_mysqlclient_integration.py @@ -19,6 +19,7 @@ import opentelemetry.instrumentation.mysqlclient from opentelemetry.instrumentation.mysqlclient import MySQLClientInstrumentor from opentelemetry.sdk import resources +from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase @@ -110,12 +111,14 @@ def test_instrument_connection_enable_commenter_dbapi_kwargs( cnx, enable_commenter=True, commenter_options={"foo": True}, + enable_attribute_commenter=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}) + self.assertEqual(kwargs["enable_attribute_commenter"], True) def test_instrument_connection_with_dbapi_sqlcomment_enabled(self): mock_connect_module = mock.MagicMock( @@ -150,6 +153,51 @@ def test_instrument_connection_with_dbapi_sqlcomment_enabled(self): 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'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_instrument_connection_with_dbapi_sqlcomment_enabled_stmt_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, + enable_attribute_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'*/;", + ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + 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, @@ -191,6 +239,10 @@ def test_instrument_connection_with_dbapi_sqlcomment_enabled_with_options( 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'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) def test_instrument_connection_with_dbapi_sqlcomment_not_enabled_default( self, @@ -221,6 +273,12 @@ def test_instrument_connection_with_dbapi_sqlcomment_not_enabled_default( mock_cursor.execute.call_args[0][0], "Select 1;", ) + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") @mock.patch("MySQLdb.connect") @@ -233,10 +291,12 @@ def test_instrument_enable_commenter_dbapi_kwargs( MySQLClientInstrumentor()._instrument( enable_commenter=True, commenter_options={"foo": True}, + enable_attribute_commenter=True, ) kwargs = mock_wrap_connect.call_args[1] self.assertEqual(kwargs["enable_commenter"], True) self.assertEqual(kwargs["commenter_options"], {"foo": True}) + self.assertEqual(kwargs["enable_attribute_commenter"], True) def test_instrument_with_dbapi_sqlcomment_enabled( self, @@ -274,6 +334,52 @@ def test_instrument_with_dbapi_sqlcomment_enabled( 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'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_instrument_with_dbapi_sqlcomment_enabled_stmt_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, + enable_attribute_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'*/;", + ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + 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, @@ -316,6 +422,10 @@ def test_instrument_with_dbapi_sqlcomment_enabled_with_options( 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'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) def test_instrument_with_dbapi_sqlcomment_not_enabled_default( self, @@ -346,6 +456,12 @@ def test_instrument_with_dbapi_sqlcomment_not_enabled_default( mock_cursor.execute.call_args[0][0], "Select 1;", ) + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) @mock.patch("MySQLdb.connect") # pylint: disable=unused-argument diff --git a/instrumentation/opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py b/instrumentation/opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py index e986ec0d46..250e65e8bd 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py @@ -80,6 +80,26 @@ :: Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ +SQLComment in span attribute +**************************** +If sqlcommenter is enabled, you can optionally configure psycopg instrumentation to append sqlcomment to query span attribute for convenience of your platform. + +.. code:: python + + from opentelemetry.instrumentation.psycopg import PsycopgInstrumentor + + PsycopgInstrumentor().instrument( + enable_commenter=True, + enable_attribute_commenter=True, + ) + + +For example, +:: + + Invoking cursor.execute("select * from auth_users") will lead to postgresql query "select * from auth_users" but when SQLCommenter and attribute_commenter are enabled + the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. + Usage ----- @@ -143,6 +163,9 @@ def _instrument(self, **kwargs): tracer_provider = kwargs.get("tracer_provider") enable_sqlcommenter = kwargs.get("enable_commenter", False) commenter_options = kwargs.get("commenter_options", {}) + enable_attribute_commenter = kwargs.get( + "enable_attribute_commenter", False + ) dbapi.wrap_connect( __name__, psycopg, @@ -154,6 +177,7 @@ def _instrument(self, **kwargs): db_api_integration_factory=DatabaseApiIntegration, enable_commenter=enable_sqlcommenter, commenter_options=commenter_options, + enable_attribute_commenter=enable_attribute_commenter, ) dbapi.wrap_connect( @@ -167,6 +191,7 @@ def _instrument(self, **kwargs): db_api_integration_factory=DatabaseApiIntegration, enable_commenter=enable_sqlcommenter, commenter_options=commenter_options, + enable_attribute_commenter=enable_attribute_commenter, ) dbapi.wrap_connect( __name__, @@ -179,6 +204,7 @@ def _instrument(self, **kwargs): db_api_integration_factory=DatabaseApiAsyncIntegration, enable_commenter=enable_sqlcommenter, commenter_options=commenter_options, + enable_attribute_commenter=enable_attribute_commenter, ) def _uninstrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py index de2e49f4c3..5d7ff9e59c 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py @@ -80,6 +80,26 @@ :: Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ +SQLComment in span attribute +**************************** +If sqlcommenter is enabled, you can optionally configure psycopg2 instrumentation to append sqlcomment to query span attribute for convenience of your platform. + +.. code:: python + + from opentelemetry.instrumentation.psycopg2 import Psycopg2Instrumentor + + Psycopg2Instrumentor().instrument( + enable_commenter=True, + enable_attribute_commenter=True, + ) + + +For example, +:: + + Invoking cursor.execute("select * from auth_users") will lead to postgresql query "select * from auth_users" but when SQLCommenter and attribute_commenter are enabled + the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. + Usage ----- @@ -140,6 +160,9 @@ def _instrument(self, **kwargs): tracer_provider = kwargs.get("tracer_provider") enable_sqlcommenter = kwargs.get("enable_commenter", False) commenter_options = kwargs.get("commenter_options", {}) + enable_attribute_commenter = kwargs.get( + "enable_attribute_commenter", False + ) dbapi.wrap_connect( __name__, psycopg2, @@ -151,6 +174,7 @@ def _instrument(self, **kwargs): db_api_integration_factory=DatabaseApiIntegration, enable_commenter=enable_sqlcommenter, commenter_options=commenter_options, + enable_attribute_commenter=enable_attribute_commenter, ) def _uninstrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py b/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py index eb4435813d..cf712c07bb 100644 --- a/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py @@ -105,6 +105,26 @@ :: Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ +SQLComment in span attribute +**************************** +If sqlcommenter is enabled, you can optionally configure PyMySQL instrumentation to append sqlcomment to query span attribute for convenience of your platform. + +.. code:: python + + from opentelemetry.instrumentation.pymysql import PyMySQLInstrumentor + + PyMySQLInstrumentor().instrument( + enable_commenter=True, + enable_attribute_commenter=True, + ) + + +For example, +:: + + Invoking cursor.execute("select * from auth_users") will lead to sql query "select * from auth_users" but when SQLCommenter and attribute_commenter are enabled + the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. + API --- """ @@ -138,6 +158,9 @@ def _instrument(self, **kwargs): # pylint: disable=no-self-use tracer_provider = kwargs.get("tracer_provider") enable_sqlcommenter = kwargs.get("enable_commenter", False) commenter_options = kwargs.get("commenter_options", {}) + enable_attribute_commenter = kwargs.get( + "enable_attribute_commenter", False + ) dbapi.wrap_connect( __name__, @@ -149,6 +172,7 @@ def _instrument(self, **kwargs): # pylint: disable=no-self-use tracer_provider=tracer_provider, enable_commenter=enable_sqlcommenter, commenter_options=commenter_options, + enable_attribute_commenter=enable_attribute_commenter, ) def _uninstrument(self, **kwargs): # pylint: disable=no-self-use @@ -161,6 +185,7 @@ def instrument_connection( tracer_provider=None, enable_commenter=None, commenter_options=None, + enable_attribute_commenter=None, ): """Enable instrumentation in a PyMySQL connection. @@ -183,6 +208,7 @@ def instrument_connection( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=pymysql, + enable_attribute_commenter=enable_attribute_commenter, ) @staticmethod diff --git a/instrumentation/opentelemetry-instrumentation-pymysql/tests/test_pymysql_integration.py b/instrumentation/opentelemetry-instrumentation-pymysql/tests/test_pymysql_integration.py index 82294236f0..ea59e5df7b 100644 --- a/instrumentation/opentelemetry-instrumentation-pymysql/tests/test_pymysql_integration.py +++ b/instrumentation/opentelemetry-instrumentation-pymysql/tests/test_pymysql_integration.py @@ -20,6 +20,7 @@ from opentelemetry import trace as trace_api from opentelemetry.instrumentation.pymysql import PyMySQLInstrumentor from opentelemetry.sdk import resources +from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase @@ -125,12 +126,14 @@ def test_instrument_connection_enable_commenter_dbapi_kwargs( cnx, enable_commenter=True, commenter_options={"foo": True}, + enable_attribute_commenter=True, ) cursor = cnx.cursor() cursor.execute("SELECT * FROM test") kwargs = mock_instrument_connection.call_args[1] self.assertEqual(kwargs["enable_commenter"], True) self.assertEqual(kwargs["commenter_options"], {"foo": True}) + self.assertEqual(kwargs["enable_attribute_commenter"], True) def test_instrument_connection_with_dbapi_sqlcomment_enabled(self): mock_connect_module = mock.MagicMock( @@ -163,6 +166,49 @@ def test_instrument_connection_with_dbapi_sqlcomment_enabled(self): mock_cursor.execute.call_args[0][0], f"Select 1 /*db_driver='pymysql%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_instrument_connection_with_dbapi_sqlcomment_enabled_stmt_enabled( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="pymysql", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module.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.pymysql.pymysql", + mock_connect_module, + ): + cnx_proxy = PyMySQLInstrumentor().instrument_connection( + mock_connection, + enable_commenter=True, + enable_attribute_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='pymysql%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", + ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + f"Select 1 /*db_driver='pymysql%%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, @@ -202,6 +248,10 @@ def test_instrument_connection_with_dbapi_sqlcomment_enabled_with_options( mock_cursor.execute.call_args[0][0], f"Select 1 /*db_driver='pymysql%%3Afoobar',dbapi_threadsafety='123',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) def test_instrument_connection_with_dbapi_sqlcomment_not_enabled_default( self, @@ -230,6 +280,12 @@ def test_instrument_connection_with_dbapi_sqlcomment_not_enabled_default( mock_cursor.execute.call_args[0][0], "Select 1;", ) + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) @mock.patch("opentelemetry.instrumentation.dbapi.wrap_connect") @mock.patch("pymysql.connect") @@ -242,10 +298,12 @@ def test_instrument_enable_commenter_dbapi_kwargs( PyMySQLInstrumentor()._instrument( enable_commenter=True, commenter_options={"foo": True}, + enable_attribute_commenter=True, ) kwargs = mock_wrap_connect.call_args[1] self.assertEqual(kwargs["enable_commenter"], True) self.assertEqual(kwargs["commenter_options"], {"foo": True}) + self.assertEqual(kwargs["enable_attribute_commenter"], True) def test_instrument_with_dbapi_sqlcomment_enabled( self, @@ -281,6 +339,50 @@ def test_instrument_with_dbapi_sqlcomment_enabled( mock_cursor.execute.call_args[0][0], f"Select 1 /*db_driver='pymysql%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_instrument_with_dbapi_sqlcomment_enabled_stmt_enabled( + self, + ): + mock_connect_module = mock.MagicMock( + __name__="pymysql", + __version__="foobar", + threadsafety="123", + apilevel="123", + paramstyle="test", + ) + mock_connect_module.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.pymysql.pymysql", + mock_connect_module, + ): + PyMySQLInstrumentor()._instrument( + enable_commenter=True, + enable_attribute_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='pymysql%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", + ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + f"Select 1 /*db_driver='pymysql%%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, @@ -321,6 +423,10 @@ def test_instrument_with_dbapi_sqlcomment_enabled_with_options( mock_cursor.execute.call_args[0][0], f"Select 1 /*db_driver='pymysql%%3Afoobar',dbapi_threadsafety='123',mysql_client_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/;", ) + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) def test_instrument_with_dbapi_sqlcomment_not_enabled_default( self, @@ -349,6 +455,12 @@ def test_instrument_with_dbapi_sqlcomment_not_enabled_default( mock_cursor.execute.call_args[0][0], "Select 1;", ) + spans_list = self.memory_exporter.get_finished_spans() + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) @mock.patch("pymysql.connect") # pylint: disable=unused-argument From c5d97b59687519e4660d07d47aea7a2d14cf7da4 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 19 Dec 2024 13:29:12 -0800 Subject: [PATCH 6/6] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d5521aece..8acf1f53d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-dbapi` including sqlcomment in `db.statement` span attribute value is now opt-in ([#3115](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3115)) +- `opentelemetry-instrumentation-psycopg2`, `opentelemetry-instrumentation-psycopg`, `opentelemetry-instrumentation-mysqlclient`, `opentelemetry-instrumentation-pymysql`: including sqlcomment in `db.statement` span attribute value is now opt-in + ([#3121](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3121)) ## Version 1.29.0/0.50b0 (2024-12-11)