From 170fbc374258ec97c0b73bef366e5cb9304e7adb Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Wed, 13 Jul 2022 13:30:14 +0530 Subject: [PATCH 1/7] ; bug fix --- .../instrumentation/sqlalchemy/engine.py | 17 ++----- .../opentelemetry/instrumentation/utils.py | 47 +++++++++++++++---- 2 files changed, 42 insertions(+), 22 deletions(-) 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 910ff7c4f6..11c403b7d2 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -21,11 +21,10 @@ ) from opentelemetry.instrumentation.sqlalchemy.version import __version__ from opentelemetry.instrumentation.utils import ( - _generate_opentelemetry_traceparent, - _generate_sql_comment, + add_sql_comment, + get_opentelemetry_values, ) from opentelemetry.semconv.trace import NetTransportValues, SpanAttributes -from opentelemetry.trace import Span from opentelemetry.trace.status import Status, StatusCode @@ -127,18 +126,12 @@ def _before_cur_exec( context._otel_span = span if self.enable_commenter: - statement = statement + EngineTracer._generate_comment(span=span) + commenter_data = {} + commenter_data.update(get_opentelemetry_values()) + statement = add_sql_comment(statement, **commenter_data) return statement, params - @staticmethod - def _generate_comment(span: Span) -> str: - span_context = span.get_span_context() - meta = {} - if span_context.is_valid: - meta.update(_generate_opentelemetry_traceparent(span)) - return _generate_sql_comment(**meta) - # pylint: disable=unused-argument def _after_cur_exec(conn, cursor, statement, params, context, executemany): diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index fea7608388..6d116534a2 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -26,6 +26,15 @@ from opentelemetry.propagate import extract from opentelemetry.trace import Span, StatusCode +try: + from opentelemetry.trace.propagation.tracecontext import ( + TraceContextTextMapPropagator, + ) + + propagator = TraceContextTextMapPropagator() +except ImportError: + propagator = None + def extract_attributes_from_object( obj: any, attributes: Sequence[str], existing: Dict[str, str] = None @@ -119,24 +128,22 @@ def _start_internal_or_server_span( return span, token -_KEY_VALUE_DELIMITER = "," - - -def _generate_sql_comment(**meta): +def generate_sql_comment(**meta) -> str: """ Return a SQL comment with comma delimited key=value pairs created from **meta kwargs. """ + key_value_delimiter = "," + if not meta: # No entries added. return "" # Sort the keywords to ensure that caching works and that testing is # deterministic. It eases visual inspection as well. - # pylint: disable=consider-using-f-string return ( " /*" - + _KEY_VALUE_DELIMITER.join( - "{}={!r}".format(_url_quote(key), _url_quote(value)) + + key_value_delimiter.join( + f"{url_quote(key)}={url_quote(value)!r}" for key, value in sorted(meta.items()) if value is not None ) @@ -144,7 +151,7 @@ def _generate_sql_comment(**meta): ) -def _url_quote(s): # pylint: disable=invalid-name +def url_quote(s): # pylint: disable=invalid-name if not isinstance(s, (str, bytes)): return s quoted = urllib.parse.quote(s) @@ -155,6 +162,17 @@ def _url_quote(s): # pylint: disable=invalid-name return quoted.replace("%", "%%") +def get_opentelemetry_values(): + """ + Return the OpenTelemetry Trace and Span IDs if Span ID is set in the + OpenTelemetry execution context. + """ + # Insert the W3C TraceContext generated + _headers = {} + propagator.inject(_headers) + return _headers + + def _generate_opentelemetry_traceparent(span: Span) -> str: meta = {} _version = "00" @@ -170,5 +188,14 @@ def _python_path_without_directory(python_path, directory, path_separator): return sub( rf"{escape(directory)}{path_separator}(?!$)", "", - python_path, - ) + python_path,) + + +def add_sql_comment(sql, **meta) -> str: + comment = generate_sql_comment(**meta) + sql = sql.rstrip() + if sql[-1] == ';': + sql = sql[:-1] + comment + ';' + else: + sql = sql + comment + return sql From 532aff6343e001ca3d0c3a4ac9ea2ba0a5dbbac1 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Fri, 15 Jul 2022 18:18:19 +0530 Subject: [PATCH 2/7] Removing test for deleted function --- .../tests/test_sqlalchemy.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index 3b3c6d735b..9547b53e58 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -186,22 +186,3 @@ async def run(): ) asyncio.get_event_loop().run_until_complete(run()) - - def test_generate_commenter(self): - logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) - engine = create_engine("sqlite:///:memory:") - SQLAlchemyInstrumentor().instrument( - engine=engine, - tracer_provider=self.tracer_provider, - enable_commenter=True, - ) - - cnx = engine.connect() - cnx.execute("SELECT 1 + 1;").fetchall() - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 1) - span = spans[0] - self.assertIn( - EngineTracer._generate_comment(span), - self.caplog.records[-2].getMessage(), - ) From ec0b60e19265169309d36a67292f9fbd9a6c431f Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Wed, 20 Jul 2022 12:39:48 +0530 Subject: [PATCH 3/7] Add ; bug fix to django --- .../instrumentation/dbapi/__init__.py | 21 +++++++------------ .../tests/test_dbapi_integration.py | 11 +++++----- .../middleware/sqlcommenter_middleware.py | 6 +++--- .../tests/test_sqlcommenter.py | 5 +++-- .../instrumentation/sqlalchemy/engine.py | 4 ++-- .../opentelemetry/instrumentation/utils.py | 20 ++++++------------ 6 files changed, 26 insertions(+), 41 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 2559466221..d23cb73c52 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -46,9 +46,9 @@ from opentelemetry import trace as trace_api from opentelemetry.instrumentation.dbapi.version import __version__ from opentelemetry.instrumentation.utils import ( - _generate_opentelemetry_traceparent, - _generate_sql_comment, + _get_opentelemetry_values, unwrap, + _add_sql_comment, ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer @@ -375,14 +375,6 @@ def get_statement(self, cursor, args): # pylint: disable=no-self-use return statement.decode("utf8", "replace") return statement - @staticmethod - def _generate_comment(span: Span) -> str: - span_context = span.get_span_context() - meta = {} - if span_context.is_valid: - meta.update(_generate_opentelemetry_traceparent(span)) - # TODO(schekuri): revisit to enrich with info such as route, db_driver etc... - return _generate_sql_comment(**meta) def traced_execution( self, @@ -405,11 +397,12 @@ def traced_execution( self._populate_span(span, cursor, *args) if args and self._commenter_enabled: try: - comment = self._generate_comment(span) - if isinstance(args[0], bytes): - comment = comment.encode("utf8") args_list = list(args) - args_list[0] += comment + commenter_data = {} + commenter_data.update(_get_opentelemetry_values()) + 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( diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 2644cb2fcd..c06713016a 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -236,12 +236,11 @@ def test_executemany_comment(self): mock_connect, {}, {} ) cursor = mock_connection.cursor() - cursor.executemany("Test query") - spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 1) - span = spans_list[0] - comment = dbapi.CursorTracer._generate_comment(span) - self.assertIn(comment, cursor.query) + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;" + ) def test_callproc(self): db_integration = dbapi.DatabaseApiIntegration( 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 5fe51fca52..10c9898cfb 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 @@ -23,8 +23,8 @@ from django.db.backends.utils import CursorDebugWrapper from opentelemetry.instrumentation.utils import ( - _generate_sql_comment, _get_opentelemetry_values, + _add_sql_comment, ) from opentelemetry.trace.propagation.tracecontext import ( TraceContextTextMapPropagator, @@ -84,7 +84,8 @@ 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 - sql_comment = _generate_sql_comment( + sql = _add_sql_comment( + sql, # Information about the controller. controller=resolver_match.view_name if resolver_match and with_controller @@ -112,7 +113,6 @@ def __call__(self, execute: Type[T], sql, params, many, context) -> T: # See: # * https://github.com/basecamp/marginalia/issues/61 # * https://github.com/basecamp/marginalia/pull/80 - sql += sql_comment # Add the query to the query log if debugging. if isinstance(context["cursor"], CursorDebugWrapper): diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index b162cc1f2a..4cd5c602f8 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py @@ -25,6 +25,7 @@ SqlCommenter, _QueryWrapper, ) +from opentelemetry.instrumentation.utils import _get_opentelemetry_values from opentelemetry.test.wsgitestutil import WsgiTestBase DJANGO_2_0 = VERSION >= (2, 0) @@ -88,7 +89,7 @@ def test_query_wrapper(self, trace_capture): execute_mock_obj = MagicMock() qw_instance( execute_mock_obj, - "Select 1", + "Select 1;", MagicMock("test"), MagicMock("test1"), MagicMock(), @@ -97,7 +98,7 @@ def test_query_wrapper(self, trace_capture): self.assertEqual( output_sql, "Select 1 /*app_name='app',controller='view',route='route',traceparent='%%2Atraceparent%%3D%%2700-0000000" - "00000000000000000deadbeef-000000000000beef-00'*/", + "00000000000000000deadbeef-000000000000beef-00'*/;", ) @patch( 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 518230dd01..3c601b01d7 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -21,7 +21,7 @@ ) from opentelemetry.instrumentation.sqlalchemy.version import __version__ from opentelemetry.instrumentation.utils import ( - add_sql_comment, + _add_sql_comment, _get_opentelemetry_values, ) from opentelemetry.semconv.trace import NetTransportValues, SpanAttributes @@ -126,7 +126,7 @@ def _before_cur_exec( if self.enable_commenter: commenter_data = {} commenter_data.update(_get_opentelemetry_values()) - statement = add_sql_comment(statement, **commenter_data) + statement = _add_sql_comment(statement, **commenter_data) context._otel_span = span diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 6b8cdc2376..065e15410a 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -147,7 +147,7 @@ def _generate_sql_comment(**meta) -> str: ) -def _url_quote(s): # pylint: disable=invalid-name +def _url_quote(s) -> str: # pylint: disable=invalid-name if not isinstance(s, (str, bytes)): return s quoted = urllib.parse.quote(s) @@ -158,7 +158,7 @@ def _url_quote(s): # pylint: disable=invalid-name return quoted.replace("%", "%%") -def _get_opentelemetry_values(): +def _get_opentelemetry_values() -> dict: """ Return the OpenTelemetry Trace and Span IDs if Span ID is set in the OpenTelemetry execution context. @@ -169,17 +169,6 @@ def _get_opentelemetry_values(): return _headers -def _generate_opentelemetry_traceparent(span: Span) -> str: - meta = {} - _version = "00" - _span_id = trace.format_span_id(span.context.span_id) - _trace_id = trace.format_trace_id(span.context.trace_id) - _flags = str(trace.TraceFlags.SAMPLED) - _traceparent = _version + "-" + _trace_id + "-" + _span_id + "-" + _flags - meta.update({"traceparent": _traceparent}) - return meta - - def _python_path_without_directory(python_path, directory, path_separator): return sub( rf"{escape(directory)}{path_separator}(?!$)", @@ -187,7 +176,10 @@ def _python_path_without_directory(python_path, directory, path_separator): python_path,) -def add_sql_comment(sql, **meta) -> str: +def _add_sql_comment(sql, **meta) -> str: + """ + Appends comments to the sql statement and returns it + """ comment = _generate_sql_comment(**meta) sql = sql.rstrip() if sql[-1] == ';': From 981e6912cb7dc28c75b1628ef7412516e5dc87e9 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Wed, 20 Jul 2022 15:06:15 +0530 Subject: [PATCH 4/7] Added unittests --- .../tests/test_utils.py | 22 +++++++++++++++++++ .../postgres/test_psycopg_sqlcommenter.py | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index e6fe80a347..c558b1ab29 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -16,6 +16,7 @@ from http import HTTPStatus from opentelemetry.instrumentation.utils import ( + _add_sql_comment, _python_path_without_directory, http_status_to_status_code, ) @@ -152,3 +153,24 @@ def test_remove_current_directory_from_python_path_linux_only_path(self): python_path, directory, path_separator ) self.assertEqual(actual_python_path, python_path) + + def test_add_sql_comments_with_semicolon(self): + sql_query_without_semicolon = "Select 1;" + comments = {"comment_1": "value 1", "comment 2": "value 3"} + commented_sql_without_semicolon = _add_sql_comment(sql_query_without_semicolon, **comments) + + self.assertEqual(commented_sql_without_semicolon, "Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/;") + + def test_add_sql_comments_without_semicolon(self): + sql_query_without_semicolon = "Select 1" + comments = {"comment_1": "value 1", "comment 2": "value 3"} + commented_sql_without_semicolon = _add_sql_comment(sql_query_without_semicolon, **comments) + + self.assertEqual(commented_sql_without_semicolon, "Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/") + + def test_add_sql_comments_without_comments(self): + sql_query_without_semicolon = "Select 1" + comments = {} + commented_sql_without_semicolon = _add_sql_comment(sql_query_without_semicolon, **comments) + + self.assertEqual(commented_sql_without_semicolon, "Select 1") diff --git a/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_sqlcommenter.py b/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_sqlcommenter.py index 9c84792256..4f5679a56b 100644 --- a/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_sqlcommenter.py +++ b/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_sqlcommenter.py @@ -50,5 +50,5 @@ def test_commenter_enabled(self): self._cursor.execute("SELECT 1;") self.assertRegex( self._cursor.query.decode("ascii"), - r"SELECT 1; /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/", + r"SELECT 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) From c596d906c5a3189d24c2428be99e47759ac71eed Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Thu, 21 Jul 2022 12:01:38 +0530 Subject: [PATCH 5/7] Linting changes --- CHANGELOG.md | 2 ++ .../instrumentation/dbapi/__init__.py | 5 +++-- .../tests/test_dbapi_integration.py | 2 +- .../tests/test_sqlcommenter.py | 2 -- .../opentelemetry/instrumentation/utils.py | 7 +++--- .../tests/test_utils.py | 22 ++++++++++++++----- 6 files changed, 27 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20cdb6cb5f..793568e433 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc2-0.32b0...HEAD) - Adding multiple db connections support for django-instrumentation's sqlcommenter ([#1187](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1187)) +- SQLCommenter semicolon bug fix + ([#1200](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1200/files)) ### Added - `opentelemetry-instrumentation-redis` add support to instrument RedisCluster clients 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 d23cb73c52..4d573ec879 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -375,7 +375,6 @@ def get_statement(self, cursor, args): # pylint: disable=no-self-use return statement.decode("utf8", "replace") return statement - def traced_execution( self, cursor, @@ -400,7 +399,9 @@ def traced_execution( args_list = list(args) commenter_data = {} commenter_data.update(_get_opentelemetry_values()) - statement = _add_sql_comment(args_list[0], **commenter_data) + 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 c06713016a..1ec5cd9df2 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -239,7 +239,7 @@ def test_executemany_comment(self): cursor.executemany("Select 1;") self.assertRegex( cursor.query, - r"Select 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;" + r"Select 1 /\*traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) def test_callproc(self): diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index 0c398084fb..8245d990f7 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -21,8 +21,6 @@ class TestSqlalchemyInstrumentationWithSQLCommenter(TestBase): - - @pytest.fixture(autouse=True) def inject_fixtures(self, caplog): self.caplog = caplog # pylint: disable=attribute-defined-outside-init diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 065e15410a..728c3ceff1 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -173,7 +173,8 @@ def _python_path_without_directory(python_path, directory, path_separator): return sub( rf"{escape(directory)}{path_separator}(?!$)", "", - python_path,) + python_path, + ) def _add_sql_comment(sql, **meta) -> str: @@ -182,8 +183,8 @@ def _add_sql_comment(sql, **meta) -> str: """ comment = _generate_sql_comment(**meta) sql = sql.rstrip() - if sql[-1] == ';': - sql = sql[:-1] + comment + ';' + if sql[-1] == ";": + sql = sql[:-1] + comment + ";" else: sql = sql + comment return sql diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index c558b1ab29..3fd52ea7c6 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -157,20 +157,32 @@ def test_remove_current_directory_from_python_path_linux_only_path(self): def test_add_sql_comments_with_semicolon(self): sql_query_without_semicolon = "Select 1;" comments = {"comment_1": "value 1", "comment 2": "value 3"} - commented_sql_without_semicolon = _add_sql_comment(sql_query_without_semicolon, **comments) + commented_sql_without_semicolon = _add_sql_comment( + sql_query_without_semicolon, **comments + ) - self.assertEqual(commented_sql_without_semicolon, "Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/;") + self.assertEqual( + commented_sql_without_semicolon, + "Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/;", + ) def test_add_sql_comments_without_semicolon(self): sql_query_without_semicolon = "Select 1" comments = {"comment_1": "value 1", "comment 2": "value 3"} - commented_sql_without_semicolon = _add_sql_comment(sql_query_without_semicolon, **comments) + commented_sql_without_semicolon = _add_sql_comment( + sql_query_without_semicolon, **comments + ) - self.assertEqual(commented_sql_without_semicolon, "Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/") + self.assertEqual( + commented_sql_without_semicolon, + "Select 1 /*comment%%202='value%%203',comment_1='value%%201'*/", + ) def test_add_sql_comments_without_comments(self): sql_query_without_semicolon = "Select 1" comments = {} - commented_sql_without_semicolon = _add_sql_comment(sql_query_without_semicolon, **comments) + commented_sql_without_semicolon = _add_sql_comment( + sql_query_without_semicolon, **comments + ) self.assertEqual(commented_sql_without_semicolon, "Select 1") From 6458d2382a3b3590b65daf26a93f3ca07dd7cc57 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Thu, 21 Jul 2022 13:38:18 +0530 Subject: [PATCH 6/7] Linting changes --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 +- .../django/middleware/sqlcommenter_middleware.py | 2 +- 2 files changed, 2 insertions(+), 2 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 4d573ec879..23f6961ce4 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -46,9 +46,9 @@ from opentelemetry import trace as trace_api from opentelemetry.instrumentation.dbapi.version import __version__ from opentelemetry.instrumentation.utils import ( + _add_sql_comment, _get_opentelemetry_values, unwrap, - _add_sql_comment, ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer 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 10c9898cfb..89c1b93008 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 @@ -23,8 +23,8 @@ from django.db.backends.utils import CursorDebugWrapper from opentelemetry.instrumentation.utils import ( - _get_opentelemetry_values, _add_sql_comment, + _get_opentelemetry_values, ) from opentelemetry.trace.propagation.tracecontext import ( TraceContextTextMapPropagator, From 606a6387ac397588494f432a52f614c81750cb75 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Thu, 21 Jul 2022 14:49:42 +0530 Subject: [PATCH 7/7] Linting changes --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 +- .../tests/test_sqlcommenter.py | 1 - .../tests/test_sqlalchemy.py | 2 -- .../src/opentelemetry/instrumentation/utils.py | 2 +- 4 files changed, 2 insertions(+), 5 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 23f6961ce4..397a80b2bd 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -51,7 +51,7 @@ unwrap, ) from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer +from opentelemetry.trace import SpanKind, TracerProvider, get_tracer _logger = logging.getLogger(__name__) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py index 4cd5c602f8..f9b8ed5233 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_sqlcommenter.py @@ -25,7 +25,6 @@ SqlCommenter, _QueryWrapper, ) -from opentelemetry.instrumentation.utils import _get_opentelemetry_values from opentelemetry.test.wsgitestutil import WsgiTestBase DJANGO_2_0 = VERSION >= (2, 0) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index d240d49280..5fa74376f9 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. import asyncio -import logging from unittest import mock import pytest @@ -21,7 +20,6 @@ from opentelemetry import trace from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor -from opentelemetry.instrumentation.sqlalchemy.engine import EngineTracer from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import TracerProvider, export from opentelemetry.test.test_base import TestBase diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 728c3ceff1..95a943afb3 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -24,7 +24,7 @@ # pylint: disable=E0611 from opentelemetry.context import _SUPPRESS_INSTRUMENTATION_KEY # noqa: F401 from opentelemetry.propagate import extract -from opentelemetry.trace import Span, StatusCode +from opentelemetry.trace import StatusCode from opentelemetry.trace.propagation.tracecontext import ( TraceContextTextMapPropagator, )