From 48f238cba005edd719481cf9b56fb0e66e4606d2 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 16 Dec 2024 14:11:43 -0800 Subject: [PATCH 01/12] db.statement inclusion of sqlcomment as opt-in --- .../instrumentation/sqlalchemy/__init__.py | 42 ++++++++++++++++++- .../instrumentation/sqlalchemy/engine.py | 27 ++++++++++-- .../tests/test_sqlcommenter.py | 34 ++++++++++++++- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 4182c0034e..92dbd33355 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -65,6 +65,28 @@ :: Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ +SQLComment in span attribute +**************************** +If sqlcommenter is enabled, you can optionally configure SQLAlchemy instrumentation to append sqlcomment to query span attribute for convenience of your platform. + +.. code:: python + + from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor + + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={}, + enable_attribute_commenter=True, + ) + + +For example, +:: + + Invoking engine.execute("select * from auth_users") will lead to sql query "select * from auth_users" but when SQLCommenter and attribute_commenter is enabled + the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. + + Usage ----- .. code:: python @@ -138,6 +160,7 @@ def _instrument(self, **kwargs): ``meter_provider``: a MeterProvider, defaults to global ``enable_commenter``: bool to enable sqlcommenter, defaults to False ``commenter_options``: dict of sqlcommenter config, defaults to {} + ``enable_attribute_commenter``: bool to enable sqlcomment addition to span attribute, defaults to False. Must also set `enable_commenter`. Returns: An instrumented engine if passed in as an argument or list of instrumented engines, None otherwise. @@ -166,19 +189,30 @@ def _instrument(self, **kwargs): enable_commenter = kwargs.get("enable_commenter", False) commenter_options = kwargs.get("commenter_options", {}) + enable_attribute_commenter = kwargs.get( + "enable_attribute_commenter", False + ) _w( "sqlalchemy", "create_engine", _wrap_create_engine( - tracer, connections_usage, enable_commenter, commenter_options + tracer, + connections_usage, + enable_commenter, + commenter_options, + enable_attribute_commenter, ), ) _w( "sqlalchemy.engine", "create_engine", _wrap_create_engine( - tracer, connections_usage, enable_commenter, commenter_options + tracer, + connections_usage, + enable_commenter, + commenter_options, + enable_attribute_commenter, ), ) # sqlalchemy.engine.create is not present in earlier versions of sqlalchemy (which we support) @@ -191,6 +225,7 @@ def _instrument(self, **kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ), ) _w( @@ -207,6 +242,7 @@ def _instrument(self, **kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ), ) if kwargs.get("engine") is not None: @@ -216,6 +252,7 @@ def _instrument(self, **kwargs): connections_usage, kwargs.get("enable_commenter", False), kwargs.get("commenter_options", {}), + kwargs.get("enable_attribute_commenter", False), ) if kwargs.get("engines") is not None and isinstance( kwargs.get("engines"), Sequence @@ -227,6 +264,7 @@ def _instrument(self, **kwargs): connections_usage, kwargs.get("enable_commenter", False), kwargs.get("commenter_options", {}), + kwargs.get("enable_attribute_commenter", False), ) for engine in kwargs.get("engines") ] 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 a20e481819..a8fa4ad089 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -43,7 +43,11 @@ def _normalize_vendor(vendor): def _wrap_create_async_engine( - tracer, connections_usage, enable_commenter=False, commenter_options=None + tracer, + connections_usage, + enable_commenter=False, + commenter_options=None, + enable_attribute_commenter=False, ): # pylint: disable=unused-argument def _wrap_create_async_engine_internal(func, module, args, kwargs): @@ -57,6 +61,7 @@ def _wrap_create_async_engine_internal(func, module, args, kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ) return engine @@ -64,7 +69,11 @@ def _wrap_create_async_engine_internal(func, module, args, kwargs): def _wrap_create_engine( - tracer, connections_usage, enable_commenter=False, commenter_options=None + tracer, + connections_usage, + enable_commenter=False, + commenter_options=None, + enable_attribute_commenter=False, ): def _wrap_create_engine_internal(func, _module, args, kwargs): """Trace the SQLAlchemy engine, creating an `EngineTracer` @@ -77,6 +86,7 @@ def _wrap_create_engine_internal(func, _module, args, kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ) return engine @@ -110,12 +120,14 @@ def __init__( connections_usage, enable_commenter=False, commenter_options=None, + enable_attribute_commenter=False, ): self.tracer = tracer self.connections_usage = connections_usage self.vendor = _normalize_vendor(engine.name) self.enable_commenter = enable_commenter self.commenter_options = commenter_options if commenter_options else {} + self.enable_attribute_commenter = enable_attribute_commenter self._engine_attrs = _get_attributes_from_engine(engine) self._leading_comment_remover = re.compile(r"^/\*.*?\*/") @@ -251,13 +263,22 @@ def _before_cur_exec( if self.commenter_options.get(k, True) } - statement = _add_sql_comment(statement, **commenter_data) + if self.enable_attribute_commenter: + 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 + and not self.enable_attribute_commenter + ): + statement = _add_sql_comment(statement, **commenter_data) + context._otel_span = span return statement, params diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index 8490721e3e..866409cc4e 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -61,13 +61,45 @@ 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): + def test_sqlcommenter_enabled_stmt_disabled_default_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}, + # enable_attribute_commenter not set + ) + 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_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}, + enable_attribute_commenter=True, ) cnx = engine.connect() cnx.execute(text("SELECT 1;")).fetchall() From fc8acd4bbab211c09dbbd11e2347c06742187e07 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 16 Dec 2024 14:13:45 -0800 Subject: [PATCH 02/12] Changlog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b8d39ff0e..59e229e14b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2969](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2969)) - `opentelemetry-instrumentation-httpx`: remove private unused `_InstrumentedClient` and `_InstrumentedAsyncClient` classes ([#3036](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3036)) +- `opentelemetry-instrumentation-sqlalchemy` including sqlcomment in `db.statement` span attribute value is now opt-in + ([#3112](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3112)) ## Version 1.28.0/0.49b0 (2024-11-05) From a05acca14efa94de5531c52f7ab9aa42bbe98839 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 11:38:18 -0800 Subject: [PATCH 03/12] Add tests --- .../tests/test_sqlcommenter.py | 174 +++++++++++++++++- 1 file changed, 173 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index 866409cc4e..d8144dadc1 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -61,7 +61,59 @@ 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_stmt_disabled_default_matches_db_statement_attribute( + def test_sqlcommenter_default_stmt_enabled_no_comments_anywhere(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + # enable_commenter not set + enable_attribute_commenter=True, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + query_log = self.caplog.records[-2].getMessage() + self.assertEqual( + query_log, + "SELECT 1;", + ) + 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_disabled_stmt_enabled_no_comments_anywhere(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=False, + enable_attribute_commenter=True, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + query_log = self.caplog.records[-2].getMessage() + self.assertEqual( + query_log, + "SELECT 1;", + ) + 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_disabled_default( self, ): engine = create_engine("sqlite:///:memory:") @@ -142,6 +194,45 @@ def test_sqlcommenter_enabled_otel_values_false(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)'\*/;", ) + 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_enabled_otel_values_false(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=True, + commenter_options={ + "db_framework": False, + "opentelemetry_values": False, + }, + enable_attribute_commenter=True, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) + 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='(.*)'*/;", + ) def test_sqlcommenter_flask_integration(self): engine = create_engine("sqlite:///:memory:") @@ -164,6 +255,49 @@ def test_sqlcommenter_flask_integration(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)',flask=1,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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_stmt_enabled_flask_integration(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=True, + commenter_options={"db_framework": False}, + enable_attribute_commenter=True, + ) + cnx = engine.connect() + + current_context = context.get_current() + sqlcommenter_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {"flask": 1}, current_context + ) + context.attach(sqlcommenter_context) + + cnx.execute(text("SELECT 1;")).fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)',flask=1,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='(.*)',flask=1,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_sqlcommenter_enabled_create_engine_after_instrumentation(self): SQLAlchemyInstrumentor().instrument( @@ -179,6 +313,44 @@ def test_sqlcommenter_enabled_create_engine_after_instrumentation(self): self.caplog.records[-2].getMessage(), 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_enabled_create_engine_after_instrumentation( + self, + ): + SQLAlchemyInstrumentor().instrument( + tracer_provider=self.tracer_provider, + enable_commenter=True, + enable_attribute_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + 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}'\*/;", + ) def test_sqlcommenter_disabled_create_engine_after_instrumentation(self): SQLAlchemyInstrumentor().instrument( From d6b625e4bf4ee562a5743131c12a3b2bb825dbbc Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 11:56:54 -0800 Subject: [PATCH 04/12] Add tests --- .../tests/test_sqlalchemy.py | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index 27a253decb..59a51baa29 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -209,6 +209,44 @@ def test_create_engine_wrapper_enable_commenter(self): self.caplog.records[-2].getMessage(), 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_create_engine_wrapper_enable_commenter_stmt_enabled(self): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={"db_framework": False}, + enable_attribute_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + # sqlcommenter + self.assertRegex( + self.caplog.records[-2].getMessage(), + 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}'\*/;", + ) def test_create_engine_wrapper_enable_commenter_otel_values_false(self): logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) @@ -229,6 +267,49 @@ def test_create_engine_wrapper_enable_commenter_otel_values_false(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)'\*/;", ) + 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_create_engine_wrapper_enable_commenter_stmt_enabled_otel_values_false( + self, + ): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={ + "db_framework": False, + "opentelemetry_values": False, + }, + enable_attribute_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + # sqlcommenter + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) + 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='(.*)'\*/;", + ) def test_custom_tracer_provider(self): provider = TracerProvider( @@ -321,6 +402,55 @@ async def run(): self.caplog.records[1].getMessage(), 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + asyncio.get_event_loop().run_until_complete(run()) + + @pytest.mark.skipif( + not sqlalchemy.__version__.startswith("1.4"), + reason="only run async tests for 1.4", + ) + def test_create_async_engine_wrapper_enable_commenter_stmt_enabled(self): + async def run(): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={ + "db_framework": False, + }, + enable_attribute_commenter=True, + ) + from sqlalchemy.ext.asyncio import ( # pylint: disable-all + create_async_engine, + ) + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.connect() as cnx: + await cnx.execute(text("SELECT 1;")) + # sqlcommenter + self.assertRegex( + self.caplog.records[1].getMessage(), + 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}'\*/;", + ) asyncio.get_event_loop().run_until_complete(run()) @@ -352,6 +482,57 @@ async def run(): self.caplog.records[1].getMessage(), r"SELECT 1 /\*db_driver='(.*)'\*/;", ) + 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.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + asyncio.get_event_loop().run_until_complete(run()) + + @pytest.mark.skipif( + not sqlalchemy.__version__.startswith("1.4"), + reason="only run async tests for 1.4", + ) + def test_create_async_engine_wrapper_enable_commenter_stmt_enabled_otel_values_false( + self, + ): + async def run(): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={ + "db_framework": False, + "opentelemetry_values": False, + }, + ) + from sqlalchemy.ext.asyncio import ( # pylint: disable-all + create_async_engine, + ) + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.connect() as cnx: + await cnx.execute(text("SELECT 1;")) + # sqlcommenter + self.assertRegex( + self.caplog.records[1].getMessage(), + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) + 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='(.*)'*/;", + ) asyncio.get_event_loop().run_until_complete(run()) From 978cfa9b7ffa1de81fea3c83497e48a6cabd2b63 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 12:09:04 -0800 Subject: [PATCH 05/12] Fix test --- .../tests/test_sqlalchemy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index 59a51baa29..8f5d0f2a94 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -510,6 +510,7 @@ async def run(): "db_framework": False, "opentelemetry_values": False, }, + enable_attribute_commenter=True, ) from sqlalchemy.ext.asyncio import ( # pylint: disable-all create_async_engine, From 63c5f121f189465206eb9498fa24fa3559a86afa Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Tue, 17 Dec 2024 17:03:34 -0800 Subject: [PATCH 06/12] Fix changelog --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85883d1965..de3f80b97f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-httpx` Fix `RequestInfo`/`ResponseInfo` type hints ([#3105](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3105)) +### Breaking changes + +- `opentelemetry-instrumentation-sqlalchemy` including sqlcomment in `db.statement` span attribute value is now opt-in + ([#3112](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3112)) ## Version 1.29.0/0.50b0 (2024-12-11) @@ -74,8 +78,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2969](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2969)) - `opentelemetry-instrumentation-httpx`: remove private unused `_InstrumentedClient` and `_InstrumentedAsyncClient` classes ([#3036](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3036)) -- `opentelemetry-instrumentation-sqlalchemy` including sqlcomment in `db.statement` span attribute value is now opt-in - ([#3112](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3112)) ## Version 1.28.0/0.49b0 (2024-11-05) From 888059fbef9754193d27d63828a42e3b987e6a72 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 18 Dec 2024 16:39:56 -0800 Subject: [PATCH 07/12] Rearrange sqlcomment and span attr setup --- .../instrumentation/sqlalchemy/engine.py | 74 ++++++++++++------- 1 file changed, 48 insertions(+), 26 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 a8fa4ad089..e2ea00daa2 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -230,6 +230,25 @@ def _operation_name(self, db_name, statement): return self.vendor return " ".join(parts) + def _get_commenter_data(self, conn) -> dict: + """Calculate sqlcomment contents from conn and configured options""" + 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) + } + return commenter_data + def _before_cur_exec( self, conn, cursor, statement, params, context, _executemany ): @@ -245,39 +264,42 @@ 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) - } + commenter_data = self._get_commenter_data(conn) if self.enable_attribute_commenter: + # sqlcomment in executed query and span attribute 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) - 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) + else: + # sqlcomment in executed query only + 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) + statement = _add_sql_comment( + statement, **commenter_data + ) - if ( - self.enable_commenter - and not self.enable_attribute_commenter - ): - statement = _add_sql_comment(statement, **commenter_data) + else: + # no sqlcomment + 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) context._otel_span = span From 7bcc404068fdc2489c1e55acfa9a97e33b0a149e Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 18 Dec 2024 16:48:41 -0800 Subject: [PATCH 08/12] More --- .../instrumentation/sqlalchemy/engine.py | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 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 e2ea00daa2..a3312bd77c 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -249,6 +249,13 @@ def _get_commenter_data(self, conn) -> dict: } return commenter_data + def _set_db_client_span_attributes(self, span, statement, attrs) -> None: + """Uses statement and attrs to set attributes of provided Otel span""" + 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) + def _before_cur_exec( self, conn, cursor, statement, params, context, _executemany ): @@ -267,39 +274,27 @@ def _before_cur_exec( commenter_data = self._get_commenter_data(conn) if self.enable_attribute_commenter: - # sqlcomment in executed query and span attribute + # sqlcomment is added to executed query and db.statement span attribute statement = _add_sql_comment( statement, **commenter_data ) - span.set_attribute( - SpanAttributes.DB_STATEMENT, statement - ) - span.set_attribute( - SpanAttributes.DB_SYSTEM, self.vendor + self._set_db_client_span_attributes( + span, statement, attrs ) - for key, value in attrs.items(): - span.set_attribute(key, value) else: - # sqlcomment in executed query only - span.set_attribute( - SpanAttributes.DB_STATEMENT, statement - ) - span.set_attribute( - SpanAttributes.DB_SYSTEM, self.vendor + # sqlcomment is only added to executed query + # so db.statement is set before add_sql_comment + self._set_db_client_span_attributes( + span, statement, attrs ) - for key, value in attrs.items(): - span.set_attribute(key, value) statement = _add_sql_comment( statement, **commenter_data ) else: - # no sqlcomment - 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) + # no sqlcomment anywhere + self._set_db_client_span_attributes(span, statement, attrs) context._otel_span = span From 3b97e641cee718ede49052a10e86096b9c2500c2 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Fri, 20 Dec 2024 08:56:11 -0800 Subject: [PATCH 09/12] Update doc with cardinality warning --- .../src/opentelemetry/instrumentation/sqlalchemy/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 92dbd33355..3676e9770f 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -86,6 +86,8 @@ Invoking engine.execute("select * from auth_users") will lead to sql query "select * from auth_users" but when SQLCommenter and attribute_commenter is enabled the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. +Warning: capture of sqlcomment in ``db.statement`` or ``db.query.text`` may have high cardinality without platform normalization. See `Semantic Conventions for database spans `_ for more information. + Usage ----- From 8f45aaf1ee8b780da5381a672593b7aaf41122c5 Mon Sep 17 00:00:00 2001 From: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:43:53 -0800 Subject: [PATCH 10/12] Update instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py Co-authored-by: Leighton Chen --- .../src/opentelemetry/instrumentation/sqlalchemy/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 3676e9770f..d951d013ce 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -83,7 +83,7 @@ For example, :: - Invoking engine.execute("select * from auth_users") will lead to sql query "select * from auth_users" but when SQLCommenter and attribute_commenter is enabled + Invoking `engine.execute("select * from auth_users")` will lead to sql query "select * from auth_users" but when SQLCommenter and `attribute_commenter` is enabled the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. Warning: capture of sqlcomment in ``db.statement`` or ``db.query.text`` may have high cardinality without platform normalization. See `Semantic Conventions for database spans `_ for more information. From 3f21e5e52e7fa38189fdec68fecb2df70ff1bb38 Mon Sep 17 00:00:00 2001 From: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:44:07 -0800 Subject: [PATCH 11/12] Update instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py Co-authored-by: Leighton Chen --- .../src/opentelemetry/instrumentation/sqlalchemy/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index d951d013ce..14a5ffd9d2 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -67,7 +67,7 @@ SQLComment in span attribute **************************** -If sqlcommenter is enabled, you can optionally configure SQLAlchemy instrumentation to append sqlcomment to query span attribute for convenience of your platform. +If sqlcommenter is enabled, you can further configure SQLAlchemy instrumentation to append sqlcomment to the `db.statement` span attribute for convenience of your platform. .. code:: python From 2c8dbe353e34d8dcf0db7c0e8e3211921c7dfb9c Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Fri, 20 Dec 2024 14:48:28 -0800 Subject: [PATCH 12/12] Rm db.query.text from docs until semconv upgrade --- .../src/opentelemetry/instrumentation/sqlalchemy/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 14a5ffd9d2..23b68a6c52 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -86,7 +86,7 @@ Invoking `engine.execute("select * from auth_users")` will lead to sql query "select * from auth_users" but when SQLCommenter and `attribute_commenter` is enabled the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. -Warning: capture of sqlcomment in ``db.statement`` or ``db.query.text`` may have high cardinality without platform normalization. See `Semantic Conventions for database spans `_ for more information. +Warning: capture of sqlcomment in ``db.statement`` may have high cardinality without platform normalization. See `Semantic Conventions for database spans `_ for more information. Usage