Add automatic scrubbing for tracing#615
Conversation
albertvaka
force-pushed
the
albertvaka/apm-obfuscation
branch
from
e737652 to
dea484e
Compare
gbbr
left a comment
gbbr
left a comment
There was a problem hiding this comment.
I don't have enough Ruby knowledge, but I'd say this looks good. The main idea is to make sure all options work, which are in the docs here: https://docs.datadoghq.com/tracing/guide/agent-obfuscation.
I think I'd feel more comfortable if the tests would be more thorough and actually verify that the full object gets translated into the file, not just a few options.
truthbk
left a comment
truthbk
left a comment
There was a problem hiding this comment.
Looks good, just a minor nit.
| } | ||
| end | ||
|
|
||
| context 'with apm_enabled set to true and apm_obfuscation specified' do |
There was a problem hiding this comment.
nit: we're missing a similar test for A5 to test the ini-style file generation (the conf footers), you might feel free to ignore, but just pointing it out.
There was a problem hiding this comment.
We don't pass this config to A5 at all, so there should be no change for A5. These settings end up in $agent_config which is only used when generating A6/7 config.
There was a problem hiding this comment.
Then why have changes to templates/datadog_apm_footer.conf.erb at all? I guess we can remove that, that's for A5 config.
There was a problem hiding this comment.
My bad, it is used by that template. I'll add a test.
There was a problem hiding this comment.
Actually the template is not working since the nested objects aren't output in ini format. I'm just going to remove it and not support this on A5, which I think is reasonable.
albertvaka
force-pushed
the
albertvaka/apm-obfuscation
branch
from
dea484e to
f03f48e
Compare
Co-authored-by: Jamie van Brunschot <j.vanbrunschot@coolblue.nl>
3 participants
Rebase of #560