diff --git a/sentry_sdk/integrations/gcp.py b/sentry_sdk/integrations/gcp.py index 29095343eb..cfd3344ab8 100644 --- a/sentry_sdk/integrations/gcp.py +++ b/sentry_sdk/integrations/gcp.py @@ -8,6 +8,7 @@ import sentry_sdk from sentry_sdk.api import continue_trace from sentry_sdk.consts import OP +from sentry_sdk.data_collection import _apply_data_collection_filtering_to_query_string from sentry_sdk.integrations import Integration from sentry_sdk.integrations._wsgi_common import _filter_headers from sentry_sdk.integrations.cloud_resource_context import CLOUD_PROVIDER @@ -20,6 +21,7 @@ TimeoutThread, capture_internal_exceptions, event_from_exception, + has_data_collection_enabled, logger, reraise, ) @@ -100,10 +102,20 @@ def sentry_func( if hasattr(gcp_event, "method"): additional_attributes["http.request.method"] = gcp_event.method - if should_send_default_pii() and hasattr(gcp_event, "query_string"): - additional_attributes["url.query"] = gcp_event.query_string.decode( - "utf-8", errors="replace" - ) + if hasattr(gcp_event, "query_string"): + query_string = gcp_event.query_string.decode("utf-8", errors="replace") + if query_string: + if has_data_collection_enabled(client.options): + filtered_qs = _apply_data_collection_filtering_to_query_string( + query_string=query_string, + behaviour=client.options["data_collection"][ + "url_query_params" + ], + ) + if filtered_qs: + additional_attributes["url.query"] = filtered_qs + elif should_send_default_pii(): + additional_attributes["url.query"] = query_string sampling_context = { "gcp_env": { @@ -235,9 +247,18 @@ def event_processor(event: "Event", hint: "Hint") -> "Optional[Event]": request["method"] = gcp_event.method if hasattr(gcp_event, "query_string"): - request["query_string"] = gcp_event.query_string.decode( - "utf-8", errors="replace" - ) + query_string = gcp_event.query_string.decode("utf-8", errors="replace") + client_options = sentry_sdk.get_client().options + if has_data_collection_enabled(client_options): + if query_string: + filtered_qs = _apply_data_collection_filtering_to_query_string( + query_string=query_string, + behaviour=client_options["data_collection"]["url_query_params"], + ) + if filtered_qs: + request["query_string"] = filtered_qs + else: + request["query_string"] = query_string if hasattr(gcp_event, "headers"): request["headers"] = _filter_headers(gcp_event.headers) diff --git a/sentry_sdk/integrations/quart.py b/sentry_sdk/integrations/quart.py index 6a5603d825..8c281f5874 100644 --- a/sentry_sdk/integrations/quart.py +++ b/sentry_sdk/integrations/quart.py @@ -5,6 +5,7 @@ from typing import TYPE_CHECKING import sentry_sdk +from sentry_sdk.data_collection import _apply_data_collection_filtering_to_query_string from sentry_sdk.integrations import DidNotEnable, Integration from sentry_sdk.integrations._wsgi_common import _filter_headers from sentry_sdk.integrations.asgi import SentryAsgiMiddleware @@ -17,6 +18,8 @@ capture_internal_exceptions, ensure_integration_enabled, event_from_exception, + has_data_collection_enabled, + parse_url, ) if TYPE_CHECKING: @@ -203,7 +206,39 @@ async def _request_websocket_started(app: "Quart", **kwargs: "Any") -> None: segment.set_attributes(header_attributes) - if should_send_default_pii(): + client_options = sentry_sdk.get_client().options + filtered_query_string = None + if has_data_collection_enabled(client_options): + query_string = request_websocket.query_string.decode( + "utf-8", errors="replace" + ) + if query_string: + filtered_query_string = ( + _apply_data_collection_filtering_to_query_string( + query_string=query_string, + behaviour=client_options["data_collection"][ + "url_query_params" + ], + ) + ) + if filtered_query_string: + segment.set_attribute( + "url.query", + filtered_query_string, + ) + + parsed_url = parse_url(request_websocket.url) + segment.set_attribute( + "url.full", + f"{parsed_url.url}?{filtered_query_string}" + if filtered_query_string + else parsed_url.url, + ) + + # TODO: Add the user properties that are seen in the branch below here once + # code is added to respect the `user_info` settings within the data collection + # configuration + elif should_send_default_pii(): segment.set_attribute("url.full", request_websocket.url) segment.set_attribute( "url.query", diff --git a/tests/integrations/gcp/test_gcp.py b/tests/integrations/gcp/test_gcp.py index daf1c6c129..b02518a413 100644 --- a/tests/integrations/gcp/test_gcp.py +++ b/tests/integrations/gcp/test_gcp.py @@ -786,3 +786,170 @@ def cloud_function(functionhandler, event): assert attrs["gcp.project.id"] == "serverless_project" assert attrs["faas.identity"] == "func_ID" assert attrs["faas.entry_point"] == "cloud_function" + + +# Each case is (send_default_pii, data_collection, expected_event, expected_span). +# ``send_default_pii`` / ``data_collection`` of None means the option is omitted. +# The two paths only diverge for the legacy (no ``data_collection``) rows: the +# event processor always records the raw query string, while the span-streaming +# path gates the ``url.query`` attribute on ``send_default_pii``. +_QUERY_STRING_DATA_COLLECTION_CASES = [ + pytest.param( + True, + None, + "toy=tennisball&color=red&auth=secret", + "toy=tennisball&color=red&auth=secret", + id="send_default_pii_true", + ), + pytest.param( + False, + None, + "toy=tennisball&color=red&auth=secret", + None, + id="send_default_pii_false", + ), + pytest.param( + None, + None, + "toy=tennisball&color=red&auth=secret", + None, + id="defaults", + ), + # data_collection configured: query string is routed through filtering. + # Spec defaults -> denylist: only the sensitive ``auth`` is redacted. + pytest.param( + None, + {}, + "toy=tennisball&color=red&auth=%5BFiltered%5D", + "toy=tennisball&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_default", + ), + pytest.param( + None, + {"url_query_params": {"mode": "denylist", "terms": ["toy"]}}, + "toy=%5BFiltered%5D&color=red&auth=%5BFiltered%5D", + "toy=%5BFiltered%5D&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_custom_terms", + ), + # allowlist with only ``toy`` allowed: ``color`` is redacted even though it + # is not sensitive, proving the redaction comes from the allowlist. + pytest.param( + None, + {"url_query_params": {"mode": "allowlist", "terms": ["toy"]}}, + "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", + "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", + id="data_collection_allowlist", + ), + pytest.param( + None, + {"url_query_params": {"mode": "off"}}, + None, + None, + id="data_collection_off", + ), +] + + +def _build_init_kwargs(send_default_pii, data_collection): + """Render the keyword-argument string passed to ``init_sdk`` in the + subprocess.""" + kwargs = [] + if send_default_pii is not None: + kwargs.append("send_default_pii=%r" % send_default_pii) + if data_collection is not None: + kwargs.append("_experiments=%r" % {"data_collection": data_collection}) + + return ", ".join(kwargs) + + +@pytest.mark.parametrize( + "send_default_pii, data_collection, expected_event, expected_span", + _QUERY_STRING_DATA_COLLECTION_CASES, +) +def test_query_string_data_collection_event_processor( + run_cloud_function, + send_default_pii, + data_collection, + expected_event, + expected_span, +): + init_kwargs = _build_init_kwargs(send_default_pii, data_collection) + envelope_items, _, _ = run_cloud_function( + dedent( + """ + functionhandler = None + + from collections import namedtuple + GCPEvent = namedtuple("GCPEvent", ["headers", "method", "query_string"]) + event = GCPEvent( + headers={}, + method="GET", + query_string=b"toy=tennisball&color=red&auth=secret", + ) + + def cloud_function(functionhandler, event): + raise Exception("something went wrong") + """ + ) + + FUNCTIONS_PRELUDE + + dedent( + """ + init_sdk(%s) + gcp_functions.worker_v1.FunctionHandler.invoke_user_function(functionhandler, event) + """ + % init_kwargs + ) + ) + + request = envelope_items[0]["request"] + if expected_event is None: + assert "query_string" not in request + else: + assert request["query_string"] == expected_event + + +@pytest.mark.parametrize( + "send_default_pii, data_collection, expected_event, expected_span", + _QUERY_STRING_DATA_COLLECTION_CASES, +) +def test_query_string_data_collection_span_streaming( + run_cloud_function, + send_default_pii, + data_collection, + expected_event, + expected_span, +): + init_kwargs = _build_init_kwargs(send_default_pii, data_collection) + _, _, span_items = run_cloud_function( + dedent( + """ + functionhandler = None + + from collections import namedtuple + GCPEvent = namedtuple("GCPEvent", ["headers", "method", "query_string"]) + event = GCPEvent( + headers={}, + method="GET", + query_string=b"toy=tennisball&color=red&auth=secret", + ) + + def cloud_function(functionhandler, event): + return "ok" + """ + ) + + FUNCTIONS_PRELUDE + + dedent( + """ + init_sdk(traces_sample_rate=1.0, trace_lifecycle="stream", %s) + gcp_functions.worker_v1.FunctionHandler.invoke_user_function(functionhandler, event) + """ + % init_kwargs + ) + ) + + assert len(span_items) == 1 + attrs = span_items[0]["attributes"] + if expected_span is None: + assert "url.query" not in attrs + else: + assert attrs["url.query"] == expected_span diff --git a/tests/integrations/quart/test_quart.py b/tests/integrations/quart/test_quart.py index b2172d80e8..286d6aeaff 100644 --- a/tests/integrations/quart/test_quart.py +++ b/tests/integrations/quart/test_quart.py @@ -1131,3 +1131,154 @@ async def test_span_streaming_sensitive_header_passthrough_with_pii_and_no_data_ segment["attributes"]["http.request.header.authorization"] == "Bearer secret-token" ) + + +_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", + ), + pytest.param( + {}, + None, + id="defaults", + ), + pytest.param( + {"_experiments": {"data_collection": {}}}, + "toy=tennisball&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_default", + ), + pytest.param( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "denylist", "terms": ["toy"]} + } + } + }, + "toy=%5BFiltered%5D&color=red&auth=%5BFiltered%5D", + id="data_collection_denylist_custom_terms", + ), + pytest.param( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "allowlist", "terms": ["toy"]} + } + } + }, + "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", + id="data_collection_allowlist", + ), + pytest.param( + { + "_experiments": { + "data_collection": { + "url_query_params": {"mode": "allowlist", "terms": ["auth"]} + } + } + }, + "toy=%5BFiltered%5D&color=%5BFiltered%5D&auth=%5BFiltered%5D", + id="data_collection_allowlist_sensitive_term", + ), + pytest.param( + {"_experiments": {"data_collection": {"url_query_params": {"mode": "off"}}}}, + None, + id="data_collection_off", + ), + pytest.param( + { + "send_default_pii": True, + "_experiments": {"data_collection": {"url_query_params": {"mode": "off"}}}, + }, + None, + id="data_collection_wins_over_send_default_pii", + ), +] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "init_kwargs, expected_query", _QUERY_PARAM_DATA_COLLECTION_CASES +) +async def test_span_streaming_url_query_data_collection( + sentry_init, capture_items, init_kwargs, expected_query +): + init_kwargs = dict(init_kwargs) + experiments = {"trace_lifecycle": "stream"} + experiments.update(init_kwargs.pop("_experiments", {})) + sentry_init( + integrations=[quart_sentry.QuartIntegration()], + traces_sample_rate=1.0, + _experiments=experiments, + **init_kwargs, + ) + items = capture_items("span") + + app = quart_app_factory() + client = app.test_client() + response = await client.get("/message?toy=tennisball&color=red&auth=secret") + assert response.status_code == 200 + + sentry_sdk.flush() + + spans = [item.payload for item in items] + assert len(spans) == 1 + + segment = spans[0] + + data_collection_enabled = "data_collection" in experiments + url_attrs_expected = data_collection_enabled or init_kwargs.get( + "send_default_pii", False + ) + + if expected_query is None: + assert "url.query" not in segment["attributes"] + if url_attrs_expected: + # When the filtered query string is empty, url.full carries the + # base URL only (no query). + assert segment["attributes"]["url.full"] == "http://localhost/message" + else: + assert "url.full" not in segment["attributes"] + else: + assert segment["attributes"]["url.query"] == expected_query + assert segment["attributes"]["url.full"] == ( + f"http://localhost/message?{expected_query}" + ) + + +@pytest.mark.asyncio +async def test_span_streaming_url_query_multi_and_blank_values( + sentry_init, capture_items +): + sentry_init( + integrations=[quart_sentry.QuartIntegration()], + traces_sample_rate=1.0, + _experiments={"trace_lifecycle": "stream", "data_collection": {}}, + ) + items = capture_items("span") + + app = quart_app_factory() + client = app.test_client() + response = await client.get("/message?foo=1&foo=2&empty=") + assert response.status_code == 200 + + sentry_sdk.flush() + + spans = [item.payload for item in items] + assert len(spans) == 1 + + segment = spans[0] + query = segment["attributes"]["url.query"] + # Repeated keys are preserved and blank values are kept. + assert "foo=1" in query + assert "foo=2" in query + assert "empty=" in query + # url.full carries the same filtered query string. + assert segment["attributes"]["url.full"] == f"http://localhost/message?{query}"