Skip to content

Commit 337d8cd

Browse files
ericapisaniericapisani
committed
feat(aiohttp): Remove request body capture from span streaming
Remove logic that captured request body data on span segments in the aiohttp integration when span streaming is enabled. This simplifies the request handling flow and removes the need for post-handler body data capture. Also removes the following test cases that validated this behavior: - test_request_body_captured_on_segment_span_streaming - test_request_body_not_read_span_streaming - test_request_body_over_size_limit_span_streaming Remove unused imports from test file. Fixes PY-2422 Fixes #6292
1 parent 75aaebf commit 337d8cd

2 files changed

Lines changed: 32 additions & 154 deletions

File tree

‎sentry_sdk/integrations/aiohttp.py‎

Lines changed: 32 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -201,60 +201,41 @@ async def sentry_app_handle(
201201

202202
with span_ctx as span:
203203
try:
204-
try:
205-
response = await old_handle(self, request)
206-
except HTTPException as e:
207-
if isinstance(span, StreamedSpan) and not isinstance(
208-
span, NoOpStreamedSpan
209-
):
210-
span.set_attribute(
211-
"http.response.status_code", e.status_code
212-
)
213-
214-
if e.status_code >= 400:
215-
span.status = SpanStatus.ERROR.value
216-
else:
217-
span.status = SpanStatus.OK.value
218-
else:
219-
# Since a NoOpStreamedSpan can end up here, we have to guard against it
220-
# so this only gets set in the legacy transaction approach.
221-
if not isinstance(span, NoOpStreamedSpan):
222-
span.set_http_status(e.status_code)
223-
224-
if (
225-
e.status_code
226-
in integration._failed_request_status_codes
227-
):
228-
_capture_exception()
229-
raise
230-
except (asyncio.CancelledError, ConnectionResetError):
231-
if isinstance(span, StreamedSpan):
232-
span.status = SpanStatus.ERROR.value
233-
else:
234-
span.set_status(SPANSTATUS.CANCELLED)
235-
raise
236-
except Exception:
237-
# This will probably map to a 500 but seems like we
238-
# have no way to tell. Do not set span status.
239-
reraise(*_capture_exception())
240-
finally:
241-
# The handler has had a chance to read the body, so
242-
# request._read_bytes may now be populated. Capture
243-
# body data on the segment regardless of outcome.
204+
response = await old_handle(self, request)
205+
except HTTPException as e:
244206
if isinstance(span, StreamedSpan) and not isinstance(
245207
span, NoOpStreamedSpan
246208
):
247-
with capture_internal_exceptions():
248-
raw_data = get_aiohttp_request_data(request)
249-
body_data = (
250-
raw_data.value
251-
if isinstance(raw_data, AnnotatedValue)
252-
else raw_data
253-
)
254-
if body_data is not None:
255-
span._segment.set_attribute(
256-
"http.request.body.data", body_data
257-
)
209+
span.set_attribute(
210+
"http.response.status_code", e.status_code
211+
)
212+
213+
if e.status_code >= 400:
214+
span.status = SpanStatus.ERROR.value
215+
else:
216+
span.status = SpanStatus.OK.value
217+
else:
218+
# Since a NoOpStreamedSpan can end up here, we have to guard against it
219+
# so this only gets set in the legacy transaction approach.
220+
if not isinstance(span, NoOpStreamedSpan):
221+
span.set_http_status(e.status_code)
222+
223+
if (
224+
e.status_code
225+
in integration._failed_request_status_codes
226+
):
227+
_capture_exception()
228+
raise
229+
except (asyncio.CancelledError, ConnectionResetError):
230+
if isinstance(span, StreamedSpan):
231+
span.status = SpanStatus.ERROR.value
232+
else:
233+
span.set_status(SPANSTATUS.CANCELLED)
234+
raise
235+
except Exception:
236+
# This will probably map to a 500 but seems like we
237+
# have no way to tell. Do not set span status.
238+
reraise(*_capture_exception())
258239

259240
try:
260241
# A valid response handler will return a valid response with a status. But, if the handler

‎tests/integrations/aiohttp/test_aiohttp.py‎

Lines changed: 0 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,8 @@
1919

2020
import sentry_sdk
2121
from sentry_sdk import capture_message, start_transaction
22-
from sentry_sdk._types import OVER_SIZE_LIMIT_SUBSTITUTE
2322
from sentry_sdk.consts import SPANDATA
2423
from sentry_sdk.integrations.aiohttp import (
25-
BODY_NOT_READ_MESSAGE,
2624
AioHttpIntegration,
2725
create_trace_config,
2826
)
@@ -1283,107 +1281,6 @@ async def hello(request):
12831281
assert server_span["attributes"]["user.ip_address"] == "127.0.0.1"
12841282

12851283

1286-
@pytest.mark.asyncio
1287-
async def test_request_body_captured_on_segment_span_streaming(
1288-
sentry_init, aiohttp_client, capture_items
1289-
):
1290-
sentry_init(
1291-
integrations=[AioHttpIntegration()],
1292-
traces_sample_rate=1.0,
1293-
_experiments={"trace_lifecycle": "stream"},
1294-
)
1295-
1296-
body = {"some": "value"}
1297-
1298-
async def hello(request):
1299-
# Reading the body populates request._read_bytes; the integration
1300-
# captures body data in a finally after the handler returns.
1301-
await request.json()
1302-
return web.Response(text="hello")
1303-
1304-
app = web.Application()
1305-
app.router.add_post("/", hello)
1306-
1307-
items = capture_items("span")
1308-
1309-
client = await aiohttp_client(app)
1310-
resp = await client.post("/", json=body)
1311-
assert resp.status == 200
1312-
1313-
sentry_sdk.flush()
1314-
1315-
server_segment, client_segment = [item.payload for item in items]
1316-
assert server_segment["is_segment"] is True
1317-
assert server_segment["attributes"]["http.request.body.data"] == json.dumps(body)
1318-
1319-
1320-
@pytest.mark.asyncio
1321-
async def test_request_body_not_read_span_streaming(
1322-
sentry_init, aiohttp_client, capture_items
1323-
):
1324-
sentry_init(
1325-
integrations=[AioHttpIntegration()],
1326-
traces_sample_rate=1.0,
1327-
_experiments={"trace_lifecycle": "stream"},
1328-
)
1329-
1330-
async def hello(request):
1331-
# Handler does not read the body; request._read_bytes stays None.
1332-
return web.Response(text="hello")
1333-
1334-
app = web.Application()
1335-
app.router.add_post("/", hello)
1336-
1337-
items = capture_items("span")
1338-
1339-
client = await aiohttp_client(app)
1340-
resp = await client.post("/", json={"some": "value"})
1341-
assert resp.status == 200
1342-
1343-
sentry_sdk.flush()
1344-
1345-
server_segment, client_segment = [item.payload for item in items]
1346-
assert server_segment["is_segment"] is True
1347-
assert (
1348-
server_segment["attributes"]["http.request.body.data"] == BODY_NOT_READ_MESSAGE
1349-
)
1350-
1351-
1352-
@pytest.mark.asyncio
1353-
async def test_request_body_over_size_limit_span_streaming(
1354-
sentry_init, aiohttp_client, capture_items
1355-
):
1356-
sentry_init(
1357-
integrations=[AioHttpIntegration()],
1358-
traces_sample_rate=1.0,
1359-
max_request_body_size="small",
1360-
_experiments={"trace_lifecycle": "stream"},
1361-
)
1362-
1363-
async def hello(request):
1364-
await request.read()
1365-
return web.Response(text="hello")
1366-
1367-
app = web.Application()
1368-
app.router.add_post("/", hello)
1369-
1370-
items = capture_items("span")
1371-
1372-
client = await aiohttp_client(app)
1373-
# "small" caps at 1 KB; send a body larger than that.
1374-
resp = await client.post("/", data=b"x" * 2000)
1375-
assert resp.status == 200
1376-
1377-
sentry_sdk.flush()
1378-
1379-
server_segment, client_segment = [item.payload for item in items]
1380-
assert server_segment["is_segment"] is True
1381-
assert (
1382-
server_segment["attributes"]["http.request.body.data"]
1383-
== OVER_SIZE_LIMIT_SUBSTITUTE
1384-
)
1385-
1386-
13871284
@pytest.mark.asyncio
13881285
async def test_url_query_attribute_span_streaming(
13891286
sentry_init, aiohttp_client, capture_items

0 commit comments

Comments
 (0)