Skip to content

Count dropped execs instead of losing them silently - #4

Merged
necco-c merged 1 commit into
mainfrom
fix-drop-accounting
Sep 3, 2026
Merged

Count dropped execs instead of losing them silently#4
necco-c merged 1 commit into
mainfrom
fix-drop-accounting

Conversation

@necco-c

@necco-c necco-c commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Follows #3. Those two seeding fixes made a third bug visible: with the scope finally covering pre-existing subtrees, a fork storm produces far more events, and most were disappearing with nothing on screen to say so.

96,000 execs fired across 24 workers. ~36,000 shown. ~60,000 gone.

The loss is a limit; the silence was the bug

Ring-buffer delivery is bound by bytes moved rather than events, and a record carries a 1 KiB argument window, so throughput caps around 2,700 execs/s. I measured that with an empty callback, so it is the runtime's ceiling and not this script's per-event work (normalize is ~4ms per 40k records, model.add ~12ms; neither is close to the bottleneck). A make -j24 can outrun it. A real npm install, at tens per second, cannot come near it.

What was wrong is that bpf_ringbuf_output's return value was discarded, so every count read as a total when it was a floor. A stats_map ARRAY now counts emitted and dropped, JS polls it, and a non-zero count appears on the verdict line:

  36,595 execs in 24s · mostly text plumbing (100%) · nothing out of place  ·  59,431 dropped

The headless report says it outright: the ring buffer filled faster than it drained, so every count below is a floor.

Tried and rejected

Recorded so nobody spends the afternoon I did:

  • ARGV_BUF 256 raises the ceiling to ~10,200/s (3.8×) but truncates 12% of real records. Measured against the checked-in captures: p50 argv is 48 bytes, p99 is 1026. Wrong trade for a tool whose job is showing you what ran.
  • Variable-length emit (header + args_len, mean 143 bytes vs a fixed 1076) delivers nothing at all. The ring is bound with btf_struct: "exec_event", so the consumer decodes fixed-size records and a short write is never seen. Comment left at the emit site.

test/drops.sh

Asserts the accounting closes rather than that the number looks plausible: captured + dropped ≥ what actually ran, drops must be non-zero at storm scale (else the drop path silently goes untested), and the report must explain what the number means.

captured=36490  dropped=59536  sum=96026  expected>=96000
PASS — every exec is either captured or counted as dropped

Also

Removes setCgroup and its unread target_cgid filter, the other loose end from #3. Nothing ever called it, so container mode has always been a plain pid subtree. Now that the existing tree is seeded from the process graph, the tgid set is the scope, and a second overlapping filter would only be a way for the two to disagree.

heuristics.test.mjs and capture.sh both still pass. Not merging.

Jacob's two seeding fixes made a third bug visible. With the scope finally
covering pre-existing subtrees, a fork storm produces far more events than
before, and most of them were disappearing with nothing on screen to say so:
96,000 execs fired across 24 workers, ~36,000 shown, ~60,000 gone.

The loss itself is a real limit rather than a defect. Ring-buffer delivery is
bound by bytes moved, not events, and a record carries a 1 KiB argument
window, so throughput caps around 2,700 execs/s. Measured with an empty
callback, so it is the runtime's ceiling and not this script's per-event work
(normalize is ~4ms per 40k records, model.add ~12ms; neither is the
bottleneck). A `make -j24` can outrun it. A normal npm install, at tens per
second, cannot come close.

What was wrong is that it was silent, so every count read as a total when it
was a floor. bpf_ringbuf_output's return value was discarded. A stats_map
ARRAY now counts emitted and dropped, JS polls it, and a non-zero drop count
appears on the verdict line and in the headless report, which also says
outright that the numbers below it are a floor.

test/drops.sh asserts the accounting closes: captured + dropped must be at
least what actually ran, drops must be non-zero at storm scale (otherwise the
drop path silently goes untested), and the report must explain what the number
means. Verified: 36,490 + 59,536 = 96,026 against 96,000 fired.

Two things tried and rejected, recorded so they are not retried:

  - Shrinking ARGV_BUF to 256 raises the ceiling to ~10,200/s, a 3.8x gain,
    but truncates 12% of real records (measured against the checked-in
    captures: p50 argv is 48 bytes, p99 is 1026). Wrong trade for a tool whose
    job is showing you what ran.
  - Emitting a variable-length record (header + args_len, mean 143 bytes
    instead of a fixed 1076) delivers nothing at all. The ring is bound with
    `btf_struct: "exec_event"`, so the consumer decodes fixed-size records and
    a short write is not seen. Left in a comment at the emit site.

Also removes setCgroup and its unread target_cgid filter. Nothing ever called
it, so container mode has always been a plain pid subtree; now that the
existing tree is seeded from the process graph, the tgid set IS the scope and
a second overlapping filter would only be a way for the two to disagree.
@necco-c
necco-c merged commit 81ddcaa into main Sep 3, 2026
5 checks passed
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.

1 participant