feat(tornado): Apply data_collection filtering to URL query strings

sentry_sdk/integrations/tornado.py

@@ -5,6 +5,7 @@
 import sentry_sdk
 from sentry_sdk.api import continue_trace
 from sentry_sdk.consts import OP, SPANDATA
+from sentry_sdk.data_collection import _apply_data_collection_filtering_to_query_string
 from sentry_sdk.integrations import DidNotEnable, Integration, _check_minimum_version
 from sentry_sdk.integrations._wsgi_common import (
     RequestExtractor,
@@ -24,6 +25,8 @@
     capture_internal_exceptions,
     ensure_integration_enabled,
     event_from_exception,
+    has_data_collection_enabled,
+    parse_url,
     transaction_from_function,
 )
 
@@ -189,6 +192,7 @@ def _handle_request_impl(self: "RequestHandler") -> "Generator[None, None, None]
 
 def _get_request_attributes(request: "Any") -> "Dict[str, Any]":
     attributes = {}  # type: Dict[str, Any]
+    client_options = sentry_sdk.get_client().options
 
     if request.method:
         attributes[SPANDATA.HTTP_REQUEST_METHOD] = request.method.upper()
@@ -197,7 +201,24 @@ 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 should_send_default_pii():
+    if has_data_collection_enabled(client_options):
+        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
+        )
+
+    elif should_send_default_pii():
         attributes[SPANDATA.URL_FULL] = request.full_url()
         attributes["url.path"] = request.path
 
@@ -273,7 +294,18 @@ def tornado_processor(event: "Event", hint: "dict[str, Any]") -> "Event":
                 request.path,
             )
 
-            request_info["query_string"] = request.query
+            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(
+                        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
+
             request_info["method"] = request.method
             request_info["env"] = {"REMOTE_ADDR": request.remote_ip}
             request_info["headers"] = _filter_headers(dict(request.headers))

tests/integrations/tornado/test_tornado.py

@@ -118,9 +118,6 @@ def test_basic(tornado_testcase, sentry_init, capture_events):
 # filtering we observe on them comes from the extractor's data-collection
 # logic, not the always-on scrubber.
 COOKIE_HEADER = "jwt=tokenval; theme=dark; lang=en; identity=alice"
-
-# Sentinel meaning "the request payload should have no ``cookies`` key at all",
-# as opposed to an empty ``{}`` dict.
 NO_COOKIES = object()
 
 
@@ -242,6 +239,257 @@ def test_cookie_data_collection(
         assert event["request"]["cookies"] == expected_cookies
 
 
+class QueryHandler(RequestHandler):
+    async def get(self):
+        self.write("ok")
+
+
+NO_QUERY_STRING = object()
+_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},
+        NO_QUERY_STRING,
+        id="send_default_pii_false",
+    ),
+    pytest.param(
+        {},
+        NO_QUERY_STRING,
+        id="defaults",
+    ),
+    pytest.param(
+        {"_experiments": {"data_collection": {}}},
+        "toy=tennisball&color=red&auth=[Filtered]",
+        id="data_collection_denylist_default",
+    ),
+    pytest.param(
+        {
+            "_experiments": {
+                "data_collection": {
+                    "url_query_params": {"mode": "denylist", "terms": ["toy"]}
+                }
+            }
+        },
+        "toy=[Filtered]&color=red&auth=[Filtered]",
+        id="data_collection_denylist_custom_terms",
+    ),
+    pytest.param(
+        {
+            "_experiments": {
+                "data_collection": {
+                    "url_query_params": {"mode": "allowlist", "terms": ["toy"]}
+                }
+            }
+        },
+        "toy=tennisball&color=[Filtered]&auth=[Filtered]",
+        id="data_collection_allowlist",
+    ),
+    pytest.param(
+        {
+            "_experiments": {
+                "data_collection": {
+                    "url_query_params": {"mode": "allowlist", "terms": ["auth"]}
+                }
+            }
+        },
+        "toy=[Filtered]&color=[Filtered]&auth=[Filtered]",
+        id="data_collection_allowlist_sensitive_term",
+    ),
+    pytest.param(
+        {"_experiments": {"data_collection": {"url_query_params": {"mode": "off"}}}},
+        NO_QUERY_STRING,
+        id="data_collection_off",
+    ),
+    pytest.param(
+        {
+            "send_default_pii": True,
+            "_experiments": {"data_collection": {"url_query_params": {"mode": "off"}}},
+        },
+        NO_QUERY_STRING,
+        id="data_collection_wins_over_send_default_pii",
+    ),
+]
+
+
+@pytest.mark.parametrize(
+    "init_kwargs, expected_query", _QUERY_PARAM_DATA_COLLECTION_CASES
+)
+def test_url_query_data_collection_span_streaming(
+    tornado_testcase, sentry_init, capture_items, init_kwargs, expected_query
+):
+    init_kwargs = dict(init_kwargs)
+    sentry_init(
+        integrations=[TornadoIntegration()],
+        traces_sample_rate=1.0,
+        trace_lifecycle="stream",
+        **init_kwargs,
+    )
+
+    items = capture_items("span")
+
+    client = tornado_testcase(Application([(r"/hi", QueryHandler)]))
+    response = client.fetch("/hi?toy=tennisball&color=red&auth=secret")
+    assert response.code == 200
+
+    sentry_sdk.flush()
+
+    (server_span,) = [item.payload for item in items]
+
+    data_collection_enabled = "data_collection" in init_kwargs.get("_experiments", {})
+    url_attrs_expected = data_collection_enabled or init_kwargs.get(
+        "send_default_pii", False
+    )
+
+    if expected_query is NO_QUERY_STRING:
+        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"]
+    else:
+        assert server_span["attributes"]["url.query"] == expected_query
+        assert server_span["attributes"]["url.full"].endswith(f"/hi?{expected_query}")
+        assert server_span["attributes"]["url.full"].startswith("http://")
+        assert server_span["attributes"]["url.path"] == "/hi"
+
+
+@pytest.mark.parametrize(
+    "init_kwargs, expected_query", _QUERY_PARAM_DATA_COLLECTION_CASES
+)
+def test_url_query_data_collection_event_processor(
+    tornado_testcase, sentry_init, capture_events, init_kwargs, expected_query
+):
+    sentry_init(
+        integrations=[TornadoIntegration()],
+        traces_sample_rate=1.0,
+        trace_lifecycle="static",
+        **init_kwargs,
+    )
+
+    events = capture_events()
+
+    client = tornado_testcase(Application([(r"/hi", QueryHandler)]))
+    response = client.fetch("/hi?toy=tennisball&color=red&auth=secret")
+    assert response.code == 200
+
+    sentry_sdk.flush()
+
+    (event,) = events
+
+    assert event["request"]["url"].endswith("/hi")
+    assert event["request"]["method"] == "GET"
+    if "data_collection" not in init_kwargs.get("_experiments", {}):
+        assert (
+            event["request"]["query_string"] == "toy=tennisball&color=red&auth=secret"
+        )
+    elif expected_query is NO_QUERY_STRING:
+        assert "query_string" not in event["request"]
+    else:
+        assert event["request"]["query_string"] == expected_query
+
+
+def test_url_query_data_collection_no_query_string(
+    tornado_testcase, sentry_init, capture_items
+):
+    sentry_init(
+        integrations=[TornadoIntegration()],
+        traces_sample_rate=1.0,
+        trace_lifecycle="stream",
+        _experiments={"data_collection": {}},
+    )
+
+    items = capture_items("span")
+
+    client = tornado_testcase(Application([(r"/hi", QueryHandler)]))
+    response = client.fetch("/hi")
+    assert response.code == 200
+
+    sentry_sdk.flush()
+
+    (server_span,) = [item.payload for item in items]
+
+    assert "url.query" not in server_span["attributes"]
+    assert server_span["attributes"]["url.full"].endswith("/hi")
+    assert server_span["attributes"]["url.path"] == "/hi"
+
+
+def test_url_query_data_collection_repeated_and_blank_params(
+    tornado_testcase, sentry_init, capture_items
+):
+    sentry_init(
+        integrations=[TornadoIntegration()],
+        traces_sample_rate=1.0,
+        trace_lifecycle="stream",
+        _experiments={"data_collection": {}},
+    )
+
+    items = capture_items("span")
+
+    client = tornado_testcase(Application([(r"/hi", QueryHandler)]))
+    response = client.fetch("/hi?a=1&a=2&b=")
+    assert response.code == 200
+
+    sentry_sdk.flush()
+
+    (server_span,) = [item.payload for item in items]
+
+    assert server_span["attributes"]["url.query"] == "a=1&a=2&b="
+
+
+def test_url_query_data_collection__event_processor_no_query_string(
+    tornado_testcase, sentry_init, capture_events
+):
+    sentry_init(
+        integrations=[TornadoIntegration()],
+        traces_sample_rate=1.0,
+        trace_lifecycle="static",
+        _experiments={"data_collection": {}},
+    )
+
+    events = capture_events()
+
+    client = tornado_testcase(Application([(r"/hi", QueryHandler)]))
+    response = client.fetch("/hi")
+    assert response.code == 200
+
+    sentry_sdk.flush()
+
+    (event,) = events
+
+    assert "query_string" not in event["request"]
+    assert event["request"]["url"].endswith("/hi")
+    assert event["request"]["method"] == "GET"
+
+
+def test_url_query_data_collection_event_processor_repeated_and_blank_params(
+    tornado_testcase, sentry_init, capture_events
+):
+    sentry_init(
+        integrations=[TornadoIntegration()],
+        traces_sample_rate=1.0,
+        trace_lifecycle="static",
+        _experiments={"data_collection": {}},
+    )
+
+    events = capture_events()
+
+    client = tornado_testcase(Application([(r"/hi", QueryHandler)]))
+    response = client.fetch("/hi?a=1&a=2&b=")
+    assert response.code == 200
+
+    sentry_sdk.flush()
+
+    (event,) = events
+
+    assert event["request"]["query_string"] == "a=1&a=2&b="
+
+
 @pytest.mark.parametrize("send_pii", [True, False])
 @pytest.mark.parametrize("span_streaming", [True, False])
 @pytest.mark.parametrize(
@@ -265,7 +513,7 @@ def test_transactions(
         integrations=[TornadoIntegration()],
         traces_sample_rate=1.0,
         send_default_pii=send_pii,
-        _experiments={"trace_lifecycle": "stream" if span_streaming else "static"},
+        trace_lifecycle="stream" if span_streaming else "static",
     )
 
     if span_streaming: