Benchmark fairness: zap’s hot path for dynamic fields

[AdmilsonCossa]

Hi zap maintainers and community!

I maintain HaloLog and its reproducible multi-logger benchmark suite. Before treating the comparison as settled, I want zap v1.27.0 represented by a configuration its maintainers consider technically fair.

The benchmark constructs zap as follows, quoted verbatim from the suite:

func newZap() *zap.Logger { return zap.New(zapcore.NewCore( zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig()), zapcore.AddSync(io.Discard), zap.InfoLevel, )) }

The manual core is deliberate. zap.NewProduction() enables 100/100 sampling by default, which would drop repeated benchmark records. It also enables work such as caller capture that is not included for the other loggers. The benchmark therefore uses the production JSON encoder without those additional behaviors.
Each comparative operation emits a complete structured JSON record containing a timestamp, level, message, and equivalent field values. The suite measures single-goroutine encoding and dispatch to io.Discard, not disk or network throughput.
For call-site fields, the timed operations use zap’s typed API:

l.Info(msg, zap.String("key", "value"))
l.Info(msg, zap.Int("k1", 1), zap.Int("k2", 2), // ... zap.Int("k10", 10), )

The request-scoped scenario binds five fields once with Logger.With(...), then adds one call-site field per record.
The twenty-field scenario constructs its []zap.Field slice before the timer starts. Caller and stack-trace capture are disabled, and the writer is intentionally not wrapped with zapcore.Lock. These choices are documented as favorable to zap.

My remaining question is narrow:
For fields whose values are available only at the call site, is Logger.Info(msg, zap.Int(...)) the idiomatic public hot path you would want represented? If not, is there another public pattern that preserves the same timestamp, level, message, and dynamic-field semantics with less per-call work?

I would add any proposed lower-level or specialized path as a separate result rather than silently replacing the public Logger.Info measurement. If an equivalent pattern exists, I will add it, rerun the suite, and publish both results.
Benchmark source:
https://github.com/Go-Gen-Ecosystem/halolog/blob/main/benchmarks/comparison_bench_test.go
Request-scoped benchmark:
https://github.com/Go-Gen-Ecosystem/halolog/blob/main/benchmarks/context_bench_test.go
Methodology and disclosed biases:
https://github.com/Go-Gen-Ecosystem/halolog/blob/main/benchmarks/comprehensive_comparison.md
Thank you.

[rabbbit]

I'm a maintainer, but this usage looks idiomatic enough - very basic, but idiomatic.

The only difference I found was that https://github.com/uber-go/zap/blob/master/benchmarks/zap_test.go#L109C26-L109C47 uses a different time encoder which per https://github.com/uber-go/zap/blob/v1.28.0/zapcore/encoder.go#L119 appears to be slightly faster. I doubt it will move the needle enough though.

Perhaps your benchmarks should contain asserts on the exact output, to verify no silly things like different time encoding is happening?

[AdmilsonCossa]

Thanks. That answers the question I came here with: the current public API path is a reasonable, idiomatic representation of Zap.
I tested the time encoder change rather than assuming the difference was negligible. On Go 1.27, Windows/amd64, with the currently committed Zap v1.27.0 and eight one-second runs, EpochNanosTimeEncoder improved the one-field case by 14.8% and the request-scoped case by 12.5%. The ten-field case improved by 2.3%; the twenty-field result was too noisy to call. Allocations were unchanged.
Good call on the output assertions. I added a contract test that uses the same constructors as the benchmarks and verifies that each operation emits exactly one valid JSON record with the expected timestamp, level, message, and typed field values. I also added fixed-clock, byte-exact tests for both Zap time encoders.
Those checks also made one detail explicit: the production default emits fractional Unix seconds, while the encoder used by Zap’s benchmark emits integer Unix nanoseconds. The records are semantically equivalent for this comparison, but the wire representation is different.
Since the wire format changes, I’m going to keep the current production-default result and add Zap’s benchmark configuration as its own named result. I’ll update the suite to Zap v1.28.0, rerun the Linux and Windows comparisons, and publish both.
Thanks for reviewing it. This improved the harness.