Refactor S3 Benchmarks

[davidh44]

Motivation and Context

The S3Express benchmarks (S3BenchmarkRunner) currently self-publish metrics directly to CloudWatch via the SDK's CloudWatchMetricPublisher attached to S3 clients. This tightly couples the runner to a specific CloudWatch namespace and account, and the publisher itself interferes with async benchmark measurements by adding CPU/thread contention.

This change decouples metric collection from publishing, outputting results as a JSON file for consumption by any CI/CD system.

Modifications

Rewrote S3BenchmarkRunner in test/s3-benchmarks to:

• Replace CloudWatchMetricPublisher with new InMemoryMetricPublisher — captures the same SDK-internal metrics (API_CALL_DURATION per operation, READ_THROUGHPUT for GetObject) but stores them in memory instead of publishing to CloudWatch
• Fail loudly on zero samples — avgDurationMs() and avgReadThroughput() throw IllegalStateException if no metrics were collected (prevents silent 0.0 results on failure)
• Propagate benchmark exceptions — removed catch (Exception e) that swallowed errors in run(); failures now propagate to JVM exit code
• Exclude warm-up from averages — metricPublisher.reset() called between warmUp() and doRunBenchmark()
• Output JMH-compatible JSON — uses Jackson ObjectMapper to write results to results.json (configurable via --output <file>)
• Change region to us-west-2 — runner executes in us-west-2 CodeBuild; legacy used us-east-1
• Remove dead code — BenchmarkSystemSetting, unused BenchmarkConfig fields, Benchmark.runWithTime/TimedResult, S3BenchmarkTestUtils dead helpers — all pre-existing from the original S3 Express launch commit, never used by this runner
• Add unit tests — InMemoryMetricPublisherTest (avg computation, zero-sample throws, reset, operation separation) and S3BenchmarkRunnerTest (verifies JSON output structure)

Testing

• Validated E2E on dev stack
• Added unit tests

[RanVaknin]

Can we use Jackson instead to do this? is there a reason we handwrite parsing logic otherwise?

[davidh44]

Good call, updated to use Jackson

[RanVaknin]

Can we add unit tests around InMemoryMetricPublisher and also the expected JSON output? It's currently hard to see what data actually flows through this

[davidh44]

Can we add unit tests around InMemoryMetricPublisher and also the expected JSON output? It's currently hard to see what data actually flows through this

Added InMemoryMetricPublisherTest and S3BenchmarkRunnerTest

[RanVaknin]

One more question. I see we added avgDurationMs to average out the per iteration samples. So isntead of rawData being something like [[8.1],[20.1],[13.5]...] it's just one subarray that hold the average of those ie: rawData = [[12.5]] this means we lose fidelity.

For the sake of the dashboarding that this feeds into, we might only care about the avg number, is this loss of fidelity is intentional?

[davidh44]

One more question. I see we added avgDurationMs to average out the per iteration samples. So isntead of rawData being something like [[8.1],[20.1],[13.5]...] it's just one subarray that hold the average of those ie: rawData = [[12.5]] this means we lose fidelity.

For the sake of the dashboarding that this feeds into, we might only care about the avg number, is this loss of fidelity is intentional?

Yes, intentional. The new benchmarks setup publishes one CloudWatch datapoint per metric per run, so we aggregate per-iteration samples into a single average. The per-iteration variance is network jitter noise; the average over 100-200 iterations is the meaningful signal for dashboarding and regression detection.