From 4ffd2dae063d00ffe882d7afff26c4bb035345f0 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 28 Oct 2024 16:20:09 -0700 Subject: [PATCH 1/6] db-api populate_span after sqlcomment creation --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 0857d2989b..19928829c3 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -423,7 +423,6 @@ def traced_execution( with self._db_api_integration._tracer.start_as_current_span( name, kind=SpanKind.CLIENT ) as span: - self._populate_span(span, cursor, *args) if args and self._commenter_enabled: try: args_list = list(args) @@ -464,6 +463,9 @@ def traced_execution( _logger.exception( "Exception while generating sql comment: %s", exc ) + + self._populate_span(span, cursor, *args) + return query_method(*args, **kwargs) From 2975c4c69ffd61e1dd53e1e01f98c01352a952b9 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 29 Oct 2024 12:38:24 -0700 Subject: [PATCH 2/6] Add unit test --- .../tests/test_dbapi_integration.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index eb2d628a3a..1ac5392d03 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -14,6 +14,7 @@ import logging +import re from unittest import mock from opentelemetry import context @@ -275,6 +276,42 @@ def test_executemany_comment(self): 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_matches_db_statement_attribute(self): + connect_module = mock.MagicMock() + 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", + "testcomponent", + enable_commenter=True, + commenter_options={"db_driver": False, "dbapi_level": False}, + connect_module=connect_module, + ) + 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}'\*/", + ) + + cursor_span_id = re.search(r"[a-zA-Z0-9_]{16}", cursor.query).group() + db_statement_span_id = re.search(r"[a-zA-Z0-9_]{16}", span.attributes[SpanAttributes.DB_STATEMENT]).group() + self.assertEqual(cursor_span_id, db_statement_span_id) + def test_compatible_build_version_psycopg_psycopg2_libpq(self): connect_module = mock.MagicMock() connect_module.__version__ = mock.MagicMock() From 0e396aa3627ec6770930991fac70a0d60fd243a9 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 29 Oct 2024 12:39:53 -0700 Subject: [PATCH 3/6] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed4671d559..5e940fa939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2635](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2635)) - `opentelemetry-instrumentation` Add support for string based dotted module paths in unwrap ([#2919](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2919)) +- `opentelemetry-instrumentation-dbapi` Add sqlcomment to `db.statement` attribute + ([#2935](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2935)) ### Fixed From 769ded8f5de74662ca0b2754d02aa4563e70b17d Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 29 Oct 2024 12:54:42 -0700 Subject: [PATCH 4/6] Appease ruff --- .../tests/test_dbapi_integration.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 1ac5392d03..cef50d1881 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -309,7 +309,9 @@ def test_executemany_comment_matches_db_statement_attribute(self): ) cursor_span_id = re.search(r"[a-zA-Z0-9_]{16}", cursor.query).group() - db_statement_span_id = re.search(r"[a-zA-Z0-9_]{16}", span.attributes[SpanAttributes.DB_STATEMENT]).group() + db_statement_span_id = re.search( + r"[a-zA-Z0-9_]{16}", span.attributes[SpanAttributes.DB_STATEMENT] + ).group() self.assertEqual(cursor_span_id, db_statement_span_id) def test_compatible_build_version_psycopg_psycopg2_libpq(self): From 6a8e9620b1ef391d1b3a8034281e8023c4ba69c7 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 4 Nov 2024 12:18:13 -0800 Subject: [PATCH 5/6] Fix test --- .../tests/test_dbapi_integration.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 019cf6b4ea..ae595fb430 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -277,7 +277,7 @@ 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}'\*/;", ) - + def test_executemany_comment_non_pep_249_compliant(self): class MockConnectModule: def __getattr__(self, name): @@ -317,7 +317,7 @@ def test_executemany_comment_matches_db_statement_attribute(self): db_integration = dbapi.DatabaseApiIntegration( "testname", - "testcomponent", + "postgresql", enable_commenter=True, commenter_options={"db_driver": False, "dbapi_level": False}, connect_module=connect_module, @@ -344,7 +344,7 @@ def test_executemany_comment_matches_db_statement_attribute(self): r"[a-zA-Z0-9_]{16}", span.attributes[SpanAttributes.DB_STATEMENT] ).group() self.assertEqual(cursor_span_id, db_statement_span_id) - + def test_compatible_build_version_psycopg_psycopg2_libpq(self): connect_module = mock.MagicMock() connect_module.__name__ = "test" From 195a1b88640bc746c2790783b7e65aa185cf59fc Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 4 Nov 2024 12:34:20 -0800 Subject: [PATCH 6/6] _populate_span and sqlcomment only if span.is_recording --- .../instrumentation/dbapi/__init__.py | 91 ++++++++++--------- 1 file changed, 47 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 7edb17f097..fb6416f52e 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -492,50 +492,53 @@ def traced_execution( with self._db_api_integration._tracer.start_as_current_span( name, kind=SpanKind.CLIENT ) as span: - 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()) - - # 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) + 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() + ) + + # 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) return query_method(*args, **kwargs)