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

[OTEL-2304] export obfuscateStatsGroup function #32064

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/trace/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,8 @@ func (a *Agent) processStats(in *pb.ClientStatsPayload, lang, tracerVersion, con
if !a.Blacklister.AllowsStat(b) {
continue
}
a.obfuscateStatsGroup(b)
obfuscator := a.lazyInitObfuscator()
Copy link
Contributor

Choose a reason for hiding this comment

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

@ajgajg1134 is there any reason why this initialization happens outside the constructor? I think it could simplify the code a bit if we move this to the Agent constructor, especially now that the obfuscation function will be exported?

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, yeah it looks like it used to be initialized in the newAgent but very recently was moved here: #31336 @sethsamuel do you happen to recall why you moved the obfuscator initialization to be lazy init outside the NewAgent func?

I'm not entirely sure why it was moved but it seems like it should probably be moved back to the agent so that a "NewAgent" can know that it safely can use the obfuscator field and doesn't have to always remember to lazy init it for every use :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems reasonable to me; we are going to be calling obfuscate.NewObfuscator in the datadog connector

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like @sethsamuel is OOO until Monday

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this may not be the exact function we need to use for datadogconnector; I think it may be missing out on more data types. We are calculating more than just sql, cassandra, and redis metrics in connector so we will either need to enhance this function, or write our own. Converting this PR to draft for now, especially since I'm out for two weeks and will be calking with APM Astra after the holidays @jdgumz

ObfuscateStatsGroup(obfuscator, b)
a.Replacer.ReplaceStatsGroup(b)
group.Stats[n] = b
n++
Expand Down
5 changes: 2 additions & 3 deletions pkg/trace/agent/obfuscate.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,8 @@ func (a *Agent) obfuscateSpan(span *pb.Span) {
}
}

func (a *Agent) obfuscateStatsGroup(b *pb.ClientGroupedStats) {
o := a.lazyInitObfuscator()

// ObfuscateStatsGroup obfuscates sensitive data in APM stats payloads
func ObfuscateStatsGroup(o *obfuscate.Obfuscator, b *pb.ClientGroupedStats) {
switch b.Type {
case "sql", "cassandra":
oq, err := o.ObfuscateSQLString(b.Resource)
Expand Down
3 changes: 2 additions & 1 deletion pkg/trace/agent/obfuscate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ func TestObfuscateStatsGroup(t *testing.T) {
} {
agnt, stop := agentWithDefaults()
defer stop()
agnt.obfuscateStatsGroup(tt.in)
obfuscator := agnt.lazyInitObfuscator()
ObfuscateStatsGroup(obfuscator, tt.in)
assert.Equal(t, tt.in.Resource, tt.out)
}
}
Expand Down
Loading