Skip to content

Commit d4a8603

Browse files
sentrivanaclaude
andcommitted
fix(stdlib): Instrument response body read for chunked HTTP responses
The HTTP client span was being finished in getresponse(), which only waits for response headers. For chunked (or large) responses, the actual data transfer happens later during read(), leaving that time uninstrumented. Defer span completion to HTTPResponse.read() for responses with a body, with HTTPResponse.close() as a safety net for responses that are never read. Responses with no body (Content-Length: 0, HEAD, 204, 304) still finish the span immediately in getresponse(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 18a5828 commit d4a8603

2 files changed

Lines changed: 139 additions & 17 deletions

File tree

sentry_sdk/integrations/stdlib.py

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import subprocess
33
import sys
44
import platform
5-
from http.client import HTTPConnection
5+
from http.client import HTTPConnection, HTTPResponse
66

77
import sentry_sdk
88
from sentry_sdk.consts import OP, SPANDATA
@@ -66,9 +66,22 @@ def add_python_runtime_context(
6666
return event
6767

6868

69+
def _finish_span(span: "Union[Span, StreamedSpan]") -> None:
70+
if isinstance(span, StreamedSpan):
71+
with capture_internal_exceptions():
72+
add_http_request_source(span)
73+
span.end()
74+
else:
75+
span.finish()
76+
with capture_internal_exceptions():
77+
add_http_request_source(span)
78+
79+
6980
def _install_httplib() -> None:
7081
real_putrequest = HTTPConnection.putrequest
7182
real_getresponse = HTTPConnection.getresponse
83+
real_read = HTTPResponse.read
84+
real_close = HTTPResponse.close
7285

7386
def putrequest(
7487
self: "HTTPConnection", method: str, url: str, *args: "Any", **kwargs: "Any"
@@ -172,29 +185,54 @@ def getresponse(self: "HTTPConnection", *args: "Any", **kwargs: "Any") -> "Any":
172185

173186
try:
174187
rv = real_getresponse(self, *args, **kwargs)
188+
except BaseException:
189+
_finish_span(span)
190+
raise
191+
192+
if isinstance(span, StreamedSpan):
193+
status_code = int(rv.status)
194+
span.status = "error" if status_code >= 400 else "ok"
195+
span.set_attribute("http.response.status_code", status_code)
196+
else:
197+
span.set_http_status(int(rv.status))
198+
span.set_data("reason", rv.reason)
175199

176-
if isinstance(span, StreamedSpan):
177-
status_code = int(rv.status)
178-
span.status = "error" if status_code >= 400 else "ok"
179-
span.set_attribute("http.response.status_code", status_code)
180-
else:
181-
span.set_http_status(int(rv.status))
182-
span.set_data("reason", rv.reason)
200+
has_body = rv.chunked or (rv.length is not None and rv.length > 0)
201+
if has_body:
202+
rv._sentrysdk_span = span # type: ignore[attr-defined]
203+
else:
204+
_finish_span(span)
205+
206+
return rv
207+
208+
def read(self: "HTTPResponse", *args: "Any", **kwargs: "Any") -> "Any":
209+
span = getattr(self, "_sentrysdk_span", None)
210+
211+
if span is None:
212+
return real_read(self, *args, **kwargs)
213+
214+
try:
215+
rv = real_read(self, *args, **kwargs)
216+
return rv
183217
finally:
184-
if isinstance(span, StreamedSpan):
185-
with capture_internal_exceptions():
186-
add_http_request_source(span)
187-
span.end()
188-
else:
189-
span.finish()
218+
if self.fp is None or self.closed or not rv:
219+
self._sentrysdk_span = None # type: ignore[attr-defined]
220+
_finish_span(span)
190221

191-
with capture_internal_exceptions():
192-
add_http_request_source(span)
222+
def close(self: "HTTPResponse") -> None:
223+
span = getattr(self, "_sentrysdk_span", None)
193224

194-
return rv
225+
try:
226+
real_close(self)
227+
finally:
228+
if span is not None:
229+
self._sentrysdk_span = None # type: ignore[attr-defined]
230+
_finish_span(span)
195231

196232
HTTPConnection.putrequest = putrequest # type: ignore[method-assign]
197233
HTTPConnection.getresponse = getresponse # type: ignore[method-assign]
234+
HTTPResponse.read = read # type: ignore[method-assign]
235+
HTTPResponse.close = close # type: ignore[method-assign]
198236

199237

200238
def _init_argument(

tests/integrations/stdlib/test_httplib.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import os
22
import socket
33
import datetime
4+
import time
45
from http.client import HTTPConnection, HTTPSConnection
56
from http.server import BaseHTTPRequestHandler, HTTPServer
67
from socket import SocketIO
@@ -1161,3 +1162,86 @@ def test_proxy_http_tunnel(
11611162
assert span["data"][SPANDATA.HTTP_METHOD] == "GET"
11621163
assert span["data"][SPANDATA.NETWORK_PEER_ADDRESS] == "localhost"
11631164
assert span["data"][SPANDATA.NETWORK_PEER_PORT] == PROXY_PORT
1165+
1166+
1167+
CHUNK_DELAY = 0.1
1168+
NUM_CHUNKS = 3
1169+
1170+
1171+
class ChunkedResponseHandler(BaseHTTPRequestHandler):
1172+
def do_GET(self):
1173+
self.send_response(200)
1174+
self.send_header("Transfer-Encoding", "chunked")
1175+
self.end_headers()
1176+
for _ in range(NUM_CHUNKS):
1177+
chunk = b"x" * 100
1178+
self.wfile.write(f"{len(chunk):x}\r\n".encode() + chunk + b"\r\n")
1179+
self.wfile.flush()
1180+
time.sleep(CHUNK_DELAY)
1181+
self.wfile.write(b"0\r\n\r\n")
1182+
1183+
def log_message(self, *args):
1184+
pass
1185+
1186+
1187+
def create_chunked_server():
1188+
port = get_free_port()
1189+
server = HTTPServer(("localhost", port), ChunkedResponseHandler)
1190+
thread = Thread(target=server.serve_forever)
1191+
thread.daemon = True
1192+
thread.start()
1193+
return port
1194+
1195+
1196+
CHUNKED_PORT = create_chunked_server()
1197+
1198+
1199+
@pytest.mark.parametrize("span_streaming", [True, False])
1200+
def test_chunked_response_span_covers_body_read(
1201+
sentry_init,
1202+
capture_events,
1203+
capture_items,
1204+
span_streaming,
1205+
):
1206+
sentry_init(
1207+
traces_sample_rate=1.0,
1208+
_experiments={"trace_lifecycle": "stream" if span_streaming else "static"},
1209+
)
1210+
1211+
min_expected_duration = CHUNK_DELAY * NUM_CHUNKS
1212+
1213+
if span_streaming:
1214+
items = capture_items("span")
1215+
1216+
with sentry_sdk.traces.start_span(name="custom parent"):
1217+
conn = HTTPConnection("localhost", CHUNKED_PORT)
1218+
conn.request("GET", "/chunked")
1219+
response = conn.getresponse()
1220+
response.read()
1221+
1222+
sentry_sdk.flush()
1223+
spans = [item.payload for item in items if item.type == "span"]
1224+
(span,) = (
1225+
span
1226+
for span in spans
1227+
if span["attributes"].get("sentry.origin") == "auto.http.stdlib.httplib"
1228+
)
1229+
1230+
duration = span["end_timestamp"] - span["start_timestamp"]
1231+
assert duration >= min_expected_duration
1232+
else:
1233+
events = capture_events()
1234+
1235+
with start_transaction(name="test_chunked"):
1236+
conn = HTTPConnection("localhost", CHUNKED_PORT)
1237+
conn.request("GET", "/chunked")
1238+
response = conn.getresponse()
1239+
response.read()
1240+
1241+
(event,) = events
1242+
(span,) = event["spans"]
1243+
1244+
start = datetime.datetime.fromisoformat(span["start_timestamp"])
1245+
end = datetime.datetime.fromisoformat(span["timestamp"])
1246+
duration = (end - start).total_seconds()
1247+
assert duration >= min_expected_duration

0 commit comments

Comments
 (0)