Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib/go-redis/redis.v8: add WithSkipRawCommand option and fix resource #1091

Merged
merged 4 commits into from Dec 16, 2021

Conversation

gbbr
Copy link
Contributor

@gbbr gbbr commented Dec 14, 2021

This change adds a new option called WithSkippedRawCommand which
determines the instrumentation code to skip adding the
redis.raw_command tag to spans. This is useful in cases when the
Datadog Agent Redis obfuscation is disabled (which it is, by default)
and commands may contain sensitive information.

Additionally, duplicate entries and an incosistency was found when
setting the resource name for Redis spans, which was fixed as part of
this PR.

Closes #1022

@dianashevchenko
Copy link
Contributor

LGTM

@felixge felixge modified the milestones: 1.35.0, 1.36.0 Dec 14, 2021
sjmtan
sjmtan previously approved these changes Dec 14, 2021
Copy link
Contributor

@sjmtan sjmtan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure my approval matters, but this will be a big help for sure!

@gbbr gbbr modified the milestones: 1.36.0, 1.35.0 Dec 15, 2021
…urce

This change adds a new option called `WithSkippedRawCommand` which
determines the instrumentation code to skip adding the
`redis.raw_command` tag to spans. This is useful in cases when the
Datadog Agent Redis obfuscation is disabled (which it is, by default)
and commands may contain sensitive information.

Additionally, duplicate entries and an incosistency was found when
setting the resource name for Redis spans, which was fixed as part of
this PR.

Closes #1022
@gbbr
Copy link
Contributor Author

gbbr commented Dec 15, 2021

@shannontan-bolt it does matter. I want to make sure this is what you wanted and what we discussed.

Copy link
Contributor

@knusbaum knusbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sjmtan
Copy link
Contributor

sjmtan commented Dec 15, 2021

@shannontan-bolt it does matter. I want to make sure this is what you wanted and what we discussed.

Ah - it is! I just mean that I don't think my approval is one that unblocks merge haha

@gbbr gbbr merged commit 58598b1 into v1 Dec 16, 2021
@gbbr gbbr deleted the gbbr/redis-skip-raw branch December 16, 2021 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

contrib/go-redis/redis.v8: Allow Option To Remove Raw Tag
5 participants