Skip to content

Commit 50ab047

Browse files
aryabharattammy-baylis-swilzchenxrmx
authored
bug_fix(1477): type handling in _add_sql_comment (#3113)
--------- Co-authored-by: Tammy Baylis <[email protected]> Co-authored-by: Leighton Chen <[email protected]> Co-authored-by: Riccardo Magliocchetti <[email protected]>
1 parent 139d787 commit 50ab047

File tree

6 files changed

+83
-0
lines changed

6 files changed

+83
-0
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
- `opentelemetry-instrumentation` Fix client address is set to server address in new semconv
1717
([#3354](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3354))
18+
- `opentelemetry-instrumentation-dbapi`, `opentelemetry-instrumentation-django`,
19+
`opentelemetry-instrumentation-sqlalchemy`: Fix sqlcomment for non string query and composable object.
20+
([#3113](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3113))
1821

1922
## Version 1.31.0/0.52b0 (2025-03-12)
2023

instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py

+5
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,11 @@ def _update_args_with_added_sql_comment(self, args, cursor) -> tuple:
498498
args_list = list(args)
499499
self._capture_mysql_version(cursor)
500500
commenter_data = self._get_commenter_data()
501+
# Convert sql statement to string, handling psycopg2.sql.Composable object
502+
if hasattr(args_list[0], "as_string"):
503+
args_list[0] = args_list[0].as_string(cursor.connection)
504+
505+
args_list[0] = str(args_list[0])
501506
statement = _add_sql_comment(args_list[0], **commenter_data)
502507
args_list[0] = statement
503508
args = tuple(args_list)

instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py

+34
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,39 @@ def test_uninstrument_connection(self):
10741074
connection4 = dbapi.uninstrument_connection(mocked_conn)
10751075
self.assertIs(connection4, mocked_conn)
10761076

1077+
def test_non_string_sql_conversion(self):
1078+
"""Test that non-string SQL statements are converted to strings before adding comments."""
1079+
# Create a mock connect_module with required attributes
1080+
connect_module = mock.MagicMock()
1081+
connect_module.__name__ = "test"
1082+
connect_module.__version__ = "1.0"
1083+
connect_module.__libpq_version__ = 123
1084+
connect_module.apilevel = "2.0"
1085+
connect_module.threadsafety = 1
1086+
connect_module.paramstyle = "test"
1087+
1088+
db_integration = dbapi.DatabaseApiIntegration(
1089+
"testname",
1090+
"postgresql",
1091+
enable_commenter=True,
1092+
connect_module=connect_module,
1093+
)
1094+
mock_connection = db_integration.wrapped_connection(
1095+
mock_connect, {}, {}
1096+
)
1097+
cursor = mock_connection.cursor()
1098+
1099+
input_sql = mock.MagicMock(as_string=lambda conn: "SELECT 2")
1100+
expected_sql = "SELECT 2"
1101+
1102+
cursor.executemany(input_sql)
1103+
# Extract the SQL part (before the comment)
1104+
actual_sql = cursor.query.split("/*")[0].strip()
1105+
self.assertEqual(actual_sql, expected_sql)
1106+
1107+
spans_list = self.memory_exporter.get_finished_spans()
1108+
self.assertEqual(len(spans_list), 1)
1109+
10771110

10781111
# pylint: disable=unused-argument
10791112
def mock_connect(*args, **kwargs):
@@ -1100,6 +1133,7 @@ class MockCursor:
11001133
def __init__(self) -> None:
11011134
self.query = ""
11021135
self.params = None
1136+
self.connection = None
11031137
# Mock mysql.connector modules and method
11041138
self._cnx = mock.MagicMock()
11051139
self._cnx._cmysql = mock.MagicMock()

instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/sqlcommenter_middleware.py

+6
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ def __call__(self, execute: Type[T], sql, params, many, context) -> T:
8080
db_driver = context["connection"].settings_dict.get("ENGINE", "")
8181
resolver_match = self.request.resolver_match
8282

83+
# Convert sql statement to string, handling psycopg2.sql.Composable object
84+
if hasattr(sql, "as_string"):
85+
sql = sql.as_string(context["connection"])
86+
87+
sql = str(sql)
88+
8389
sql = _add_sql_comment(
8490
sql,
8591
# Information about the controller.

instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py

+32
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,38 @@ def test_query_wrapper(self, trace_capture):
131131
"00000000000000000deadbeef-000000000000beef-00'*/;",
132132
)
133133

134+
@patch(
135+
"opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware._get_opentelemetry_values"
136+
)
137+
def test_query_wrapper_non_string_queries(self, trace_capture):
138+
"""Test that non-string queries and psycopg2 composable objects are handled correctly."""
139+
requests_mock = MagicMock()
140+
requests_mock.resolver_match.view_name = "view"
141+
requests_mock.resolver_match.route = "route"
142+
requests_mock.resolver_match.app_name = "app"
143+
144+
trace_capture.return_value = {
145+
"traceparent": "*traceparent='00-000000000000000000000000deadbeef-000000000000beef-00"
146+
}
147+
qw_instance = _QueryWrapper(requests_mock)
148+
execute_mock_obj = MagicMock()
149+
150+
input_query = MagicMock(as_string=lambda conn: "SELECT 2")
151+
expected_query_start = "SELECT 2"
152+
153+
qw_instance(
154+
execute_mock_obj,
155+
input_query,
156+
MagicMock("test"),
157+
MagicMock("test1"),
158+
MagicMock(),
159+
)
160+
output_sql = execute_mock_obj.call_args[0][0]
161+
self.assertTrue(
162+
str(output_sql).startswith(str(expected_query_start)),
163+
f"Query should start with {expected_query_start!r}, got {output_sql!r}",
164+
)
165+
134166
@patch(
135167
"opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware._QueryWrapper"
136168
)

instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py

+3
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ def _before_cur_exec(
274274
commenter_data = self._get_commenter_data(conn)
275275

276276
if self.enable_attribute_commenter:
277+
# just to handle type safety
278+
statement = str(statement)
279+
277280
# sqlcomment is added to executed query and db.statement span attribute
278281
statement = _add_sql_comment(
279282
statement, **commenter_data

0 commit comments

Comments
 (0)