fix data loss for handler, connection, account

core/logger/buffer_logger_test.go

@@ -86,18 +86,48 @@ func (l *bufferLogger) WithLazy(fields ...zapcore.Field) Logger {
 }
 
 // ForHandler returns a new logger that is associated with the given handler.
+// This will replace "handler" field if it was set before.
+// Note: chain calls like ForHandler().With().ForHandler() will DUPLICATE handler field!
 func (l *bufferLogger) ForHandler(handler any) Logger {
-	return l.clone(l.parentOrCurrent().WithLazy(zap.Any(HandlerAttr, handler)))
+	if l.previous != previousFieldHandler {
+		result := l.With(zap.Any(HandlerAttr, handler))
+		result.(*bufferLogger).setPrevious(previousFieldHandler)
+		result.(*bufferLogger).parent = l.Logger
+		return result
+	}
+	result := l.clone(l.parentOrCurrent().With(zap.Any(HandlerAttr, handler)))
+	result.(*bufferLogger).setPrevious(previousFieldHandler)
+	return result
 }
 
 // ForConnection returns a new logger that is associated with the given connection.
+// This will replace "connection" field if it was set before.
+// Note: chain calls like ForConnection().With().ForConnection() will DUPLICATE connection field!
 func (l *bufferLogger) ForConnection(conn any) Logger {
-	return l.clone(l.parentOrCurrent().WithLazy(zap.Any(ConnectionAttr, conn)))
+	if l.previous != previousFieldConnection {
+		result := l.With(zap.Any(ConnectionAttr, conn))
+		result.(*bufferLogger).setPrevious(previousFieldConnection)
+		result.(*bufferLogger).parent = l.Logger
+		return result
+	}
+	result := l.clone(l.parentOrCurrent().With(zap.Any(ConnectionAttr, conn)))
+	result.(*bufferLogger).setPrevious(previousFieldConnection)
+	return result
 }
 
 // ForAccount returns a new logger that is associated with the given account.
+// This will replace "account" field if it was set before.
+// Note: chain calls like ForAccount().With().ForAccount() will DUPLICATE account field!
 func (l *bufferLogger) ForAccount(acc any) Logger {
-	return l.clone(l.parentOrCurrent().WithLazy(zap.Any(AccountAttr, acc)))
+	if l.previous != previousFieldAccount {
+		result := l.With(zap.Any(AccountAttr, acc))
+		result.(*bufferLogger).setPrevious(previousFieldAccount)
+		result.(*bufferLogger).parent = l.Logger
+		return result
+	}
+	result := l.clone(l.parentOrCurrent().With(zap.Any(AccountAttr, acc)))
+	result.(*bufferLogger).setPrevious(previousFieldAccount)
+	return result
 }
 
 // Read bytes from the logger buffer. io.Reader implementation.

core/logger/default.go

@@ -44,10 +44,19 @@ type Logger interface {
 	Sync() error
 }
 
+type previousField uint8
+
+const (
+	previousFieldHandler previousField = iota + 1
+	previousFieldConnection
+	previousFieldAccount
+)
+
 // Default is a default logger implementation.
 type Default struct {
 	*zap.Logger
-	parent *zap.Logger
+	parent   *zap.Logger
+	previous previousField
 }
 
 // NewDefault creates a new default logger with the given format and debug level.
@@ -68,18 +77,48 @@ func (l *Default) WithLazy(fields ...zap.Field) Logger {
 }
 
 // ForHandler returns a new logger that is associated with the given handler.
+// This will replace "handler" field if it was set before.
+// Note: chain calls like ForHandler().With().ForHandler() will DUPLICATE handler field!
 func (l *Default) ForHandler(handler any) Logger {
-	return l.clone(l.parentOrCurrent().WithLazy(zap.Any(HandlerAttr, handler)))
+	if l.previous != previousFieldHandler {
+		result := l.With(zap.Any(HandlerAttr, handler))
+		result.(*Default).setPrevious(previousFieldHandler)
+		result.(*Default).parent = l.Logger
+		return result
+	}
+	result := l.clone(l.parentOrCurrent().With(zap.Any(HandlerAttr, handler)))
+	result.(*Default).setPrevious(previousFieldHandler)
+	return result
 }
 
 // ForConnection returns a new logger that is associated with the given connection.
+// This will replace "connection" field if it was set before.
+// Note: chain calls like ForConnection().With().ForConnection() will DUPLICATE connection field!
 func (l *Default) ForConnection(conn any) Logger {
-	return l.clone(l.parentOrCurrent().WithLazy(zap.Any(ConnectionAttr, conn)))
+	if l.previous != previousFieldConnection {
+		result := l.With(zap.Any(ConnectionAttr, conn))
+		result.(*Default).setPrevious(previousFieldConnection)
+		result.(*Default).parent = l.Logger
+		return result
+	}
+	result := l.clone(l.parentOrCurrent().With(zap.Any(ConnectionAttr, conn)))
+	result.(*Default).setPrevious(previousFieldConnection)
+	return result
 }
 
 // ForAccount returns a new logger that is associated with the given account.
+// This will replace "account" field if it was set before.
+// Note: chain calls like ForAccount().With().ForAccount() will DUPLICATE account field!
 func (l *Default) ForAccount(acc any) Logger {
-	return l.clone(l.parentOrCurrent().WithLazy(zap.Any(AccountAttr, acc)))
+	if l.previous != previousFieldAccount {
+		result := l.With(zap.Any(AccountAttr, acc))
+		result.(*Default).setPrevious(previousFieldAccount)
+		result.(*Default).parent = l.Logger
+		return result
+	}
+	result := l.clone(l.parentOrCurrent().With(zap.Any(AccountAttr, acc)))
+	result.(*Default).setPrevious(previousFieldAccount)
+	return result
 }
 
 // clone creates a copy of the given logger.
@@ -99,6 +138,10 @@ func (l *Default) parentOrCurrent() *zap.Logger {
 	return l.Logger
 }
 
+func (l *Default) setPrevious(prev previousField) {
+	l.previous = prev
+}
+
 // AnyZapFields converts an array of values to zap fields.
 func AnyZapFields(args []interface{}) []zap.Field {
 	fields := make([]zap.Field, len(args))

core/logger/default_test.go

@@ -105,6 +105,48 @@ func (s *TestDefaultSuite) TestForAccountNoDuplicate() {
 	s.Assert().NotContains(log.String(), "acc1")
 }
 
+func (s *TestDefaultSuite) TestNoDuplicatesPersistRecords() {
+	log := newBufferLogger()
+	log.
+		ForHandler("handler1").
+		ForHandler("handler2").
+		ForConnection("conn1").
+		ForConnection("conn2").
+		ForAccount("acc1").
+		ForAccount("acc2").
+		Info("test")
+
+	s.Assert().Contains(log.String(), "handler2")
+	s.Assert().NotContains(log.String(), "handler1")
+	s.Assert().Contains(log.String(), "conn2")
+	s.Assert().NotContains(log.String(), "conn1")
+	s.Assert().Contains(log.String(), "acc2")
+	s.Assert().NotContains(log.String(), "acc1")
+}
+
+// TestPersistRecordsIncompatibleWith is not a unit test, but rather a demonstration how you shouldn't use For* methods.
+func (s *TestDefaultSuite) TestPersistRecordsIncompatibleWith() {
+	log := newBufferLogger()
+	log.
+		ForHandler("handler1").
+		With(zap.Int("f1", 1)).
+		ForHandler("handler2").
+		ForConnection("conn1").
+		With(zap.Int("f2", 2)).
+		ForConnection("conn2").
+		ForAccount("acc1").
+		With(zap.Int("f3", 3)).
+		ForAccount("acc2").
+		Info("test")
+
+	s.Assert().Contains(log.String(), "handler2")
+	s.Assert().Contains(log.String(), "handler1")
+	s.Assert().Contains(log.String(), "conn2")
+	s.Assert().Contains(log.String(), "conn1")
+	s.Assert().Contains(log.String(), "acc2")
+	s.Assert().Contains(log.String(), "acc1")
+}
+
 func TestAnyZapFields(t *testing.T) {
 	fields := AnyZapFields([]interface{}{zap.String("k0", "v0"), "ooga", "booga"})
 	require.Len(t, fields, 3)