loggerWith.WithCallerSkip expand keyvals (#1267)

loggerWith.WithCallerSkip expand keyvals
temporalio · Oct 13, 2023 · b484666 · b484666
1 parent a6cdcd7
commit b484666
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 19 deletions.
diff --git a/internal/log/logger_test.go b/internal/log/logger_test.go
@@ -42,6 +42,16 @@ func TestMemoryLogger_With(t *testing.T) {
 	assert.Equal(t, "INFO  message2 p4 4\n", logger.Lines()[1])
 }
 
+func TestMemoryLoggerWithoutWith_With(t *testing.T) {
+	logger := NewMemoryLoggerWithoutWith()
+	withLogger := log.With(logger, "p1", 1, "p2", "v2")
+	withLogger.Info("message", "p3", float64(3))
+	logger.Info("message2", "p4", 4)
+
+	assert.Equal(t, "INFO  message p1 1 p2 v2 p3 3\n", logger.Lines()[0])
+	assert.Equal(t, "INFO  message2 p4 4\n", logger.Lines()[1])
+}
+
 func TestDefaultLogger_With(t *testing.T) {
 	logger := NewDefaultLogger()
 
@@ -88,3 +98,26 @@ func TestReplayLogger_With(t *testing.T) {
 	assert.Equal(t, "INFO  message p1 1 p2 v2 p3 3\n", logger.Lines()[0])
 	assert.Equal(t, "INFO  message2 p4 4\n", logger.Lines()[1])
 }
+
+func TestReplayLogger_Skip(t *testing.T) {
+	logger := NewMemoryLogger()
+	isReplay, enableLoggingInReplay := false, false
+
+	replayLogger := NewReplayLogger(logger, &isReplay, &enableLoggingInReplay)
+	withReplayLogger := log.With(replayLogger, "p1", 1, "p2", "v2")
+	withReplayLogger = log.With(withReplayLogger, "p3", float64(3), "p4", true)
+	skipLogger := log.Skip(withReplayLogger, 2)
+	skipLogger = log.With(skipLogger, "p5", 5, "p6", 6)
+	skipLogger.Info("message", "p7", 7)
+	assert.Equal(t, "INFO  message p1 1 p2 v2 p3 3 p4 true p5 5 p6 6 p7 7\n", logger.Lines()[0])
+
+	loggerWithoutWith := NewMemoryLoggerWithoutWith()
+	replayLogger = NewReplayLogger(loggerWithoutWith, &isReplay, &enableLoggingInReplay)
+	withReplayLogger = log.With(replayLogger, "p1", 1, "p2", "v2")
+	withReplayLogger = log.With(withReplayLogger, "p3", float64(3), "p4", true)
+	skipLogger = log.Skip(withReplayLogger, 2)
+	skipLogger = log.With(skipLogger, "p5", 5, "p6", 6)
+	skipLogger.Info("message", "p7", 7)
+	assert.Equal(t, "INFO  message p1 1 p2 v2 p3 3 p4 true p5 5 p6 6 p7 7\n", loggerWithoutWith.Lines()[0])
+
+}
diff --git a/internal/log/memory_logger.go b/internal/log/memory_logger.go
@@ -31,21 +31,21 @@ import (
 	"go.temporal.io/sdk/log"
 )
 
-// MemoryLogger is Logger implementation that stores logs in memory (useful for testing). Use Lines() to get log lines.
-type MemoryLogger struct {
+// MemoryLoggerWithoutWith is a Logger implementation that stores logs in memory (useful for testing). Use Lines() to get log lines.
+type MemoryLoggerWithoutWith struct {
 	lines         *[]string
 	globalKeyvals string
 }
 
-// NewMemoryLogger creates new instance of MemoryLogger.
-func NewMemoryLogger() *MemoryLogger {
+// NewMemoryLoggerWithoutWith creates new instance of MemoryLoggerWithoutWith.
+func NewMemoryLoggerWithoutWith() *MemoryLoggerWithoutWith {
 	var lines []string
-	return &MemoryLogger{
+	return &MemoryLoggerWithoutWith{
 		lines: &lines,
 	}
 }
 
-func (l *MemoryLogger) println(level, msg string, keyvals []interface{}) {
+func (l *MemoryLoggerWithoutWith) println(level, msg string, keyvals []interface{}) {
 	// To avoid extra space when globalKeyvals is not specified.
 	if l.globalKeyvals == "" {
 		*l.lines = append(*l.lines, fmt.Sprintln(append([]interface{}{level, msg}, keyvals...)...))
@@ -55,28 +55,44 @@ func (l *MemoryLogger) println(level, msg string, keyvals []interface{}) {
 }
 
 // Debug appends message to the log.
-func (l *MemoryLogger) Debug(msg string, keyvals ...interface{}) {
+func (l *MemoryLoggerWithoutWith) Debug(msg string, keyvals ...interface{}) {
 	l.println("DEBUG", msg, keyvals)
 }
 
 // Info appends message to the log.
-func (l *MemoryLogger) Info(msg string, keyvals ...interface{}) {
+func (l *MemoryLoggerWithoutWith) Info(msg string, keyvals ...interface{}) {
 	l.println("INFO ", msg, keyvals)
 }
 
 // Warn appends message to the log.
-func (l *MemoryLogger) Warn(msg string, keyvals ...interface{}) {
+func (l *MemoryLoggerWithoutWith) Warn(msg string, keyvals ...interface{}) {
 	l.println("WARN ", msg, keyvals)
 }
 
 // Error appends message to the log.
-func (l *MemoryLogger) Error(msg string, keyvals ...interface{}) {
+func (l *MemoryLoggerWithoutWith) Error(msg string, keyvals ...interface{}) {
 	l.println("ERROR", msg, keyvals)
 }
 
-// With returns new logger the prepend every log entry with keyvals.
+// Lines returns written log lines.
+func (l *MemoryLoggerWithoutWith) Lines() []string {
+	return *l.lines
+}
+
+type MemoryLogger struct {
+	*MemoryLoggerWithoutWith
+}
+
+// NewMemoryLogger creates new instance of MemoryLogger.
+func NewMemoryLogger() *MemoryLogger {
+	return &MemoryLogger{
+		NewMemoryLoggerWithoutWith(),
+	}
+}
+
+// With returns new logger that prepend every log entry with keyvals.
 func (l *MemoryLogger) With(keyvals ...interface{}) log.Logger {
-	logger := &MemoryLogger{
+	logger := &MemoryLoggerWithoutWith{
 		lines: l.lines,
 	}
 
@@ -88,8 +104,3 @@ func (l *MemoryLogger) With(keyvals ...interface{}) log.Logger {
 
 	return logger
 }
-
-// Lines returns written log lines.
-func (l *MemoryLogger) Lines() []string {
-	return *l.lines
-}
diff --git a/internal/log/replay_logger.go b/internal/log/replay_logger.go
@@ -80,7 +80,7 @@ func (l *ReplayLogger) Error(msg string, keyvals ...interface{}) {
 	}
 }
 
-// With returns new logger the prepend every log entry with keyvals.
+// With returns new logger that prepend every log entry with keyvals.
 func (l *ReplayLogger) With(keyvals ...interface{}) log.Logger {
 	return NewReplayLogger(log.With(l.logger, keyvals...), l.isReplay, l.enableLoggingInReplay)
 }

diff --git a/log/with_logger.go b/log/with_logger.go
@@ -86,7 +86,7 @@ func (l *withLogger) Error(msg string, keyvals ...interface{}) {
 
 func (l *withLogger) WithCallerSkip(depth int) Logger {
 	if sl, ok := l.logger.(WithSkipCallers); ok {
-		return newWithLogger(sl.WithCallerSkip(depth), l.keyvals)
+		return newWithLogger(sl.WithCallerSkip(depth), l.keyvals...)
 	}
 	return l
 }
diff --git a/log/with_logger_test.go b/log/with_logger_test.go
@@ -37,3 +37,11 @@ func TestWithLogger(t *testing.T) {
 	allKeys := wl.prependKeyvals([]interface{}{"p4", 4})
 	assert.Equal(t, []interface{}{"p1", 1, "p2", "v2", "p4", 4}, allKeys)
 }
+
+func TestWithLoggerSkip(t *testing.T) {
+	wl := &withLogger{}
+	wl2 := With(wl, "p1", 1, "p2", "v2")
+	sl := Skip(wl2, 0)
+	wl, _ = sl.(*withLogger)
+	assert.Equal(t, []interface{}{"p1", 1, "p2", "v2"}, wl.keyvals)
+}