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

Default to disable http.client_ip tag collection #2321

Merged
merged 5 commits into from
Oct 26, 2022

Conversation

lloeki
Copy link
Contributor

@lloeki lloeki commented Oct 19, 2022

What does this PR do?

Change http.client_ip (implemented in #2248) collection to opt-in.

Motivation

http.client_ip should not be collected by default by the tracer.

@lloeki lloeki requested a review from a team October 19, 2022 13:41
@github-actions github-actions bot added the core Involves Datadog core libraries label Oct 19, 2022
TonyCTHsu
TonyCTHsu previously approved these changes Oct 19, 2022
@lloeki lloeki force-pushed the default-to-disable-client-ip branch from 0c751bf to a42806d Compare October 24, 2022 14:44
@github-actions github-actions bot added appsec Application Security monitoring product integrations Involves tracing integrations tracing labels Oct 24, 2022
@lloeki lloeki requested a review from TonyCTHsu October 25, 2022 12:42
@lloeki lloeki dismissed TonyCTHsu’s stale review October 25, 2022 12:43

It was a bit early to review as more changes were needed.

@lloeki lloeki force-pushed the default-to-disable-client-ip branch from 9fe33f4 to 2d0c039 Compare October 25, 2022 14:52
@lloeki
Copy link
Contributor Author

lloeki commented Oct 25, 2022

Rebased to benefit from #2327

@lloeki lloeki requested a review from a team October 25, 2022 14:54
Comment on lines 57 to 58
ENV_DISABLED = 'DD_TRACE_CLIENT_IP_HEADER_DISABLED'.freeze
ENV_ENABLED = 'DD_TRACE_CLIENT_IP_HEADER_ENABLED'.freeze
ENV_DISABLED = 'DD_TRACE_CLIENT_IP_HEADER_DISABLED'.freeze # TODO: deprecated, remove later
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there might be a mistake in the original one and it's supposed to be DD_TRACE_CLIENT_IP_ENABLED. Checking.

This removes the `_HEADER` part inconsistent with the configuration
setting.
@lloeki lloeki merged commit 7c03bb3 into master Oct 26, 2022
@lloeki lloeki deleted the default-to-disable-client-ip branch October 26, 2022 11:15
@github-actions github-actions bot added this to the 1.6.0 milestone Oct 26, 2022
@lloeki lloeki mentioned this pull request Oct 26, 2022
lloeki added a commit that referenced this pull request Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appsec Application Security monitoring product core Involves Datadog core libraries integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants