Implement sensitive data sanitization for redis instrumentation #1548
Labels
Comments
7 tasks
7 tasks
tombruijn
added a commit
to tombruijn/opentelemetry-python-contrib
that referenced
this issue
Jan 10, 2023
Add a Redis query sanitizer. This can be disabled with the `sanitize_query = False` config option. Given the query `SET key value`, the sanitized query becomes `SET ? ?`. Both the keys and values are sanitized, as both can contain PII data. The Redis queries are sanitized by default. This changes the default behavior of this instrumentation. Previously it reported unsanitized Redis queries. This was previously discussed in the previous implementation of this PR in PR open-telemetry#1571 Closes open-telemetry#1548
tombruijn
added a commit
to tombruijn/opentelemetry-python-contrib
that referenced
this issue
Jan 10, 2023
Add a query sanitizer to the Redis instrumentation. This can be disabled with the `sanitize_query = False` config option. Given the query `SET key value`, the sanitized query becomes `SET ? ?`. Both the keys and values are sanitized, as both can contain PII data. The Redis queries are sanitized by default. This changes the default behavior of this instrumentation. Previously it reported unsanitized Redis queries. This was previously discussed in the previous implementation of this PR in PR open-telemetry#1571 Closes open-telemetry#1548
8 tasks
srikanthccv
added a commit
that referenced
this issue
Feb 4, 2023
* Add Redis instrumentation query sanitization Add a query sanitizer to the Redis instrumentation. This can be disabled with the `sanitize_query = False` config option. Given the query `SET key value`, the sanitized query becomes `SET ? ?`. Both the keys and values are sanitized, as both can contain PII data. The Redis queries are sanitized by default. This changes the default behavior of this instrumentation. Previously it reported unsanitized Redis queries. This was previously discussed in the previous implementation of this PR in PR #1571 Closes #1548 * Update Redis sanitize_query option documentation Changes suggested in #1572 (comment) * Remove uninstrument & instrument from test setup The Redis test that performs the tests with the default options, doesn't need to uninstrument and then instrument the instrumentor. This commit removes the unnecessary setup code. The setup code is already present at the top of the file. * Fix code style formatting * 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. * Test Redis unsanitized queries by default Change the Redis functional tests so that they test the unsanitized query by default, and test the sanitized query results in the separate test functions. This is a partial revert of the previous commit 8d56c2f * Fix formatting issue in Redis utils * 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. open-telemetry/opentelemetry-specification#3104 * Fix pylint issue Remove else statement. * Update changelog about Redis query sanitization default [ci skip] Co-authored-by: Srikanth Chekuri <[email protected]> * Fix potential error on Redis args being 0 Check the length of the args array and return an empty string if there are no args. That way it won't cause an IndexError if the args array is empty and it tries to fetch the first element, which should be the Redis command. --------- Co-authored-by: Srikanth Chekuri <[email protected]>
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Aggregated by #1543
The text was updated successfully, but these errors were encountered: