From b75a9773281aaab57021e101b02d1dea4fba71a3 Mon Sep 17 00:00:00 2001 From: bharatarya Date: Tue, 17 Dec 2024 22:45:09 +0530 Subject: [PATCH 1/9] bug_fix(1477): type handling in _add_sql_comment --- .../instrumentation/sqlcommenter_utils.py | 2 ++ opentelemetry-instrumentation/tests/test_utils.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py index 1eeefbf206..f3a217fd10 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py @@ -22,6 +22,8 @@ def _add_sql_comment(sql, **meta) -> str: """ meta.update(**_add_framework_tags()) comment = _generate_sql_comment(**meta) + # converting to str to handle any type errors + sql = str(sql) sql = sql.rstrip() if sql[-1] == ";": sql = sql[:-1] + comment + ";" diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index 5ddd45d692..e61a0fa774 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -208,6 +208,21 @@ def test_add_sql_comments_without_comments(self): self.assertEqual(commented_sql_without_semicolon, "Select 1") + def test_psycopg2_sql_composable(self): + """Test handling of psycopg2.sql.Composable input""" + # Mock psycopg2.sql.Composable object + class MockComposable: + def __str__(self): + return "SELECT * FROM table_name" + + sql_query = MockComposable() + comments = {"trace_id": "abc123"} + + result = _add_sql_comment(sql_query, **comments) + expected = "SELECT * FROM table_name /*trace_id='abc123'*/" + + self.assertEqual(result, expected) + def test_is_instrumentation_enabled_by_default(self): self.assertTrue(is_instrumentation_enabled()) self.assertTrue(is_http_instrumentation_enabled()) From 03561c80fc291d97833dcf55d0cb4d13f11010cb Mon Sep 17 00:00:00 2001 From: bharatarya Date: Tue, 17 Dec 2024 22:45:09 +0530 Subject: [PATCH 2/9] bug_fix(1477): type handling in _add_sql_comment --- .../instrumentation/sqlcommenter_utils.py | 2 ++ opentelemetry-instrumentation/tests/test_utils.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py index 1eeefbf206..f3a217fd10 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py @@ -22,6 +22,8 @@ def _add_sql_comment(sql, **meta) -> str: """ meta.update(**_add_framework_tags()) comment = _generate_sql_comment(**meta) + # converting to str to handle any type errors + sql = str(sql) sql = sql.rstrip() if sql[-1] == ";": sql = sql[:-1] + comment + ";" diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index 5ddd45d692..e61a0fa774 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -208,6 +208,21 @@ def test_add_sql_comments_without_comments(self): self.assertEqual(commented_sql_without_semicolon, "Select 1") + def test_psycopg2_sql_composable(self): + """Test handling of psycopg2.sql.Composable input""" + # Mock psycopg2.sql.Composable object + class MockComposable: + def __str__(self): + return "SELECT * FROM table_name" + + sql_query = MockComposable() + comments = {"trace_id": "abc123"} + + result = _add_sql_comment(sql_query, **comments) + expected = "SELECT * FROM table_name /*trace_id='abc123'*/" + + self.assertEqual(result, expected) + def test_is_instrumentation_enabled_by_default(self): self.assertTrue(is_instrumentation_enabled()) self.assertTrue(is_http_instrumentation_enabled()) From ba3b51ae40c97c2e53fec99fac809602b6b4f8ac Mon Sep 17 00:00:00 2001 From: bharatarya Date: Tue, 17 Dec 2024 22:45:09 +0530 Subject: [PATCH 3/9] bug_fix(1477): type handling in _add_sql_comment --- .../instrumentation/sqlcommenter_utils.py | 2 ++ opentelemetry-instrumentation/tests/test_utils.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py index 1eeefbf206..f3a217fd10 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py @@ -22,6 +22,8 @@ def _add_sql_comment(sql, **meta) -> str: """ meta.update(**_add_framework_tags()) comment = _generate_sql_comment(**meta) + # converting to str to handle any type errors + sql = str(sql) sql = sql.rstrip() if sql[-1] == ";": sql = sql[:-1] + comment + ";" diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index 5ddd45d692..e61a0fa774 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -208,6 +208,21 @@ def test_add_sql_comments_without_comments(self): self.assertEqual(commented_sql_without_semicolon, "Select 1") + def test_psycopg2_sql_composable(self): + """Test handling of psycopg2.sql.Composable input""" + # Mock psycopg2.sql.Composable object + class MockComposable: + def __str__(self): + return "SELECT * FROM table_name" + + sql_query = MockComposable() + comments = {"trace_id": "abc123"} + + result = _add_sql_comment(sql_query, **comments) + expected = "SELECT * FROM table_name /*trace_id='abc123'*/" + + self.assertEqual(result, expected) + def test_is_instrumentation_enabled_by_default(self): self.assertTrue(is_instrumentation_enabled()) self.assertTrue(is_http_instrumentation_enabled()) From 341899ddb19de527a441b2afb4ec4161eb550371 Mon Sep 17 00:00:00 2001 From: aryabharat Date: Tue, 4 Feb 2025 10:50:45 +0530 Subject: [PATCH 4/9] bug_fix(1477): updated type handling for _add_sql_comment --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 ++ .../django/middleware/sqlcommenter_middleware.py | 2 ++ .../src/opentelemetry/instrumentation/sqlalchemy/engine.py | 6 +++--- 3 files changed, 7 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 1c9f34fadb..6f51a929a8 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -491,6 +491,8 @@ def _update_args_with_added_sql_comment(self, args, cursor) -> tuple: # 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-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py index 9fe34ea2ae..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 @@ -84,6 +84,8 @@ def __call__(self, execute: Type[T], sql, params, many, context) -> T: 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-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py index f0d502487f..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,9 +274,9 @@ def _before_cur_exec( commenter_data = self._get_commenter_data(conn) if self.enable_attribute_commenter: - # Convert sql statement to string, handling psycopg2.sql.Composable object - if hasattr(statement, "as_string"): - statement = statement.as_string(conn) + # 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 From 226c8e6a1bbe1ba7b88e9b1856f3178407629fff Mon Sep 17 00:00:00 2001 From: aryabharat Date: Tue, 4 Feb 2025 10:50:45 +0530 Subject: [PATCH 5/9] bug_fix(1477): updated type handling for _add_sql_comment --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 5 +++++ .../django/middleware/sqlcommenter_middleware.py | 6 ++++++ .../src/opentelemetry/instrumentation/sqlalchemy/engine.py | 3 +++ 3 files changed, 14 insertions(+) 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 27aafc7308..6f51a929a8 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -488,6 +488,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-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-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 From 2c8d0084a85f66ed11251fb5bd7d145c9d89d788 Mon Sep 17 00:00:00 2001 From: aryabharat Date: Sat, 15 Feb 2025 23:38:52 +0530 Subject: [PATCH 6/9] bug_fix(1477): added unit tests --- .../tests/test_dbapi_integration.py | 34 +++++++++++++++++++ .../tests/test_sqlcommenter.py | 32 +++++++++++++++++ 2 files changed, 66 insertions(+) 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/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index eec02d7a54..61c3b8048c 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" ) From 592903c85fe0e703c688c8fdd9c71465992b93ea Mon Sep 17 00:00:00 2001 From: Bharat Arya Date: Wed, 19 Feb 2025 21:19:44 +0530 Subject: [PATCH 7/9] updated change log file --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 101cafd361..5f2d72ca1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `opentelemetry-instrumentation-dbapi` Add support for non string query and composable object. + ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) +- `opentelemetry-instrumentation-django` Add support for non string query and composable object. + ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) +- `opentelemetry-instrumentation-sqlalchemy` Add support for non string query. + ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) - `opentelemetry-instrumentation-botocore` Add support for GenAI user events and lazy initialize tracer ([#3258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3258)) - `opentelemetry-instrumentation-botocore` Add support for GenAI system events From 113222a58021a262f404e16236dd1cf1abb7ed53 Mon Sep 17 00:00:00 2001 From: aryabharat Date: Sun, 23 Feb 2025 11:09:09 +0530 Subject: [PATCH 8/9] bug_fix(1477): updated change log. --- CHANGELOG.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f2d72ca1e..929996ae33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,12 +13,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `opentelemetry-instrumentation-dbapi` Add support for non string query and composable object. - ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) -- `opentelemetry-instrumentation-django` Add support for non string query and composable object. - ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) -- `opentelemetry-instrumentation-sqlalchemy` Add support for non string query. - ([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113)) - `opentelemetry-instrumentation-botocore` Add support for GenAI user events and lazy initialize tracer ([#3258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3258)) - `opentelemetry-instrumentation-botocore` Add support for GenAI system events @@ -26,6 +20,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `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)) - `opentelemetry-instrumentation-redis` Add missing entry in doc string for `def _instrument` ([#3247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3247)) - `opentelemetry-instrumentation-asyncpg` Fix fallback for empty queries. From b1e84d91f91806144184cc814dfa22e89fe336c7 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Wed, 26 Mar 2025 18:09:46 +0100 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 054a66456f..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) @@ -43,9 +46,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `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)) - `opentelemetry-instrumentation-redis` Add missing entry in doc string for `def _instrument` ([#3247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3247)) - `opentelemetry-instrumentation-botocore` sns-extension: Change destination name attribute