diff --git a/CHANGELOG.md b/CHANGELOG.md index c136481511..15fc2248c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2976](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2976)) - Add `opentelemetry-instrumentation-openai-v2` to `opentelemetry-bootstrap` ([#2996](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2996)) +- `opentelemetry-instrumentation-sqlalchemy` Add sqlcomment to `db.statement` attribute + ([#2937](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2937)) +- `opentelemetry-instrumentation-dbapi` Add sqlcomment to `db.statement` attribute + ([#2935](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2935)) - `opentelemetry-instrumentation-mysql` Add sqlcommenter support - ([#2943](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2943)) + ([#3023](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3023)) ### Fixed diff --git a/instrumentation/opentelemetry-instrumentation-confluent-kafka/README.rst b/instrumentation/opentelemetry-instrumentation-confluent-kafka/README.rst index 163c2a4393..1ce6dcbd26 100644 --- a/instrumentation/opentelemetry-instrumentation-confluent-kafka/README.rst +++ b/instrumentation/opentelemetry-instrumentation-confluent-kafka/README.rst @@ -19,5 +19,5 @@ Installation References ---------- -* `OpenTelemetry confluent-kafka/ Tracing `_ +* `OpenTelemetry confluent-kafka/ Tracing `_ * `OpenTelemetry Project `_ 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 79e69a1439..da99167444 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -499,49 +499,54 @@ 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) - - # 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 - ) + 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) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index e29a8ad380..ae595fb430 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 @@ -306,6 +307,44 @@ def __getattr__(self, name): r"Select 1 /\*dbapi_level='1.0',dbapi_threadsafety='unknown',driver_paramstyle='unknown',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", + "postgresql", + 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.__name__ = "test" 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 172c1193f3..b64af796d1 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -219,28 +219,31 @@ def _before_cur_exec( ) with trace.use_span(span, end_on_exit=False): if span.is_recording(): + if self.enable_commenter: + commenter_data = { + "db_driver": conn.engine.driver, + # Driver/framework centric information. + "db_framework": f"sqlalchemy:{sqlalchemy.__version__}", + } + + 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(statement, **commenter_data) + span.set_attribute(SpanAttributes.DB_STATEMENT, statement) span.set_attribute(SpanAttributes.DB_SYSTEM, self.vendor) for key, value in attrs.items(): span.set_attribute(key, value) - if self.enable_commenter: - commenter_data = { - "db_driver": conn.engine.driver, - # Driver/framework centric information. - "db_framework": f"sqlalchemy:{sqlalchemy.__version__}", - } - - 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(statement, **commenter_data) context._otel_span = span diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index ec2fc51e5b..8490721e3e 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +import re import pytest from sqlalchemy import ( @@ -21,6 +22,7 @@ from opentelemetry import context from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor +from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase @@ -59,6 +61,38 @@ def test_sqlcommenter_enabled(self): r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + def test_sqlcommenter_enabled_matches_db_statement_attribute(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=True, + commenter_options={"db_framework": False}, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + query_log = self.caplog.records[-2].getMessage() + self.assertRegex( + query_log, + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + cnx_span_id = re.search(r"[a-zA-Z0-9_]{16}", query_log).group() + db_statement_span_id = re.search( + r"[a-zA-Z0-9_]{16}", + query_span.attributes[SpanAttributes.DB_STATEMENT], + ).group() + self.assertEqual(cnx_span_id, db_statement_span_id) + def test_sqlcommenter_enabled_otel_values_false(self): engine = create_engine("sqlite:///:memory:") SQLAlchemyInstrumentor().instrument(