Simplify debug level check

common/log/logger.go

@@ -25,8 +25,6 @@ import (
 	"math/rand"
 	"path/filepath"
 	"runtime"
-	"sync/atomic"
-	"time"
 
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
@@ -35,31 +33,25 @@ import (
 )
 
 type loggerImpl struct {
-	zapLogger                  *zap.Logger
-	skip                       int
-	sampleLocalFn              func(int) bool
-	debugOn                    int32
-	debugOnCheckTimestampNanos int64
-	debugCheckInterval         time.Duration
+	zapLogger     *zap.Logger
+	skip          int
+	sampleLocalFn func(int) bool
 }
 
 const (
 	skipForDefaultLogger = 3
 	// we put a default message when it is empty so that the log can be searchable/filterable
 	defaultMsgForEmpty = "none"
-	// debugCheckInterval is the interval to check if debug level is on
-	debugCheckInterval = 10 * time.Second
 )
 
 var defaultSampleFn = func(i int) bool { return rand.Intn(i) == 0 }
 
 // NewLogger returns a new logger
 func NewLogger(zapLogger *zap.Logger, opts ...Option) Logger {
 	impl := &loggerImpl{
-		zapLogger:          zapLogger,
-		skip:               skipForDefaultLogger,
-		sampleLocalFn:      defaultSampleFn,
-		debugCheckInterval: debugCheckInterval,
+		zapLogger:     zapLogger,
+		skip:          skipForDefaultLogger,
+		sampleLocalFn: defaultSampleFn,
 	}
 	for _, opt := range opts {
 		opt(impl)
@@ -159,23 +151,8 @@ func (lg *loggerImpl) WithTags(tags ...tag.Tag) Logger {
 }
 
 // DebugOn checks if debug level is on.
-// This is useful to avoid expensive debugging serializations etc. in production.
-// It caches the result for debugOnCheckInterval and checks again if the interval has passed.
-// Log level changes not reflected immediately because of the cache but it's acceptable.
 func (lg *loggerImpl) DebugOn() bool {
-	if time.Since(time.Unix(0, atomic.LoadInt64(&lg.debugOnCheckTimestampNanos))) < lg.debugCheckInterval {
-		return atomic.LoadInt32(&lg.debugOn) != 0
-	}
-
-	on := int32(0)
-	if lg.zapLogger.Check(zap.DebugLevel, "test") != nil {
-		on = 1
-	}
-
-	// no locking to avoid performance overhead. there's chance of redundant computation but it's acceptable.
-	atomic.StoreInt32(&lg.debugOn, on)
-	atomic.StoreInt64(&lg.debugOnCheckTimestampNanos, time.Now().UnixNano())
-	return on != 0
+	return lg.zapLogger.Check(zap.DebugLevel, "test") != nil
 }
 
 func (lg *loggerImpl) SampleInfo(msg string, sampleRate int, tags ...tag.Tag) {

common/log/logger_test.go

@@ -25,7 +25,6 @@ import (
 	"strconv"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/zap"
@@ -197,21 +196,18 @@ func TestDebugOn(t *testing.T) {
 		EncodeDuration: zapcore.StringDurationEncoder,
 	}), zapcore.AddSync(buf), l))
 
-	logger := NewLogger(zapLogger, WithDebugCheckInterval(time.Millisecond))
+	logger := NewLogger(zapLogger)
 
 	// Set level to debug and check if debugOn is true
 	l.SetLevel(zap.DebugLevel)
-	time.Sleep(time.Millisecond)
 	assert.True(t, logger.DebugOn())
 
 	// Set level to info and check if debugOn is false
 	l.SetLevel(zap.InfoLevel)
-	time.Sleep(time.Millisecond)
 	assert.False(t, logger.DebugOn())
 
 	// Set level to debug again and check if debugOn is true
 	l.SetLevel(zap.DebugLevel)
-	time.Sleep(time.Millisecond)
 	assert.True(t, logger.DebugOn())
 }
 

common/log/options.go

@@ -22,8 +22,6 @@
 
 package log
 
-import "time"
-
 // Option is used to set options for the logger.
 type Option func(impl *loggerImpl)
 
@@ -33,9 +31,3 @@ func WithSampleFunc(fn func(int) bool) Option {
 		impl.sampleLocalFn = fn
 	}
 }
-
-func WithDebugCheckInterval(interval time.Duration) Option {
-	return func(impl *loggerImpl) {
-		impl.debugCheckInterval = interval
-	}
-}