Fix: Slog Sublogger (#6)

* Fixing slog sublogger and adding comprehensive test cases Signed-off-by: Shivansh Vij <[email protected]> * Renaming `SubLogger` to `Logger` and original `Logger` to `RootLogger` Signed-off-by: Shivansh Vij <[email protected]> --------- Signed-off-by: Shivansh Vij <[email protected]>
loopholelabs · Aug 30, 2024 · ef47fb8 · ef47fb8
1 parent ba1e898
commit ef47fb8
Show file tree

Hide file tree

Showing 9 changed files with 189 additions and 21 deletions.
diff --git a/loggers/noop/context.go b/loggers/noop/context.go
@@ -14,7 +14,7 @@ type Context struct {
 	l *Logger
 }
 
-func (c *Context) Logger() types.SubLogger {
+func (c *Context) Logger() types.Logger {
 	return c.l
 }
 

diff --git a/loggers/noop/noop.go b/loggers/noop/noop.go
@@ -4,7 +4,7 @@ package noop
 
 import "github.com/loopholelabs/logging/types"
 
-var _ types.Logger = (*Logger)(nil)
+var _ types.RootLogger = (*Logger)(nil)
 
 type Logger struct {
 	level types.Level
@@ -22,7 +22,7 @@ func (s *Logger) Level() types.Level {
 	return s.level
 }
 
-func (s *Logger) SubLogger(string) types.SubLogger { return s }
+func (s *Logger) SubLogger(string) types.Logger { return s }
 
 func (s *Logger) With() types.Context {
 	return &Context{l: s}

diff --git a/loggers/slog/context.go b/loggers/slog/context.go
@@ -16,9 +16,12 @@ type Context struct {
 	attrs []any
 }
 
-func (c *Context) Logger() types.SubLogger {
+func (c *Context) Logger() types.Logger {
 	l := New(c.l.source, c.l.level, c.l.output)
-	l.logger = c.l.logger.With(c.attrs...)
+	if c.attrs != nil {
+		l.logger = l.logger.With(c.attrs...)
+		l.attrs = c.attrs
+	}
 	return l
 }
 

diff --git a/loggers/slog/slog.go b/loggers/slog/slog.go
@@ -10,7 +10,7 @@ import (
 	"github.com/loopholelabs/logging/types"
 )
 
-var _ types.Logger = (*Logger)(nil)
+var _ types.RootLogger = (*Logger)(nil)
 
 var (
 	ReplaceAttr = func(_ []string, a slog.Attr) slog.Attr {
@@ -28,6 +28,7 @@ type Logger struct {
 	slogLevel *slog.LevelVar
 	output    io.Writer
 	source    string
+	attrs     []any
 }
 
 func New(source string, level types.Level, output io.Writer) *Logger {
@@ -44,13 +45,12 @@ func newSlog(source string, slogLevel *slog.LevelVar, output io.Writer) *Logger
 	}).WithAttrs([]slog.Attr{
 		{Key: types.SourceKey, Value: slog.StringValue(source)},
 	}))
-	s := &Logger{
+	return &Logger{
 		logger:    sl,
 		output:    output,
 		slogLevel: slogLevel,
 		source:    source,
 	}
-	return s
 }
 
 func (s *Logger) Level() types.Level {
@@ -77,14 +77,20 @@ func (s *Logger) SetLevel(level types.Level) {
 	s.slogLevel.Set(slogLevel)
 }
 
-func (s *Logger) SubLogger(source string) types.SubLogger {
+func (s *Logger) SubLogger(source string) types.Logger {
 	sloglevel := new(slog.LevelVar)
 	sloglevel.Set(s.slogLevel.Level())
-	return newSlog(fmt.Sprintf("%s:%s", s.source, source), sloglevel, s.output)
+	l := newSlog(fmt.Sprintf("%s:%s", s.source, source), sloglevel, s.output)
+	l.level = s.level
+	if s.attrs != nil {
+		l.logger = l.logger.With(s.attrs...)
+		l.attrs = s.attrs
+	}
+	return l
 }
 
 func (s *Logger) With() types.Context {
-	return &Context{l: s}
+	return &Context{l: s, attrs: s.attrs}
 }
 
 func (s *Logger) Fatal() types.Event {

diff --git a/loggers/zerolog/context.go b/loggers/zerolog/context.go
@@ -16,7 +16,7 @@ type Context struct {
 	zeroCtx zerolog.Context
 }
 
-func (c *Context) Logger() types.SubLogger {
+func (c *Context) Logger() types.Logger {
 	return &Logger{
 		logger: c.zeroCtx.Logger(),
 		source: c.l.source,

diff --git a/loggers/zerolog/zerolog.go b/loggers/zerolog/zerolog.go
@@ -11,7 +11,7 @@ import (
 	"github.com/loopholelabs/logging/types"
 )
 
-var _ types.Logger = (*Logger)(nil)
+var _ types.RootLogger = (*Logger)(nil)
 
 type Logger struct {
 	level  types.Level
@@ -58,7 +58,7 @@ func (z *Logger) SetLevel(level types.Level) {
 	z.logger.Level(zerologLevel)
 }
 
-func (z *Logger) SubLogger(source string) types.SubLogger {
+func (z *Logger) SubLogger(source string) types.Logger {
 	return &Logger{
 		logger: z.logger,
 		source: fmt.Sprintf("%s:%s", z.source, source),

diff --git a/logging.go b/logging.go
@@ -25,7 +25,7 @@ const (
 
 // New creates a new logger based on the given kind, source, and output
 // and a default level of Info.
-func New(kind Kind, source string, output io.Writer) types.Logger {
+func New(kind Kind, source string, output io.Writer) types.RootLogger {
 	switch kind {
 	case Noop:
 		return noop.New(types.InfoLevel)
@@ -38,7 +38,7 @@ func New(kind Kind, source string, output io.Writer) types.Logger {
 	}
 }
 
-func Test(t testing.TB, kind Kind, source string) types.Logger {
+func Test(t testing.TB, kind Kind, source string) types.RootLogger {
 	switch kind {
 	case Noop:
 		return noop.New(types.InfoLevel)

diff --git a/logging_test.go b/logging_test.go
@@ -38,10 +38,28 @@ func fillZerologTestFields(t *testing.T, format string) string {
 	return fmt.Sprintf(format, zeroTime.Format(zerolog.TimeFieldFormat), t.Name())
 }
 
+func fillZerologSubloggerTestFields(t *testing.T, format string, depth int) string {
+	args := make([]interface{}, 0, depth+2)
+	args = append(args, zeroTime.Format(zerolog.TimeFieldFormat), t.Name())
+	for i := 0; i < depth; i++ {
+		args = append(args, t.Name())
+	}
+	return fmt.Sprintf(format, args...)
+}
+
 func fillSlogTestFields(t *testing.T, format string) string {
 	return fmt.Sprintf(format, t.Name())
 }
 
+func fillSlogSubloggerTestFields(t *testing.T, format string, depth int) string {
+	args := make([]interface{}, 0, depth+1)
+	args = append(args, t.Name())
+	for i := 0; i < depth; i++ {
+		args = append(args, t.Name())
+	}
+	return fmt.Sprintf(format, args...)
+}
+
 func TestInfo(t *testing.T) {
 	t.Run("empty", func(t *testing.T) {
 		t.Run("noop", func(t *testing.T) {
@@ -199,3 +217,144 @@ func TestInfo(t *testing.T) {
 		})
 	})
 }
+
+func TestSubLoggers(t *testing.T) {
+	t.Run("depth=1", func(t *testing.T) {
+		t.Run("zerolog", func(t *testing.T) {
+			out := &bytes.Buffer{}
+			log := New(Zerolog, t.Name(), out)
+			sublogger := log.SubLogger(t.Name())
+			sublogger.Info().Str("foo", "bar").Msg("")
+			assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"time\":\"%s\",\"source\":\"%s:%s\",\"foo\":\"bar\"}\n", 1), out.String())
+		})
+
+		t.Run("slog", func(t *testing.T) {
+			out := &bytes.Buffer{}
+			log := New(Slog, t.Name(), out)
+			sublogger := log.SubLogger(t.Name())
+			sublogger.Info().Str("foo", "bar").Msg("")
+			assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s\" foo=bar\n", 1), out.String())
+		})
+	})
+
+	t.Run("depth=2", func(t *testing.T) {
+		t.Run("zerolog", func(t *testing.T) {
+			out := &bytes.Buffer{}
+			log := New(Zerolog, t.Name(), out)
+			sublogger0 := log.SubLogger(t.Name())
+			sublogger1 := sublogger0.SubLogger(t.Name())
+			sublogger1.Info().Str("foo", "bar").Msg("")
+			assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"time\":\"%s\",\"source\":\"%s:%s:%s\",\"foo\":\"bar\"}\n", 2), out.String())
+		})
+
+		t.Run("slog", func(t *testing.T) {
+			out := &bytes.Buffer{}
+			log := New(Slog, t.Name(), out)
+			sublogger0 := log.SubLogger(t.Name())
+			sublogger1 := sublogger0.SubLogger(t.Name())
+			sublogger1.Info().Str("foo", "bar").Msg("")
+			assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s:%s\" foo=bar\n", 2), out.String())
+		})
+	})
+
+	t.Run("depth=3", func(t *testing.T) {
+		t.Run("zerolog", func(t *testing.T) {
+			out := &bytes.Buffer{}
+			log := New(Zerolog, t.Name(), out)
+			sublogger0 := log.SubLogger(t.Name())
+			sublogger1 := sublogger0.SubLogger(t.Name())
+			sublogger2 := sublogger1.SubLogger(t.Name())
+			sublogger2.Info().Str("foo", "bar").Msg("")
+			assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"time\":\"%s\",\"source\":\"%s:%s:%s:%s\",\"foo\":\"bar\"}\n", 3), out.String())
+		})
+
+		t.Run("slog", func(t *testing.T) {
+			out := &bytes.Buffer{}
+			log := New(Slog, t.Name(), out)
+			sublogger0 := log.SubLogger(t.Name())
+			sublogger1 := sublogger0.SubLogger(t.Name())
+			sublogger2 := sublogger1.SubLogger(t.Name())
+			sublogger2.Info().Str("foo", "bar").Msg("")
+			assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s:%s:%s\" foo=bar\n", 3), out.String())
+		})
+	})
+
+	t.Run("with", func(t *testing.T) {
+
+		t.Run("before-depth=1", func(t *testing.T) {
+			t.Run("zerolog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Zerolog, t.Name(), out).With().Str("foo", "bar").Logger()
+				sublogger := log.SubLogger(t.Name())
+				sublogger.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"foo\":\"bar\",\"time\":\"%s\",\"source\":\"%s:%s\",\"foo1\":\"bar1\"}\n", 1), out.String())
+			})
+
+			t.Run("slog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Slog, t.Name(), out).With().Str("foo", "bar").Logger()
+				sublogger := log.SubLogger(t.Name())
+				sublogger.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s\" foo=bar foo1=bar1\n", 1), out.String())
+			})
+		})
+
+		t.Run("before-depth=2", func(t *testing.T) {
+			t.Run("zerolog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Zerolog, t.Name(), out).With().Str("foo", "bar").Logger()
+				sublogger0 := log.SubLogger(t.Name())
+				sublogger1 := sublogger0.SubLogger(t.Name())
+				sublogger1.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"foo\":\"bar\",\"time\":\"%s\",\"source\":\"%s:%s:%s\",\"foo1\":\"bar1\"}\n", 2), out.String())
+			})
+
+			t.Run("slog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Slog, t.Name(), out).With().Str("foo", "bar").Logger()
+				sublogger0 := log.SubLogger(t.Name())
+				sublogger1 := sublogger0.SubLogger(t.Name())
+				sublogger1.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s:%s\" foo=bar foo1=bar1\n", 2), out.String())
+			})
+		})
+
+		t.Run("after-depth=1", func(t *testing.T) {
+			t.Run("zerolog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Zerolog, t.Name(), out)
+				sublogger := log.SubLogger(t.Name()).With().Str("foo", "bar").Logger()
+				sublogger.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"foo\":\"bar\",\"time\":\"%s\",\"source\":\"%s:%s\",\"foo1\":\"bar1\"}\n", 1), out.String())
+			})
+
+			t.Run("slog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Slog, t.Name(), out)
+				sublogger := log.SubLogger(t.Name()).With().Str("foo", "bar").Logger()
+				sublogger.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s\" foo=bar foo1=bar1\n", 1), out.String())
+			})
+		})
+
+		t.Run("after-depth=2", func(t *testing.T) {
+			t.Run("zerolog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Zerolog, t.Name(), out)
+				sublogger0 := log.SubLogger(t.Name())
+				sublogger1 := sublogger0.SubLogger(t.Name()).With().Str("foo", "bar").Logger()
+				sublogger1.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillZerologSubloggerTestFields(t, "{\"level\":\"info\",\"foo\":\"bar\",\"time\":\"%s\",\"source\":\"%s:%s:%s\",\"foo1\":\"bar1\"}\n", 2), out.String())
+			})
+
+			t.Run("slog", func(t *testing.T) {
+				out := &bytes.Buffer{}
+				log := New(Slog, t.Name(), out)
+				sublogger0 := log.SubLogger(t.Name())
+				sublogger1 := sublogger0.SubLogger(t.Name()).With().Str("foo", "bar").Logger()
+				sublogger1.Info().Str("foo1", "bar1").Msg("")
+				assert.Equal(t, fillSlogSubloggerTestFields(t, "level=INFO msg=\"\" source=\"%s:%s:%s\" foo=bar foo1=bar1\n", 2), out.String())
+			})
+		})
+	})
+}
diff --git a/types/types.go b/types/types.go
@@ -21,14 +21,14 @@ const (
 	SourceKey    = "source"
 )
 
-type Logger interface {
+type RootLogger interface {
 	SetLevel(level Level)
-	SubLogger
+	Logger
 }
 
-type SubLogger interface {
+type Logger interface {
 	Level() Level
-	SubLogger(source string) SubLogger
+	SubLogger(source string) Logger
 	With() Context
 
 	Fatal() Event
@@ -49,7 +49,7 @@ type Event interface {
 type Context interface {
 	taggable[Context]
 
-	Logger() SubLogger
+	Logger() Logger
 }
 
 // taggable represents values that can receive structured fields.