diff --git a/CHANGELOG.md b/CHANGELOG.md index b754fccc41..0c41a54fba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation` Fix client address is set to server address in new semconv ([#3354](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3354)) +- `opentelemetry-instrumentation-dbapi`, `opentelemetry-instrumentation-django`, + `opentelemetry-instrumentation-sqlalchemy`: Fix sqlcomment for non string query and composable object. + ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) ## Version 1.31.0/0.52b0 (2025-03-12) 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 c7b1dee3b2..f87401ff32 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -498,6 +498,11 @@ def _update_args_with_added_sql_comment(self, args, cursor) -> tuple: args_list = list(args) self._capture_mysql_version(cursor) commenter_data = self._get_commenter_data() + # Convert sql statement to string, handling psycopg2.sql.Composable object + if hasattr(args_list[0], "as_string"): + args_list[0] = args_list[0].as_string(cursor.connection) + + args_list[0] = str(args_list[0]) statement = _add_sql_comment(args_list[0], **commenter_data) args_list[0] = statement args = tuple(args_list) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 97d53f33ec..3f30f3a897 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -1074,6 +1074,39 @@ def test_uninstrument_connection(self): connection4 = dbapi.uninstrument_connection(mocked_conn) self.assertIs(connection4, mocked_conn) + def test_non_string_sql_conversion(self): + """Test that non-string SQL statements are converted to strings before adding comments.""" + # Create a mock connect_module with required attributes + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = "1.0" + connect_module.__libpq_version__ = 123 + connect_module.apilevel = "2.0" + connect_module.threadsafety = 1 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + connect_module=connect_module, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + + input_sql = mock.MagicMock(as_string=lambda conn: "SELECT 2") + expected_sql = "SELECT 2" + + cursor.executemany(input_sql) + # Extract the SQL part (before the comment) + actual_sql = cursor.query.split("/*")[0].strip() + self.assertEqual(actual_sql, expected_sql) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + # pylint: disable=unused-argument def mock_connect(*args, **kwargs): @@ -1100,6 +1133,7 @@ class MockCursor: def __init__(self) -> None: self.query = "" self.params = None + self.connection = None # Mock mysql.connector modules and method self._cnx = mock.MagicMock() self._cnx._cmysql = mock.MagicMock() diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py index ef53d5dc38..8167724564 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py @@ -80,6 +80,12 @@ def __call__(self, execute: Type[T], sql, params, many, context) -> T: db_driver = context["connection"].settings_dict.get("ENGINE", "") resolver_match = self.request.resolver_match + # Convert sql statement to string, handling psycopg2.sql.Composable object + if hasattr(sql, "as_string"): + sql = sql.as_string(context["connection"]) + + sql = str(sql) + sql = _add_sql_comment( sql, # Information about the controller. diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index a21cc36ceb..63f7c5e883 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py @@ -131,6 +131,38 @@ def test_query_wrapper(self, trace_capture): "00000000000000000deadbeef-000000000000beef-00'*/;", ) + @patch( + "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware._get_opentelemetry_values" + ) + def test_query_wrapper_non_string_queries(self, trace_capture): + """Test that non-string queries and psycopg2 composable objects are handled correctly.""" + requests_mock = MagicMock() + requests_mock.resolver_match.view_name = "view" + requests_mock.resolver_match.route = "route" + requests_mock.resolver_match.app_name = "app" + + trace_capture.return_value = { + "traceparent": "*traceparent='00-000000000000000000000000deadbeef-000000000000beef-00" + } + qw_instance = _QueryWrapper(requests_mock) + execute_mock_obj = MagicMock() + + input_query = MagicMock(as_string=lambda conn: "SELECT 2") + expected_query_start = "SELECT 2" + + qw_instance( + execute_mock_obj, + input_query, + MagicMock("test"), + MagicMock("test1"), + MagicMock(), + ) + output_sql = execute_mock_obj.call_args[0][0] + self.assertTrue( + str(output_sql).startswith(str(expected_query_start)), + f"Query should start with {expected_query_start!r}, got {output_sql!r}", + ) + @patch( "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware._QueryWrapper" ) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py index a3312bd77c..52a8949c47 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -274,6 +274,9 @@ def _before_cur_exec( commenter_data = self._get_commenter_data(conn) if self.enable_attribute_commenter: + # just to handle type safety + statement = str(statement) + # sqlcomment is added to executed query and db.statement span attribute statement = _add_sql_comment( statement, **commenter_data