Measure request latency to the last byte (read the full response body) - #227
Merged
Merged
Conversation
zoosky
force-pushed
the
force-read-response-body
branch
from
May 31, 2026 16:05
64db7ed to
ad49543
Compare
Owner
|
Awesome! Can you solve the merge conflict? |
drill stopped its timer as soon as the response headers arrived (`client.execute().await`), but reqwest streams the body lazily, so body-transfer time was never measured. The body was only read when a request used `assign`, and even then after the timer had already stopped. Endpoints serving non-trivial bodies (files, large JSON) were reported as completing far faster than they really did. Read the full response body before stopping the timer, so latency now measures time-to-last-byte, matching wrk, k6 and vegeta. Status, headers and cookies are snapshotted into an owned struct before the body stream is consumed. The body is drained one chunk at a time and only buffered when a request uses `assign` (then decoded with the response Content-Type charset, via encoding_rs, as reqwest's `text()` did); otherwise each chunk is dropped as it arrives, so peak memory stays O(chunk) rather than O(body) for large responses. Timing and keep-alive are unchanged -- the body is still read to the last byte. Adds regression tests: a server that delays its body (latency must include the delay) and a 1 MiB non-assign body (drained but not retained). Fixes fcsonline#74.
zoosky
force-pushed
the
force-read-response-body
branch
from
June 1, 2026 08:08
ad49543 to
ad91f14
Compare
Contributor
Author
|
Done. Please verify. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
drill stops its timer as soon as the response headers arrive (
client.execute().await), but reqwest streams the body lazily. The body is only read when a request usesassign— and even then after the timer has already stopped. So any endpoint serving a non-trivial body (files, large JSON) is reported as completing far faster than it really does. This is the accuracy issue reported in #74, and the same root cause as #182.Fix
Read the entire response body before stopping the timer, so latency now measures time-to-last-byte — matching
wrk,k6, andvegeta. Status, headers and cookies are snapshotted into an ownedResponseDatabefore the body stream is consumed, so theassignpath and verbose logging keep working unchanged. Theassignbody is decoded with the responseContent-Typecharset viaencoding_rs(exactly what reqwest'stext()did internally), so non-UTF-8 bodies aren't corrupted by reading the body ourselves.encoding_rsis already a transitive dependency through reqwest, so this adds no new crates to the tree.Test
Adds
measures_full_body_transfer_time: a bare HTTP server sends the response head immediately, then delays 300 ms before the body; the measured duration must be ≥ 250 ms. This fails against the old header-only timing and passes now. Also adds unit tests for the charset parsing/decoding.cargo fmt --check,cargo clippy --all-targets -- -D warnings, andcargo testare all green.Note on approach
#74 asked for an option to force-read the body. I've made accurate measurement the default instead (no flag), since a load tester reporting time-to-headers is misleading and every mainstream tool measures to last byte. The trade-off is that reported latencies for body-heavy endpoints will increase — they become correct. One follow-on behavior worth flagging: a body that doesn't finish transferring within
--timeoutis now reported as a connection error (status520) rather than its HTTP status, because the body read is part of the timed request.If you'd prefer to preserve existing numbers by default and gate this behind a
--read-body/--force-bodyflag, I'm happy to adjust — your call as maintainer.Fixes #74.