Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 32 additions & 51 deletions sentry_sdk/integrations/aiohttp.py
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an indentation change after the removal of the finally block that was setting the http.request.body.data attribute - it might be easier to review this outside the diff view

Original file line number Diff line number Diff line change
Expand Up @@ -201,60 +201,41 @@ async def sentry_app_handle(

with span_ctx as span:
try:
try:
response = await old_handle(self, request)
except HTTPException as e:
if isinstance(span, StreamedSpan) and not isinstance(
span, NoOpStreamedSpan
):
span.set_attribute(
"http.response.status_code", e.status_code
)

if e.status_code >= 400:
span.status = SpanStatus.ERROR.value
else:
span.status = SpanStatus.OK.value
else:
# Since a NoOpStreamedSpan can end up here, we have to guard against it
# so this only gets set in the legacy transaction approach.
if not isinstance(span, NoOpStreamedSpan):
span.set_http_status(e.status_code)

if (
e.status_code
in integration._failed_request_status_codes
):
_capture_exception()
raise
except (asyncio.CancelledError, ConnectionResetError):
if isinstance(span, StreamedSpan):
span.status = SpanStatus.ERROR.value
else:
span.set_status(SPANSTATUS.CANCELLED)
raise
except Exception:
# This will probably map to a 500 but seems like we
# have no way to tell. Do not set span status.
reraise(*_capture_exception())
finally:
# The handler has had a chance to read the body, so
# request._read_bytes may now be populated. Capture
# body data on the segment regardless of outcome.
response = await old_handle(self, request)
except HTTPException as e:
if isinstance(span, StreamedSpan) and not isinstance(
span, NoOpStreamedSpan
):
with capture_internal_exceptions():
raw_data = get_aiohttp_request_data(request)
body_data = (
raw_data.value
if isinstance(raw_data, AnnotatedValue)
else raw_data
)
if body_data is not None:
span._segment.set_attribute(
"http.request.body.data", body_data
)
span.set_attribute(
"http.response.status_code", e.status_code
)

if e.status_code >= 400:
span.status = SpanStatus.ERROR.value
else:
span.status = SpanStatus.OK.value
else:
# Since a NoOpStreamedSpan can end up here, we have to guard against it
# so this only gets set in the legacy transaction approach.
if not isinstance(span, NoOpStreamedSpan):
span.set_http_status(e.status_code)

if (
e.status_code
in integration._failed_request_status_codes
):
_capture_exception()
raise
except (asyncio.CancelledError, ConnectionResetError):
if isinstance(span, StreamedSpan):
span.status = SpanStatus.ERROR.value
else:
span.set_status(SPANSTATUS.CANCELLED)
raise
except Exception:
# This will probably map to a 500 but seems like we
# have no way to tell. Do not set span status.
reraise(*_capture_exception())

try:
# A valid response handler will return a valid response with a status. But, if the handler
Expand Down
103 changes: 0 additions & 103 deletions tests/integrations/aiohttp/test_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@

import sentry_sdk
from sentry_sdk import capture_message, start_transaction
from sentry_sdk._types import OVER_SIZE_LIMIT_SUBSTITUTE
from sentry_sdk.consts import SPANDATA
from sentry_sdk.integrations.aiohttp import (
BODY_NOT_READ_MESSAGE,
AioHttpIntegration,
create_trace_config,
)
Expand Down Expand Up @@ -1283,107 +1281,6 @@ async def hello(request):
assert server_span["attributes"]["user.ip_address"] == "127.0.0.1"


@pytest.mark.asyncio
async def test_request_body_captured_on_segment_span_streaming(
sentry_init, aiohttp_client, capture_items
):
sentry_init(
integrations=[AioHttpIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)

body = {"some": "value"}

async def hello(request):
# Reading the body populates request._read_bytes; the integration
# captures body data in a finally after the handler returns.
await request.json()
return web.Response(text="hello")

app = web.Application()
app.router.add_post("/", hello)

items = capture_items("span")

client = await aiohttp_client(app)
resp = await client.post("/", json=body)
assert resp.status == 200

sentry_sdk.flush()

server_segment, client_segment = [item.payload for item in items]
assert server_segment["is_segment"] is True
assert server_segment["attributes"]["http.request.body.data"] == json.dumps(body)


@pytest.mark.asyncio
async def test_request_body_not_read_span_streaming(
sentry_init, aiohttp_client, capture_items
):
sentry_init(
integrations=[AioHttpIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)

async def hello(request):
# Handler does not read the body; request._read_bytes stays None.
return web.Response(text="hello")

app = web.Application()
app.router.add_post("/", hello)

items = capture_items("span")

client = await aiohttp_client(app)
resp = await client.post("/", json={"some": "value"})
assert resp.status == 200

sentry_sdk.flush()

server_segment, client_segment = [item.payload for item in items]
assert server_segment["is_segment"] is True
assert (
server_segment["attributes"]["http.request.body.data"] == BODY_NOT_READ_MESSAGE
)


@pytest.mark.asyncio
async def test_request_body_over_size_limit_span_streaming(
sentry_init, aiohttp_client, capture_items
):
sentry_init(
integrations=[AioHttpIntegration()],
traces_sample_rate=1.0,
max_request_body_size="small",
_experiments={"trace_lifecycle": "stream"},
)

async def hello(request):
await request.read()
return web.Response(text="hello")

app = web.Application()
app.router.add_post("/", hello)

items = capture_items("span")

client = await aiohttp_client(app)
# "small" caps at 1 KB; send a body larger than that.
resp = await client.post("/", data=b"x" * 2000)
assert resp.status == 200

sentry_sdk.flush()

server_segment, client_segment = [item.payload for item in items]
assert server_segment["is_segment"] is True
assert (
server_segment["attributes"]["http.request.body.data"]
== OVER_SIZE_LIMIT_SUBSTITUTE
)


@pytest.mark.asyncio
async def test_url_query_attribute_span_streaming(
sentry_init, aiohttp_client, capture_items
Expand Down
Loading