Skip to content

Redis:Omit command arguments from span.resource by default#3235

Merged
marcotc merged 2 commits into
masterfrom
redis-only-command-default
Nov 1, 2023
Merged

Redis:Omit command arguments from span.resource by default#3235
marcotc merged 2 commits into
masterfrom
redis-only-command-default

Conversation

@marcotc

@marcotc marcotc commented Oct 30, 2023

Copy link
Copy Markdown
Member

For release notes

The Datadog Agent removes command arguments from the resource name. However there are cases, like compressed keys, where this obfuscation cannot correctly remove command arguments. To safeguard that situation, the resource name set by the tracer will only be the command (e.g. SET) with no arguments. To retain the previous behavior and keep arguments in the span resource, with the potential risk of some command arguments not being fully obfuscated, set DD_REDIS_COMMAND_ARGS=true or option c.instrument :redis, command_args: true.

What does this PR do?

This PR changes the value of span.resource for the Redis tracing instrumentation to only capture the command name by default (e.g. GET, BLPOP).

It's possible to revert back to the previous default, capturing command arguments, with the environment variable DD_REDIS_COMMAND_ARGS or option command_args.

Motivation:

Redis command arguments can contain sensitive information and thus must not be captured by default.

Additional Notes:

How to test the change?

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@github-actions github-actions Bot added integrations Involves tracing integrations tracing labels Oct 30, 2023
@marcotc
marcotc force-pushed the redis-only-command-default branch from 3bc30b4 to 648bbc8 Compare October 30, 2023 20:40
@marcotc
marcotc force-pushed the redis-only-command-default branch from 648bbc8 to d1a30df Compare October 30, 2023 21:44
@marcotc marcotc self-assigned this Oct 30, 2023
@marcotc marcotc added this to the 1.16.0 milestone Oct 30, 2023
@marcotc
marcotc marked this pull request as ready for review October 30, 2023 21:55
@marcotc
marcotc requested review from a team as code owners October 30, 2023 21:55
Comment thread docs/GettingStarted.md Outdated
@codecov-commenter

codecov-commenter commented Oct 31, 2023

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.22%. Comparing base (4d22403) to head (47d136c).
⚠️ Report is 5165 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3235   +/-   ##
=======================================
  Coverage   98.22%   98.22%           
=======================================
  Files        1252     1252           
  Lines       72201    72173   -28     
  Branches     3353     3348    -5     
=======================================
- Hits        70917    70890   -27     
+ Misses       1284     1283    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marcotc
marcotc merged commit 713fb1e into master Nov 1, 2023
@marcotc
marcotc deleted the redis-only-command-default branch November 1, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrations Involves tracing integrations tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants