Skip to content

Commit 8d56c2f

Browse files
tombruijntombruijn
committed
Update Redis functional tests
- 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.
1 parent 8462981 commit 8d56c2f

File tree

2 files changed

+116
-11
lines changed
  • instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis
  • tests/opentelemetry-docker-tests/tests/redis

2 files changed

+116
-11
lines changed

Diff for: ‎instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py

+7-3
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,17 @@ def _extract_conn_attributes(conn_kwargs):
5050

5151
def _format_command_args(args, sanitize_query):
5252
"""Format and sanitize command arguments, and trim them as needed"""
53+
cmd_max_len = 1000
54+
value_too_long_mark = "..."
5355
if sanitize_query:
5456
# Sanitized query format: "COMMAND ? ?"
5557
out = [str(args[0])] + ["?"] * (len(args) - 1)
58+
out_str = " ".join(out)
59+
if len(out_str) > cmd_max_len:
60+
out_str = out_str[:cmd_max_len - len(value_too_long_mark)] + value_too_long_mark
61+
return out_str
5662
else:
5763
value_max_len = 100
58-
value_too_long_mark = "..."
59-
cmd_max_len = 1000
6064
length = 0
6165
out = []
6266
for arg in args:
@@ -73,4 +77,4 @@ def _format_command_args(args, sanitize_query):
7377
out.append(cmd)
7478
length += len(cmd)
7579

76-
return " ".join(out)
80+
return " ".join(out)

Diff for: ‎tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py

+109-8
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,27 @@ def _check_span(self, span, name):
4646
self.assertEqual(span.attributes[SpanAttributes.NET_PEER_PORT], 6379)
4747

4848
def test_long_command(self):
49+
self.redis_client.mget(*range(2000))
50+
51+
spans = self.memory_exporter.get_finished_spans()
52+
self.assertEqual(len(spans), 1)
53+
span = spans[0]
54+
self._check_span(span, "MGET")
55+
self.assertTrue(
56+
span.attributes.get(SpanAttributes.DB_STATEMENT).startswith(
57+
"MGET ? ? ? ?"
58+
)
59+
)
60+
self.assertTrue(
61+
span.attributes.get(SpanAttributes.DB_STATEMENT).endswith("...")
62+
)
63+
64+
def test_long_command_unsanitized(self):
65+
RedisInstrumentor().uninstrument()
66+
RedisInstrumentor().instrument(
67+
tracer_provider=self.tracer_provider, sanitize_query=False
68+
)
69+
4970
self.redis_client.mget(*range(1000))
5071

5172
spans = self.memory_exporter.get_finished_spans()
@@ -62,6 +83,22 @@ def test_long_command(self):
6283
)
6384

6485
def test_basics(self):
86+
self.assertIsNone(self.redis_client.get("cheese"))
87+
spans = self.memory_exporter.get_finished_spans()
88+
self.assertEqual(len(spans), 1)
89+
span = spans[0]
90+
self._check_span(span, "GET")
91+
self.assertEqual(
92+
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
93+
)
94+
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)
95+
96+
def test_basics_unsanitized(self):
97+
RedisInstrumentor().uninstrument()
98+
RedisInstrumentor().instrument(
99+
tracer_provider=self.tracer_provider, sanitize_query=False
100+
)
101+
65102
self.assertIsNone(self.redis_client.get("cheese"))
66103
spans = self.memory_exporter.get_finished_spans()
67104
self.assertEqual(len(spans), 1)
@@ -73,6 +110,28 @@ def test_basics(self):
73110
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)
74111

75112
def test_pipeline_traced(self):
113+
with self.redis_client.pipeline(transaction=False) as pipeline:
114+
pipeline.set("blah", 32)
115+
pipeline.rpush("foo", "éé")
116+
pipeline.hgetall("xxx")
117+
pipeline.execute()
118+
119+
spans = self.memory_exporter.get_finished_spans()
120+
self.assertEqual(len(spans), 1)
121+
span = spans[0]
122+
self._check_span(span, "SET RPUSH HGETALL")
123+
self.assertEqual(
124+
span.attributes.get(SpanAttributes.DB_STATEMENT),
125+
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
126+
)
127+
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)
128+
129+
def test_pipeline_traced_unsanitized(self):
130+
RedisInstrumentor().uninstrument()
131+
RedisInstrumentor().instrument(
132+
tracer_provider=self.tracer_provider, sanitize_query=False
133+
)
134+
76135
with self.redis_client.pipeline(transaction=False) as pipeline:
77136
pipeline.set("blah", 32)
78137
pipeline.rpush("foo", "éé")
@@ -90,6 +149,27 @@ def test_pipeline_traced(self):
90149
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)
91150

92151
def test_pipeline_immediate(self):
152+
with self.redis_client.pipeline() as pipeline:
153+
pipeline.set("a", 1)
154+
pipeline.immediate_execute_command("SET", "b", 2)
155+
pipeline.execute()
156+
157+
spans = self.memory_exporter.get_finished_spans()
158+
# expecting two separate spans here, rather than a
159+
# single span for the whole pipeline
160+
self.assertEqual(len(spans), 2)
161+
span = spans[0]
162+
self._check_span(span, "SET")
163+
self.assertEqual(
164+
span.attributes.get(SpanAttributes.DB_STATEMENT), "SET ? ?"
165+
)
166+
167+
def test_pipeline_immediate_unsanitized(self):
168+
RedisInstrumentor().uninstrument()
169+
RedisInstrumentor().instrument(
170+
tracer_provider=self.tracer_provider, sanitize_query=False
171+
)
172+
93173
with self.redis_client.pipeline() as pipeline:
94174
pipeline.set("a", 1)
95175
pipeline.immediate_execute_command("SET", "b", 2)
@@ -150,7 +230,7 @@ def test_basics(self):
150230
span = spans[0]
151231
self._check_span(span, "GET")
152232
self.assertEqual(
153-
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET cheese"
233+
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
154234
)
155235
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)
156236

@@ -167,7 +247,7 @@ def test_pipeline_traced(self):
167247
self._check_span(span, "SET RPUSH HGETALL")
168248
self.assertEqual(
169249
span.attributes.get(SpanAttributes.DB_STATEMENT),
170-
"SET blah 32\nRPUSH foo éé\nHGETALL xxx",
250+
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
171251
)
172252
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)
173253

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

222302
def test_long_command(self):
303+
self.redis_client.mget(*range(2000))
304+
305+
spans = self.memory_exporter.get_finished_spans()
306+
self.assertEqual(len(spans), 1)
307+
span = spans[0]
308+
self._check_span(span, "MGET")
309+
self.assertTrue(
310+
span.attributes.get(SpanAttributes.DB_STATEMENT).startswith(
311+
"MGET ? ? ? ?"
312+
)
313+
)
314+
self.assertTrue(
315+
span.attributes.get(SpanAttributes.DB_STATEMENT).endswith("...")
316+
)
317+
318+
def test_long_command_unsanitized(self):
319+
RedisInstrumentor().uninstrument()
320+
RedisInstrumentor().instrument(
321+
tracer_provider=self.tracer_provider, sanitize_query=False
322+
)
323+
223324
async_call(self.redis_client.mget(*range(1000)))
224325

225326
spans = self.memory_exporter.get_finished_spans()
@@ -242,7 +343,7 @@ def test_basics(self):
242343
span = spans[0]
243344
self._check_span(span, "GET")
244345
self.assertEqual(
245-
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET cheese"
346+
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
246347
)
247348
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)
248349

@@ -264,7 +365,7 @@ async def pipeline_simple():
264365
self._check_span(span, "SET RPUSH HGETALL")
265366
self.assertEqual(
266367
span.attributes.get(SpanAttributes.DB_STATEMENT),
267-
"SET blah 32\nRPUSH foo éé\nHGETALL xxx",
368+
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
268369
)
269370
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)
270371

@@ -284,7 +385,7 @@ async def pipeline_immediate():
284385
span = spans[0]
285386
self._check_span(span, "SET")
286387
self.assertEqual(
287-
span.attributes.get(SpanAttributes.DB_STATEMENT), "SET b 2"
388+
span.attributes.get(SpanAttributes.DB_STATEMENT), "SET ? ?"
288389
)
289390

290391
def test_parent(self):
@@ -332,7 +433,7 @@ def test_basics(self):
332433
span = spans[0]
333434
self._check_span(span, "GET")
334435
self.assertEqual(
335-
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET cheese"
436+
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
336437
)
337438
self.assertEqual(span.attributes.get("db.redis.args_length"), 2)
338439

@@ -354,7 +455,7 @@ async def pipeline_simple():
354455
self._check_span(span, "SET RPUSH HGETALL")
355456
self.assertEqual(
356457
span.attributes.get(SpanAttributes.DB_STATEMENT),
357-
"SET blah 32\nRPUSH foo éé\nHGETALL xxx",
458+
"SET ? ?\nRPUSH ? ?\nHGETALL ?",
358459
)
359460
self.assertEqual(span.attributes.get("db.redis.pipeline_length"), 3)
360461

@@ -408,5 +509,5 @@ def test_get(self):
408509
span = spans[0]
409510
self._check_span(span, "GET")
410511
self.assertEqual(
411-
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET foo"
512+
span.attributes.get(SpanAttributes.DB_STATEMENT), "GET ?"
412513
)

0 commit comments

Comments
 (0)