From cfd017e5ec49d4584bc2bc6810c7c330e7ff53ab Mon Sep 17 00:00:00 2001 From: avzis <107620508+avzis@users.noreply.github.com> Date: Tue, 6 Dec 2022 16:52:38 +0200 Subject: [PATCH] fix enable_commenter functionality (#1440) --- CHANGELOG.md | 2 ++ .../instrumentation/sqlalchemy/__init__.py | 11 +++++--- .../instrumentation/sqlalchemy/engine.py | 12 +++++---- .../tests/test_sqlcommenter.py | 27 +++++++++++++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7746f5fe67..b72b9d09bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1461](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1461)) - Add grpc.aio instrumentation to package entry points ([#1442](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1442)) +- Fix a bug in SQLAlchemy instrumentation - support disabling enable_commenter variable + ([#1440](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1440)) ## Version 1.14.0/0.35b0 (2022-11-03) 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 e56485ca77..6c91ae16e0 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -136,11 +136,16 @@ def _instrument(self, **kwargs): An instrumented engine if passed in as an argument or list of instrumented engines, None otherwise. """ tracer_provider = kwargs.get("tracer_provider") - _w("sqlalchemy", "create_engine", _wrap_create_engine(tracer_provider)) + enable_commenter = kwargs.get("enable_commenter", False) + _w( + "sqlalchemy", + "create_engine", + _wrap_create_engine(tracer_provider, enable_commenter), + ) _w( "sqlalchemy.engine", "create_engine", - _wrap_create_engine(tracer_provider), + _wrap_create_engine(tracer_provider, enable_commenter), ) _w( "sqlalchemy.engine.base", @@ -151,7 +156,7 @@ def _instrument(self, **kwargs): _w( "sqlalchemy.ext.asyncio", "create_async_engine", - _wrap_create_async_engine(tracer_provider), + _wrap_create_async_engine(tracer_provider, enable_commenter), ) if kwargs.get("engine") is not None: return EngineTracer( 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 2965f45085..33d0183ef0 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -49,27 +49,29 @@ def _get_tracer(tracer_provider=None): ) -def _wrap_create_async_engine(tracer_provider=None): +def _wrap_create_async_engine(tracer_provider=None, enable_commenter=False): # pylint: disable=unused-argument def _wrap_create_async_engine_internal(func, module, args, kwargs): """Trace the SQLAlchemy engine, creating an `EngineTracer` object that will listen to SQLAlchemy events. """ engine = func(*args, **kwargs) - EngineTracer(_get_tracer(tracer_provider), engine.sync_engine) + EngineTracer( + _get_tracer(tracer_provider), engine.sync_engine, enable_commenter + ) return engine return _wrap_create_async_engine_internal -def _wrap_create_engine(tracer_provider=None): +def _wrap_create_engine(tracer_provider=None, enable_commenter=False): # pylint: disable=unused-argument def _wrap_create_engine_internal(func, module, args, kwargs): """Trace the SQLAlchemy engine, creating an `EngineTracer` object that will listen to SQLAlchemy events. """ engine = func(*args, **kwargs) - EngineTracer(_get_tracer(tracer_provider), engine) + EngineTracer(_get_tracer(tracer_provider), engine, enable_commenter) return engine return _wrap_create_engine_internal @@ -94,7 +96,7 @@ def _wrap_connect_internal(func, module, args, kwargs): class EngineTracer: def __init__( - self, tracer, engine, enable_commenter=True, commenter_options=None + self, tracer, engine, enable_commenter=False, commenter_options=None ): self.tracer = tracer self.engine = engine diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index 6cdc5b2f67..5f9e75a1aa 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -77,3 +77,30 @@ 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}'\*/;", ) + + def test_sqlcommenter_enabled_create_engine_after_instrumentation(self): + SQLAlchemyInstrumentor().instrument( + tracer_provider=self.tracer_provider, + enable_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute("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}'\*/;", + ) + + def test_sqlcommenter_disabled_create_engine_after_instrumentation(self): + SQLAlchemyInstrumentor().instrument( + tracer_provider=self.tracer_provider, + enable_commenter=False, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute("SELECT 1;").fetchall() + self.assertEqual(self.caplog.records[-2].getMessage(), "SELECT 1;")