From 5fc865f50573589df2c180deae92390f2e8fbccf Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Wed, 25 Jan 2023 16:11:58 +0100 Subject: [PATCH] Disable Redis query sanitization by default Update the Redis instrumentation library to not change the default behavior for the Redis instrumentation. This can be enabled at a later time when the spec discussion about this topic has concluded. https://github.com/open-telemetry/opentelemetry-specification/issues/3104 --- .../instrumentation/redis/__init__.py | 6 ++-- .../tests/test_redis.py | 12 +++---- .../tests/redis/test_redis_functional.py | 36 +++++++++---------- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index 37cf76e7d6..0f18639bd2 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -64,7 +64,7 @@ async def redis_get(): response_hook (Callable) - a function with extra user-defined logic to be performed after performing the request this function signature is: def response_hook(span: Span, instance: redis.connection.Connection, response) -> None -sanitize_query (Boolean) - default True, enable the Redis query sanitization +sanitize_query (Boolean) - default False, enable the Redis query sanitization for example: @@ -141,7 +141,7 @@ def _instrument( tracer, request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, - sanitize_query: bool = True, + sanitize_query: bool = False, ): def _traced_execute_command(func, instance, args, kwargs): query = _format_command_args(args, sanitize_query) @@ -287,7 +287,7 @@ def _instrument(self, **kwargs): tracer, request_hook=kwargs.get("request_hook"), response_hook=kwargs.get("response_hook"), - sanitize_query=kwargs.get("sanitize_query", True), + sanitize_query=kwargs.get("sanitize_query", False), ) def _uninstrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py index 696bee02ee..1c64fcb13e 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -153,6 +153,12 @@ def test_query_sanitizer_enabled(self): connection = redis.connection.Connection() redis_client.connection = connection + RedisInstrumentor().uninstrument() + RedisInstrumentor().instrument( + tracer_provider=self.tracer_provider, + sanitize_query=True, + ) + with mock.patch.object(redis_client, "connection"): redis_client.set("key", "value") @@ -167,12 +173,6 @@ def test_query_sanitizer_disabled(self): connection = redis.connection.Connection() redis_client.connection = connection - RedisInstrumentor().uninstrument() - RedisInstrumentor().instrument( - tracer_provider=self.tracer_provider, - sanitize_query=False, - ) - with mock.patch.object(redis_client, "connection"): redis_client.set("key", "value") diff --git a/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py b/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py index 87bc3265c2..675a37fa9f 100644 --- a/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py +++ b/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py @@ -28,9 +28,7 @@ def setUp(self): super().setUp() self.redis_client = redis.Redis(port=6379) self.redis_client.flushall() - RedisInstrumentor().instrument( - tracer_provider=self.tracer_provider, sanitize_query=False - ) + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) def tearDown(self): RedisInstrumentor().uninstrument() @@ -49,7 +47,9 @@ def _check_span(self, span, name): def test_long_command_sanitized(self): RedisInstrumentor().uninstrument() - RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + RedisInstrumentor().instrument( + tracer_provider=self.tracer_provider, sanitize_query=True + ) self.redis_client.mget(*range(2000)) @@ -84,7 +84,9 @@ def test_long_command(self): def test_basics_sanitized(self): RedisInstrumentor().uninstrument() - RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + RedisInstrumentor().instrument( + tracer_provider=self.tracer_provider, sanitize_query=True + ) self.assertIsNone(self.redis_client.get("cheese")) spans = self.memory_exporter.get_finished_spans() @@ -109,7 +111,9 @@ def test_basics(self): def test_pipeline_traced_sanitized(self): RedisInstrumentor().uninstrument() - RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + RedisInstrumentor().instrument( + tracer_provider=self.tracer_provider, sanitize_query=True + ) with self.redis_client.pipeline(transaction=False) as pipeline: pipeline.set("blah", 32) @@ -146,7 +150,9 @@ def test_pipeline_traced(self): def test_pipeline_immediate_sanitized(self): RedisInstrumentor().uninstrument() - RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + RedisInstrumentor().instrument( + tracer_provider=self.tracer_provider, sanitize_query=True + ) with self.redis_client.pipeline() as pipeline: pipeline.set("a", 1) @@ -207,9 +213,7 @@ def setUp(self): host="localhost", port=7000 ) self.redis_client.flushall() - RedisInstrumentor().instrument( - tracer_provider=self.tracer_provider, sanitize_query=False - ) + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) def tearDown(self): super().tearDown() @@ -278,9 +282,7 @@ def setUp(self): super().setUp() self.redis_client = redis.asyncio.Redis(port=6379) async_call(self.redis_client.flushall()) - RedisInstrumentor().instrument( - tracer_provider=self.tracer_provider, sanitize_query=False - ) + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) def tearDown(self): RedisInstrumentor().uninstrument() @@ -393,9 +395,7 @@ def setUp(self): host="localhost", port=7000 ) async_call(self.redis_client.flushall()) - RedisInstrumentor().instrument( - tracer_provider=self.tracer_provider, sanitize_query=False - ) + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) def tearDown(self): super().tearDown() @@ -464,9 +464,7 @@ def setUp(self): super().setUp() self.redis_client = redis.Redis(port=6379, db=10) self.redis_client.flushall() - RedisInstrumentor().instrument( - tracer_provider=self.tracer_provider, sanitize_query=False - ) + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) def tearDown(self): RedisInstrumentor().uninstrument()