diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index abad953de3..4e7559e751 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -63,6 +63,7 @@ get_sdk_name, get_type_name, handle_in_app, + has_data_collection_enabled, has_logs_enabled, has_metrics_enabled, logger, @@ -351,12 +352,20 @@ def _get_options(*args: "Optional[str]", **kwargs: "Any") -> "Dict[str, Any]": rv["data_collection"] = _resolve_data_collection(rv) - if rv["event_scrubber"] is None: + # Do not add the event scrubber if data collection is enabled as it can remove data that's + # collected under data collection config + if not has_data_collection_enabled(rv) and rv["event_scrubber"] is None: rv["event_scrubber"] = EventScrubber( send_default_pii=False if rv["send_default_pii"] is None else rv["send_default_pii"] ) + elif has_data_collection_enabled(rv) and rv["event_scrubber"]: + warnings.warn( + "Event scrubbers are not enabled when data collection configuration is provided. Ignoring event_scrubber...", + stacklevel=2, + ) + rv["event_scrubber"] = None if rv["socket_options"] and not isinstance(rv["socket_options"], list): logger.warning( diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 5ee60bcc41..e2cb6e36bc 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -136,7 +136,14 @@ def __call__( ) Scope.set_custom_sampling_context({"wsgi_environ": environ}) - if should_send_default_pii(): + if has_data_collection_enabled(client.options): + if client.options["data_collection"]["user_info"]: + client_ip = get_client_ip(environ) + if client_ip: + scope.set_attribute( + SPANDATA.USER_IP_ADDRESS, client_ip + ) + elif should_send_default_pii(): client_ip = get_client_ip(environ) if client_ip: scope.set_attribute( @@ -243,9 +250,14 @@ def _get_environ(environ: "Dict[str, str]") -> "Iterator[Tuple[str, str]]": capture (server name, port and remote addr if pii is enabled). """ keys = ["SERVER_NAME", "SERVER_PORT"] - if should_send_default_pii(): - # make debugging of proxy setup easier. Proxy headers are - # in headers. + client_options = sentry_sdk.get_client().options + + # make debugging of proxy setup easier. Proxy headers are + # in headers. + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + keys += ["REMOTE_ADDR"] + elif should_send_default_pii(): keys += ["REMOTE_ADDR"] for key in keys: @@ -364,7 +376,12 @@ def event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event": # if the code below fails halfway through we at least have some data request_info = event.setdefault("request", {}) - if should_send_default_pii(): + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + user_info = event.setdefault("user", {}) + if client_ip: + user_info.setdefault("ip_address", client_ip) + elif should_send_default_pii(): user_info = event.setdefault("user", {}) if client_ip: user_info.setdefault("ip_address", client_ip) @@ -442,6 +459,11 @@ def _get_request_attributes( attributes["url.full"] = get_request_url(environ, use_x_forwarded_for) + if client_options["data_collection"]["user_info"]: + client_ip = get_client_ip(environ) + if client_ip: + attributes["client.address"] = client_ip + elif should_send_default_pii(): client_ip = get_client_ip(environ) if client_ip: diff --git a/tests/integrations/django/test_data_scrubbing.py b/tests/integrations/django/test_data_scrubbing.py index 0c9497c18d..049226d0d6 100644 --- a/tests/integrations/django/test_data_scrubbing.py +++ b/tests/integrations/django/test_data_scrubbing.py @@ -396,3 +396,112 @@ def test_empty_query_string_is_dropped_with_data_collection( (event,) = events assert "query_string" not in event["request"] + + +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", + ), +] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS) +def test_user_info_span_attributes_data_collection( + sentry_init, client, capture_items, init_kwargs, expect_ip +): + init_kwargs = dict(init_kwargs) # shallow copy so we can mutate + experiments = init_kwargs.pop("_experiments", {}) + + sentry_init( + integrations=[DjangoIntegration()], + traces_sample_rate=1.0, + trace_lifecycle="stream", + _experiments=experiments, + **init_kwargs, + ) + + items = capture_items("span") + + unpack_werkzeug_response( + client.get(reverse("message"), environ_base={"REMOTE_ADDR": "127.0.0.1"}) + ) + + sentry_sdk.flush() + + spans = [item.payload for item in items] + root_span = spans[-1] + + if expect_ip: + assert root_span["attributes"][SPANDATA.USER_IP_ADDRESS] == "127.0.0.1" + assert root_span["attributes"]["client.address"] == "127.0.0.1" + else: + assert SPANDATA.USER_IP_ADDRESS not in root_span["attributes"] + assert "client.address" not in root_span["attributes"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS) +def test_user_info_error_event_data_collection( + sentry_init, client, capture_events, init_kwargs, expect_ip +): + sentry_init(integrations=[DjangoIntegration()], **init_kwargs) + events = capture_events() + + client.get(reverse("view_exc"), environ_base={"REMOTE_ADDR": "127.0.0.1"}) + + (event,) = events + + if expect_ip: + assert event["user"]["ip_address"] == "127.0.0.1" + assert event["request"]["env"]["REMOTE_ADDR"] == "127.0.0.1" + else: + assert "ip_address" not in event.get("user", {}) + assert "REMOTE_ADDR" not in event["request"]["env"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_error_event_no_user_ip_address_without_remote_addr( + sentry_init, client, capture_events +): + sentry_init( + integrations=[DjangoIntegration()], + _experiments={"data_collection": {"user_info": True}}, + ) + events = capture_events() + + client.get(reverse("view_exc")) + + (event,) = events + + assert "ip_address" not in event.get("user", {}) diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index 6ff170de2d..df736a932c 100644 --- a/tests/integrations/flask/test_flask.py +++ b/tests/integrations/flask/test_flask.py @@ -1398,3 +1398,128 @@ def test_empty_query_string_is_dropped_with_data_collection( (event,) = events 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) +def test_user_info_span_attributes_data_collection( + sentry_init, app, capture_items, monkeypatch, init_kwargs, expect_ip +): + init_kwargs = dict(init_kwargs) # shallow copy so we can mutate + experiments = init_kwargs.pop("_experiments", {}) + + sentry_init( + integrations=[flask_sentry.FlaskIntegration()], + traces_sample_rate=1.0, + trace_lifecycle="stream", + _experiments=experiments, + **init_kwargs, + ) + # This test is about user IP collection, not flask_login. Disable + # flask_login so the module-level login manager does not interfere. + monkeypatch.setattr(flask_sentry, "flask_login", None) + + items = capture_items("span") + + client = app.test_client() + client.get("/message", environ_overrides={"REMOTE_ADDR": "127.0.0.1"}) + + sentry_sdk.flush() + + spans = [item.payload for item in items if item.type == "span"] + (segment,) = spans + + if expect_ip: + assert segment["attributes"][SPANDATA.USER_IP_ADDRESS] == "127.0.0.1" + assert segment["attributes"]["client.address"] == "127.0.0.1" + else: + assert SPANDATA.USER_IP_ADDRESS not in segment["attributes"] + assert "client.address" not in segment["attributes"] + + +@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS) +def test_user_info_error_event_data_collection( + sentry_init, app, capture_events, monkeypatch, init_kwargs, expect_ip +): + sentry_init(integrations=[flask_sentry.FlaskIntegration()], **init_kwargs) + monkeypatch.setattr(flask_sentry, "flask_login", None) + + @app.route("/crash") + def crash(): + 1 / 0 + + events = capture_events() + + client = app.test_client() + with pytest.raises(ZeroDivisionError): + client.get("/crash", environ_overrides={"REMOTE_ADDR": "127.0.0.1"}) + + (event,) = events + + if expect_ip: + assert event["user"]["ip_address"] == "127.0.0.1" + assert event["request"]["env"]["REMOTE_ADDR"] == "127.0.0.1" + else: + assert "ip_address" not in event.get("user", {}) + assert "REMOTE_ADDR" not in event["request"]["env"] + + +def test_error_event_no_user_ip_address_without_remote_addr( + sentry_init, app, capture_events, monkeypatch +): + sentry_init( + integrations=[flask_sentry.FlaskIntegration()], + _experiments={"data_collection": {"user_info": True}}, + ) + monkeypatch.setattr(flask_sentry, "flask_login", None) + + @app.route("/crash") + def crash(): + 1 / 0 + + events = capture_events() + + client = app.test_client() + with pytest.raises(ZeroDivisionError): + client.get("/crash", environ_overrides={"REMOTE_ADDR": ""}) + + (event,) = events + + assert "ip_address" not in event.get("user", {}) diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index ebef296e56..77fa42e12a 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -1316,3 +1316,119 @@ def dogpark(environ, start_response): else: assert "user.ip_address" not in server_span["attributes"] assert "user.ip_address" not in child_span["attributes"] + + +# 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) +def test_user_info_span_attributes_data_collection( + sentry_init, capture_items, init_kwargs, expect_ip +): + def dogpark(environ, start_response): + with sentry_sdk.traces.start_span(name="child-span"): + pass + start_response("200 OK", []) + return ["Go get the ball! Good dog!"] + + init_kwargs = dict(init_kwargs) # shallow copy so we can mutate + experiments = init_kwargs.pop("_experiments", {}) + + sentry_init( + traces_sample_rate=1.0, + trace_lifecycle="stream", + _experiments=experiments, + **init_kwargs, + ) + app = SentryWsgiMiddleware(dogpark) + client = Client(app) + + items = capture_items("span") + + client.get("/dogs/are/great/", environ_base={"REMOTE_ADDR": "127.0.0.1"}) + + sentry_sdk.flush() + + child_span, server_span = [item.payload for item in items] + + 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" + assert server_span["attributes"]["client.address"] == "127.0.0.1" + else: + assert "user.ip_address" not in server_span["attributes"] + assert "user.ip_address" not in child_span["attributes"] + assert "client.address" not in server_span["attributes"] + + +@pytest.mark.parametrize("init_kwargs, expect_ip", USER_INFO_INIT_KWARGS) +def test_user_info_error_event_data_collection( + sentry_init, crashing_app, capture_events, init_kwargs, expect_ip +): + sentry_init(**init_kwargs) + app = SentryWsgiMiddleware(crashing_app) + client = Client(app) + events = capture_events() + + with pytest.raises(ZeroDivisionError): + client.get("/", environ_base={"REMOTE_ADDR": "127.0.0.1"}) + + (event,) = events + + if expect_ip: + assert event["user"]["ip_address"] == "127.0.0.1" + assert event["request"]["env"]["REMOTE_ADDR"] == "127.0.0.1" + else: + assert "ip_address" not in event.get("user", {}) + assert "REMOTE_ADDR" not in event["request"]["env"] + + +def test_error_event_no_user_ip_address_without_remote_addr( + sentry_init, crashing_app, capture_events +): + sentry_init(_experiments={"data_collection": {"user_info": True}}) + app = SentryWsgiMiddleware(crashing_app) + client = Client(app) + events = capture_events() + + with pytest.raises(ZeroDivisionError): + client.get("/") + + (event,) = events + + assert "ip_address" not in event.get("user", {})