Add Close method to logp.Logger

logp/core.go

@@ -63,6 +63,11 @@ type coreLogger struct {
 	observedLogs *observer.ObservedLogs // Contains events generated while in observation mode (a testing mode).
 }
 
+type closerCore struct {
+	zapcore.Core
+	io.Closer
+}
+
 // Configure configures the logp package.
 func Configure(cfg Config) error {
 	return ConfigureWithOutputs(cfg)
@@ -314,14 +319,35 @@ func makeFileOutput(cfg Config, enab zapcore.LevelEnabler) (zapcore.Core, error)
 		return nil, fmt.Errorf("failed to create file rotator: %w", err)
 	}
 
-	return newCore(buildEncoder(cfg), rotator, enab), nil
+	// Keep the same behaviour from before we introduced the closerCore.
+	core, err := newCore(buildEncoder(cfg), rotator, enab), nil
+	if err != nil {
+		return core, err
+	}
+
+	cc := closerCore{
+		Core:   core,
+		Closer: rotator,
+	}
+
+	return cc, err
 }
 
 func newCore(enc zapcore.Encoder, ws zapcore.WriteSyncer, enab zapcore.LevelEnabler) zapcore.Core {
 	return wrappedCore(zapcore.NewCore(enc, ws, enab))
 }
 func wrappedCore(core zapcore.Core) zapcore.Core {
-	return ecszap.WrapCore(core)
+	wc := ecszap.WrapCore(core)
+
+	if closeCore, ok := core.(io.Closer); ok {
+		cc := closerCore{
+			Core:   wc,
+			Closer: closeCore,
+		}
+		return cc
+	}
+
+	return wc
 }
 
 func globalLogger() *zap.Logger {
@@ -406,3 +432,17 @@ func (m multiCore) Sync() error {
 	}
 	return errors.Join(errs...)
 }
+
+// Close calls Close on any core that implements io.Closer.
+// All returned errors are joined by errors.Join and returned.
+func (m multiCore) Close() error {
+	errs := []error{}
+	for _, core := range m.cores {
+		if closer, ok := core.(io.Closer); ok {
+			closeErr := closer.Close()
+			errs = append(errs, closeErr)
+		}
+	}
+
+	return errors.Join(errs...)
+}

logp/core_test.go

@@ -25,7 +25,6 @@ import (
 	golog "log"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"testing"
 
@@ -222,33 +221,10 @@ func TestLoggingECSFields(t *testing.T) {
 }
 
 func TestCreatingNewLoggerWithDifferentOutput(t *testing.T) {
-	var tempDir1, tempDir2 string
-	// Because of the way logp and zap work, when the test finishes, the log
-	// file is still open, this creates a problem on Windows because the
-	// temporary directory cannot be removed if a file inside it is still
-	// open.
-	// See https://github.com/elastic/elastic-agent-libs/issues/179
-	// for more details
-	//
-	// To circumvent this problem on Windows we use os.MkdirTemp
-	// leaving it behind and delegating to the OS the responsibility
-	// of cleaning it up (usually on restart).
-	if runtime.GOOS == "windows" {
-		var err error
-		tempDir1, err = os.MkdirTemp("", t.Name()+"-*")
-		if err != nil {
-			t.Fatalf("could not create temporary directory: %s", err)
-		}
-		tempDir2, err = os.MkdirTemp("", t.Name()+"-*")
-		if err != nil {
-			t.Fatalf("could not create temporary directory: %s", err)
-		}
-	} else {
-		// We have no problems on Linux and Darwin, so we can rely on t.TempDir
-		// that will remove the files once the tests finishes.
-		tempDir1 = t.TempDir()
-		tempDir2 = t.TempDir()
-	}
+	// We have no problems on Linux and Darwin, so we can rely on t.TempDir
+	// that will remove the files once the tests finishes.
+	tempDir1 := t.TempDir()
+	tempDir2 := t.TempDir()
 
 	secondLoggerMessage := "this is a log message"
 	secondLoggerName := t.Name() + "-second"
@@ -279,6 +255,11 @@ func TestCreatingNewLoggerWithDifferentOutput(t *testing.T) {
 	if err := logger.Sync(); err != nil {
 		t.Fatalf("could not sync log file from fist logger: %s", err)
 	}
+	t.Cleanup(func() {
+		if err := logger.Close(); err != nil {
+			t.Fatalf("could not close first logger: %s", err)
+		}
+	})
 
 	// Actually clones the logger and use the "WithFileOutput" function
 	secondCfg := DefaultConfig(DefaultEnvironment)
@@ -303,6 +284,11 @@ func TestCreatingNewLoggerWithDifferentOutput(t *testing.T) {
 	if err := secondLogger.Sync(); err != nil {
 		t.Fatalf("could not sync log file from second logger: %s", err)
 	}
+	t.Cleanup(func() {
+		if err := secondLogger.Close(); err != nil {
+			t.Fatalf("could not close second logger: %s", err)
+		}
+	})
 
 	// Write again with the first logger to ensure it has not been affected
 	// by the new configuration on the second logger.
@@ -631,6 +617,29 @@ func TestCreateLogOutputAllDisabled(t *testing.T) {
 	}
 }
 
+func TestCoresCanBeClosed(t *testing.T) {
+	cfg := DefaultConfig(DefaultEnvironment)
+	cfg.ToFiles = true
+
+	cores := map[string]zapcore.Core{}
+	var err error
+	cores["File Output"], err = makeFileOutput(cfg, zapcore.DebugLevel)
+	if err != nil {
+		t.Fatalf("cannot create file output: %s", err)
+	}
+
+	cores["Syslog Output"], err = makeSyslogOutput(cfg, zapcore.DebugLevel)
+	if err != nil {
+		t.Fatalf("cannot create syslog output: %s", err)
+	}
+
+	for name, c := range cores {
+		if _, ok := c.(io.Closer); !ok {
+			t.Fatalf("the %s does not implement io.Closer", name)
+		}
+	}
+}
+
 func strField(key, val string) zapcore.Field {
 	return zapcore.Field{Type: zapcore.StringType, Key: key, String: val}
 }

logp/core_windows_test.go

@@ -0,0 +1,24 @@
+//go:build windows
+
+package logp
+
+import (
+	"io"
+	"testing"
+
+	"go.uber.org/zap/zapcore"
+)
+
+func TestEventLogOutputCanBeClosed(t *testing.T) {
+	cfg := DefaultConfig(DefaultEnvironment)
+	cfg.ToFiles = true
+
+	eventLog, err := makeEventLogOutput(cfg, zapcore.DebugLevel)
+	if err != nil {
+		t.Fatalf("cannot create eventLog output: %s", err)
+	}
+
+	if _, ok := eventLog.(io.Closer); !ok {
+		t.Fatal("the EventLog Output does not implement io.Closer")
+	}
+}

logp/logger.go

@@ -20,6 +20,7 @@ package logp
 import (
 	"bytes"
 	"fmt"
+	"io"
 
 	"go.elastic.co/ecszap"
 	"go.uber.org/zap"
@@ -259,6 +260,15 @@ func (l *Logger) Core() zapcore.Core {
 	return l.logger.Core()
 }
 
+// Close closes the underlying logger core/writer.
+func (l *Logger) Close() error {
+	if closer, ok := l.logger.Core().(io.Closer); ok {
+		return closer.Close()
+	}
+
+	return nil
+}
+
 // L returns an unnamed global logger.
 func L() *Logger {
 	return loadLogger().logger

logp/selective.go

@@ -18,6 +18,8 @@
 package logp
 
 import (
+	"io"
+
 	"go.uber.org/zap/zapcore"
 )
 
@@ -87,3 +89,12 @@ func (c *selectiveCore) Write(ent zapcore.Entry, fields []zapcore.Field) error {
 func (c *selectiveCore) Sync() error {
 	return c.core.Sync()
 }
+
+// Close calls Close on c.core if it implements io.Closer
+func (c *selectiveCore) Close() error {
+	if closer, ok := c.core.(io.Closer); ok {
+		return closer.Close()
+	}
+
+	return nil
+}

logp/syslog_unix.go

@@ -101,6 +101,11 @@ func (c *syslogCore) Clone() *syslogCore {
 	return &clone
 }
 
+// Close calls close in the syslog writer
+func (c *syslogCore) Close() error {
+	return c.writer.Close()
+}
+
 func replaceTabsWithSpaces(b []byte, n int) {
 	var count = 0
 	for i, v := range b {

logp/typedloggercore.go

@@ -18,7 +18,9 @@
 package logp
 
 import (
+	"errors"
 	"fmt"
+	"io"
 
 	"go.uber.org/zap/zapcore"
 )
@@ -102,3 +104,20 @@ func (t *typedLoggerCore) Write(e zapcore.Entry, fields []zapcore.Field) error {
 
 	return t.defaultCore.Write(e, fields)
 }
+
+// Close calls Close on any core that implements io.Close
+// all errors are joined by errors.Join and returned
+func (t *typedLoggerCore) Close() error {
+	errs := []error{}
+	if closer, ok := t.defaultCore.(io.Closer); ok {
+		closeErr := closer.Close()
+		errs = append(errs, closeErr)
+	}
+
+	if closer, ok := t.typedCore.(io.Closer); ok {
+		closeErr := closer.Close()
+		errs = append(errs, closeErr)
+	}
+
+	return errors.Join(errs...)
+}