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

Add automatic scrubbing for tracing #560

Closed

Conversation

jvanbrunschot
Copy link

Hi,

I would love to add Agent Trace Obfuscation configuration. While this PR is not done. I would like to have your opinion on how to test (spec) and how to get the data (type checking) into the module. I can of course write a Struct to verify if all the data is correctly set but looking at all the different values of the obfuscation configuration this seems far from ideal and it would introduce some maintenance as well.

Or should I go for something like extra_apm_data which just accepts random data and puts that in the configuration? I think other modules takes this approach as well.

Thanks!

Copy link
Contributor

@albertvaka albertvaka left a comment

Choose a reason for hiding this comment

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

There is always a trade-off between flexibility (adding new fields without having to change this module) and type safety (checking those fields exist/are correct). In this case, I think we can do with a generic apm_obfuscation map, and just output its content under apm_conf -> obfuscation like your PR does. Maybe adding an example in the field description. Apart from that, the functionality is there and the PR looks quite good already :)

Sorry for the late reply and thanks for sending this!

@albertvaka
Copy link
Contributor

albertvaka commented Feb 19, 2020

Rebased in #615

@jvanbrunschot Would it be enough for your use case if we merge it as it is?

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.

2 participants