Skip to content

fix(stdlib): Instrument response body read for chunked HTTP responses#6202

Merged
sentrivana merged 16 commits intomasterfrom
ivana/requests-missing-instrumentation
May 7, 2026
Merged

fix(stdlib): Instrument response body read for chunked HTTP responses#6202
sentrivana merged 16 commits intomasterfrom
ivana/requests-missing-instrumentation

Conversation

@sentrivana
Copy link
Copy Markdown
Contributor

@sentrivana sentrivana commented May 5, 2026

The HTTP client span in the stdlib integration was finishing in getresponse(), which only waits for response headers, not the actual response. For chunked or large responses, the actual data transfer happens during read(), leaving that time uninstrumented.

Defer span completion to HTTPResponse.read() for responses with a body (chunked or Content-Length > 0), with HTTPResponse.close() as a safety net for responses that are never read.

⚠️ Note that this means we might report some requests to be longer than they actually were, since in some cases we only close them once the GC gets to them (and close() is called). In a sense we're essentially flipping the current situation (where we report requests to be much shorter than they are, since they don't include the response part) -- but the current situation was incorrect for all spans, while this close() fallback should hopefully only kick in for edge cases.

Responses with no body (Content-Length: 0, HEAD, 204, 304) still finish the span immediately in getresponse().

sentrivana and others added 2 commits May 5, 2026 13:30
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>
Comment thread sentry_sdk/integrations/stdlib.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Codecov Results 📊

13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 8.21s

All tests are passing successfully.

❌ Patch coverage is 19.51%. Project has 15021 uncovered lines.

Files with missing lines (1)
File Patch % Lines
stdlib.py 50.26% ⚠️ 97 Missing and 15 partials

Generated by Codecov Action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Codecov Results 📊

6 passed | Total: 6 | Pass Rate: 100% | Execution Time: 1.01s

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

❌ Patch coverage is 18.18%. Project has 17057 uncovered lines.
❌ Project coverage is 21.31%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
stdlib.py 28.79% ⚠️ 141 Missing and 2 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    22.11%    21.31%     -0.8%
==========================================
  Files          190       190         —
  Lines        21789     21675      -114
  Branches      7302      7619      +317
==========================================
+ Hits          4817      4618      -199
- Misses       16972     17057       +85
- Partials       386       385        -1

Generated by Codecov Action

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 5, 2026

Comment thread sentry_sdk/integrations/stdlib.py Outdated
Comment thread sentry_sdk/integrations/stdlib.py
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread tests/integrations/stdlib/test_httplib.py Outdated
sentrivana and others added 3 commits May 5, 2026 14:04
Drop the `not rv` check from the read() wrapper's span-finishing
condition. read(0) legitimately returns b"" without consuming the body,
which would falsely trigger span completion. The fp and closed checks
are sufficient to detect when the body is fully consumed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sentrivana sentrivana marked this pull request as ready for review May 5, 2026 12:22
@sentrivana sentrivana requested a review from a team as a code owner May 5, 2026 12:22
Comment thread sentry_sdk/integrations/stdlib.py
@sentrivana sentrivana marked this pull request as draft May 5, 2026 12:30
@sentrivana sentrivana marked this pull request as ready for review May 5, 2026 12:32
Comment thread sentry_sdk/integrations/stdlib.py
Comment thread sentry_sdk/integrations/stdlib.py Outdated
Comment thread sentry_sdk/integrations/stdlib.py Outdated
Comment thread tests/integrations/stdlib/test_httplib.py Outdated
Comment thread sentry_sdk/integrations/stdlib.py
Comment thread sentry_sdk/integrations/stdlib.py
Copy link
Copy Markdown
Contributor

@alexander-alderman-webb alexander-alderman-webb left a comment

Choose a reason for hiding this comment

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

Looks good overall; blocking on the span management comment.

Comment thread sentry_sdk/integrations/stdlib.py Outdated
Comment thread sentry_sdk/integrations/stdlib.py
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread sentry_sdk/integrations/stdlib.py
Co-authored-by: Alex Alderman Webb <alexander.webb@sentry.io>
Comment thread sentry_sdk/integrations/stdlib.py Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5e768d8. Configure here.

Comment thread sentry_sdk/integrations/stdlib.py
Comment thread sentry_sdk/integrations/stdlib.py
@sentrivana sentrivana merged commit 3971a12 into master May 7, 2026
156 checks passed
@sentrivana sentrivana deleted the ivana/requests-missing-instrumentation branch May 7, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentry does not fully instrument requests library requests

3 participants