diff --git a/sentry_sdk/integrations/fastapi.py b/sentry_sdk/integrations/fastapi.py index 5833e5f290..e0061a2340 100644 --- a/sentry_sdk/integrations/fastapi.py +++ b/sentry_sdk/integrations/fastapi.py @@ -4,15 +4,16 @@ from typing import TYPE_CHECKING import sentry_sdk +from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations import DidNotEnable from sentry_sdk.scope import should_send_default_pii -from sentry_sdk.traces import StreamedSpan +from sentry_sdk.traces import StreamedSpan, _get_current_streamed_span from sentry_sdk.tracing import SOURCE_FOR_STYLE, TransactionSource from sentry_sdk.tracing_utils import has_span_streaming_enabled from sentry_sdk.utils import transaction_from_function if TYPE_CHECKING: - from typing import Any, Callable, Dict + from typing import Any, Awaitable, Callable, Dict from sentry_sdk._types import Event @@ -20,7 +21,7 @@ from sentry_sdk.integrations.starlette import ( StarletteIntegration, StarletteRequestExtractor, - _set_request_body_data_on_streaming_segment, + _get_cached_request_body_attribute, ) except DidNotEnable: raise DidNotEnable("Starlette is not installed") @@ -75,6 +76,66 @@ def _set_transaction_name_and_source( scope.set_transaction_name(name, source=source) +async def _wrap_async_handler( + handler: "Callable[..., Awaitable[Any]]", *args: "Any", **kwargs: "Any" +) -> "Any": + """ + Wraps an asynchronous handler function to attach request info to errors and the server segment span. + The request body cached on the Starlette Request object is attached to streamed spans, but consuming the request body in the event + processor can still cause application hangs. + """ + client = sentry_sdk.get_client() + integration = client.get_integration(FastApiIntegration) + if integration is None: + return await handler(*args, **kwargs) + + request = args[0] + + _set_transaction_name_and_source( + sentry_sdk.get_current_scope(), integration.transaction_style, request + ) + sentry_scope = sentry_sdk.get_isolation_scope() + extractor = StarletteRequestExtractor(request) + info = await extractor.extract_request_info() + + def _make_request_event_processor( + req: "Any", integration: "Any" + ) -> "Callable[[Event, Dict[str, Any]], Event]": + def event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event": + # Extract information from request + request_info = event.get("request", {}) + if info: + if "cookies" in info and should_send_default_pii(): + request_info["cookies"] = info["cookies"] + if "data" in info: + request_info["data"] = info["data"] + event["request"] = deepcopy(request_info) + + return event + + return event_processor + + sentry_scope._name = FastApiIntegration.identifier + sentry_scope.add_event_processor( + _make_request_event_processor(request, integration) + ) + + try: + return await handler(*args, **kwargs) + finally: + current_span = _get_current_streamed_span() + + if type(current_span) is StreamedSpan: + request_body = _get_cached_request_body_attribute( + client=client, request=request + ) + if request_body: + current_span._segment.set_attribute( + SPANDATA.HTTP_REQUEST_BODY_DATA, + request_body, + ) + + def patch_get_request_handler() -> None: old_get_request_handler = fastapi.routing.get_request_handler @@ -113,46 +174,7 @@ def _sentry_call(*args: "Any", **kwargs: "Any") -> "Any": old_app = old_get_request_handler(*args, **kwargs) async def _sentry_app(*args: "Any", **kwargs: "Any") -> "Any": - client = sentry_sdk.get_client() - integration = client.get_integration(FastApiIntegration) - if integration is None: - return await old_app(*args, **kwargs) - - request = args[0] - - _set_transaction_name_and_source( - sentry_sdk.get_current_scope(), integration.transaction_style, request - ) - sentry_scope = sentry_sdk.get_isolation_scope() - extractor = StarletteRequestExtractor(request) - info = await extractor.extract_request_info() - - def _make_request_event_processor( - req: "Any", integration: "Any" - ) -> "Callable[[Event, Dict[str, Any]], Event]": - def event_processor(event: "Event", hint: "Dict[str, Any]") -> "Event": - # Extract information from request - request_info = event.get("request", {}) - if info: - if "cookies" in info and should_send_default_pii(): - request_info["cookies"] = info["cookies"] - if "data" in info: - request_info["data"] = info["data"] - event["request"] = deepcopy(request_info) - - return event - - return event_processor - - sentry_scope._name = FastApiIntegration.identifier - sentry_scope.add_event_processor( - _make_request_event_processor(request, integration) - ) - - if has_span_streaming_enabled(client.options): - _set_request_body_data_on_streaming_segment(info) - - return await old_app(*args, **kwargs) + return await _wrap_async_handler(old_app, *args, **kwargs) return _sentry_app diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index 691a3dc0bb..e5c79b763a 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -254,18 +254,6 @@ def _default(value: "Any") -> "Any": return json.dumps(data, default=_default) -def _set_request_body_data_on_streaming_segment( - info: "Optional[Dict[str, Any]]", -) -> None: - current_span = _get_current_streamed_span() - if info and "data" in info and type(current_span) is StreamedSpan: - with capture_internal_exceptions(): - current_span._segment.set_attribute( - "http.request.body.data", - _serialize_request_body_data(info["data"]), - ) - - @ensure_integration_enabled(StarletteIntegration) def _capture_exception(exception: BaseException, handled: "Any" = False) -> None: event, hint = event_from_exception( diff --git a/tests/integrations/fastapi/test_fastapi.py b/tests/integrations/fastapi/test_fastapi.py index 845b948e71..e6c7d962a9 100644 --- a/tests/integrations/fastapi/test_fastapi.py +++ b/tests/integrations/fastapi/test_fastapi.py @@ -15,6 +15,7 @@ import sentry_sdk from sentry_sdk import capture_message +from sentry_sdk.consts import SPANDATA from sentry_sdk.feature_flags import add_feature_flag from sentry_sdk.integrations.asgi import SentryAsgiMiddleware from sentry_sdk.integrations.fastapi import FastApiIntegration @@ -112,117 +113,232 @@ async def body_form( @pytest.mark.asyncio -async def test_request_info_json_body(sentry_init, capture_events): +@pytest.mark.parametrize("span_streaming", [True, False]) +async def test_request_info_json_body( + sentry_init, capture_events, capture_items, span_streaming +): sentry_init( traces_sample_rate=1.0, send_default_pii=True, integrations=[StarletteIntegration()], + _experiments={ + "trace_lifecycle": "stream" if span_streaming else "static", + }, ) app = fastapi_app_factory() client = TestClient(app) - events = capture_events() + if span_streaming: + items = capture_items("event", "span") + + client.post( + "/body/json", + json=BODY_JSON, + headers={ + "cookie": "yummy_cookie=choco; tasty_cookie=strawberry", + }, + ) - client.post( - "/body/json", - json=BODY_JSON, - headers={ - "cookie": "yummy_cookie=choco; tasty_cookie=strawberry", - }, - ) + (event,) = (item.payload for item in items if item.type == "event") + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert event["request"]["data"] == BODY_JSON + + sentry_sdk.flush() + spans = [item.payload for item in items if item.type == "span"] + server_span = next( + span for span in spans if span["attributes"]["sentry.op"] == "http.server" + ) + + assert json.loads( + server_span["attributes"][SPANDATA.HTTP_REQUEST_BODY_DATA] + ) == {"some": "json", "for": "testing", "nested": {"numbers": 123}} + else: + events = capture_events() + + client.post( + "/body/json", + json=BODY_JSON, + headers={ + "cookie": "yummy_cookie=choco; tasty_cookie=strawberry", + }, + ) - (event, transaction_event) = events + (event, transaction_event) = events - assert event["request"]["cookies"] == { - "tasty_cookie": "strawberry", - "yummy_cookie": "choco", - } - assert event["request"]["data"] == BODY_JSON + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert event["request"]["data"] == BODY_JSON - assert transaction_event["request"]["cookies"] == { - "tasty_cookie": "strawberry", - "yummy_cookie": "choco", - } - assert transaction_event["request"]["data"] == BODY_JSON + assert transaction_event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert transaction_event["request"]["data"] == BODY_JSON @pytest.mark.asyncio -async def test_formdata_request_body(sentry_init, capture_events): +@pytest.mark.parametrize("span_streaming", [True, False]) +async def test_formdata_request_body( + sentry_init, capture_events, capture_items, span_streaming +): sentry_init( traces_sample_rate=1.0, send_default_pii=True, max_request_body_size="always", integrations=[StarletteIntegration()], + _experiments={ + "trace_lifecycle": "stream" if span_streaming else "static", + }, ) app = fastapi_app_factory() client = TestClient(app) - events = capture_events() + if span_streaming: + items = capture_items("event", "span") + + client.post( + "/body/form", + data=BODY_FORM.encode("utf-8"), + headers={ + "content-type": "multipart/form-data; boundary=fd721ef49ea403a6", + }, + ) - client.post( - "/body/form", - data=BODY_FORM.encode("utf-8"), - headers={ - "content-type": "multipart/form-data; boundary=fd721ef49ea403a6", - }, - ) + (event,) = (item.payload for item in items if item.type == "event") + assert event["request"]["data"].keys() == PARSED_FORM.keys() + assert event["request"]["data"]["username"] == PARSED_FORM["username"] + assert event["request"]["data"]["password"] == "[Filtered]" + assert event["request"]["data"]["photo"] == "" + + sentry_sdk.flush() + spans = [item.payload for item in items if item.type == "span"] + server_span = next( + span for span in spans if span["attributes"]["sentry.op"] == "http.server" + ) - (event, transaction_event) = events - assert event["request"]["data"].keys() == PARSED_FORM.keys() - assert event["request"]["data"]["username"] == PARSED_FORM["username"] - assert event["request"]["data"]["password"] == "[Filtered]" - assert event["request"]["data"]["photo"] == "" - assert event["_meta"]["request"]["data"]["photo"] == {"": {"rem": [["!raw", "x"]]}} + # Going forward, the sanitization of data will need to happen within the `before_send_span` hooks + # See https://sentry.slack.com/archives/C09RR0KD2N7/p1776951331206129?thread_ts=1776951227.440659&cid=C09RR0KD2N7 + parsed_form_attribute = json.loads( + server_span["attributes"][SPANDATA.HTTP_REQUEST_BODY_DATA] + ) + assert parsed_form_attribute.keys() == PARSED_FORM.keys() + assert parsed_form_attribute["username"] == PARSED_FORM["username"] + assert parsed_form_attribute["password"] == "hello123" + assert parsed_form_attribute["photo"] == "[Unparsable]" + else: + events = capture_events() + + client.post( + "/body/form", + data=BODY_FORM.encode("utf-8"), + headers={ + "content-type": "multipart/form-data; boundary=fd721ef49ea403a6", + }, + ) - assert transaction_event["request"]["data"].keys() == PARSED_FORM.keys() - assert transaction_event["request"]["data"]["username"] == PARSED_FORM["username"] - assert transaction_event["request"]["data"]["password"] == "[Filtered]" - assert transaction_event["request"]["data"]["photo"] == "" - assert transaction_event["_meta"]["request"]["data"]["photo"] == { - "": {"rem": [["!raw", "x"]]} - } + (event, transaction_event) = events + assert event["request"]["data"].keys() == PARSED_FORM.keys() + assert event["request"]["data"]["username"] == PARSED_FORM["username"] + assert event["request"]["data"]["password"] == "[Filtered]" + assert event["request"]["data"]["photo"] == "" + assert event["_meta"]["request"]["data"]["photo"] == { + "": {"rem": [["!raw", "x"]]} + } + + assert transaction_event["request"]["data"].keys() == PARSED_FORM.keys() + assert ( + transaction_event["request"]["data"]["username"] == PARSED_FORM["username"] + ) + assert transaction_event["request"]["data"]["password"] == "[Filtered]" + assert transaction_event["request"]["data"]["photo"] == "" + assert transaction_event["_meta"]["request"]["data"]["photo"] == { + "": {"rem": [["!raw", "x"]]} + } @pytest.mark.asyncio -async def test_request_body_too_big(sentry_init, capture_events): +@pytest.mark.parametrize("span_streaming", [True, False]) +async def test_request_body_too_big( + sentry_init, capture_events, capture_items, span_streaming +): sentry_init( traces_sample_rate=1.0, send_default_pii=True, integrations=[StarletteIntegration()], + _experiments={ + "trace_lifecycle": "stream" if span_streaming else "static", + }, ) app = fastapi_app_factory() client = TestClient(app) - events = capture_events() + if span_streaming: + items = capture_items("event", "span") + + client.post( + "/body/form", + data=BODY_FORM.encode("utf-8"), + headers={ + "content-type": "multipart/form-data; boundary=fd721ef49ea403a6", + "cookie": "yummy_cookie=choco; tasty_cookie=strawberry", + }, + ) - client.post( - "/body/form", - data=BODY_FORM.encode("utf-8"), - headers={ - "content-type": "multipart/form-data; boundary=fd721ef49ea403a6", - "cookie": "yummy_cookie=choco; tasty_cookie=strawberry", - }, - ) + (event,) = (item.payload for item in items if item.type == "event") + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + # Because request is too big only the AnnotatedValue is extracted. + assert event["_meta"]["request"]["data"] == {"": {"rem": [["!config", "x"]]}} + + sentry_sdk.flush() + spans = [item.payload for item in items if item.type == "span"] + server_span = next( + span for span in spans if span["attributes"]["sentry.op"] == "http.server" + ) + + # Because request is too big only the AnnotatedValue is extracted. + assert ( + server_span["attributes"][SPANDATA.HTTP_REQUEST_BODY_DATA] + == "[Exceeds maximum size]" + ) + else: + events = capture_events() + + client.post( + "/body/form", + data=BODY_FORM.encode("utf-8"), + headers={ + "content-type": "multipart/form-data; boundary=fd721ef49ea403a6", + "cookie": "yummy_cookie=choco; tasty_cookie=strawberry", + }, + ) - (event, transaction_event) = events - assert event["request"]["cookies"] == { - "tasty_cookie": "strawberry", - "yummy_cookie": "choco", - } - # Because request is too big only the AnnotatedValue is extracted. - assert event["_meta"]["request"]["data"] == {"": {"rem": [["!config", "x"]]}} + (event, transaction_event) = events + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + # Because request is too big only the AnnotatedValue is extracted. + assert event["_meta"]["request"]["data"] == {"": {"rem": [["!config", "x"]]}} - assert transaction_event["request"]["cookies"] == { - "tasty_cookie": "strawberry", - "yummy_cookie": "choco", - } - # Because request is too big only the AnnotatedValue is extracted. - assert transaction_event["_meta"]["request"]["data"] == { - "": {"rem": [["!config", "x"]]} - } + assert transaction_event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + # Because request is too big only the AnnotatedValue is extracted. + assert transaction_event["_meta"]["request"]["data"] == { + "": {"rem": [["!config", "x"]]} + } @pytest.mark.asyncio @@ -398,142 +514,6 @@ def test_active_thread_id_span_streaming(sentry_init, capture_items, endpoint): assert str(data["active"]) == segments[0]["attributes"]["thread.id"] -def _post_body_fastapi_app(handler_awaitable): - app = FastAPI() - - @app.post("/body") - async def _route(request: Request): - await handler_awaitable(request) - return {"ok": True} - - return app - - -@pytest.mark.parametrize("middleware_spans", [False, True]) -def test_request_body_data_does_not_scrub_pii_span_streaming( - sentry_init, capture_items, middleware_spans -): - sentry_init( - auto_enabling_integrations=False, - integrations=[ - StarletteIntegration(middleware_spans=middleware_spans), - FastApiIntegration(middleware_spans=middleware_spans), - ], - traces_sample_rate=1.0, - _experiments={"trace_lifecycle": "stream"}, - ) - - async def _read_json(request): - await request.json() - - items = capture_items("span") - - client = TestClient(_post_body_fastapi_app(_read_json)) - response = client.post( - "/body", - json={ - "password": "ohno", - "authorization": "Bearer token", - "message": "hello", - }, - ) - assert response.status_code == 200 - - sentry_sdk.flush() - - segments = [item.payload for item in items if item.payload.get("is_segment")] - assert len(segments) == 1 - attr = segments[0]["attributes"]["http.request.body.data"] - - # Going forward, the sanitization of data will need to happen within the `before_send_span` hooks - # See https://sentry.slack.com/archives/C09RR0KD2N7/p1776951331206129?thread_ts=1776951227.440659&cid=C09RR0KD2N7 - assert "ohno" in attr - assert "Bearer token" in attr - assert "hello" in attr - - -@pytest.mark.skipif( - STARLETTE_VERSION < (0, 21), - reason="Requires Starlette >= 0.21, because earlier versions use a requests-based TestClient which does not support the 'content' kwarg", -) -@pytest.mark.parametrize("middleware_spans", [False, True]) -def test_request_body_data_annotated_value_top_level_span_streaming( - sentry_init, capture_items, middleware_spans -): - sentry_init( - auto_enabling_integrations=False, - integrations=[ - StarletteIntegration(middleware_spans=middleware_spans), - FastApiIntegration(middleware_spans=middleware_spans), - ], - traces_sample_rate=1.0, - _experiments={"trace_lifecycle": "stream"}, - ) - - async def _read_body(request): - await request.body() - - items = capture_items("span") - - client = TestClient(_post_body_fastapi_app(_read_body)) - response = client.post( - "/body", - content=b"not json and not form", - headers={"content-type": "application/octet-stream"}, - ) - assert response.status_code == 200 - - sentry_sdk.flush() - - segments = [item.payload for item in items if item.payload.get("is_segment")] - assert len(segments) == 1 - attr = segments[0]["attributes"]["http.request.body.data"] - - assert isinstance(attr, str) - assert attr == '""' - - -@pytest.mark.parametrize("middleware_spans", [False, True]) -def test_request_body_data_annotated_value_nested_span_streaming( - sentry_init, capture_items, middleware_spans -): - pytest.importorskip("multipart") - - sentry_init( - auto_enabling_integrations=False, - integrations=[ - StarletteIntegration(middleware_spans=middleware_spans), - FastApiIntegration(middleware_spans=middleware_spans), - ], - traces_sample_rate=1.0, - _experiments={"trace_lifecycle": "stream"}, - ) - - async def _read_form(request): - await request.form() - - items = capture_items("span") - - client = TestClient(_post_body_fastapi_app(_read_form)) - response = client.post( - "/body", - data={"name": "erica"}, - files={"avatar": ("photo.jpg", b"fake-bytes", "image/jpeg")}, - ) - assert response.status_code == 200 - - sentry_sdk.flush() - - segments = [item.payload for item in items if item.payload.get("is_segment")] - assert len(segments) == 1 - attr = segments[0]["attributes"]["http.request.body.data"] - - assert isinstance(attr, str) - parsed = json.loads(attr) - assert parsed["name"] == "erica" - assert "fake-bytes" not in attr - - @pytest.mark.parametrize("span_streaming", [True, False]) @pytest.mark.asyncio async def test_original_request_not_scrubbed(