From 0f58e133d5ab319eb88fd7339f674761b87d9c46 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Fri, 17 Jul 2026 14:14:55 -0400 Subject: [PATCH] feat(asgi): Gate client IP/user info behind data_collection config Apply data_collection.user_info filtering to REMOTE_ADDR/client.address, falling back to send_default_pii when data_collection is not configured. Also convert earlier query parameter data collection tests to end-to-end assertions via a real ASGI app. Ref PY-2586 Ref #6746 --- sentry_sdk/integrations/_asgi_common.py | 21 +- tests/integrations/asgi/test_asgi.py | 293 +++++++++++++++++------- 2 files changed, 231 insertions(+), 83 deletions(-) diff --git a/sentry_sdk/integrations/_asgi_common.py b/sentry_sdk/integrations/_asgi_common.py index 57ce4d54ca..bfd494554f 100644 --- a/sentry_sdk/integrations/_asgi_common.py +++ b/sentry_sdk/integrations/_asgi_common.py @@ -148,8 +148,12 @@ def _get_request_data( ) client = asgi_scope.get("client") - if client and should_send_default_pii(): - request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} + if client: + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} + elif should_send_default_pii(): + request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)} return request_data @@ -226,9 +230,14 @@ def _get_request_attributes( else url_without_query_string ) - client = asgi_scope.get("client") - if client and should_send_default_pii(): - ip = _get_ip(asgi_scope) - attributes["client.address"] = ip + asgi_scope_client = asgi_scope.get("client") + if asgi_scope_client: + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + ip = _get_ip(asgi_scope) + attributes["client.address"] = ip + elif should_send_default_pii(): + ip = _get_ip(asgi_scope) + attributes["client.address"] = ip return attributes diff --git a/tests/integrations/asgi/test_asgi.py b/tests/integrations/asgi/test_asgi.py index 224fe49a51..35f6c26091 100644 --- a/tests/integrations/asgi/test_asgi.py +++ b/tests/integrations/asgi/test_asgi.py @@ -8,9 +8,6 @@ from sentry_sdk.integrations._asgi_common import ( _get_headers, _get_ip, - _get_request_attributes, - _get_request_data, - _RootPathInPath, ) from sentry_sdk.integrations.asgi import SentryAsgiMiddleware, _looks_like_asgi3 from sentry_sdk.tracing import TransactionSource @@ -837,82 +834,94 @@ def test_get_headers(): } -def test_get_request_data_url_with_filtered_host(sentry_init): - # allowlist mode in data collection that does not allow "host" scrubs the host header value, - # but the reported URL must still resolve via rather than embedding the substituted "[Filtered]" value. +@pytest.mark.asyncio +async def test_get_request_data_url_with_filtered_host( + sentry_init, capture_events, asgi3_app +): + # allowlist mode in data collection that does not allow "host" scrubs the host + # header value, but the reported URL must still resolve rather than embedding the + # substituted "[Filtered]" value. sentry_init( + traces_sample_rate=1.0, _experiments={ "data_collection": { "http_headers": {"request": {"mode": "allowlist", "terms": []}} } - } + }, ) + app = SentryAsgiMiddleware(asgi3_app) - scope = { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": b"", - "headers": [(b"host", b"example.com")], - } + events = capture_events() + scope = {"server": ("example.com", 80), "scheme": "http"} + async with TestClient(app, scope=scope) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() - request_data = _get_request_data(scope, _RootPathInPath.EXCLUDED) + (transaction_event,) = events - assert request_data["headers"]["host"] == "[Filtered]" - assert request_data["url"] == "http://example.com/foo" + assert transaction_event["request"]["headers"]["host"] == "[Filtered]" + assert transaction_event["request"]["url"] == "http://example.com/foo" -def test_get_request_attributes_url_with_filtered_host(sentry_init): - # As with _get_request_data, an allowlist mode that does not allow "host" - # scrubs the host header value, but "url.full" must still resolve rather than embedding the substituted value. +@pytest.mark.asyncio +async def test_get_request_attributes_url_with_filtered_host( + sentry_init, capture_items, asgi3_app +): + # As with the request data, an allowlist mode that does not allow "host" scrubs + # the host header value, but "url.full" must still resolve rather than embedding + # the substituted value. sentry_init( send_default_pii=True, + traces_sample_rate=1.0, _experiments={ + "trace_lifecycle": "stream", "data_collection": { "http_headers": {"request": {"mode": "allowlist", "terms": []}} - } + }, }, ) + app = SentryAsgiMiddleware(asgi3_app) - scope = { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": b"somevalue=123", - "headers": [(b"host", b"example.com")], - } + items = capture_items("span") + scope = {"server": ("example.com", 80), "scheme": "http"} + async with TestClient(app, scope=scope) as client: + await client.get("/foo?somevalue=123", headers={"host": "example.com"}) + + sentry_sdk.flush() - attributes = _get_request_attributes(scope, _RootPathInPath.EXCLUDED) + assert len(items) == 1 + attributes = items[0].payload["attributes"] assert attributes["http.request.header.host"] == "[Filtered]" assert attributes["url.full"] == "http://example.com/foo?somevalue=123" -def test_get_request_attributes_url_with_headers_off(sentry_init): - # "off" mode in data collection captures no headers at all, but "url.full" - # must still resolve via the (uncaptured) host header rather than being dropped. +@pytest.mark.asyncio +async def test_get_request_attributes_url_with_headers_off( + sentry_init, capture_items, asgi3_app +): + # "off" mode in data collection captures no headers at all, but "url.full" must + # still resolve via the (uncaptured) host header rather than being dropped. sentry_init( send_default_pii=True, + traces_sample_rate=1.0, _experiments={ - "data_collection": {"http_headers": {"request": {"mode": "off"}}} + "trace_lifecycle": "stream", + "data_collection": {"http_headers": {"request": {"mode": "off"}}}, }, ) + app = SentryAsgiMiddleware(asgi3_app) - scope = { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": b"somevalue=123", - "headers": [(b"host", b"example.com")], - } + items = capture_items("span") + scope = {"server": ("example.com", 80), "scheme": "http"} + async with TestClient(app, scope=scope) as client: + await client.get("/foo?somevalue=123", headers={"host": "example.com"}) + + sentry_sdk.flush() - attributes = _get_request_attributes(scope, _RootPathInPath.EXCLUDED) + assert len(items) == 1 + attributes = items[0].payload["attributes"] assert not any(key.startswith("http.request.header.") for key in attributes) assert attributes["url.full"] == "http://example.com/foo?somevalue=123" @@ -922,18 +931,11 @@ def test_get_request_attributes_url_with_headers_off(sentry_init): QUERY_STRING = "token=abc&theme=dark&lang=en&session=xyz" -def _http_scope(query_string=QUERY_STRING): - return { - "type": "http", - "method": "GET", - "scheme": "http", - "server": ("example.com", 80), - "path": "/foo", - "query_string": query_string.encode("latin-1"), - "headers": [(b"host", b"example.com")], - } +def _http_scope(): + return {"server": ("example.com", 80), "scheme": "http"} +@pytest.mark.asyncio @pytest.mark.parametrize( "init_kwargs, expected_query_string", [ @@ -1012,12 +1014,20 @@ def _http_scope(query_string=QUERY_STRING): ), ], ) -def test_get_request_data_query_string_data_collection( - sentry_init, init_kwargs, expected_query_string +async def test_get_request_data_query_string_data_collection( + sentry_init, capture_events, asgi3_app, init_kwargs, expected_query_string ): - sentry_init(**init_kwargs) + sentry_init(traces_sample_rate=1.0, **init_kwargs) + app = SentryAsgiMiddleware(asgi3_app) + + events = capture_events() + async with TestClient(app, scope=_http_scope()) as client: + await client.get(f"/foo?{QUERY_STRING}", headers={"host": "example.com"}) + + sentry_sdk.flush() - request_data = _get_request_data(_http_scope(), _RootPathInPath.EXCLUDED) + (transaction_event,) = events + request_data = transaction_event["request"] if expected_query_string is NO_QUERY_STRING: assert "query_string" not in request_data @@ -1025,27 +1035,42 @@ def test_get_request_data_query_string_data_collection( assert request_data["query_string"] == expected_query_string -def test_get_request_data_query_string_empty_legacy_is_none(sentry_init): +@pytest.mark.asyncio +async def test_get_request_data_query_string_empty_legacy_is_none( + sentry_init, capture_events, asgi3_app +): # Legacy path: the query string is always set even when empty (``None``). - sentry_init(send_default_pii=True) + sentry_init(send_default_pii=True, traces_sample_rate=1.0) + app = SentryAsgiMiddleware(asgi3_app) - request_data = _get_request_data( - _http_scope(query_string=""), _RootPathInPath.EXCLUDED - ) + events = capture_events() + async with TestClient(app, scope=_http_scope()) as client: + await client.get("/foo", headers={"host": "example.com"}) - assert request_data["query_string"] is None + sentry_sdk.flush() + (transaction_event,) = events + assert transaction_event["request"]["query_string"] is None -def test_get_request_data_empty_query_string_dropped_with_data_collection(sentry_init): - sentry_init(_experiments={"data_collection": {}}) - request_data = _get_request_data( - _http_scope(query_string=""), _RootPathInPath.EXCLUDED - ) +@pytest.mark.asyncio +async def test_get_request_data_empty_query_string_dropped_with_data_collection( + sentry_init, capture_events, asgi3_app +): + sentry_init(traces_sample_rate=1.0, _experiments={"data_collection": {}}) + app = SentryAsgiMiddleware(asgi3_app) + + events = capture_events() + async with TestClient(app, scope=_http_scope()) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() - assert "query_string" not in request_data + (transaction_event,) = events + assert "query_string" not in transaction_event["request"] +@pytest.mark.asyncio @pytest.mark.parametrize( "init_kwargs, expected_query, expected_url_full", [ @@ -1108,12 +1133,27 @@ def test_get_request_data_empty_query_string_dropped_with_data_collection(sentry ), ], ) -def test_get_request_attributes_query_data_collection( - sentry_init, init_kwargs, expected_query, expected_url_full +async def test_get_request_attributes_query_data_collection( + sentry_init, + capture_items, + asgi3_app, + init_kwargs, + expected_query, + expected_url_full, ): - sentry_init(**init_kwargs) + kwargs = {k: v for k, v in init_kwargs.items() if k != "_experiments"} + experiments = {"trace_lifecycle": "stream", **init_kwargs.get("_experiments", {})} + sentry_init(traces_sample_rate=1.0, _experiments=experiments, **kwargs) + app = SentryAsgiMiddleware(asgi3_app) - attributes = _get_request_attributes(_http_scope(), _RootPathInPath.EXCLUDED) + items = capture_items("span") + async with TestClient(app, scope=_http_scope()) as client: + await client.get(f"/foo?{QUERY_STRING}", headers={"host": "example.com"}) + + sentry_sdk.flush() + + assert len(items) == 1 + attributes = items[0].payload["attributes"] if expected_query is NO_QUERY_STRING: assert "http.query" not in attributes @@ -1128,6 +1168,105 @@ def test_get_request_attributes_query_data_collection( assert attributes["url.path"] == "/foo" +USER_INFO_CASES = [ + pytest.param( + {"_experiments": {"data_collection": {"user_info": False}}}, + True, + False, + id="dc_user_info_false", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + True, + True, + id="dc_default_user_info", + ), + pytest.param( + { + "send_default_pii": True, + "_experiments": {"data_collection": {"user_info": False}}, + }, + True, + False, + id="dc_wins_over_pii", + ), + pytest.param( + {"send_default_pii": True}, + True, + True, + id="legacy_pii_true", + ), + pytest.param( + {"send_default_pii": False}, + True, + False, + id="legacy_pii_false", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + False, + False, + id="no_client", + ), +] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("init_kwargs, has_client, expect_ip", USER_INFO_CASES) +async def test_get_request_data_env_user_info( + sentry_init, capture_events, asgi3_app, init_kwargs, has_client, expect_ip +): + sentry_init(traces_sample_rate=1.0, **init_kwargs) + app = SentryAsgiMiddleware(asgi3_app) + + scope = _http_scope() + if has_client: + scope["client"] = ("127.0.0.1", 60457) + + events = capture_events() + async with TestClient(app, scope=scope) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() + + (transaction_event,) = events + request_data = transaction_event["request"] + + if expect_ip: + assert request_data["env"] == {"REMOTE_ADDR": "127.0.0.1"} + else: + assert "env" not in request_data + + +@pytest.mark.asyncio +@pytest.mark.parametrize("init_kwargs, has_client, expect_ip", USER_INFO_CASES) +async def test_get_request_attributes_client_address_user_info( + sentry_init, capture_items, asgi3_app, init_kwargs, has_client, expect_ip +): + kwargs = {k: v for k, v in init_kwargs.items() if k != "_experiments"} + experiments = {"trace_lifecycle": "stream", **init_kwargs.get("_experiments", {})} + sentry_init(traces_sample_rate=1.0, _experiments=experiments, **kwargs) + app = SentryAsgiMiddleware(asgi3_app) + + scope = _http_scope() + if has_client: + scope["client"] = ("127.0.0.1", 60457) + + items = capture_items("span") + async with TestClient(app, scope=scope) as client: + await client.get("/foo", headers={"host": "example.com"}) + + sentry_sdk.flush() + + assert len(items) == 1 + attributes = items[0].payload["attributes"] + + if expect_ip: + assert attributes["client.address"] == "127.0.0.1" + else: + assert "client.address" not in attributes + + @pytest.mark.asyncio @pytest.mark.parametrize( "request_url,transaction_style,expected_transaction_name,expected_transaction_source",