Restore per-transaction .csv.zst output via --full flag

[russfellows]

bench: restore per-transaction .csv.zst output via --full flag

Summary

Prior to commit 483f844, warp wrote a per-transaction CSV/TSV file after every
benchmark run, enabling fine-grained post-hoc analysis with warp analyze. That
commit introduced addCollector() but registered the --full flag only in
analyzeFlags — never in benchFlags — so the flag silently had no effect and
the per-transaction file was never written.

This PR restores the intended behavior with a minimal, targeted fix across three
files, adds unit tests, and updates documentation.

---

Problem

After 483f844, running any benchmark with --full:

warp put --host=... --full

produced only a .json.zst aggregate file. The .csv.zst per-transaction file was
never written, making post-hoc per-operation analysis impossible. The flag appeared
to be accepted but was completely ignored.

---

Changes

cli/benchmark.go — Core fix (single-node mode)

addCollector() now gates on ctx.Bool("full"):

• Without --full (default): A LiveCollector only. Live throughput display and
  auto-termination work as before. Output: .json.zst only.
• With --full: Ops are fanned to both a NewOpsCollector (capturing every
  individual request) and the LiveCollector (for live display). After the benchmark,
  .csv.zst is written first, then .json.zst is also written. --full is strictly
  additive — both files are always produced together.

A duplicate --full flag registration in benchFlags that caused a startup panic
(since --full already lives in analyzeFlags, which is combined with benchFlags
for all benchmark commands) has been removed.

cli/benchserver.go — Distributed mode mirror

• With --full: Downloads raw ops → writes .csv.zst → downloads aggregate →
  writes .json.zst.
• Without --full: Downloads aggregate → writes .json.zst only.

cli/analyze.go — User-facing warning

warp analyze file.csv.zst without --full re-aggregates individual operations into
1-second buckets before reporting, producing results equivalent to reading the
.json.zst aggregate. This loses per-operation resolution — latency percentiles and
throughput figures can differ, and per-request filtering flags have no additional
effect.

A warning is now printed in this case:

WARNING: Analyzing .csv.zst without --full produces aggregated results (1-second
         buckets), not accurate per-operation statistics.
         For precise latency percentiles and throughput, use: warp analyze --full <file>

The warning is suppressed when --quiet or --json output is active.

Note: Passing --full to warp analyze on a .json.zst file is silently
ignored — aggregate files contain no individual operations.

cli/addcollector_test.go — New unit tests (7 tests)

Test	Validates
TestAddCollectorDefaultNoOps	Default mode stores no individual ops
TestAddCollectorFullCollectsOps	--full captures all operations
TestAddCollectorFullFansOutToLive	--full also feeds the LiveCollector
TestAddCollectorDefaultUpdatesChannel	Default mode updates channel is functional
TestAddCollectorDiscardOutput	DiscardOutput suppresses collection in both modes
TestAddCollectorFullIsAdditive	--full always produces both files
TestAddCollectorOpValues	Op field values are preserved through the collector

README.md — Documentation

Added an Analyzing Results subsection under Full Per-Transaction Logging
documenting:

• Use warp analyze --full file.csv.zst for accurate per-operation stats
• Default analyze on .csv.zst re-aggregates (same as .json.zst) and now warns
• --full is silently ignored when analyzing .json.zst

---

Behavior Before / After

Scenario	Before this PR	After this PR
warp put (no flags)	.json.zst written	.json.zst written ✓
warp put --full	.json.zst written (bug)	.csv.zst and .json.zst written ✓
warp analyze file.csv.zst	Silent re-aggregation	Re-aggregation with warning ✓
warp analyze --full file.csv.zst	Full per-op analysis	Full per-op analysis ✓
warp analyze --full file.json.zst	Ignored silently	Ignored silently (noted in docs) ✓

---

Testing

Validated against a real MinIO S3 cluster. With --full:

$ ls -lh warp-put-*.{csv,json}.zst
-rw-r--r-- 1 user user 142K warp-put-2026-04-07[162451]-abcd.csv.zst
-rw-r--r-- 1 user user 1.2K warp-put-2026-04-07[162451]-abcd.json.zst

$ warp analyze warp-put-2026-04-07[162451]-abcd.csv.zst
WARNING: Analyzing .csv.zst without --full produces aggregated results...

$ warp analyze --full warp-put-2026-04-07[162451]-abcd.csv.zst
# → exact per-op percentiles, 4x1s throughput windows

All 7 unit tests pass: go test ./cli/ -run TestAddCollector -v

[klauspost]

Sure. It is overly dramatic, though - and lacks mentioning the main downside.

[russfellows]

Klaus, I didn’t want to presume to own the warning messages to your clients coming from your code. I am happy to craft the output messages, but being that you all own the product, I would think you would prefer to control the messages. I will run some analysis of the memory usage and time issues you cited. I don’t find them to be of much concern, but you seem to feel they are. So, in order to provide fact based messages, I will run some analysis. If you already have data points you would rather output, that is of course up to you. To be clear, this is just fixing a documented feature that was recently broken. A better solution all the way around would be to utilize histograms, such as HDR histograms to collect the metrics. That way you can combine the results from multiple clients accurately and maintain statistical accuracy of all data points such as mean, median, p90, p95, p99 values. The current 5 second averaging method you use by default loses statistical validity. In contrast, the detailed zst trace log files you can collect, do provide statistically valid results, as you well know, but at the expense of processing time at the end. Regards, —Russ

…

On Apr 16, 2026, at 8:47 AM, Klaus Post ***@***.***> wrote: @klauspost commented on this pull request. In README.md <#475 (comment)>: > +using the `--benchdata` parameter. + +## Full Per-Transaction Logging (`--full`) + +Adding `--full` to any benchmark command enables full per-transaction logging. With this +flag warp writes **both** output files: + +| File | Contents | +|------|----------| +| `<benchdata>.csv.zst` | Every individual request — one row per operation, [zstandard](https://facebook.github.io/zstd/)-compressed CSV/TSV | +| `<benchdata>.json.zst` | Aggregated summary (same as default) | + +```bash +warp put --host=... --full +``` + Then the issue just gets closed as not planned. If you want to take ownership of the fix, I am offering code review. That is it. — Reply to this email directly, view it on GitHub <#475 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AF64UJ2D5KZTJFKY2GCYJUD4WDW6PAVCNFSM6AAAAACXQBCJPGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCMRRHA2DONJQHA>. You are receiving this because you authored the thread.

[russfellows]

Thanks for the feedback. Pushed an updated commit that:

• Softens the analyze.go warning from WARNING: to Note: and shortens it to two lines (also fixes the trailing \n flagged by govet)
• Fixes behaviour → behavior in the test file (misspell) and removes the redundant b.Common.Collector selector (staticcheck QF1008)
• Replaces the vague memory note in the README with actual measurements from a local MinIO server — the overhead is negligible for short runs (+9 MB at 30s/8c) but grows to +124 MB at 60s/32c and projects to ~8 GB for a 1-hour high-concurrency run, so the caution is warranted at scale

[klauspost]

lgtm

[Copilot]

Pull request overview

Restores the intended --full behavior for benchmarks so per-transaction .csv.zst output is produced again (alongside the aggregate .json.zst), and adds a user-facing warning when analyzing .csv.zst without --full.

Changes:

• Reworks collector wiring so --full collects per-op data while preserving live aggregation for UI/autoterm.
• Mirrors --full behavior for remote benchmarks by downloading ops for .csv.zst and still writing .json.zst.
• Adds an analyze-time warning for .csv.zst without --full, plus new unit tests and README updates.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cli/benchmark.go	Wires --full to collect ops + keep live updates; writes .json.zst alongside .csv.zst in full mode.
cli/benchserver.go	In remote mode, downloads ops for .csv.zst when --full and also writes the aggregate .json.zst.
cli/analyze.go	Emits a warning when analyzing .csv.zst without --full (unless quiet/JSON).
cli/addcollector_test.go	Adds unit tests covering default/full/discard-output collector behavior.
README.md	Documents --full outputs and analysis behavior differences for .csv.zst vs .json.zst.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.