From cdc70c91b26754e0b8985dcb4a8bb061b0171c54 Mon Sep 17 00:00:00 2001 From: Selcuk Ayguney Date: Fri, 28 Feb 2025 15:26:00 +1000 Subject: [PATCH 1/3] Handle empty strings Safer check for a trailing semicolon that could handle empty strings as well. --- .../src/opentelemetry/instrumentation/sqlcommenter_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py index 1eeefbf206..18630d9187 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/sqlcommenter_utils.py @@ -23,7 +23,7 @@ def _add_sql_comment(sql, **meta) -> str: meta.update(**_add_framework_tags()) comment = _generate_sql_comment(**meta) sql = sql.rstrip() - if sql[-1] == ";": + if sql.endswith(";"): sql = sql[:-1] + comment + ";" else: sql = sql + comment From 18bbfddcfa1537ba48f0c1223e758539bdc86d96 Mon Sep 17 00:00:00 2001 From: Selcuk Ayguney Date: Fri, 28 Feb 2025 15:35:00 +1000 Subject: [PATCH 2/3] Updated CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 280d0dbe46..5ad1a6330f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3247)) - `opentelemetry-instrumentation-asyncpg` Fix fallback for empty queries. ([#3253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3253)) +- `opentelemetry-instrumentation` Fix a traceback in sqlcommenter when psycopg connection pooling is enabled. + ([#3309](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3309)) ## Version 1.30.0/0.51b0 (2025-02-03) From 7be5095200b2432acada4743ffe2c1b73940ddc9 Mon Sep 17 00:00:00 2001 From: Selcuk Ayguney Date: Sat, 8 Mar 2025 13:39:08 +1000 Subject: [PATCH 3/3] Test for empty SQL strings in sqlcommenter --- .../tests/test_sqlcommenter.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index eec02d7a54..a21cc36ceb 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py @@ -146,3 +146,31 @@ def test_multiple_connection_support(self, query_wrapper): # check if query_wrapper is added to the context for 2 databases self.assertEqual(query_wrapper.call_count, 2) + + @patch( + "opentelemetry.instrumentation.django.middleware.sqlcommenter_middleware._get_opentelemetry_values" + ) + def test_empty_sql(self, trace_capture): + 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() + qw_instance( + execute_mock_obj, + "", + MagicMock("test"), + MagicMock("test1"), + MagicMock(), + ) + output_sql = execute_mock_obj.call_args[0][0] + self.assertEqual( + output_sql, + " /*app_name='app',controller='view',route='route',traceparent='%%2Atraceparent%%3D%%2700-0000000" + "00000000000000000deadbeef-000000000000beef-00'*/", + )