Skip to content

Commit

Permalink
Discard counters for out-of-bounds levels (#975)
Browse files Browse the repository at this point in the history
Previously this just crashed.

Co-authored-by: Prashant Varanasi <prashant@uber.com>
  • Loading branch information
thockin and prashantv authored Jul 1, 2021
1 parent 120b08c commit 007a55e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
14 changes: 8 additions & 6 deletions zapcore/sampler.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,14 @@ func (s *sampler) Check(ent Entry, ce *CheckedEntry) *CheckedEntry {
return ce
}

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, LogDropped)
return ce
if ent.Level >= _minLevel && ent.Level <= _maxLevel {
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, LogDropped)
return ce
}
s.hook(ent, LogSampled)
}
s.hook(ent, LogSampled)
return s.Core.Check(ent, ce)
}
20 changes: 20 additions & 0 deletions zapcore/sampler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,23 @@ func TestSamplerRaces(t *testing.T) {
close(start)
wg.Wait()
}

func TestSamplerUnknownLevels(t *testing.T) {
// Prove that out-of-bounds levels don't panic.
unknownLevels := []Level{
DebugLevel - 1,
FatalLevel + 1,
}

for _, lvl := range unknownLevels {
t.Run(lvl.String(), func(t *testing.T) {
sampler, logs := fakeSampler(lvl, time.Minute, 2, 3)
for i := 1; i < 10; i++ {
writeSequence(sampler, i, lvl)
}

// Expect no sampling for unknown levels.
assertSequence(t, logs.TakeAll(), lvl, 1, 2, 3, 4, 5, 6, 7, 8, 9)
})
}
}

0 comments on commit 007a55e

Please sign in to comment.