feat(sanic): Gate user info behind data_collection config

sentry_sdk/integrations/sanic.py

@@ -196,8 +196,12 @@ async def _context_enter(request: "Request") -> None:
         sentry_sdk.traces.continue_trace(dict(request.headers))
         scope.set_custom_sampling_context({"sanic_request": request})
 
-        if should_send_default_pii() and request.remote_addr:
-            scope.set_attribute(SPANDATA.USER_IP_ADDRESS, request.remote_addr)
+        if request.remote_addr:
+            if has_data_collection_enabled(client.options):
+                if client.options["data_collection"]["user_info"]:
+                    scope.set_attribute(SPANDATA.USER_IP_ADDRESS, request.remote_addr)
+            elif should_send_default_pii():
+                scope.set_attribute(SPANDATA.USER_IP_ADDRESS, request.remote_addr)
 
         span = sentry_sdk.traces.start_span(
             # Unless the request results in a 404 error, the name and source
@@ -401,19 +405,23 @@ def _get_request_attributes(request: "Request") -> "Dict[str, Any]":
             query=filtered_query or ""
         ).geturl()
 
+        if request.remote_addr:
+            if client_options["data_collection"]["user_info"]:
+                attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_addr
+
     elif should_send_default_pii():
         attributes[SPANDATA.URL_FULL] = request.url
         attributes["url.path"] = urlparts.path
 
         if urlparts.query:
             attributes[SPANDATA.HTTP_QUERY] = urlparts.query
 
+        if request.remote_addr:
+            attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_addr
+
     if urlparts.scheme:
         attributes[SPANDATA.NETWORK_PROTOCOL_NAME] = urlparts.scheme
 
-    if should_send_default_pii() and request.remote_addr:
-        attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_addr
-
     return attributes
 
 

tests/integrations/django/asgi/test_asgi.py

@@ -16,10 +16,8 @@
 from sentry_sdk.integrations.django import DjangoIntegration
 from sentry_sdk.integrations.django.asgi import _asgi_middleware_mixin_factory
 from tests.integrations.django.myapp.asgi import channels_application
-from tests.integrations.django.utils import (
-    USER_INFO_INIT_KWARGS,
-    pytest_mark_django_db_decorator,
-)
+from tests.integrations.django.utils import pytest_mark_django_db_decorator
+from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES
 
 try:
     from django.urls import reverse
@@ -1108,7 +1106,7 @@ async def test_async_middleware_process_exception_is_awaited(
 @pytest.mark.skipif(
     django.VERSION < (3, 0), reason="Django ASGI support shipped in 3.0"
 )
-@pytest.mark.parametrize("init_kwargs, expect_user", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES)
 @pytest_mark_django_db_decorator()
 async def test_user_identity_error_event_data_collection(
     sentry_init, capture_events, application, init_kwargs, expect_user

tests/integrations/django/test_data_scrubbing.py

@@ -6,10 +6,8 @@
 from sentry_sdk.integrations.django import DjangoIntegration
 from tests.conftest import unpack_werkzeug_response, werkzeug_set_cookie
 from tests.integrations.django.myapp.wsgi import application
-from tests.integrations.django.utils import (
-    USER_INFO_INIT_KWARGS,
-    pytest_mark_django_db_decorator,
-)
+from tests.integrations.django.utils import pytest_mark_django_db_decorator
+from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES
 
 try:
     from django.urls import reverse
@@ -397,7 +395,7 @@ def test_empty_query_string_is_dropped_with_data_collection(
 
 @pytest.mark.forked
 @pytest_mark_django_db_decorator()
-@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_info_span_attributes_data_collection(
     sentry_init, client, capture_items, init_kwargs, expect_ip
 ):
@@ -433,7 +431,7 @@ def test_user_info_span_attributes_data_collection(
 
 @pytest.mark.forked
 @pytest_mark_django_db_decorator()
-@pytest.mark.parametrize("init_kwargs, expect_user", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_identity_span_attributes_data_collection(
     sentry_init, client, capture_items, init_kwargs, expect_user
 ):
@@ -469,7 +467,7 @@ def test_user_identity_span_attributes_data_collection(
 
 @pytest.mark.forked
 @pytest_mark_django_db_decorator()
-@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_info_error_event_data_collection(
     sentry_init, client, capture_events, init_kwargs, expect_ip
 ):
@@ -490,7 +488,7 @@ def test_user_info_error_event_data_collection(
 
 @pytest.mark.forked
 @pytest_mark_django_db_decorator()
-@pytest.mark.parametrize("init_kwargs, expect_user", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_identity_error_event_data_collection(
     sentry_init, client, capture_events, init_kwargs, expect_user
 ):

tests/integrations/django/utils.py

@@ -19,40 +19,3 @@
         )
 except AttributeError:
     pass
-
-
-# Shared parametrization test matrix exercising the precedence between the legacy
-# ``send_default_pii`` boolean and the ``data_collection.user_info`` setting.
-# The second value indicates whether user info is expected to be collected.
-USER_INFO_INIT_KWARGS = [
-    pytest.param({"send_default_pii": True}, True, id="legacy_send_default_pii_true"),
-    pytest.param(
-        {"send_default_pii": False}, False, id="legacy_send_default_pii_false"
-    ),
-    pytest.param(
-        {"_experiments": {"data_collection": {"user_info": True}}},
-        True,
-        id="data_collection_user_info_true",
-    ),
-    pytest.param(
-        {"_experiments": {"data_collection": {"user_info": False}}},
-        False,
-        id="data_collection_user_info_false",
-    ),
-    pytest.param(
-        {
-            "send_default_pii": True,
-            "_experiments": {"data_collection": {"user_info": False}},
-        },
-        False,
-        id="data_collection_wins_over_send_default_pii_true",
-    ),
-    pytest.param(
-        {
-            "send_default_pii": False,
-            "_experiments": {"data_collection": {"user_info": True}},
-        },
-        True,
-        id="data_collection_wins_over_send_default_pii_false",
-    ),
-]

tests/integrations/flask/test_flask.py

@@ -32,6 +32,7 @@
 from sentry_sdk.consts import SPANDATA
 from sentry_sdk.integrations.logging import LoggingIntegration
 from sentry_sdk.serializer import MAX_DATABAG_BREADTH
+from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES
 
 # Query string used across the query-param filtering tests below. ``auth`` is a
 # built-in sensitive term, so it is redacted by the default denylist.
@@ -1398,45 +1399,7 @@ def test_empty_query_string_is_dropped_with_data_collection(
     assert "query_string" not in event["request"]
 
 
-# Parametrization shared by the user_info tests below. ``expect_ip`` is
-# whether the client IP may be collected under the given init kwargs.
-USER_INFO_INIT_KWARGS = [
-    pytest.param({"send_default_pii": True}, True, id="legacy_send_default_pii_true"),
-    pytest.param(
-        {"send_default_pii": False}, False, id="legacy_send_default_pii_false"
-    ),
-    pytest.param(
-        {"_experiments": {"data_collection": {"user_info": True}}},
-        True,
-        id="data_collection_user_info_true",
-    ),
-    pytest.param(
-        {"_experiments": {"data_collection": {"user_info": False}}},
-        False,
-        id="data_collection_user_info_false",
-    ),
-    # ``data_collection`` is the single source of truth: it must win over
-    # ``send_default_pii`` when both are configured.
-    pytest.param(
-        {
-            "send_default_pii": True,
-            "_experiments": {"data_collection": {"user_info": False}},
-        },
-        False,
-        id="data_collection_wins_over_send_default_pii_true",
-    ),
-    pytest.param(
-        {
-            "send_default_pii": False,
-            "_experiments": {"data_collection": {"user_info": True}},
-        },
-        True,
-        id="data_collection_wins_over_send_default_pii_false",
-    ),
-]
-
-
-@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_info_span_attributes_data_collection(
     sentry_init, app, capture_items, monkeypatch, init_kwargs, expect_ip
 ):
@@ -1472,7 +1435,7 @@ def test_user_info_span_attributes_data_collection(
         assert "client.address" not in segment["attributes"]
 
 
-@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_info_error_event_data_collection(
     sentry_init, app, capture_events, monkeypatch, init_kwargs, expect_ip
 ):
@@ -1523,7 +1486,7 @@ def crash():
     assert "ip_address" not in event.get("user", {})
 
 
-@pytest.mark.parametrize("init_kwargs, expect_user", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES)
 def test_flask_login_user_identity_error_event_data_collection(
     sentry_init, app, capture_events, init_kwargs, expect_user
 ):
@@ -1571,7 +1534,7 @@ def crash():
         assert "username" not in user
 
 
-@pytest.mark.parametrize("init_kwargs, expect_user", USER_INFO_INIT_KWARGS)
+@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES)
 def test_flask_login_user_identity_span_attributes_data_collection(
     sentry_init, app, capture_items, init_kwargs, expect_user
 ):

tests/integrations/sanic/test_sanic.py

@@ -16,6 +16,7 @@
 from sentry_sdk.integrations.sanic import SanicIntegration
 from sentry_sdk.tracing import TransactionSource
 from tests.conftest import get_free_port
+from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES
 
 try:
     from sanic_testing import TestManager
@@ -559,9 +560,9 @@ def test_span_origin(sentry_init, app, capture_events, capture_items, span_strea
 @pytest.mark.skipif(
     not PERFORMANCE_SUPPORTED, reason="Performance not supported on this Sanic version"
 )
-@pytest.mark.parametrize("send_default_pii", [True, False])
+@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES)
 def test_user_ip_address_on_all_spans(
-    sentry_init, app, capture_items, send_default_pii
+    sentry_init, app, capture_items, init_kwargs, expect_ip
 ):
     app.config.FORWARDED_SECRET = "test"
 
@@ -575,8 +576,8 @@ def child_span_handler(request):
         integrations=[SanicIntegration()],
         default_integrations=False,
         traces_sample_rate=1.0,
-        send_default_pii=send_default_pii,
         trace_lifecycle="stream",
+        **init_kwargs,
     )
 
     items = capture_items("span")
@@ -592,14 +593,54 @@ def child_span_handler(request):
 
     child_span, server_span = [item.payload for item in items]
 
-    if send_default_pii:
+    if expect_ip:
         assert server_span["attributes"]["user.ip_address"] == "127.0.0.1"
         assert child_span["attributes"]["user.ip_address"] == "127.0.0.1"
     else:
         assert "user.ip_address" not in server_span["attributes"]
         assert "user.ip_address" not in child_span["attributes"]
 
 
+@pytest.mark.skipif(
+    not PERFORMANCE_SUPPORTED, reason="Performance not supported on this Sanic version"
+)
+@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES)
+def test_client_address_span_attribute_data_collection(
+    sentry_init, app, capture_items, init_kwargs, expect_ip
+):
+    app.config.FORWARDED_SECRET = "test"
+
+    sentry_init(
+        integrations=[SanicIntegration()],
+        default_integrations=False,
+        traces_sample_rate=1.0,
+        trace_lifecycle="stream",
+        **init_kwargs,
+    )
+
+    items = capture_items("span")
+
+    c = get_client(app)
+    with c as client:
+        client.get(
+            "/message",
+            headers={"Forwarded": "for=127.0.0.1;secret=test"},
+        )
+
+    sentry_sdk.flush()
+
+    (server_span,) = [
+        item.payload
+        for item in items
+        if item.payload["attributes"].get("sentry.origin") == "auto.http.sanic"
+    ]
+
+    if expect_ip:
+        assert server_span["attributes"]["client.address"] == "127.0.0.1"
+    else:
+        assert "client.address" not in server_span["attributes"]
+
+
 _QUERY_PARAM_DATA_COLLECTION_CASES = [
     pytest.param(
         {"send_default_pii": True},

tests/integrations/utils.py

@@ -0,0 +1,38 @@
+import pytest
+
+# Shared parametrization test matrix exercising the precedence between the legacy
+# ``send_default_pii`` boolean and the ``data_collection.user_info`` setting.
+# Each case is ``(init_kwargs, expect_user_info)`` where the second element indicates
+# whether user info (IP address, user identity, etc.) is expected to be collected.
+DATA_COLLECTION_USER_INFO_CASES = [
+    pytest.param({"send_default_pii": True}, True, id="legacy_send_default_pii_true"),
+    pytest.param(
+        {"send_default_pii": False}, False, id="legacy_send_default_pii_false"
+    ),
+    pytest.param(
+        {"_experiments": {"data_collection": {"user_info": True}}},
+        True,
+        id="data_collection_user_info_true",
+    ),
+    pytest.param(
+        {"_experiments": {"data_collection": {"user_info": False}}},
+        False,
+        id="data_collection_user_info_false",
+    ),
+    pytest.param(
+        {
+            "send_default_pii": True,
+            "_experiments": {"data_collection": {"user_info": False}},
+        },
+        False,
+        id="data_collection_wins_over_send_default_pii_true",
+    ),
+    pytest.param(
+        {
+            "send_default_pii": False,
+            "_experiments": {"data_collection": {"user_info": True}},
+        },
+        True,
+        id="data_collection_wins_over_send_default_pii_false",
+    ),
+]