zerolog: fix log level (#7)

zerolog.Logger.Level() returns a new logger, which was being ignored, causing SetLevel() to be a no-op. Store the current logger in a pointer that is updated with the new logger whenever SetLevel() is called. Signed-off-by: Luiz Aoqui <luizaoqui@loopholelabs.io>
loopholelabs · Oct 16, 2024 · b1c518d · b1c518d
1 parent f631b02
commit b1c518d
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 15 deletions.
diff --git a/loggers/zerolog/context.go b/loggers/zerolog/context.go
@@ -17,11 +17,7 @@ type Context struct {
 }
 
 func (c *Context) Logger() types.Logger {
-	return &Logger{
-		logger: c.zeroCtx.Logger(),
-		source: c.l.source,
-		level:  c.l.level,
-	}
+	return newLogger(c.l.source, c.l.level, c.zeroCtx.Logger())
 }
 
 func (c *Context) Str(key string, val string) types.Context {

diff --git a/loggers/zerolog/zerolog.go b/loggers/zerolog/zerolog.go
@@ -5,6 +5,7 @@ package zerolog
 import (
 	"fmt"
 	"io"
+	"sync"
 
 	"github.com/rs/zerolog"
 
@@ -14,9 +15,10 @@ import (
 var _ types.RootLogger = (*Logger)(nil)
 
 type Logger struct {
-	level  types.Level
-	logger zerolog.Logger
-	source string
+	level      types.Level
+	logger     *zerolog.Logger
+	loggerLock sync.RWMutex
+	source     string
 }
 
 func init() {
@@ -25,9 +27,12 @@ func init() {
 }
 
 func New(source string, level types.Level, output io.Writer) *Logger {
-	zl := zerolog.New(output)
+	return newLogger(source, level, zerolog.New(output))
+}
+
+func newLogger(source string, level types.Level, zl zerolog.Logger) *Logger {
 	z := &Logger{
-		logger: zl,
+		logger: &zl,
 		source: source,
 	}
 	z.SetLevel(level)
@@ -54,44 +59,68 @@ func (z *Logger) SetLevel(level types.Level) {
 	case types.TraceLevel:
 		zerologLevel = zerolog.TraceLevel
 	}
+
+	newLogger := z.logger.Level(zerologLevel)
+
+	z.loggerLock.Lock()
+	defer z.loggerLock.Unlock()
+
+	z.logger = &newLogger
 	z.level = level
-	z.logger.Level(zerologLevel)
 }
 
 func (z *Logger) SubLogger(source string) types.Logger {
-	return &Logger{
-		logger: z.logger,
-		source: fmt.Sprintf("%s:%s", z.source, source),
-	}
+	return newLogger(fmt.Sprintf("%s:%s", z.source, source), z.level, *z.logger)
 }
 
 func (z *Logger) With() types.Context {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return &Context{
 		l:       z,
 		zeroCtx: z.logger.With(),
 	}
 }
 
 func (z *Logger) Fatal() types.Event {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return (*Event)(z.logger.Fatal().Timestamp().Str(types.SourceKey, z.source))
 }
 
 func (z *Logger) Error() types.Event {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return (*Event)(z.logger.Error().Timestamp().Str(types.SourceKey, z.source))
 }
 
 func (z *Logger) Warn() types.Event {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return (*Event)(z.logger.Warn().Timestamp().Str(types.SourceKey, z.source))
 }
 
 func (z *Logger) Info() types.Event {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return (*Event)(z.logger.Info().Timestamp().Str(types.SourceKey, z.source))
 }
 
 func (z *Logger) Debug() types.Event {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return (*Event)(z.logger.Debug().Timestamp().Str(types.SourceKey, z.source))
 }
 
 func (z *Logger) Trace() types.Event {
+	z.loggerLock.RLock()
+	defer z.loggerLock.RUnlock()
+
 	return (*Event)(z.logger.Trace().Timestamp().Str(types.SourceKey, z.source))
 }
diff --git a/logging_test.go b/logging_test.go
@@ -66,20 +66,23 @@ func TestInfo(t *testing.T) {
 			out := &bytes.Buffer{}
 			log := New(Noop, t.Name(), out)
 			log.Info().Msg("")
+			log.Debug().Msg("") // Should not be logged.
 			assert.Equal(t, "", out.String())
 		})
 
 		t.Run("zerolog", func(t *testing.T) {
 			out := &bytes.Buffer{}
 			log := New(Zerolog, t.Name(), out)
 			log.Info().Msg("")
+			log.Debug().Msg("") // Should not be logged.
 			assert.Equal(t, fillZerologTestFields(t, "{\"level\":\"info\",\"time\":\"%s\",\"source\":\"%s\"}\n"), out.String())
 		})
 
 		t.Run("slog", func(t *testing.T) {
 			out := &bytes.Buffer{}
 			log := New(Slog, t.Name(), out)
 			log.Info().Msg("")
+			log.Debug().Msg("") // Should not be logged.
 			assert.Equal(t, fillSlogTestFields(t, "level=INFO msg=\"\" source=%s\n"), out.String())
 		})
 	})
@@ -89,20 +92,23 @@ func TestInfo(t *testing.T) {
 			out := &bytes.Buffer{}
 			log := New(Noop, t.Name(), out)
 			log.Info().Str("foo", "bar").Msg("")
+			log.Debug().Str("foo", "bar").Msg("") // Should not be logged.
 			assert.Equal(t, "", out.String())
 		})
 
 		t.Run("zerolog", func(t *testing.T) {
 			out := &bytes.Buffer{}
 			log := New(Zerolog, t.Name(), out)
 			log.Info().Str("foo", "bar").Msg("")
+			log.Debug().Str("foo", "bar").Msg("") // Should not be logged.
 			assert.Equal(t, fillZerologTestFields(t, "{\"level\":\"info\",\"time\":\"%s\",\"source\":\"%s\",\"foo\":\"bar\"}\n"), out.String())
 		})
 
 		t.Run("slog", func(t *testing.T) {
 			out := &bytes.Buffer{}
 			log := New(Slog, t.Name(), out)
 			log.Info().Str("foo", "bar").Msg("")
+			log.Debug().Str("foo", "bar").Msg("") // Should not be logged.
 			assert.Equal(t, fillSlogTestFields(t, "level=INFO msg=\"\" source=%s foo=bar\n"), out.String())
 		})
 	})
@@ -115,6 +121,10 @@ func TestInfo(t *testing.T) {
 				Str("foo", "bar").
 				Int("n", 123).
 				Msg("")
+			log.Debug(). // Should not be logged.
+					Str("foo", "bar").
+					Int("n", 123).
+					Msg("")
 			assert.Equal(t, "", out.String())
 		})
 
@@ -125,6 +135,10 @@ func TestInfo(t *testing.T) {
 				Str("foo", "bar").
 				Int("n", 123).
 				Msg("")
+			log.Debug(). // Should not be logged.
+					Str("foo", "bar").
+					Int("n", 123).
+					Msg("")
 			assert.Equal(t, fillZerologTestFields(t, "{\"level\":\"info\",\"time\":\"%s\",\"source\":\"%s\",\"foo\":\"bar\",\"n\":123}\n"), out.String())
 		})
 
@@ -135,6 +149,10 @@ func TestInfo(t *testing.T) {
 				Str("foo", "bar").
 				Int("n", 123).
 				Msg("")
+			log.Debug(). // Should not be logged.
+					Str("foo", "bar").
+					Int("n", 123).
+					Msg("")
 			assert.Equal(t, fillSlogTestFields(t, "level=INFO msg=\"\" source=%s foo=bar n=123\n"), out.String())
 		})
 	})