feat(asm): add the DD_TRACE_CLIENT_IP_ENABLED env var to fetch request IPs with ASM disabled (#4971)

Backport of #4949 to 1.8

+tests.

Signed-off-by: Juanjo Alvarez <[email protected]>

Signed-off-by: Juanjo Alvarez <[email protected]>
Co-authored-by: Brett Langdon <[email protected]>

## Description
<!-- Briefly describe the change and why it was required. -->

<!-- If this is a breaking change, explain why it is necessary. Breaking
changes must append `!` after the type/scope. See
https://ddtrace.readthedocs.io/en/stable/contributing.html for more
details. -->

## Checklist
- [ ] Followed the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines)
when writing a release note.
- [ ] Add additional sections for `feat` and `fix` pull requests.
- [ ] [Library
documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs)
and/or [Datadog's documentation
site](https://github.com/DataDog/documentation/) is updated. Link to doc
PR in description.

<!-- Copy and paste the relevant snippet based on the type of pull
request -->

<!-- START feat -->

## Motivation
<!-- Expand on why the change is required, include relevant context for
reviewers -->

## Design 
<!-- Include benefits from the change as well as possible drawbacks and
trade-offs -->

## Testing strategy
<!-- Describe the automated tests and/or the steps for manual testing.

<!-- END feat -->

<!-- START fix -->

## Relevant issue(s)
<!-- Link the pull request to any issues related to the fix. Use
keywords for links to automate closing the issues once the pull request
is merged. -->

## Testing strategy
<!-- Describe any added regression tests and/or the manual testing
performed. -->

<!-- END fix -->

## Reviewer Checklist
- [ ] Title is accurate.
- [ ] Description motivates each change.
- [ ] No unnecessary changes were introduced in this PR.
- [ ] Avoid breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [ ] Tests provided or description of manual testing performed is
included in the code or PR.
- [ ] Release note has been added and follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines),
or else `changelog/no-changelog` label added.
- [ ] All relevant GitHub issues are correctly linked.
- [ ] Change contains telemetry where appropriate (logs, metrics, etc.).
- [ ] Telemetry is meaningful, actionable and does not have the
potential to leak sensitive data.

Signed-off-by: Juanjo Alvarez <[email protected]>
Co-authored-by: Juanjo Alvarez Martinez <[email protected]>
Co-authored-by: Brett Langdon <[email protected]>

Author: 3 people

ddtrace/contrib/trace_utils.py

@@ -480,7 +480,7 @@ def set_http_meta(
 
         # We always collect the IP if appsec is enabled to report it on potential vulnerabilities.
         # https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2118779066/Client+IP+addresses+resolution
-        if config._appsec_enabled:
+        if config._appsec_enabled or config.retrieve_client_ip:
             ip = _get_request_header_client_ip(span, request_headers, peer_ip, headers_are_case_sensitive)
             if ip:
                 span.set_tag_str(http.CLIENT_IP, ip)

ddtrace/settings/config.py

@@ -195,6 +195,7 @@ def __init__(self):
         legacy_config_value = os.getenv("DD_ANALYTICS_ENABLED", default=False)
 
         self.analytics_enabled = asbool(os.getenv("DD_TRACE_ANALYTICS_ENABLED", default=legacy_config_value))
+        self.retrieve_client_ip = asbool(os.getenv("DD_TRACE_CLIENT_IP_ENABLED", default=False))
 
         self.tags = parse_tags_str(os.getenv("DD_TAGS") or "")
 

releasenotes/notes/asm-dd-client-enabled-env-var-0d418972a04728f2.yaml

@@ -0,0 +1,3 @@
+features:
+  - |
+    tracing: Add support for enabling collecting of HTTP request client IP addresses as the ``http.client_ip`` span tag. You can set the ``DD_TRACE_CLIENT_IP_ENABLED`` environment variable to ``true`` to enable. This feature is disabled by default.

tests/tracer/test_trace_utils.py

@@ -580,6 +580,49 @@ def test_set_http_meta_headers_ip(
             mock_store_headers.assert_called()
 
 
+def test_set_http_meta_headers_ip_asm_disabled_env_default_false(span, int_config):
+    with override_global_config(dict(_appsec_enabled=False)):
+        int_config.myint.http._header_tags = {"enabled": True}
+        assert int_config.myint.is_header_tracing_configured is True
+        trace_utils.set_http_meta(
+            span,
+            int_config.myint,
+            request_headers={"via": "8.8.8.8"},
+        )
+        result_keys = list(span.get_tags().keys())
+        result_keys.sort(reverse=True)
+        assert result_keys == ["runtime-id"]
+
+
+def test_set_http_meta_headers_ip_asm_disabled_env_false(span, int_config):
+    with override_global_config(dict(_appsec_enabled=False, retrieve_client_ip=False)):
+        int_config.myint.http._header_tags = {"enabled": True}
+        assert int_config.myint.is_header_tracing_configured is True
+        trace_utils.set_http_meta(
+            span,
+            int_config.myint,
+            request_headers={"via": "8.8.8.8"},
+        )
+        result_keys = list(span.get_tags().keys())
+        result_keys.sort(reverse=True)
+        assert result_keys == ["runtime-id"]
+
+
+def test_set_http_meta_headers_ip_asm_disabled_env_true(span, int_config):
+    with override_global_config(dict(_appsec_enabled=False, retrieve_client_ip=True)):
+        int_config.myint.http._header_tags = {"enabled": True}
+        assert int_config.myint.is_header_tracing_configured is True
+        trace_utils.set_http_meta(
+            span,
+            int_config.myint,
+            request_headers={"via": "8.8.8.8"},
+        )
+        result_keys = list(span.get_tags().keys())
+        result_keys.sort(reverse=True)
+        assert result_keys == ["runtime-id", "network.client.ip", http.CLIENT_IP]
+        assert span.get_tag(http.CLIENT_IP) == "8.8.8.8"
+
+
 def test_ip_subnet_regression():
     del_ip = "1.2.3.4/32"
     req_ip = "10.2.3.4"

tests/utils.py

@@ -86,6 +86,7 @@ def override_global_config(values):
     # DEV: We do not do `ddtrace.config.keys()` because we have all of our integrations
     global_config_keys = [
         "analytics_enabled",
+        "retrieve_client_ip",
         "report_hostname",
         "health_metrics_enabled",
         "_propagation_style_extract",