My own test, and it broke the release build. CI reported
`assert 239800 == 0` on the very commit that was supposed to ship v1.11.0, so
no image was pushed and the tag and release were never cut.
The test drained a 50-row queue with `batch_size=100` and `flush_ms=10`, then
asserted the byte counter was back to zero. `_collect()` stops at whichever
comes first, `batch_size` rows or the flush deadline - and with a batch size
larger than the row count, the deadline is the only thing that can end it. It
was measuring the scheduler, not the sink.
The arithmetic is exact: a row here weighs 1400 + 4096 + 4096 = 9592 bytes, and
239 800 is 25 of them. `_collect()` returned half the queue because 25
iterations of `asyncio.wait_for` were enough to exhaust 10 ms on that runner.
The workflow builds `linux/amd64,linux/arm64`, so one of the two runs under qemu
emulation; a local `docker build` compiles the native platform only and never
sees that path. I could not reproduce the failure even building both platforms
here - this machine fits 49 iterations inside 10 ms - which is the point: a test
whose result depends on how fast the host is will pass everywhere it is
convenient and fail where it matters.
Fixed structurally rather than by widening the window: `batch_size` now EQUALS
the row count, so the collect loop exits on the count and never consults the
deadline at all. The flush window is generous as a backstop, the drain runs in
a loop instead of a single call, and the row count is asserted on the way in
and on the way out so a future change cannot make it vacuous.
Verified on both platforms the workflow builds: 1667 passed / 152 skipped on
linux/arm64 and on linux/amd64 under emulation.
No production code changes.
The auth_pass / stats-auth / userlist / URI patterns added by the redaction
fixes on this branch run on the writer task, on every string value of every
captured body, so their cost is paid per row forever. Measured, they were:
writer-side redaction, before the redaction fixes 12.2 us/row
writer-side redaction, with them 359.1 us/row
A 29x regression, and none of it was spent matching anything - almost every
body contains none of these keywords. Profiling the four patterns on an 8 KB
config body, 300 iterations:
one combined alternation, IGNORECASE, \b-anchored 308.2 us
the same four run separately (sum) 219.7 us
text.lower() once + four substring pre-checks 5.3 us
`\b` and IGNORECASE each defeat the regex engine's literal-prefix scan, so
every alphanumeric position in 8 KB became a candidate start and the engine
walked the whole body four times to find nothing. Isolated, `auth_pass` costs
27.5 us with IGNORECASE and 2.4 us without.
Split the alternation and gate each pattern behind a substring test on one
lowercased copy. `str.lower()` and `in` are C-level scans; a pattern now runs
only when its keyword is actually present, and then on text that genuinely
contains it. Cost becomes O(total string bytes) instead of O(bytes x patterns).
The URI pattern also drops IGNORECASE and `\b` - its character class already
covers both cases, and `://` gives the engine a literal to scan for.
writer-side redaction, after 18.8 us/row
= 3.4s of CPU/day
at 180 000 rows/day
6.6 us/row over the pre-fix baseline, for four secret classes that were
previously written to the table in cleartext.
The pre-checks are on the lowercased copy, so the patterns stay IGNORECASE: the
marker may well have been `AUTH_PASS` in the original. Request-path cost is
unchanged at 27.7 us p50 - none of this ever ran there.
All 123 redaction and payload tests still pass, so the behaviour is identical;
only the path to it is cheaper.
The branch shipped 245 tests and 72 of them covered redaction, all passing,
while six real endpoints of this application still wrote secrets to
`request_logs`. That is not a gap in effort, it is a gap in kind: those tests
pin the RULES - which key names match, which value shapes fire - and a rule test
proves the rule, not the coverage. Nothing was measuring what this system
actually sends.
51 tests in two files, every case built from a real handler's request or
response shape with the field names taken from the source and cited in the
docstring.
test_request_log_real_payloads.py drives payloads through `decode_body()`, the
same entry point the writer uses, rather than calling `redact()` on a dict. That
is load-bearing: a config upload is routinely larger than the capture cap, so it
never reaches redaction as a dict at all - it arrives as one truncated `_raw`
string where the line breaks are still the escape `\n`. A test that starts from
a dict reports a pass on a payload that leaks, and on one that gets masked into
uselessness. Both properties are asserted on both paths: the secret is gone AND
the rest of the config is still readable.
test_request_log_fleet_scale.py pins the four behavioural fixes, each of which
only appears at scale or at the edge of a setting's documented range:
* successful agent polls are dropped and failures never are, including a
transport error with no HTTP response at all;
* agent traffic is identified from headers, and `offer()` is asserted to
contain no `await` and no connection call, because it runs on the request
coroutine;
* `requestlog.read` scoping admits agent rows but NOT `user_id IS NULL`, so
anonymous traffic and the usernames in failed logins stay admin-only;
* background passes get one id each, and unwrapped background code does not
collapse onto one either;
* queue memory stays inside its budget with `max_body_bytes` at its 256 KB
ceiling, and the budget is released as rows drain - a budget that only
counts up is a leak, not a limit.
Also closes a hole in the branch's own auth tests: they asserted that every
endpoint calls `_require`, but not that it is called BEFORE the try block. The
repo's GHSA-3p5c pattern exists because a permission check inside `try` is
swallowed by the handler's `except Exception -> 500`, which turns a 403 into a
server error and hides that the check ran. Now asserted per endpoint.
Both halves of each trade are pinned: alongside every "this must be redacted"
there is an "and this must not be", so a later tightening cannot quietly blank
the fields the feature exists to show.
taylanbakircioglu