Remove code 0 latency metrics from latency average calculations#71
Open
meecethereese wants to merge 27 commits into
Open
Remove code 0 latency metrics from latency average calculations#71meecethereese wants to merge 27 commits into
meecethereese wants to merge 27 commits into
Conversation
…_rate Extract per-second bucketing logic into modules/vegeta/aggregate.awk shared between the live runner and merge.sh. Latency percentiles now ignore status_code=0 records (vegeta's marker for transport-level failures: connection refused, EOF, timeout) so percentiles reflect service response time rather than failure-detection time. Adds successful_count and success_rate fields so consumers can tell whether p99 was computed over many samples or a handful of survivors. rps, code.hist, and byte sums still count every record, so sum(code.hist[*]) == rps remains an invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
modules/vegeta/run/run.sh now pipes vegeta encode --to csv through
gawk -f aggregate.awk, the same aggregator merge.sh uses. Live and
merged JSON outputs now share one schema, and the live path inherits
the code-0 latency filter plus the new successful_count and
success_rate fields.
Code histogram keys are now exact status codes (e.g. "0", "200",
"503") rather than jaggr's binned families ("100"..."500").
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New synthetic-data test in modules/vegeta/test/test.sh exercises three
buckets: mixed (5 successes + 5 code-0 timeouts), all successes, all
failures. Asserts that p99 in the mixed bucket excludes the 30s code-0
timeouts, that successful_count and success_rate match expected values,
that the all-failure bucket emits latency:{0,0,0} without crashing, and
that the live and merge paths produce byte-identical output for in-order
input.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Data flow for results section now describes the gawk-based aggregator pipeline (replacing the previous jaggr step), the unified live/merge schema, and the latency-filter behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Other vegeta artifacts live in subdirectories (run/, merge/, output/, test/, install/); aggregate.awk now lives under lib/ rather than directly at modules/vegeta/. Updates path references in run.sh, merge.sh, test.sh, and CLAUDE.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The live path streams records in attack-completion order and may emit a record whose timestamp is slightly earlier than the current bucket when latency variance is high. Previously the aggregator treated any mismatch as a forward boundary and flushed, which produced two output lines for the same wall-second under saturation. Now flushing only happens when this_second > current_second; late records (this_second < current_second) are absorbed into the current bucket. Slight count drift across buckets is preferable to a single wall-second appearing on two lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The vegeta aggregator now keys code.hist on exact status codes rather than jaggr's binned 100/200/300/400/500 families, so the previous jplot args silently lost visibility into transport failures (code 0) and any non-200 successes. Plot 0/200/502/503 — the codes actually seen in this repo's tests — and add successful_count next to rps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The live runner now uses gawk -f lib/aggregate.awk instead of jaggr, so jaggr is no longer needed by anything in this repo. Remove the jaggr install steps from install.sh and the jaggr verification / version checks from test.sh. Also clean up two stale comments that still referred to "jaggr output format". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new aggregator buckets by embedded request timestamps (vs. jaggr's wall-clock arrival), so the first and last second of an attack are usually partial — vegeta rarely starts and stops exactly on a second boundary. Test 13 previously asserted every line's RPS fell within 38..63 for a --rate=50 attack, which can flake when an edge bucket legitimately has a low partial count. Skip the lower-bound check on the first and last lines, mirroring the pattern in test 14. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace string-equality assertions like [[ "$B0_RATE" == "0.5000" ]] with jq -e numeric comparisons (jq -e '.success_rate == 0.5'). The prior form was fragile to printf format changes in aggregate.awk — flipping the format from %.4f to %.3f or %g would silently break the tests without changing semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After encoding the synthetic CSV to gob, decode back to CSV and assert the row count matches. Catches silent encoding loss before downstream assertions, which would otherwise just see fewer records and produce confusing failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Data flow for results section now uses fenced code blocks for the streaming command and the per-second JSON shape, instead of inlining both as long backtick spans. Easier to scan and copy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
code.hist preserves the exact status codes seen this second (e.g.
"200":5,"503":2). code.family aggregates those into hundreds families
("0", "100", "200", "300", "400", "500") and always emits all six
keys, even with zero counts — downstream plotters (jplot etc.) need a
stable schema so series don't disappear when a controller's exact
codes vary across runs.
sum(code.hist[*]) == sum(code.family[*]) == rps remains an invariant.
Test #17 schema invariants now also assert code.family is present,
all six keys are emitted, and per-bucket counts match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The aggregator now emits two count/rate pairs that describe distinct
phases of the request lifecycle:
responded_count / response_rate
Out of the rps requests sent this second, how many came back as a
non-0 status (server actually replied). Latency percentiles are
computed over these records — any non-0 status is a real
measurement of server response time. Code-0 transport failures
(connection refused, EOF, timeout) are excluded because their
"latency" is failure-detection time. response_rate = responded / rps.
successful_count / success_rate
Out of the responded_count records, how many were 2xx (server
accepted and fulfilled the request). 1xx interim, 3xx redirects,
4xx client errors, and 5xx server errors are NOT counted as
successful. success_rate = successful / responded, or 0 when
responded is 0.
This separates "did we hear back?" from "was the request fulfilled?",
which the previous single success_rate conflated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Test #17 now asserts responded_count, response_rate, successful_count, and success_rate per bucket under the new semantics: - bucket 0 (5 × 200 + 5 × code-0): resp=5, rate=0.5; succ=5, rate=1.0 - bucket 1 (10 × 200): resp=10, rate=1; succ=10, rate=1 - bucket 2 (10 × code-0): resp=0, rate=0; succ=0, rate=0 Schema invariants gain successful_count ≤ responded_count ≤ rps and sum(code.family) == rps. New test #18 sends two distinct codes per non-zero hundreds family (101+102 → family.100=2, 200+204 → family.200=2, 301+302 → family.300=2, 400+404 → family.400=2, 500+503 → family.500=2) and asserts code.hist preserves all 10 distinct codes while code.family aggregates to 2 each. Strict-2xx successful_count=2 (only 200 and 204). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…E.md Reflect the response/success split and the dual code.hist (exact) / code.family (hundreds-binned) histogram in the Data flow section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
code.family always emits all six hundreds keys (0/100/200/300/400/500), so jplot has a stable schema regardless of which exact codes a controller returns this run. Adds two new panels: - rps + responded_count + successful_count (counts at each layer) - response_rate + success_rate (the two rates from aggregate.awk) The exact-code histogram is still in code.hist for log inspection, but plotting against family is reliable across controllers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drift between modules/jplot/run/run.sh's hardcoded jplot args and the
JSON schema emitted by modules/vegeta/lib/aggregate.awk silently
breaks the live dashboard. The new test step:
1. Builds a sample CSV with one record per hundreds family (101, 200,
301, 404, 503, 0) so every code.family bin is populated.
2. Runs aggregate.awk on it and asserts code.family == {"0":1,
"100":1,"200":1,"300":1,"400":1,"500":1} — catches any future
binning regression.
3. Sources jplot/run/run.sh with `jplot` overridden to print its
arguments, then resolves each captured field path against the
sample JSON via jq. Numeric path segments (e.g. code.family.0)
are translated to bracket form (.code.family["0"]).
Any path that fails to resolve fails CI before a broken dashboard
ships.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PROJECT_ROOT was computed as MODULE_DIR/.. which resolves to <repo>/modules, not the repo root. The aggregator path then double-resolved as <repo>/modules/modules/vegeta/lib/aggregate.awk, causing CI to fail with "vegeta aggregator not found at …/modules/modules/vegeta/…". Use MODULE_DIR/../.. to escape both the module subdir and the modules/ container, matching the pattern in modules/vegeta/test/test.sh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
trap … RETURN only fires on function return, but the test is top-level shell code, so the temp .csv and .json files were never being cleaned up. Switch to trap … EXIT so the rm runs whenever the test exits (success, failure, or signal). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous capture relied on a literal string match against "Plotting vegeta results..." to skip the run script's echo line. Any future change to that echo (extra prefix, removal, additional log lines) would silently leak non-arg text into the panel parser. Replace the stub with one that prefixes every captured arg with __JPLOT_ARG__\t, then sed out only those lines into JPLOT_ARGS. The parser now sees only the actual jplot panels regardless of what else the run script writes to stdout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `else out = "" out` branch existed for hypothetical paths that start with a numeric segment, but none of the jplot panel paths look like that (rps, code.family.X, latency.p99, bytes_in.sum). The else clause was also a no-op — concatenating an empty string. Simplify to unconditional `print "." out` and note the assumption inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
meecethereese
marked this pull request as draft
June 11, 2026 22:26
When responded_count is 0 (an entire second was transport failures), success_rate is emitted as 0 to keep the JSON shape stable — but that "0" is "0/0", not "0% of responses succeeded". A consumer reading success_rate alone could draw the wrong conclusion. Add an explicit note in both CLAUDE.md and the aggregate.awk header telling consumers to check responded_count > 0 before treating success_rate as meaningful. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When no responses came back this second, the ratio successful_count / responded_count is undefined (0/0). Previously the aggregator emitted 0, which a consumer reading success_rate alone could mistake for "0% of responses succeeded". Emit JSON null instead so the undefined case is explicit. - aggregate.awk: format the field as "null" when responded_count is 0 - test #17 bucket 2: assert .success_rate == null - schema invariants: success_rate is null iff responded_count == 0, otherwise a [0,1] float - CLAUDE.md: document the null sentinel and how consumers should interpret it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Test #17 already diffs the live (gawk on unsorted CSV) and merge (sort | gawk) outputs. Test #18 only ran the merge path. The two share aggregate.awk so equivalence is structurally guaranteed today, but the explicit diff catches any future divergence — e.g. if a preprocessing step is added to merge.sh but not to the live runner. Also expand the cleanup glob to /tmp/vegeta-test-allfam*.json so the new live tmp file is removed alongside the merge output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
set -ex echoes every command to stderr, which makes scenario logs in master.sh extremely noisy and buries the actual vegeta progress output. Keep -e (exit on error) and -o pipefail (catch failures inside pipelines), drop the trace. Matches modules/vegeta/merge/merge.sh which already uses -eo pipefail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cleanup() trap had grown to six space-separated patterns (*.bin, *.csv, merge-*.json, synthetic*, latfilt-*.json, allfam*.json), and every new test had to remember to extend the list — a maintenance trap that already let the latfilt-live.json file leak briefly. Every test artifact already uses the /tmp/vegeta-test- prefix, so a single rm -f /tmp/vegeta-test-* covers all current and future files without further bookkeeping. Verified no test references a /tmp path outside that prefix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
meecethereese
marked this pull request as ready for review
June 15, 2026 15:51
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.
This PR updates our latency metrics to only report latency for requests that received a response from the server. It accomplishes this by reusing the logic from the merge script and filtering out code 0 latency data. We also add success count and success rate metrics too.