Update lazy logger not to materialize unless it's being written to (#1519)

Lazy logger is frequently used to delay (expensive) logger
initialization unless we actually write to it.

This works for a pattern like:
```golang
logger := logger.With(.../*fields)

err := foo()
if err != nil {
   logger.Info("foo: %w, err)
}
```

Another common pattern, though, is
```golang
logger := logger.With(.../*fields)

// a lot of code

logger.Debug("something happened") // only Info is enabled
```

Here, despite the logs being discarded, we still initialize the logger
even though it might be expensive.

This diff attempts to mitigate this by using the original core for level
checking. This, afaiu, should be safe as `With` is only for field
creation, and should not be changing log levels.

Preserving the original core to avoid race conditions (torn interfaces)
on actual `With` initialization.

Author: rabbbit

zapcore/lazy_with.go

@@ -23,7 +23,8 @@ package zapcore
 import "sync"
 
 type lazyWithCore struct {
-	core   Core
+	core         Core
+	originalCore Core
 	sync.Once
 	fields []Field
 }
@@ -32,14 +33,15 @@ type lazyWithCore struct {
 // the logger is written to (or is further chained in a lon-lazy manner).
 func NewLazyWith(core Core, fields []Field) Core {
 	return &lazyWithCore{
-		core:   core,
-		fields: fields,
+		core:         nil, // core is allocated once `initOnce` is called.
+		originalCore: core,
+		fields:       fields,
 	}
 }
 
 func (d *lazyWithCore) initOnce() {
 	d.Once.Do(func() {
-		d.core = d.core.With(d.fields)
+		d.core = d.originalCore.With(d.fields)
 	})
 }
 
@@ -49,13 +51,19 @@ func (d *lazyWithCore) With(fields []Field) Core {
 }
 
 func (d *lazyWithCore) Check(e Entry, ce *CheckedEntry) *CheckedEntry {
+	// This is safe because `lazyWithCore` doesn't change the level.
+	// So we can delagate the level check, any not `initOnce`
+	// just for the check.
+	if !d.originalCore.Enabled(e.Level) {
+		return ce
+	}
 	d.initOnce()
 	return d.core.Check(e, ce)
 }
 
 func (d *lazyWithCore) Enabled(level Level) bool {
-	d.initOnce()
-	return d.core.Enabled(level)
+	// Like above, this is safe because `lazyWithCore` doesn't change the level.
+	return d.originalCore.Enabled(level)
 }
 
 func (d *lazyWithCore) Write(e Entry, fields []Field) error {

zapcore/lazy_with_test.go

@@ -65,6 +65,7 @@ func TestLazyCore(t *testing.T) {
 		initialFields []zapcore.Field
 		withChains    [][]zapcore.Field
 		wantLogs      []observer.LoggedEntry
+		wantSkipInit  bool
 	}{
 		{
 			name:     "no logging, no with, inner core with never called, inner core check never called",
@@ -94,6 +95,38 @@ func TestLazyCore(t *testing.T) {
 			},
 			wantLogs: []observer.LoggedEntry{},
 		},
+		{
+			name: "1 log, discarded, no-chains, inner core with never called",
+			entries: []zapcore.Entry{
+				{Level: zapcore.DebugLevel, Message: "log-at-debug"},
+			},
+			initialFields: []zapcore.Field{
+				makeInt64Field("a", 11), makeInt64Field("b", 22),
+			},
+			wantLogs:     []observer.LoggedEntry{},
+			wantSkipInit: true,
+		},
+		{
+			name: "1 log, logged, no-chains, inner core with called",
+			entries: []zapcore.Entry{
+				{Level: zapcore.WarnLevel, Message: "log-at-warn"},
+			},
+			initialFields: []zapcore.Field{
+				makeInt64Field("a", 11), makeInt64Field("b", 22),
+			},
+			wantLogs: []observer.LoggedEntry{
+				{
+					Entry: zapcore.Entry{
+						Level:   zapcore.WarnLevel,
+						Message: "log-at-warn",
+					},
+					Context: []zapcore.Field{
+						makeInt64Field("a", 11),
+						makeInt64Field("b", 22),
+					},
+				},
+			},
+		},
 		{
 			name: "2 logs, 1 dropped, 2-chained with, inner core with called once, inner core check never called",
 			entries: []zapcore.Entry{
@@ -142,7 +175,7 @@ func TestLazyCore(t *testing.T) {
 						ce.Write()
 					}
 				}
-				if len(tt.entries) > 0 || len(tt.withChains) > 0 {
+				if !tt.wantSkipInit && (len(tt.entries) > 0 || len(tt.withChains) > 0) {
 					checkCounts(1, "expected with calls because the logger had entries or with chains")
 				} else {
 					checkCounts(0, "expected no with calls because the logger is not used yet")