feat: add newzaplogger and rework logptest (#335)

* feat: add newzaplogger and rework logptest

properly use zaptest logger instead of rewrapping the zap core

this should make full use of zaptest logger and only log
if a test fails

* Update logger.go

* Update logger.go

* Update mapstr_test.go

* refactor: add test helper for observer logs

* Update mapstr_test.go

* Update logger.go

* Update logp/logger.go

Co-authored-by: Khushi Jain <[email protected]>

* Update logp/logptest/logptest.go

Co-authored-by: Khushi Jain <[email protected]>

---------

Co-authored-by: Khushi Jain <[email protected]>

Author: kruskall

logp/logger.go

@@ -88,6 +88,11 @@ func NewDevelopmentLogger(selector string, options ...LogOption) (*Logger, error
 	return &Logger{logger, logger.Sugar(), make(map[string]struct{})}, nil
 }
 
+// NewZapLogger returns a logger based on the provided zap logger
+func NewZapLogger(logger *zap.Logger) (*Logger, error) {
+	return &Logger{logger, logger.Sugar(), make(map[string]struct{})}, nil
+}
+
 // NewInMemory returns a new in-memory logger along with the buffer to which it
 // logs. It's goroutine safe, but operating directly on the returned buffer is not.
 // This logger is primary intended for short and simple use-cases such as printing

logp/logptest/logptest.go

@@ -24,19 +24,27 @@ import (
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 	"go.uber.org/zap/zaptest"
+	"go.uber.org/zap/zaptest/observer"
 )
 
 // NewTestingLogger returns a testing suitable logp.Logger.
 func NewTestingLogger(t testing.TB, selector string, options ...logp.LogOption) *logp.Logger {
-	log := zaptest.NewLogger(t)
+	log := zaptest.NewLogger(t, zaptest.WrapOptions(options...))
 	log = log.Named(selector)
-	wrapCore := zap.WrapCore(func(zapcore.Core) zapcore.Core {
-		return log.Core()
-	})
-	options = append([]logp.LogOption{wrapCore}, options...)
-	logger, err := logp.NewDevelopmentLogger(selector, options...)
+
+	logger, err := logp.NewZapLogger(log)
 	if err != nil {
 		t.Fatal(err)
 	}
 	return logger
 }
+
+// NewTestingLoggerWithObserver returns a testing suitable logp.Logger and an observer
+func NewTestingLoggerWithObserver(t testing.TB, selector string) (*logp.Logger, *observer.ObservedLogs) {
+	observedCore, observedLogs := observer.New(zapcore.DebugLevel)
+	logger := NewTestingLogger(t, selector, zap.WrapCore(func(core zapcore.Core) zapcore.Core {
+		return observedCore
+	}))
+
+	return logger, observedLogs
+}

mapstr/mapstr_test.go

@@ -29,6 +29,7 @@ import (
 	"go.uber.org/zap/zapcore"
 
 	"github.com/elastic/elastic-agent-libs/logp"
+	"github.com/elastic/elastic-agent-libs/logp/logptest"
 )
 
 func TestMapStrUpdate(t *testing.T) {
@@ -382,7 +383,7 @@ func incrementMapstrValues(m M) {
 		case int:
 			m[k] = v + 1
 		case M:
-			incrementMapstrValues(m[k].(M))
+			incrementMapstrValues(v)
 		case []M:
 			for _, c := range v {
 				incrementMapstrValues(c)
@@ -956,9 +957,6 @@ func BenchmarkMapStrFlatten(b *testing.B) {
 
 // Ensure the MapStr is marshaled in logs the same way it is by json.Marshal.
 func TestMapStrJSONLog(t *testing.T) {
-	err := logp.DevelopmentSetup(logp.ToObserverOutput())
-	require.Nil(t, err)
-
 	m := M{
 		"test": 15,
 		"hello": M{
@@ -977,8 +975,10 @@ func TestMapStrJSONLog(t *testing.T) {
 	}
 	expectedJSON := string(data)
 
-	logp.NewLogger("test").Infow("msg", "m", m)
-	logs := logp.ObserverLogs().TakeAll()
+	logger, observedLogs := logptest.NewTestingLoggerWithObserver(t, "test")
+
+	logger.Infow("msg", "m", m)
+	logs := observedLogs.TakeAll()
 	if assert.Len(t, logs, 1) {
 		log := logs[0]
 

transport/logging_test.go

@@ -25,24 +25,20 @@ import (
 	"testing"
 	"time"
 
-	"github.com/elastic/elastic-agent-libs/logp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/elastic/elastic-agent-libs/logp/logptest"
 )
 
 func TestCloseConnectionError(t *testing.T) {
-	// observe all logs
-	if err := logp.DevelopmentSetup(logp.ToObserverOutput()); err != nil {
-		t.Fatalf("cannot initialise logger on development mode: %+v", err)
-	}
-
 	// Set up a test HTTP server
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		fmt.Fprintln(w, `{"status": "ok"}`)
 	}))
 	defer server.Close()
 
-	logger := logp.NewLogger("test")
+	logger, observedLogs := logptest.NewTestingLoggerWithObserver(t, "test")
 	// Set IdleConnTimeout to 2 seconds and a custom dialer
 	transport := &http.Transport{
 		IdleConnTimeout: 2 * time.Second,
@@ -70,7 +66,7 @@ func TestCloseConnectionError(t *testing.T) {
 	_, _ = io.ReadAll(resp.Body)
 	resp.Body.Close()
 
-	logs := logp.ObserverLogs().FilterMessageSnippet("Error reading from connection:").TakeAll()
+	logs := observedLogs.FilterMessageSnippet("Error reading from connection:").TakeAll()
 	assert.Equal(t, 0, len(logs), "did not ignore use of closed connection error")
 
 }