Improve recording accuracy#20
Merged
Merged
Conversation
- keep_successful_responses=false now still records the status, outcome and duration of successful responses - only the body and headers are omitted, so rows no longer read as failures - Measure the max_response_size limit in bytes rather than characters, so multibyte bodies cannot exceed the configured limit on disk - Subtract raw float timestamps before rounding durations, removing the +-1ms truncation error; still stored as whole milliseconds
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
1 participant
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.
Overview of change
Last of the three follow-ups from the review pass behind #17 (feature / schema / accuracy fixes).
keep_successful_responses=falseno longer makes successful requests look failedPreviously the response recording was skipped entirely, leaving rows at the request-time default of
successful = falsewith no status — so filtering for failures returned every successful request too. Now the status, outcome, and duration are always recorded; only the body and headers are omitted for successful responses. Failed responses are still kept in full. Config comment and README updated to describe the corrected behaviour.Byte-accurate
max_response_sizeThe size check used
mb_strlen(characters), so multibyte bodies could exceed the configured kilobyte limit by 2–4× on disk (a 100k-character CJK/emoji body is 300–400KB). The limit is about storage, and storage is measured in bytes, so the check now usesstrlen. No string slicing is involved, so there is no multibyte-corruption risk.Correct duration rounding — still whole milliseconds
The old code truncated each timestamp to whole ms before subtracting, introducing up to ±1ms of error — a sub-millisecond request could record as 0ms. Timestamps are now subtracted as raw floats and rounded once at the end. Durations remain clean whole-millisecond integers; no schema, signature, or display changes.
Testing
Four new tests: successful-outcome recording and failed-response fullness under
keep_successful_responses=false, byte-vs-character sizing with a multibyte body, and duration rounding across truncation-error cases. Full suite: 47 passed (185 assertions), PHPStan level 8 clean, Pint clean.