Simplify default constructor, add a sample event, do not return error

for hook.

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...,
 			)
 		}))
 	}

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())
 }

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)
 }