Skip to content

Commit

Permalink
Simplify default constructor, add a sample event, do not return error
Browse files Browse the repository at this point in the history
for hook.
  • Loading branch information
shirchen committed Apr 7, 2020
1 parent 5cc987e commit fd20684
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 28 deletions.
14 changes: 7 additions & 7 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...,
)
}))
}
Expand Down
25 changes: 16 additions & 9 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.")
Expand Down Expand Up @@ -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())
}
25 changes: 13 additions & 12 deletions zapcore/sampler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
})
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -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)
}

0 comments on commit fd20684

Please sign in to comment.