Skip to content

Commit

Permalink
Disable Redis query sanitization by default
Browse files Browse the repository at this point in the history
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.

open-telemetry/opentelemetry-specification#3104
  • Loading branch information
tombruijn committed Jan 25, 2023
1 parent dcc34c1 commit 5fc865f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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))

Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 5fc865f

Please sign in to comment.