diff --git a/sentry_sdk/integrations/tornado.py b/sentry_sdk/integrations/tornado.py index 0dba1f8e46..5ccc3b19e2 100644 --- a/sentry_sdk/integrations/tornado.py +++ b/sentry_sdk/integrations/tornado.py @@ -12,14 +12,12 @@ request_body_within_bounds, ) from sentry_sdk.integrations.logging import ignore_logger_for_events -from sentry_sdk.scope import should_send_default_pii from sentry_sdk.traces import SegmentNameSource from sentry_sdk.utils import ( AnnotatedValue, capture_internal_exceptions, ensure_integration_enabled, event_from_exception, - has_data_collection_enabled, parse_url, transaction_from_function, ) @@ -98,14 +96,8 @@ def _handle_request_impl(self: "RequestHandler") -> "Generator[None, None, None] sentry_sdk.continue_trace(dict(headers)) scope.set_custom_sampling_context({"tornado_request": self.request}) - 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) + if self.request.remote_ip and client.options["data_collection"]["user_info"]: + scope.set_attribute(SPANDATA.USER_IP_ADDRESS, self.request.remote_ip) with sentry_sdk.start_span( name=_DEFAULT_ROOT_SPAN_NAME, @@ -151,47 +143,31 @@ def _get_request_attributes(request: "Any") -> "Dict[str, Any]": for header, value in headers.items(): attributes[f"{SPANDATA.HTTP_REQUEST_HEADER}.{header.lower()}"] = value - if has_data_collection_enabled(client_options): - attributes["url.path"] = request.path + attributes["url.path"] = request.path - filtered_query = None - if request.query: - filtered_query = _apply_data_collection_filtering_to_query_string( - query_string=request.query, - behaviour=client_options["data_collection"]["url_query_params"], - ) - if filtered_query: - attributes[SPANDATA.URL_QUERY] = filtered_query - - parsed_url = parse_url(request.full_url()) - attributes[SPANDATA.URL_FULL] = ( - f"{parsed_url.url}?{filtered_query}" if filtered_query else parsed_url.url + filtered_query = None + if request.query: + filtered_query = _apply_data_collection_filtering_to_query_string( + query_string=request.query, + behaviour=client_options["data_collection"]["url_query_params"], ) + if filtered_query: + attributes[SPANDATA.URL_QUERY] = filtered_query - 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 - - if request.query: - attributes[SPANDATA.URL_QUERY] = request.query + parsed_url = parse_url(request.full_url()) + attributes[SPANDATA.URL_FULL] = ( + f"{parsed_url.url}?{filtered_query}" if filtered_query else parsed_url.url + ) - if request.remote_ip: - attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_ip + if request.remote_ip and client_options["data_collection"]["user_info"]: + attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_ip if request.protocol: attributes[SPANDATA.NETWORK_PROTOCOL_NAME] = request.protocol - # The request data was unconditionally set pre-data collection which is - # why we're defaulting to True - record_incoming_request_data = True - if has_data_collection_enabled(client_options): - record_incoming_request_data = ( - "incoming_request" in client_options["data_collection"]["http_bodies"] - ) + record_incoming_request_data = ( + "incoming_request" in client_options["data_collection"]["http_bodies"] + ) if record_incoming_request_data: with capture_internal_exceptions(): @@ -260,38 +236,21 @@ def tornado_processor(event: "Event", hint: "dict[str, Any]") -> "Event": request.path, ) - if has_data_collection_enabled(client_options): - if request.query: - filtered_query = _apply_data_collection_filtering_to_query_string( - query_string=request.query, - behaviour=client_options["data_collection"]["url_query_params"], - ) - if filtered_query: - request_info["query_string"] = filtered_query - else: - request_info["query_string"] = request.query + if request.query: + filtered_query = _apply_data_collection_filtering_to_query_string( + query_string=request.query, + behaviour=client_options["data_collection"]["url_query_params"], + ) + if filtered_query: + request_info["query_string"] = filtered_query request_info["method"] = request.method - # REMOTE_ADDR was unconditionally set pre-data collection, so it - # continues to be set when data collection is not enabled. - if ( - not has_data_collection_enabled(client_options) - or client_options["data_collection"]["user_info"] - ): + if client_options["data_collection"]["user_info"]: request_info["env"] = {"REMOTE_ADDR": request.remote_ip} request_info["headers"] = _filter_headers(dict(request.headers)) - 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(): + if client_options["data_collection"]["user_info"]: try: current_user = handler.current_user except Exception: diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index 0750d49747..145ab389a6 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -87,24 +87,24 @@ def get(self): "init_kwargs, expected_cookies", [ pytest.param( - {"send_default_pii": True}, + {"data_collection": {}}, { - "jwt": "tokenval", + "jwt": SENSITIVE_DATA_SUBSTITUTE, "theme": "dark", "lang": "en", - "identity": "alice", + "identity": SENSITIVE_DATA_SUBSTITUTE, }, - id="send_default_pii_true", + id="data_collection_default", ), pytest.param( - {"send_default_pii": False}, - None, - id="send_default_pii_false", - ), - pytest.param( - {}, - None, - id="defaults", + {"data_collection": {"cookies": {"mode": "denylist", "terms": []}}}, + { + "jwt": SENSITIVE_DATA_SUBSTITUTE, + "theme": "dark", + "lang": "en", + "identity": SENSITIVE_DATA_SUBSTITUTE, + }, + id="data_collection_denylist_empty_terms", ), pytest.param( {"data_collection": {"cookies": {"mode": "off"}}}, @@ -155,19 +155,6 @@ def get(self): }, id="data_collection_allowlist_sensitive_term", ), - pytest.param( - { - "send_default_pii": False, - "data_collection": {"cookies": {"mode": "denylist"}}, - }, - { - "jwt": SENSITIVE_DATA_SUBSTITUTE, - "theme": "dark", - "lang": "en", - "identity": SENSITIVE_DATA_SUBSTITUTE, - }, - id="data_collection_wins_over_send_default_pii", - ), ], ) def test_cookie_data_collection( @@ -194,19 +181,14 @@ async def get(self): _QUERY_PARAM_DATA_COLLECTION_CASES = [ pytest.param( - {"send_default_pii": True}, - "toy=tennisball&color=red&auth=secret", - id="send_default_pii_true", - ), - pytest.param( - {"send_default_pii": False}, - None, - id="send_default_pii_false", + {"data_collection": {"url_query_params": {"mode": "denylist", "terms": []}}}, + "toy=tennisball&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_empty_terms", ), pytest.param( - {}, + {"data_collection": {"url_query_params": {"mode": "off"}}}, None, - id="defaults", + id="data_collection_off", ), pytest.param( {"data_collection": {}}, @@ -240,19 +222,6 @@ async def get(self): "toy=%5BFiltered%5D&color=%5BFiltered%5D&auth=%5BFiltered%5D", id="data_collection_allowlist_sensitive_term", ), - pytest.param( - {"data_collection": {"url_query_params": {"mode": "off"}}}, - None, - id="data_collection_off", - ), - pytest.param( - { - "send_default_pii": True, - "data_collection": {"url_query_params": {"mode": "off"}}, - }, - None, - id="data_collection_wins_over_send_default_pii", - ), ] @@ -279,19 +248,10 @@ def test_url_query_data_collection( (server_span,) = [item.payload for item in items] - data_collection_enabled = "data_collection" in init_kwargs - url_attrs_expected = data_collection_enabled or init_kwargs.get( - "send_default_pii", False - ) - if expected_query is None: assert "url.query" not in server_span["attributes"] - if url_attrs_expected: - assert server_span["attributes"]["url.full"].endswith("/hi") - assert server_span["attributes"]["url.path"] == "/hi" - else: - assert "url.full" not in server_span["attributes"] - assert "url.path" not in server_span["attributes"] + assert server_span["attributes"]["url.full"].endswith("/hi") + assert server_span["attributes"]["url.path"] == "/hi" else: assert server_span["attributes"]["url.query"] == expected_query assert server_span["attributes"]["url.full"].endswith(f"/hi?{expected_query}") @@ -465,7 +425,17 @@ def test_request_body_data_collection_event_processor( assert "data" not in event["request"] -@pytest.mark.parametrize("send_pii", [True, False]) +@pytest.mark.parametrize( + "data_collection, expect_query", + [ + pytest.param({}, True, id="data_collection_default"), + pytest.param( + {"url_query_params": {"mode": "off"}}, + False, + id="data_collection_url_query_params_off", + ), + ], +) @pytest.mark.parametrize( "handler,code", [ @@ -480,12 +450,13 @@ def test_transactions( capture_items, handler, code, - send_pii, + data_collection, + expect_query, ): sentry_init( integrations=[TornadoIntegration()], traces_sample_rate=1.0, - send_default_pii=send_pii, + data_collection=data_collection, ) items = capture_items("event", "span") @@ -533,15 +504,14 @@ def test_transactions( assert server_segment["status"] == ("ok" if code == 200 else "error") assert client_segment["trace_id"] == server_segment["trace_id"] - if send_pii: + assert server_segment["attributes"]["url.path"] == "/hi" + if expect_query: assert server_segment["attributes"]["url.query"] == "foo=bar" assert server_segment["attributes"]["url.full"].endswith("/hi?foo=bar") assert server_segment["attributes"]["url.full"].startswith("http://") - assert server_segment["attributes"]["url.path"] == "/hi" else: assert "url.query" not in server_segment["attributes"] - assert "url.full" not in server_segment["attributes"] - assert "url.path" not in server_segment["attributes"] + assert server_segment["attributes"]["url.full"].endswith("/hi") def test_400_not_logged(tornado_testcase, sentry_init, capture_events): @@ -561,7 +531,10 @@ def get(self): def test_user_auth(tornado_testcase, sentry_init, capture_events): - sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) + sentry_init( + integrations=[TornadoIntegration()], + data_collection={"user_info": True}, + ) events = capture_events() class UserHandler(RequestHandler): @@ -629,7 +602,7 @@ def get_current_user(self): def test_formdata(tornado_testcase, sentry_init, capture_events): - sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) + sentry_init(integrations=[TornadoIntegration()], data_collection={}) events = capture_events() class FormdataHandler(RequestHandler): @@ -654,7 +627,7 @@ def post(self): def test_json(tornado_testcase, sentry_init, capture_events): - sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) + sentry_init(integrations=[TornadoIntegration()], data_collection={}) events = capture_events() class FormdataHandler(RequestHandler):