-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Reusing sampler counters across multiple sinks #1498
Description
Is your feature request related to a problem? Please describe.
The OpenTelemetry (OTel) Collector uses Zap extensively for internal logging. These internal logs may be directed to the custom Zap core defined in the otelzap library, which allows exporting Zap logs through the OTel Logger API. We additionally have log sampling enabled by default, using a sampler core created by zapcore.NewSamplerWithOptions, which is wrapped around the otelzap core.
In order to set certain parameters associated with the OTel Logger (instrumentation scope attributes, specifically), we recreate the bridge core with those new parameters, then call zapcore.NewSamplerWithOptions to wrap it (code link 1, code link 2). This has some issues:
- This creates a new pipeline with its own independent sampling counters.
- The counters allocated by each call take up about half a megabyte of memory, which can quickly accumulate.
Describe the solution you'd like
For these reasons, we would like to be able to create a new sampler core with a different sink, while reusing an existing sampling counter allocation. This would presumably be done using a new method on the sampler struct.
The existing With method on the sampler core does something similar, as it allows changing the fields associated with the core while reusing the same counters. It does not allow changing the underlying sink, however.
Describe alternatives you've considered
Alternative solutions to our immediate problem would include modifying the otelzap bridge to allow setting these custom parameters by encoding them in regular Zap fields. This could cause accidental name collisions, and would be inefficient since the parameters should be static once an OTel Logger is created.
Moreover, this sort of solution may not necessarily work for other users of Zap who might want to have multiple pipelines behind a sampler core.
Is this a breaking change?
I believe this would not constitute a breaking change.