chore(asm): enable ip collection if not disabled and appsec is enabled (#4333)

Enable IP collection if `DD_TRACE_CLIENT_IP_HEADER_DISABLED` is not set but appsec is enabled as per https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2118779066/Client+IP+addresses+resolution#Activation

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



## Checklist
- [ ] Title must conform to [conventional commit](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional).
- [ ] Add additional sections for `feat` and `fix` pull requests.
- [ ] Ensure tests are passing for affected code.
- [ ] [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.





## Motivation


## Design 


## Testing strategy




## Relevant issue(s)


## Testing strategy




## Reviewer Checklist
- [ ] Title is accurate.
- [ ] Description motivates each change.
- [ ] No unnecessary changes were introduced in this PR.
- [ ] PR cannot be broken up into smaller PRs.
- [ ] 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 for fixes and features, or else `changelog/no-changelog` label added.
- [ ] All relevant GitHub issues are correctly linked.
- [ ] Backports are identified and tagged with Mergifyio.
- [ ] Add to milestone.

Author: juanjux

ddtrace/contrib/trace_utils.py

@@ -177,7 +177,10 @@ def _get_request_header_client_ip(span, headers, peer_ip=None, headers_are_case_
     # type: (Span, Mapping[str, str], Optional[str], bool) -> str
     global _USED_IP_HEADER
 
-    if asbool(os.getenv("DD_TRACE_CLIENT_IP_HEADER_DISABLED", default=False)):
+    ip_collection_disabled = os.getenv("DD_TRACE_CLIENT_IP_HEADER_DISABLED", default=None)
+    if (ip_collection_disabled is None and not config._appsec_enabled) or asbool(ip_collection_disabled):
+        # IP Collection will honor the environment var is set. Otherwise it will be enabled
+        # only if appsec is enabled
         return ""
 
     def get_header_value(key):  # type: (str) -> Optional[str]

docs/spelling_wordlist.txt

@@ -22,6 +22,7 @@ agentless
 analytics
 api
 app
+appsec
 aredis
 args
 asgi
@@ -198,4 +199,4 @@ libddwaf
 Serverless
 serverless
 cattrs
-IAST
+IAST

releasenotes/notes/asm-ipaddress-consider-appsec-value-9a0948896645a4b1.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    ASM: ip address collection will be enabled if not explicitly disabled and appsec is enabled.

tests/contrib/django/test_django_appsec.py

@@ -248,7 +248,7 @@ def test_django_useragent(client, test_spans, tracer):
         assert root_span.get_tag(http.USER_AGENT) == "test/1.2.3"
 
 
-def test_django_client_ip_disabled(client, test_spans, tracer):
+def test_django_client_ip_env_var_disabled_appsec_enabled_must_be_disabled(client, test_spans, tracer):
     with override_global_config(dict(_appsec_enabled=True)), override_env(
         dict(DD_TRACE_CLIENT_IP_HEADER_DISABLED="True")
     ):
@@ -257,6 +257,29 @@ def test_django_client_ip_disabled(client, test_spans, tracer):
         assert not root_span.get_tag(http.CLIENT_IP)
 
 
+def test_django_client_ip_env_var_not_disabled_appsec_disabled_must_be_enabled(client, test_spans, tracer):
+    with override_global_config(dict(_appsec_enabled=False)), override_env(
+        dict(DD_TRACE_CLIENT_IP_HEADER_DISABLED="False")
+    ):
+        client.get("/?a=1&b&c=d", HTTP_X_REAL_IP="8.8.8.8")
+        root_span = test_spans.spans[0]
+        assert root_span.get_tag(http.CLIENT_IP)
+
+
+def test_django_client_ip_env_var_missing_appsec_enabled_must_be_enabled(client, test_spans, tracer):
+    with override_global_config(dict(_appsec_enabled=True)):
+        client.get("/?a=1&b&c=d", HTTP_X_REAL_IP="8.8.8.8")
+        root_span = test_spans.spans[0]
+        assert root_span.get_tag(http.CLIENT_IP)
+
+
+def test_django_client_ip_env_var_missing_appsec_disabled_must_be_disabled(client, test_spans, tracer):
+    with override_global_config(dict(_appsec_enabled=False)):
+        client.get("/?a=1&b&c=d", HTTP_X_REAL_IP="8.8.8.8")
+        root_span = test_spans.spans[0]
+        assert not root_span.get_tag(http.CLIENT_IP)
+
+
 def test_django_client_ip_header_set_by_env_var_empty(client, test_spans, tracer):
     with override_global_config(dict(_appsec_enabled=True)), override_env(
         dict(DD_TRACE_CLIENT_IP_HEADER="Fooipheader")

tests/contrib/flask/test_flask_appsec.py

@@ -126,7 +126,9 @@ def test_flask_useragent(self):
         assert root_span.get_tag(http.USER_AGENT) == "test/1.2.3"
 
     def test_flask_client_ip_header_set_by_env_var_valid(self):
-        with override_env(dict(DD_TRACE_CLIENT_IP_HEADER="X-Use-This")):
+        with override_global_config(dict(_appsec_enabled=True)), override_env(
+            dict(DD_TRACE_CLIENT_IP_HEADER="X-Use-This")
+        ):
             self.client.get("/?a=1&b&c=d", headers={"HTTP_CLIENT_IP": "8.8.8.8", "X-Use-This": "4.4.4.4"})
             spans = self.pop_spans()
             root_span = spans[0]