Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/20250729175928.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
:bug: `logs` Fix missing writer close skip
12 changes: 9 additions & 3 deletions utils/logs/json_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func (l *JSONLoggers) GetSource() string {
defer l.mu.RUnlock()
return l.source
}

func (l *JSONLoggers) GetLoggerSource() string {
l.mu.RLock()
defer l.mu.RUnlock()
Expand All @@ -71,15 +72,15 @@ func (l *JSONLoggers) Configure() error {
}

// Log logs to the output stream.
func (l *JSONLoggers) Log(output ...interface{}) {
func (l *JSONLoggers) Log(output ...any) {
if len(output) == 1 && output[0] == "\n" {
return
}
l.zerologger.Info().Str("source", l.GetLoggerSource()).Msg(fmt.Sprint(output...))
}

// LogError logs to the error stream.
func (l *JSONLoggers) LogError(err ...interface{}) {
func (l *JSONLoggers) LogError(err ...any) {
if len(err) == 1 && err[0] == "\n" {
return
}
Expand All @@ -98,7 +99,7 @@ func NewJSONLogger(writer WriterWithSource, loggerSource string, source string)
return newJSONLogger(true, writer, loggerSource, source)
}

// NewJSONLogger creates a Json logger. It is similar to NewJSONLogger but does not close the writer on Close().
// NewJSONLoggerWithWriter creates a Json logger. It is similar to NewJSONLogger but does not close the writer on Close().
func NewJSONLoggerWithWriter(writer WriterWithSource, loggerSource string, source string) (Loggers, error) {
return newJSONLogger(false, writer, loggerSource, source)
}
Expand Down Expand Up @@ -156,5 +157,10 @@ func NewJSONLoggerForSlowWriter(slowWriter WriterWithSource, ringBufferSize int,
// droppedMessagesLogger : logger for dropped messages
// If pollInterval is greater than 0, a poller is used otherwise a waiter is used.
func NewJSONLoggerForSlowWriterWithoutClosingWriter(slowWriter WriterWithSource, ringBufferSize int, pollInterval time.Duration, loggerSource string, source string, droppedMessagesLogger Loggers) (loggers Loggers, err error) {
// We call NewJSONLogger which closes the writer, because in this case the writer is a DiodeWriter that doesn't close the writer that is passed to it.
// JSONLogger.Close()
// -> DiodeWriterWithoutClosing.Close()
// -> does the proper steps for closing it's own goroutines
// -> does not close the writer passed as it's passed a non-closeable writer
return NewJSONLogger(NewDiodeWriterForSlowWriterWithoutClosing(slowWriter, ringBufferSize, pollInterval, droppedMessagesLogger), loggerSource, source)
}
7 changes: 2 additions & 5 deletions utils/logs/json_logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ func TestLogMessage(t *testing.T) {
})
t.Run("without writer closing", func(t *testing.T) {
writer := NewStdWriterWithSource()
defer func() { _ = writer.Close() }()
defer func() { require.NoError(t, writer.Close()) }()
loggers, err := NewJSONLoggerWithWriter(writer, "Test", "TestLogMessage")
require.NoError(t, err)
testLog(t, loggers)
require.NoError(t, writer.Close())
})
}

Expand All @@ -34,7 +33,6 @@ func TestLogMessageToSlowLogger(t *testing.T) {
stdloggers, err := NewStdLogger("ERR:")
require.NoError(t, err)

defer goleak.VerifyNone(t)
t.Run("with writer closing", func(t *testing.T) {
loggers, err := NewJSONLoggerForSlowWriter(NewTestSlowWriter(t), 1024, 2*time.Millisecond, "Test", "TestLogMessageToSlowLogger", stdloggers)
require.NoError(t, err)
Expand All @@ -43,11 +41,10 @@ func TestLogMessageToSlowLogger(t *testing.T) {
})
t.Run("without writer closing", func(t *testing.T) {
writer := NewTestSlowWriter(t)
defer func() { _ = writer.Close() }()
defer func() { require.NoError(t, writer.Close()) }()
loggers, err := NewJSONLoggerForSlowWriterWithoutClosingWriter(writer, 1024, 2*time.Millisecond, "Test", "TestLogMessageToSlowLogger", stdloggers)
require.NoError(t, err)
testLog(t, loggers)
time.Sleep(100 * time.Millisecond)
require.NoError(t, writer.Close())
})
}
4 changes: 1 addition & 3 deletions utils/logs/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func testLog(t *testing.T, loggers Loggers) {
t.Helper()
err := loggers.Check()
require.NoError(t, err)
defer func() { _ = loggers.Close() }()
defer func() { require.NoError(t, loggers.Close()) }()

err = loggers.SetLogSource("source1")
require.NoError(t, err)
Expand Down Expand Up @@ -60,6 +60,4 @@ func testLog(t *testing.T, loggers Loggers) {
loggers.LogError(commonerrors.ErrUnexpected, "some error")
loggers.LogError("some error", commonerrors.ErrUnexpected)
loggers.LogError(nil, "no error")
err = loggers.Close()
require.NoError(t, err)
}
13 changes: 4 additions & 9 deletions utils/logs/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,10 @@ func (w *DiodeWriter) Write(p []byte) (n int, err error) {
}

func (w *DiodeWriter) Close() error {
err := w.slowWriter.Close()
if err != nil {
return err
}
if diodeCloser, ok := w.diodeWriter.(io.Closer); ok {
return diodeCloser.Close()
if w.closeStore == nil {
return nil
}
return err
return w.closeStore.Close()
}

func (w *DiodeWriter) SetSource(source string) error {
Expand Down Expand Up @@ -150,8 +146,7 @@ func newDiodeWriterForSlowWriter(closeWriterOnClose bool, slowWriter WriterWithS

// We pass a wrapper that "overrides" the close method to do nothing,
// this is because we control the writer's close behaviour through the `closeWriterOnClose` check
ncw := nonCloseableWriter{slowWriter}
d := diode.NewWriter(ncw, ringBufferSize, pollInterval, func(missed int) {
d := diode.NewWriter(nonCloseableWriter{Writer: slowWriter}, ringBufferSize, pollInterval, func(missed int) {
if droppedMessagesLogger != nil {
droppedMessagesLogger.LogError(fmt.Sprintf("Logger dropped %d messages", missed))
}
Expand Down
Loading