diff --git a/config.go b/config.go index 3223f8f15..d0ee43e5b 100644 --- a/config.go +++ b/config.go @@ -40,7 +40,7 @@ import ( type SamplingConfig struct { Initial int `json:"initial" yaml:"initial"` Thereafter int `json:"thereafter" yaml:"thereafter"` - Hook func(zapcore.Entry, zapcore.SamplingDecision) error + Hook func(zapcore.Entry, zapcore.SamplingDecision) } // Config offers a declarative way to construct a logger. It doesn't do @@ -212,19 +212,19 @@ func (cfg Config) buildOptions(errSink zapcore.WriteSyncer) []Option { if !cfg.DisableStacktrace { opts = append(opts, AddStacktrace(stackLevel)) } - if cfg.Sampling != nil && cfg.Sampling.Hook == nil { - // Assign a default nop sampling hook. - cfg.Sampling.Hook = zapcore.NopSamplingHook - } - if cfg.Sampling != nil { + if scfg := cfg.Sampling; scfg != nil { opts = append(opts, WrapCore(func(core zapcore.Core) zapcore.Core { + var samplerOpts []zapcore.SamplerOption + if scfg.Hook != nil { + samplerOpts = append(samplerOpts, zapcore.SamplerHook(scfg.Hook)) + } return zapcore.NewSamplerWithOptions( core, time.Second, cfg.Sampling.Initial, cfg.Sampling.Thereafter, - zapcore.SamplerHook(cfg.Sampling.Hook), + samplerOpts..., ) })) } diff --git a/config_test.go b/config_test.go index bb67470c9..fa15ffdd7 100644 --- a/config_test.go +++ b/config_test.go @@ -146,17 +146,22 @@ func TestConfigWithMissingAttributes(t *testing.T) { } } -func makeSamplerCountingHook() (func(zapcore.Entry, zapcore.SamplingDecision) error, *atomic.Int64) { - count := &atomic.Int64{} - h := func(zapcore.Entry, zapcore.SamplingDecision) error { - count.Inc() - return nil +func makeSamplerCountingHook() (func(zapcore.Entry, zapcore.SamplingDecision), + *atomic.Int64, *atomic.Int64) { + droppedCount := &atomic.Int64{} + sampledCount := &atomic.Int64{} + h := func(_ zapcore.Entry, dec zapcore.SamplingDecision) { + if dec == zapcore.LogDropped { + droppedCount.Inc() + } else if dec == zapcore.LogSampled { + sampledCount.Inc() + } } - return h, count + return h, droppedCount, sampledCount } func TestConfigWithSamplingHook(t *testing.T) { - shook, scount := makeSamplerCountingHook() + shook, dcount, scount := makeSamplerCountingHook() cfg := Config{ Level: NewAtomicLevelAt(InfoLevel), Development: false, @@ -173,7 +178,8 @@ func TestConfigWithSamplingHook(t *testing.T) { expectN := 2 + 100 + 1 // 2 from initial logs, 100 initial sampled logs, 1 from off-by-one in sampler expectRe := `{"level":"info","caller":"zap/config_test.go:\d+","msg":"info","k":"v","z":"zz"}` + "\n" + `{"level":"warn","caller":"zap/config_test.go:\d+","msg":"warn","k":"v","z":"zz"}` + "\n" - expectDropped := 99 // 200 - 100 initial - 1 thereafter + expectDropped := 99 // 200 - 100 initial - 1 thereafter + expectSampled := 103 // 2 from initial + 100 + 1 thereafter temp, err := ioutil.TempFile("", "zap-prod-config-test") require.NoError(t, err, "Failed to create temp file.") @@ -205,5 +211,6 @@ func TestConfigWithSamplingHook(t *testing.T) { logger.Info("sampling") } assert.Equal(t, int64(expectN), count.Load(), "Hook called an unexpected number of times.") - assert.Equal(t, int64(expectDropped), scount.Load()) + assert.Equal(t, int64(expectDropped), dcount.Load()) + assert.Equal(t, int64(expectSampled), scount.Load()) } diff --git a/zapcore/sampler.go b/zapcore/sampler.go index f1c60d0ac..c9ee8d971 100644 --- a/zapcore/sampler.go +++ b/zapcore/sampler.go @@ -85,8 +85,10 @@ func (c *counter) IncCheckReset(t time.Time, tick time.Duration) uint64 { type SamplingDecision uint8 const ( - // Dropped means that a log was dropped. - Dropped SamplingDecision = iota + // LogDropped means that a log was dropped. + LogDropped SamplingDecision = iota + // LogSampled means that a log was successfully sampled. + LogSampled ) // optionFunc wraps a func so it satisfies the SamplerOption interface. @@ -102,19 +104,17 @@ type SamplerOption interface { } // NopSamplingHook is the default hook used by sampler. -func NopSamplingHook(_ Entry, _ SamplingDecision) error { - return nil -} +func NopSamplingHook(_ Entry, _ SamplingDecision) {} -// SamplerHook registers a which will be called when Sampler makes a decision. -// Currently a hook is called when a log is dropped and zapcore.Dropped decision -// is emitted. +// SamplerHook registers a function which will be called when Sampler makes a +// decision. Currently a hook is called when a log is dropped and +// zapcore.LogDropped decision is emitted. // // This hook is useful for side effects, for example emitting number of dropped // logs. Note, there is no access to Fields in this hook. In the future, this // hook can be expanded to emit whether this is first entry that was dropped, // first after a period, etc. -func SamplerHook(hook func(entry Entry, dec SamplingDecision) error) SamplerOption { +func SamplerHook(hook func(entry Entry, dec SamplingDecision)) SamplerOption { return optionFunc(func(s *sampler) { s.hook = hook }) @@ -156,7 +156,7 @@ type sampler struct { counts *counters tick time.Duration first, thereafter uint64 - hook func(Entry, SamplingDecision) error + hook func(Entry, SamplingDecision) } // NewSampler creates a Core that samples incoming entries, which @@ -170,6 +170,7 @@ type sampler struct { // Keep in mind that zap's sampling implementation is optimized for speed over // absolute precision; under load, each tick may be slightly over- or // under-sampled. +// // Deprecated: use NewSamplerWithOptions. func NewSampler(core Core, tick time.Duration, first, thereafter int) Core { return &sampler{ @@ -201,9 +202,9 @@ func (s *sampler) Check(ent Entry, ce *CheckedEntry) *CheckedEntry { counter := s.counts.get(ent.Level, ent.Message) n := counter.IncCheckReset(ent.Time, s.tick) if n > s.first && (n-s.first)%s.thereafter != 0 { - _ = s.hook(ent, Dropped) + s.hook(ent, LogDropped) return ce } - + s.hook(ent, LogSampled) return s.Core.Check(ent, ce) }