🐛 logs Fix missing writer close skip (#660)

<!--
Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors.
All rights reserved.
SPDX-License-Identifier: Apache-2.0
-->
### Description

<!--
Please add any detail or context that would be useful to a reviewer.
-->



### Test Coverage

<!--
Please put an `x` in the correct box e.g. `[x]` to indicate the testing
coverage of this change.
-->

- [ ]  This change is covered by existing or additional automated tests.
- [ ] Manual testing has been performed (and evidence provided) as
automated testing was not feasible.
- [ ] Additional tests are not required for this change (e.g.
documentation update).

Author: aorabdel

changes/20250729175928.bugfix

@@ -0,0 +1 @@
+:bug: `logs` Fix missing writer close skip

utils/logs/json_logger.go

@@ -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()
@@ -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
 	}
@@ -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)
 }
@@ -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)
 }

utils/logs/json_logger_test.go

@@ -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())
 	})
 }
 
@@ -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)
@@ -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())
 	})
 }

utils/logs/log_test.go

@@ -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)
@@ -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)
 }

utils/logs/writer.go

@@ -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 {
@@ -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))
 		}