diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 2c561164e6..0f8dfcd24a 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -30,6 +30,7 @@ capture_internal_exceptions, ensure_integration_enabled, event_from_exception, + has_data_collection_enabled, logger, transaction_from_function, walk_exception_chain, @@ -457,6 +458,39 @@ def _attempt_resolve_again( _set_transaction_name_and_source(scope, transaction_style, request) +def _get_user_from_request_and_set_on_scope(request: "WSGIRequest") -> None: + user = getattr(request, "user", None) + + # Evaluating a SimpleLazyObject in an async view can raise django.core.exceptions.SynchronousOnlyOperation. + # Exit early if the user has not been materialized yet. + is_lazy = isinstance(user, SimpleLazyObject) + if is_lazy and hasattr(request, "_cached_user"): + user = request._cached_user + elif is_lazy: + return + + if user is None or not is_authenticated(user): + return + + user_info = {} + try: + user_info["id"] = str(user.pk) + except Exception: + pass + + try: + user_info["email"] = user.email + except Exception: + pass + + try: + user_info["username"] = user.get_username() + except Exception: + pass + + sentry_sdk.set_user(user_info) + + def _after_get_response(request: "WSGIRequest") -> None: client = sentry_sdk.get_client() integration = client.get_integration(DjangoIntegration) @@ -468,37 +502,12 @@ def _after_get_response(request: "WSGIRequest") -> None: _attempt_resolve_again(request, scope, integration.transaction_style) span_streaming = has_span_streaming_enabled(client.options) - if span_streaming and should_send_default_pii(): - user = getattr(request, "user", None) - - # Evaluating a SimpleLazyObject in an async view can raise django.core.exceptions.SynchronousOnlyOperation. - # Exit early if the user has not been materialized yet. - is_lazy = isinstance(user, SimpleLazyObject) - if is_lazy and hasattr(request, "_cached_user"): - user = request._cached_user - elif is_lazy: - return - - if user is None or not is_authenticated(user): - return - - user_info = {} - try: - user_info["id"] = str(user.pk) - except Exception: - pass - - try: - user_info["email"] = user.email - except Exception: - pass - - try: - user_info["username"] = user.get_username() - except Exception: - pass - - sentry_sdk.set_user(user_info) + if span_streaming: + if has_data_collection_enabled(client.options): + if client.options["data_collection"]["user_info"]: + _get_user_from_request_and_set_on_scope(request) + elif should_send_default_pii(): + _get_user_from_request_and_set_on_scope(request) def _patch_get_response() -> None: @@ -544,7 +553,12 @@ def wsgi_request_event_processor(event: "Event", hint: "dict[str, Any]") -> "Eve with capture_internal_exceptions(): DjangoRequestExtractor(request).extract_into_event(event) - if should_send_default_pii(): + client_options = sentry_sdk.get_client().options + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + with capture_internal_exceptions(): + _set_user_info(request, event) + elif should_send_default_pii(): with capture_internal_exceptions(): _set_user_info(request, event) diff --git a/sentry_sdk/integrations/django/asgi.py b/sentry_sdk/integrations/django/asgi.py index 785fa2af34..df287e6436 100644 --- a/sentry_sdk/integrations/django/asgi.py +++ b/sentry_sdk/integrations/django/asgi.py @@ -22,6 +22,7 @@ from sentry_sdk.utils import ( capture_internal_exceptions, ensure_integration_enabled, + has_data_collection_enabled, ) if TYPE_CHECKING: @@ -70,7 +71,12 @@ def asgi_request_event_processor(event: "Event", hint: "dict[str, Any]") -> "Eve with capture_internal_exceptions(): DjangoRequestExtractor(request).extract_into_event(event) - if should_send_default_pii(): + client_options = sentry_sdk.get_client().options + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + with capture_internal_exceptions(): + _set_user_info(request, event) + elif should_send_default_pii(): with capture_internal_exceptions(): _set_user_info(request, event) diff --git a/sentry_sdk/integrations/pyramid.py b/sentry_sdk/integrations/pyramid.py index 6837d8345c..5ab6ef90e9 100644 --- a/sentry_sdk/integrations/pyramid.py +++ b/sentry_sdk/integrations/pyramid.py @@ -15,6 +15,7 @@ capture_internal_exceptions, ensure_integration_enabled, event_from_exception, + has_data_collection_enabled, reraise, ) @@ -86,10 +87,16 @@ def sentry_patched_call_view( scope = sentry_sdk.get_isolation_scope() - if should_send_default_pii() and has_span_streaming_enabled(client.options): - user_id = authenticated_userid(request) - if user_id: - scope.set_user({"id": user_id}) + if has_span_streaming_enabled(client.options): + if has_data_collection_enabled(client.options): + if client.options["data_collection"]["user_info"]: + user_id = authenticated_userid(request) + if user_id: + scope.set_user({"id": user_id}) + elif should_send_default_pii(): + user_id = authenticated_userid(request) + if user_id: + scope.set_user({"id": user_id}) scope.add_event_processor( _make_event_processor(weakref.ref(request), integration) @@ -229,7 +236,13 @@ def pyramid_event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event": with capture_internal_exceptions(): PyramidRequestExtractor(request).extract_into_event(event) - if should_send_default_pii(): + client_options = sentry_sdk.get_client().options + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + with capture_internal_exceptions(): + user_info = event.setdefault("user", {}) + user_info.setdefault("id", authenticated_userid(request)) + elif should_send_default_pii(): with capture_internal_exceptions(): user_info = event.setdefault("user", {}) user_info.setdefault("id", authenticated_userid(request)) diff --git a/sentry_sdk/integrations/sanic.py b/sentry_sdk/integrations/sanic.py index 3f584b37ed..2d839d1c61 100644 --- a/sentry_sdk/integrations/sanic.py +++ b/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,6 +405,10 @@ 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 @@ -408,12 +416,12 @@ def _get_request_attributes(request: "Request") -> "Dict[str, Any]": 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 diff --git a/sentry_sdk/integrations/tornado.py b/sentry_sdk/integrations/tornado.py index 836f3a4b25..e71bf94d12 100644 --- a/sentry_sdk/integrations/tornado.py +++ b/sentry_sdk/integrations/tornado.py @@ -133,8 +133,16 @@ def _handle_request_impl(self: "RequestHandler") -> "Generator[None, None, None] sentry_sdk.traces.continue_trace(dict(headers)) scope.set_custom_sampling_context({"tornado_request": self.request}) - if should_send_default_pii() and self.request.remote_ip: - scope.set_attribute(SPANDATA.USER_IP_ADDRESS, self.request.remote_ip) + if self.request.remote_ip: + if has_data_collection_enabled(client.options): + if client.options["data_collection"]["user_info"]: + scope.set_attribute( + SPANDATA.USER_IP_ADDRESS, self.request.remote_ip + ) + elif should_send_default_pii(): + scope.set_attribute( + SPANDATA.USER_IP_ADDRESS, self.request.remote_ip + ) span_ctx = sentry_sdk.traces.start_span( name=_DEFAULT_ROOT_SPAN_NAME, @@ -218,6 +226,10 @@ def _get_request_attributes(request: "Any") -> "Dict[str, Any]": f"{parsed_url.url}?{filtered_query}" if filtered_query else parsed_url.url ) + if request.remote_ip: + if client_options["data_collection"]["user_info"]: + attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_ip + elif should_send_default_pii(): attributes[SPANDATA.URL_FULL] = request.full_url() attributes["url.path"] = request.path @@ -225,12 +237,12 @@ def _get_request_attributes(request: "Any") -> "Dict[str, Any]": if request.query: attributes[SPANDATA.URL_QUERY] = request.query + if request.remote_ip: + attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_ip + if request.protocol: attributes[SPANDATA.NETWORK_PROTOCOL_NAME] = request.protocol - if should_send_default_pii() and request.remote_ip: - attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_ip - with capture_internal_exceptions(): raw_data = _get_tornado_request_data(request) body_data = raw_data.value if isinstance(raw_data, AnnotatedValue) else raw_data @@ -282,6 +294,7 @@ def tornado_processor(event: "Event", hint: "dict[str, Any]") -> "Event": event["transaction"] = transaction_from_function(method) or "" event["transaction_info"] = {"source": TransactionSource.COMPONENT} + client_options = sentry_sdk.get_client().options with capture_internal_exceptions(): extractor = TornadoRequestExtractor(request) extractor.extract_into_event(event) @@ -294,7 +307,6 @@ def tornado_processor(event: "Event", hint: "dict[str, Any]") -> "Event": request.path, ) - client_options = sentry_sdk.get_client().options if has_data_collection_enabled(client_options): if request.query: filtered_query = _apply_data_collection_filtering_to_query_string( @@ -310,7 +322,16 @@ def tornado_processor(event: "Event", hint: "dict[str, Any]") -> "Event": request_info["env"] = {"REMOTE_ADDR": request.remote_ip} request_info["headers"] = _filter_headers(dict(request.headers)) - if should_send_default_pii(): + if has_data_collection_enabled(client_options): + if client_options["data_collection"]["user_info"]: + try: + current_user = handler.current_user + except Exception: + current_user = None + + if current_user: + event.setdefault("user", {}).setdefault("is_authenticated", True) + elif should_send_default_pii(): try: current_user = handler.current_user except Exception: diff --git a/tests/integrations/django/asgi/test_asgi.py b/tests/integrations/django/asgi/test_asgi.py index 700704ad4f..752dba702a 100644 --- a/tests/integrations/django/asgi/test_asgi.py +++ b/tests/integrations/django/asgi/test_asgi.py @@ -16,6 +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 pytest_mark_django_db_decorator +from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES try: from django.urls import reverse @@ -1096,3 +1098,33 @@ async def test_async_middleware_process_exception_is_awaited( assert response["status"] == 200 assert response["body"] == b"handled by async process_exception" + + +@pytest.mark.forked +@pytest.mark.parametrize("application", APPS) +@pytest.mark.asyncio +@pytest.mark.skipif( + django.VERSION < (3, 0), reason="Django ASGI support shipped in 3.0" +) +@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 +): + sentry_init(integrations=[DjangoIntegration()], **init_kwargs) + events = capture_events() + + comm = HttpCommunicator(application, "GET", "/mylogin-with-exception") + await comm.get_response() + await comm.wait() + + event = events[-1] + + if expect_user: + assert event["user"]["id"] == "1" + assert event["user"]["email"] == "lennon@thebeatles.com" + assert event["user"]["username"] == "john" + else: + assert "id" not in event.get("user", {}) + assert "email" not in event.get("user", {}) + assert "username" not in event.get("user", {}) diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 87e9750889..2c1cad4298 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -47,6 +47,11 @@ def path(path, *args, **kwargs): path("nomessage", views.nomessage, name="nomessage"), path("view-with-signal", views.view_with_signal, name="view_with_signal"), path("mylogin", views.mylogin, name="mylogin"), + path( + "mylogin-with-exception", + views.mylogin_with_exception, + name="mylogin_with_exception", + ), path("classbased", views.ClassBasedView.as_view(), name="classbased"), path("sentryclass", views.SentryClassBasedView(), name="sentryclass"), path( diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py index 80587acaa3..ebaa3b37eb 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -135,6 +135,16 @@ def mylogin(request): return HttpResponse("ok") +@csrf_exempt +def mylogin_with_exception(request): + user, _ = User.objects.get_or_create( + username="john", defaults={"email": "lennon@thebeatles.com"} + ) + user.backend = "django.contrib.auth.backends.ModelBackend" + login(request, user) + 1 / 0 + + @csrf_exempt def handler500(request): return HttpResponseServerError("Sentry error.") diff --git a/tests/integrations/django/test_data_scrubbing.py b/tests/integrations/django/test_data_scrubbing.py index 5e0eb03508..559b9b0466 100644 --- a/tests/integrations/django/test_data_scrubbing.py +++ b/tests/integrations/django/test_data_scrubbing.py @@ -7,6 +7,7 @@ from tests.conftest import unpack_werkzeug_response, werkzeug_set_cookie from tests.integrations.django.myapp.wsgi import application 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 @@ -392,43 +393,9 @@ def test_empty_query_string_is_dropped_with_data_collection( 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) +@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 ): @@ -464,7 +431,43 @@ def test_user_info_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_user", DATA_COLLECTION_USER_INFO_CASES) +def test_user_identity_span_attributes_data_collection( + sentry_init, client, capture_items, init_kwargs, expect_user +): + 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, + ) + + unpack_werkzeug_response(client.get(reverse("mylogin"))) + + items = capture_items("span") + unpack_werkzeug_response(client.get(reverse("template_test"))) + sentry_sdk.flush() + + spans = [item.payload for item in items] + (span,) = (s for s in spans if s["name"] == "/template-test") + + if expect_user: + assert span["attributes"][SPANDATA.USER_ID] == "1" + assert span["attributes"][SPANDATA.USER_EMAIL] == "lennon@thebeatles.com" + assert span["attributes"][SPANDATA.USER_NAME] == "john" + else: + assert SPANDATA.USER_ID not in span["attributes"] + assert SPANDATA.USER_EMAIL not in span["attributes"] + assert SPANDATA.USER_NAME not in span["attributes"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@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 ): @@ -483,6 +486,30 @@ def test_user_info_error_event_data_collection( assert "REMOTE_ADDR" not in event["request"]["env"] +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@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 +): + sentry_init(integrations=[DjangoIntegration()], **init_kwargs) + events = capture_events() + + client.get(reverse("mylogin")) + client.get(reverse("view_exc")) + + event = events[-1] + + if expect_user: + assert event["user"]["id"] == "1" + assert event["user"]["email"] == "lennon@thebeatles.com" + assert event["user"]["username"] == "john" + else: + assert "id" not in event.get("user", {}) + assert "email" not in event.get("user", {}) + assert "username" not in event.get("user", {}) + + @pytest.mark.forked @pytest_mark_django_db_decorator() def test_error_event_no_user_ip_address_without_remote_addr( diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index c091e23f61..71c56d3fef 100644 --- a/tests/integrations/flask/test_flask.py +++ b/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 ): diff --git a/tests/integrations/pyramid/test_pyramid.py b/tests/integrations/pyramid/test_pyramid.py index 06c38eccce..bdd5cb5049 100644 --- a/tests/integrations/pyramid/test_pyramid.py +++ b/tests/integrations/pyramid/test_pyramid.py @@ -15,6 +15,7 @@ from sentry_sdk.serializer import MAX_DATABAG_BREADTH from sentry_sdk.traces import SpanStatus from tests.conftest import unpack_werkzeug_response +from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES try: from importlib.metadata import version @@ -559,19 +560,20 @@ def test_span_origin( assert event["contexts"]["trace"]["origin"] == "auto.http.pyramid" -@pytest.mark.parametrize("send_default_pii", [True, False]) +@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES) def test_span_sets_user_id_on_segment( sentry_init, pyramid_config, capture_items, get_client, - send_default_pii, + init_kwargs, + expect_user, ): sentry_init( integrations=[PyramidIntegration()], traces_sample_rate=1.0, - send_default_pii=send_default_pii, trace_lifecycle="stream", + **init_kwargs, ) class AuthenticationPolicy: @@ -592,7 +594,43 @@ def authenticated_userid(self, request): assert len(spans) == 1 (segment,) = spans - if send_default_pii: + if expect_user: assert segment["attributes"]["user.id"] == "123-abc" else: assert "user.id" not in segment["attributes"] + + +@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES) +def test_user_id_error_event_data_collection( + sentry_init, + pyramid_config, + capture_events, + route, + get_client, + init_kwargs, + expect_user, +): + sentry_init(integrations=[PyramidIntegration()], **init_kwargs) + events = capture_events() + + class AuthenticationPolicy: + def authenticated_userid(self, request): + return "123-abc" + + pyramid_config.set_authorization_policy(ACLAuthorizationPolicy()) + pyramid_config.set_authentication_policy(AuthenticationPolicy()) + + @route("/crash") + def crash(request): + 1 / 0 + + client = get_client() + with pytest.raises(ZeroDivisionError): + client.get("/crash") + + (event,) = events + + if expect_user: + assert event["user"]["id"] == "123-abc" + else: + assert "id" not in event.get("user", {}) diff --git a/tests/integrations/sanic/test_sanic.py b/tests/integrations/sanic/test_sanic.py index 1f030c425f..0bc6133744 100644 --- a/tests/integrations/sanic/test_sanic.py +++ b/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,7 +593,7 @@ 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: @@ -600,6 +601,46 @@ def child_span_handler(request): 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}, diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index 23fbc316d6..2268a791ee 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -8,6 +8,7 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk._types import SENSITIVE_DATA_SUBSTITUTE from sentry_sdk.integrations.tornado import TornadoIntegration +from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES @pytest.fixture @@ -701,6 +702,32 @@ def get(self): assert "user" not in event +@pytest.mark.parametrize("init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES) +def test_user_auth_data_collection( + tornado_testcase, sentry_init, capture_events, init_kwargs, expect_user +): + sentry_init(integrations=[TornadoIntegration()], **init_kwargs) + events = capture_events() + + class UserHandler(RequestHandler): + def get(self): + 1 / 0 + + def get_current_user(self): + return 42 + + client = tornado_testcase(Application([(r"/auth", UserHandler)])) + + response = client.fetch("/auth") + assert response.code == 500 + + (event,) = events + if expect_user: + assert event["user"] == {"is_authenticated": True} + else: + assert "is_authenticated" not in event.get("user", {}) + + def test_formdata(tornado_testcase, sentry_init, capture_events): sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) events = capture_events() @@ -927,15 +954,15 @@ def test_span_origin( assert event["contexts"]["trace"]["origin"] == "auto.http.tornado" -@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( - tornado_testcase, sentry_init, capture_items, send_default_pii + tornado_testcase, sentry_init, capture_items, init_kwargs, expect_ip ): sentry_init( integrations=[TornadoIntegration()], traces_sample_rate=1.0, - send_default_pii=send_default_pii, trace_lifecycle="stream", + **init_kwargs, ) items = capture_items("span") @@ -947,9 +974,39 @@ def test_user_ip_address_on_all_spans( 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.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES) +def test_client_address_span_attribute_data_collection( + tornado_testcase, sentry_init, capture_items, init_kwargs, expect_ip +): + sentry_init( + integrations=[TornadoIntegration()], + traces_sample_rate=1.0, + trace_lifecycle="stream", + **init_kwargs, + ) + + items = capture_items("span") + + client = tornado_testcase(Application([(r"/hi", ChildSpanHandler)])) + client.fetch("/hi") + + sentry_sdk.flush() + + (server_span,) = [ + item.payload + for item in items + if item.payload["attributes"].get("sentry.origin") == "auto.http.tornado" + ] + + if expect_ip: + assert server_span["attributes"]["client.address"] == "127.0.0.1" + else: + assert "client.address" not in server_span["attributes"] diff --git a/tests/integrations/utils.py b/tests/integrations/utils.py new file mode 100644 index 0000000000..557ed3b62c --- /dev/null +++ b/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", + ), +] diff --git a/tests/integrations/wsgi/test_wsgi.py b/tests/integrations/wsgi/test_wsgi.py index 084abe0745..eb73be1499 100644 --- a/tests/integrations/wsgi/test_wsgi.py +++ b/tests/integrations/wsgi/test_wsgi.py @@ -11,6 +11,7 @@ _ScopedResponse, get_request_url, ) +from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES @pytest.fixture @@ -1303,45 +1304,7 @@ def dogpark(environ, start_response): 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) +@pytest.mark.parametrize("init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES) def test_user_info_span_attributes_data_collection( sentry_init, capture_items, init_kwargs, expect_ip ): @@ -1381,7 +1344,7 @@ def dogpark(environ, start_response): assert "client.address" not in server_span["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, crashing_app, capture_events, init_kwargs, expect_ip ):