Skip to content

Commit

Permalink
Update Redis functional tests
Browse files Browse the repository at this point in the history 
- Update the sanitizer to also account for a max `db.statement`
  attribute value length. No longer than 1000 characters.
- Update the functional tests to assume the queries are sanitized by
  default.
- Add new tests that test the behavior with sanitization turned off.
  Only for the tests in the first test class. I don't think it's needed
  to duplicate this test for the clustered and async setup combinations.
  • Loading branch information
@tombruijn
tombruijn committed Jan 16, 2023
1 parent 8462981 commit 8d56c2f
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 11 deletions.
10 changes: 7 additions & 3 deletions ...ation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,17 @@ def _extract_conn_attributes(conn_kwargs):

def _format_command_args(args, sanitize_query):
"""Format and sanitize command arguments, and trim them as needed"""
cmd_max_len = 1000
value_too_long_mark = "..."
if sanitize_query:
# Sanitized query format: "COMMAND ? ?"
out = [str(args[0])] + ["?"] * (len(args) - 1)
out_str = " ".join(out)
if len(out_str) > cmd_max_len:
out_str = out_str[:cmd_max_len - len(value_too_long_mark)] + value_too_long_mark
return out_str
else:
value_max_len = 100
value_too_long_mark = "..."
cmd_max_len = 1000
length = 0
out = []
for arg in args:
Expand All @@ -73,4 +77,4 @@ def _format_command_args(args, sanitize_query):
out.append(cmd)
length += len(cmd)

return " ".join(out)
return " ".join(out)
117 changes: 109 additions & 8 deletions tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,27 @@ def _check_span(self, span, name):
self.assertEqual(span.attributes[SpanAttributes.NET_PEER_PORT], 6379)

def test_long_command(self):
self.redis_client.mget(*range(2000))

spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self._check_span(span, "MGET")
self.assertTrue(
span.attributes.get(SpanAttributes.DB_STATEMENT).startswith(
"MGET ? ? ? ?"
)
)
self.assertTrue(
span.attributes.get(SpanAttributes.DB_STATEMENT).endswith("...")
)

def test_long_command_unsanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=False
)

self.redis_client.mget(*range(1000))

spans = self.memory_exporter.get_finished_spans()
Expand All @@ -62,6 +83,22 @@ def test_long_command(self):
)

def test_basics(self):
self.assertIsNone(self.redis_client.get("cheese"))
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self._check_span(span, "GET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
)
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)

def test_basics_unsanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=False
)

self.assertIsNone(self.redis_client.get("cheese"))
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
Expand All @@ -73,6 +110,28 @@ def test_basics(self):
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)

def test_pipeline_traced(self):
with self.redis_client.pipeline(transaction=False) as pipeline:
pipeline.set("blah", 32)
pipeline.rpush("foo", "éé")
pipeline.hgetall("xxx")
pipeline.execute()

spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self._check_span(span, "SET RPUSH HGETALL")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT),
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
)
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)

def test_pipeline_traced_unsanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=False
)

with self.redis_client.pipeline(transaction=False) as pipeline:
pipeline.set("blah", 32)
pipeline.rpush("foo", "éé")
Expand All @@ -90,6 +149,27 @@ def test_pipeline_traced(self):
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)

def test_pipeline_immediate(self):
with self.redis_client.pipeline() as pipeline:
pipeline.set("a", 1)
pipeline.immediate_execute_command("SET", "b", 2)
pipeline.execute()

spans = self.memory_exporter.get_finished_spans()
# expecting two separate spans here, rather than a
# single span for the whole pipeline
self.assertEqual(len(spans), 2)
span = spans[0]
self._check_span(span, "SET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "SET ? ?"
)

def test_pipeline_immediate_unsanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=False
)

with self.redis_client.pipeline() as pipeline:
pipeline.set("a", 1)
pipeline.immediate_execute_command("SET", "b", 2)
Expand Down Expand Up @@ -150,7 +230,7 @@ def test_basics(self):
span = spans[0]
self._check_span(span, "GET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET cheese"
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
)
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)

Expand All @@ -167,7 +247,7 @@ def test_pipeline_traced(self):
self._check_span(span, "SET RPUSH HGETALL")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT),
"SET blah 32\nRPUSH foo éé\nHGETALL xxx",
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
)
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)

Expand Down Expand Up @@ -220,6 +300,27 @@ def _check_span(self, span, name):
self.assertEqual(span.attributes[SpanAttributes.NET_PEER_PORT], 6379)

def test_long_command(self):
self.redis_client.mget(*range(2000))

spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self._check_span(span, "MGET")
self.assertTrue(
span.attributes.get(SpanAttributes.DB_STATEMENT).startswith(
"MGET ? ? ? ?"
)
)
self.assertTrue(
span.attributes.get(SpanAttributes.DB_STATEMENT).endswith("...")
)

def test_long_command_unsanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=False
)

async_call(self.redis_client.mget(*range(1000)))

spans = self.memory_exporter.get_finished_spans()
Expand All @@ -242,7 +343,7 @@ def test_basics(self):
span = spans[0]
self._check_span(span, "GET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET cheese"
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
)
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)

Expand All @@ -264,7 +365,7 @@ async def pipeline_simple():
self._check_span(span, "SET RPUSH HGETALL")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT),
"SET blah 32\nRPUSH foo éé\nHGETALL xxx",
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
)
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)

Expand All @@ -284,7 +385,7 @@ async def pipeline_immediate():
span = spans[0]
self._check_span(span, "SET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "SET b 2"
span.attributes.get(SpanAttributes.DB_STATEMENT), "SET ? ?"
)

def test_parent(self):
Expand Down Expand Up @@ -332,7 +433,7 @@ def test_basics(self):
span = spans[0]
self._check_span(span, "GET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET cheese"
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
)
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)

Expand All @@ -354,7 +455,7 @@ async def pipeline_simple():
self._check_span(span, "SET RPUSH HGETALL")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT),
"SET blah 32\nRPUSH foo éé\nHGETALL xxx",
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
)
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)

Expand Down Expand Up @@ -408,5 +509,5 @@ def test_get(self):
span = spans[0]
self._check_span(span, "GET")
self.assertEqual(
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET foo"
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
)

0 comments on commit 8d56c2f

Please sign in to comment.