ddtrace/tracer: obfuscate stats #1069
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gbbr
force-pushed
the
gbbr/obfuscate-stats
branch
from
b58f915
to
588e13a
Compare
piochelepiotr
previously approved these changes
Nov 29, 2021
piochelepiotr
previously approved these changes
Nov 29, 2021
pkalmakis
reviewed
Nov 29, 2021
pkalmakis
requested changes
Nov 29, 2021
There was a problem hiding this comment.
Let's please hold off on enabling client-computed stats by default. We currently have 0 production customers using this feature. We should get a few to validate that the go tracer is working properly before enabling this by default for 100% of customers who upgrade.
knusbaum
reviewed
Nov 30, 2021
Comment on lines
+408
to
+426
| func obfuscatedResource(o *obfuscate.Obfuscator, typ, resource string) string { | ||
| if o == nil { | ||
| return resource | ||
| } | ||
| switch typ { | ||
| case "sql", "cassandra": | ||
| oq, err := o.ObfuscateSQLString(resource) | ||
| if err != nil { | ||
| log.Error("Error obfuscating stats group resource %q: %v", resource, err) | ||
| return textNonParsable | ||
| } | ||
| return oq.Query | ||
| case "redis": | ||
| return o.QuantizeRedisString(resource) | ||
| default: | ||
| return resource | ||
| } | ||
| } | ||
|
|
| @@ -3,6 +3,7 @@ module gopkg.in/DataDog/dd-trace-go.v1 | |||
| go 1.12 | |||
|
|
|||
| require ( | |||
| github.com/DataDog/datadog-agent/pkg/obfuscate v0.0.0-20211129110424-6491aa3bf583 | |||
There was a problem hiding this comment.
Any chance we can get a proper mod version here?
There was a problem hiding this comment.
Not yet. We'll have to use the pseudo-version format until the Datadog Agent is released, and an official tag is assigned to this module. It should be fine. Do you see any risk associated with this approach, until that happens?
|
@pkalmakis that's done in the latest commit. PTAL |
This change introduces obfuscation of stats aggregation key resources by using the same code as used in the trace-agent. For now, it is disabled by default and under the "obfuscate_stats" feature flag.
gbbr
force-pushed
the
gbbr/obfuscate-stats
branch
from
09f9690
to
719c849
Compare
piochelepiotr
previously approved these changes
Dec 3, 2021
piochelepiotr
approved these changes
Dec 3, 2021
pkalmakis
approved these changes
Dec 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change introduces obfuscation of stats aggregation key resources by
using the same code as used in the trace-agent.
This feature continues to be off by default, enablable using the
discoveryfeature flag.