Sanitize redis db_statement by default (#1776)

Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
Co-authored-by: Shalev Roda <65566801+shalevr@users.noreply.github.com>
This commit is contained in:
Nimrod Shlagman
2023-06-13 15:37:55 +03:00
committed by GitHub
parent 818ef43223
commit 37d85f0745
6 changed files with 36 additions and 134 deletions

View File

@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Fix redis db.statements to be sanitized by default
([#1778](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1778))
- Fix elasticsearch db.statement attribute to be sanitized by default
([#1758](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1758))
- Fix `AttributeError` when AWS Lambda handler receives a list event

View File

@ -64,8 +64,6 @@ this function signature is: def request_hook(span: Span, instance: redis.connec
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 False, enable the Redis query sanitization
for example:
.. code: python
@ -88,27 +86,11 @@ for example:
client = redis.StrictRedis(host="localhost", port=6379)
client.get("my-key")
Configuration
-------------
Query sanitization
******************
To enable query sanitization with an environment variable, set
``OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS`` to "true".
For example,
::
export OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS="true"
will result in traced queries like "SET ? ?".
API
---
"""
import typing
from os import environ
from typing import Any, Collection
import redis
@ -116,9 +98,6 @@ from wrapt import wrap_function_wrapper
from opentelemetry import trace
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.redis.environment_variables import (
OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS,
)
from opentelemetry.instrumentation.redis.package import _instruments
from opentelemetry.instrumentation.redis.util import (
_extract_conn_attributes,
@ -161,10 +140,9 @@ def _instrument(
tracer,
request_hook: _RequestHookT = None,
response_hook: _ResponseHookT = None,
sanitize_query: bool = False,
):
def _traced_execute_command(func, instance, args, kwargs):
query = _format_command_args(args, sanitize_query)
query = _format_command_args(args)
if len(args) > 0 and args[0]:
name = args[0]
@ -194,7 +172,7 @@ def _instrument(
cmds = [
_format_command_args(
c.args if hasattr(c, "args") else c[0], sanitize_query
c.args if hasattr(c, "args") else c[0],
)
for c in command_stack
]
@ -307,15 +285,6 @@ class RedisInstrumentor(BaseInstrumentor):
tracer,
request_hook=kwargs.get("request_hook"),
response_hook=kwargs.get("response_hook"),
sanitize_query=kwargs.get(
"sanitize_query",
environ.get(
OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS, "false"
)
.lower()
.strip()
== "true",
),
)
def _uninstrument(self, **kwargs):

View File

@ -1,17 +0,0 @@
# Copyright The OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS = (
"OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS"
)

View File

@ -48,41 +48,23 @@ def _extract_conn_attributes(conn_kwargs):
return attributes
def _format_command_args(args, sanitize_query):
def _format_command_args(args):
"""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 ? ?"
args_length = len(args)
if args_length > 0:
out = [str(args[0])] + ["?"] * (args_length - 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
)
else:
out_str = ""
return out_str
# Sanitized query format: "COMMAND ? ?"
args_length = len(args)
if args_length > 0:
out = [str(args[0])] + ["?"] * (args_length - 1)
out_str = " ".join(out)
value_max_len = 100
length = 0
out = []
for arg in args:
cmd = str(arg)
if len(out_str) > cmd_max_len:
out_str = (
out_str[: cmd_max_len - len(value_too_long_mark)]
+ value_too_long_mark
)
else:
out_str = ""
if len(cmd) > value_max_len:
cmd = cmd[:value_max_len] + value_too_long_mark
if length + len(cmd) > cmd_max_len:
prefix = cmd[: cmd_max_len - length]
out.append(f"{prefix}{value_too_long_mark}")
break
out.append(cmd)
length += len(cmd)
return " ".join(out)
return out_str

View File

@ -168,22 +168,11 @@ class TestRedis(TestBase):
span = spans[0]
self.assertEqual(span.attributes.get("db.statement"), "SET ? ?")
def test_query_sanitizer_enabled_env(self):
def test_query_sanitizer(self):
redis_client = redis.Redis()
connection = redis.connection.Connection()
redis_client.connection = connection
RedisInstrumentor().uninstrument()
env_patch = mock.patch.dict(
"os.environ",
{"OTEL_PYTHON_INSTRUMENTATION_SANITIZE_REDIS": "true"},
)
env_patch.start()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider,
)
with mock.patch.object(redis_client, "connection"):
redis_client.set("key", "value")
@ -192,21 +181,6 @@ class TestRedis(TestBase):
span = spans[0]
self.assertEqual(span.attributes.get("db.statement"), "SET ? ?")
env_patch.stop()
def test_query_sanitizer_disabled(self):
redis_client = redis.Redis()
connection = redis.connection.Connection()
redis_client.connection = connection
with mock.patch.object(redis_client, "connection"):
redis_client.set("key", "value")
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 1)
span = spans[0]
self.assertEqual(span.attributes.get("db.statement"), "SET key value")
def test_no_op_tracer_provider(self):
RedisInstrumentor().uninstrument()

View File

@ -47,9 +47,7 @@ class TestRedisInstrument(TestBase):
def test_long_command_sanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=True
)
RedisInstrumentor().instrument(tracer_provider=self.tracer_provider)
self.redis_client.mget(*range(2000))
@ -75,7 +73,7 @@ class TestRedisInstrument(TestBase):
self._check_span(span, "MGET")
self.assertTrue(
span.attributes.get(SpanAttributes.DB_STATEMENT).startswith(
"MGET 0 1 2 3"
"MGET ? ? ? ?"
)
)
self.assertTrue(
@ -84,9 +82,7 @@ class TestRedisInstrument(TestBase):
def test_basics_sanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=True
)
RedisInstrumentor().instrument(tracer_provider=self.tracer_provider)
self.assertIsNone(self.redis_client.get("cheese"))
spans = self.memory_exporter.get_finished_spans()
@ -105,15 +101,13 @@ class TestRedisInstrument(TestBase):
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)
def test_pipeline_traced_sanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=True
)
RedisInstrumentor().instrument(tracer_provider=self.tracer_provider)
with self.redis_client.pipeline(transaction=False) as pipeline:
pipeline.set("blah", 32)
@ -144,15 +138,13 @@ class TestRedisInstrument(TestBase):
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)
def test_pipeline_immediate_sanitized(self):
RedisInstrumentor().uninstrument()
RedisInstrumentor().instrument(
tracer_provider=self.tracer_provider, sanitize_query=True
)
RedisInstrumentor().instrument(tracer_provider=self.tracer_provider)
with self.redis_client.pipeline() as pipeline:
pipeline.set("a", 1)
@ -182,7 +174,7 @@ class TestRedisInstrument(TestBase):
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):
@ -230,7 +222,7 @@ class TestRedisClusterInstrument(TestBase):
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)
@ -247,7 +239,7 @@ class TestRedisClusterInstrument(TestBase):
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)
@ -308,7 +300,7 @@ class TestAsyncRedisInstrument(TestBase):
self._check_span(span, "MGET")
self.assertTrue(
span.attributes.get(SpanAttributes.DB_STATEMENT).startswith(
"MGET 0 1 2 3"
"MGET ? ? ? ?"
)
)
self.assertTrue(
@ -322,7 +314,7 @@ class TestAsyncRedisInstrument(TestBase):
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)
@ -344,7 +336,7 @@ class TestAsyncRedisInstrument(TestBase):
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)
@ -364,7 +356,7 @@ class TestAsyncRedisInstrument(TestBase):
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):
@ -412,7 +404,7 @@ class TestAsyncRedisClusterInstrument(TestBase):
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)
@ -434,7 +426,7 @@ class TestAsyncRedisClusterInstrument(TestBase):
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)
@ -488,5 +480,5 @@ class TestRedisDBIndexInstrument(TestBase):
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 ?"
)