Add APIs to support buffering (#364)

This is a breaking change that adds a `Sync` method to `zapcore.Core`. The
intention here is to expose `Sync` all the way up to the loggers themselves;
this lets implementations buffer I/O, since the end user can `Sync` at the end
of main.

(Of course, nothing can save users if they directly panic or os.Exit from a
different goroutine.)

This fixes #355.

internal/observer/observer.go

@@ -124,6 +124,10 @@ func (o *observer) Write(ent zapcore.Entry, fields []zapcore.Field) error {
 	return o.sink(LoggedEntry{ent, fields})
 }
 
+func (o *observer) Sync() error {
+	return nil
+}
+
 type contextObserver struct {
 	zapcore.LevelEnabler
 	sink    func(LoggedEntry) error
@@ -151,3 +155,7 @@ func (co *contextObserver) Write(ent zapcore.Entry, fields []zapcore.Field) erro
 	all = append(all, fields...)
 	return co.sink(LoggedEntry{ent, all})
 }
+
+func (co *contextObserver) Sync() error {
+	return nil
+}

logger.go

@@ -210,6 +210,11 @@ func (log *Logger) Fatal(msg string, fields ...zapcore.Field) {
 	}
 }
 
+// Sync flushes any buffered log entries.
+func (log *Logger) Sync() error {
+	return log.core.Sync()
+}
+
 // Core returns the underlying zapcore.Core.
 func (log *Logger) Core() zapcore.Core {
 	return log.core

logger_test.go

@@ -21,6 +21,7 @@
 package zap
 
 import (
+	"errors"
 	"sync"
 	"testing"
 
@@ -313,6 +314,26 @@ func TestLoggerWriteFailure(t *testing.T) {
 	assert.True(t, errSink.Called(), "Expected logging an internal error to call Sync the error sink.")
 }
 
+func TestLoggerSync(t *testing.T) {
+	withLogger(t, DebugLevel, nil, func(logger *Logger, _ *observer.ObservedLogs) {
+		assert.NoError(t, logger.Sync(), "Expected syncing a test logger to succeed.")
+		assert.NoError(t, logger.Sugar().Sync(), "Expected syncing a sugared logger to succeed.")
+	})
+}
+
+func TestLoggerSyncFail(t *testing.T) {
+	noSync := &testutils.Buffer{}
+	err := errors.New("fail")
+	noSync.SetError(err)
+	logger := New(zapcore.NewCore(
+		zapcore.NewJSONEncoder(zapcore.EncoderConfig{}),
+		noSync,
+		DebugLevel,
+	))
+	assert.Equal(t, err, logger.Sync(), "Expected Logger.Sync to propagate errors.")
+	assert.Equal(t, err, logger.Sugar().Sync(), "Expected SugaredLogger.Sync to propagate errors.")
+}
+
 func TestLoggerAddCaller(t *testing.T) {
 	tests := []struct {
 		options []Option

sugar.go

@@ -200,6 +200,11 @@ func (s *SugaredLogger) Fatalw(msg string, keysAndValues ...interface{}) {
 	s.log(FatalLevel, msg, nil, keysAndValues)
 }
 
+// Sync flushes any buffered log entries.
+func (s *SugaredLogger) Sync() error {
+	return s.base.Sync()
+}
+
 func (s *SugaredLogger) log(lvl zapcore.Level, template string, fmtArgs []interface{}, context []interface{}) {
 	// If logging at this level is completely disabled, skip the overhead of
 	// string formatting.

zapcore/core.go

@@ -42,6 +42,8 @@ type Core interface {
 	// If called, Write should always log the Entry and Fields; it should not
 	// replicate the logic of Check.
 	Write(Entry, []Field) error
+	// Sync flushes buffered logs (if any).
+	Sync() error
 }
 
 type nopCore struct{}
@@ -52,6 +54,7 @@ func (nopCore) Enabled(Level) bool                            { return false }
 func (n nopCore) With([]Field) Core                           { return n }
 func (nopCore) Check(_ Entry, ce *CheckedEntry) *CheckedEntry { return ce }
 func (nopCore) Write(Entry, []Field) error                    { return nil }
+func (nopCore) Sync() error                                   { return nil }
 
 // NewCore creates a Core that writes logs to a WriteSyncer.
 func NewCore(enc Encoder, ws WriteSyncer, enab LevelEnabler) Core {
@@ -93,11 +96,15 @@ func (c *ioCore) Write(ent Entry, fields []Field) error {
 	}
 	if ent.Level > ErrorLevel {
 		// Since we may be crashing the program, sync the output.
-		return c.out.Sync()
+		return c.Sync()
 	}
 	return nil
 }
 
+func (c *ioCore) Sync() error {
+	return c.out.Sync()
+}
+
 func (c *ioCore) clone() *ioCore {
 	return &ioCore{
 		LevelEnabler: c.LevelEnabler,

zapcore/core_test.go

@@ -21,6 +21,7 @@
 package zapcore_test
 
 import (
+	"errors"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -62,6 +63,7 @@ func TestNopCore(t *testing.T) {
 		assert.False(t, core.Enabled(level), "Expected all levels to be disabled in no-op core.")
 		assert.Equal(t, ce, core.Check(entry, ce), "Expected no-op Check to return checked entry unchanged.")
 		assert.NoError(t, core.Write(entry, nil), "Expected no-op Writes to always succeed.")
+		assert.NoError(t, core.Sync(), "Expected no-op Syncs to always succeed.")
 	}
 }
 
@@ -80,6 +82,7 @@ func TestIOCore(t *testing.T) {
 		temp,
 		InfoLevel,
 	).With([]Field{makeInt64Field("k", 1)})
+	defer assert.NoError(t, core.Sync(), "Expected Syncing a temp file to succeed.")
 
 	if ce := core.Check(Entry{Level: DebugLevel, Message: "debug"}, nil); ce != nil {
 		ce.Write(makeInt64Field("k", 2))
@@ -102,6 +105,25 @@ func TestIOCore(t *testing.T) {
 	)
 }
 
+func TestIOCoreSyncFail(t *testing.T) {
+	sink := &testutils.Discarder{}
+	err := errors.New("failed")
+	sink.SetError(err)
+
+	core := NewCore(
+		NewJSONEncoder(testEncoderConfig()),
+		sink,
+		DebugLevel,
+	)
+
+	assert.Equal(
+		t,
+		err,
+		core.Sync(),
+		"Expected core.Sync to return errors from underlying WriteSyncer.",
+	)
+}
+
 func TestIOCoreSyncsOutput(t *testing.T) {
 	tests := []struct {
 		entry      Entry

zapcore/sampler_test.go

@@ -139,10 +139,6 @@ type countingCore struct {
 	logs atomic.Uint32
 }
 
-func (c *countingCore) Enabled(Level) bool {
-	return true
-}
-
 func (c *countingCore) Check(ent Entry, ce *CheckedEntry) *CheckedEntry {
 	return ce.AddCore(ent, c)
 }
@@ -152,9 +148,9 @@ func (c *countingCore) Write(Entry, []Field) error {
 	return nil
 }
 
-func (c *countingCore) With([]Field) Core {
-	return c
-}
+func (c *countingCore) With([]Field) Core { return c }
+func (*countingCore) Enabled(Level) bool  { return true }
+func (*countingCore) Sync() error         { return nil }
 
 func TestSamplerConcurrent(t *testing.T) {
 	const (

zapcore/tee.go

@@ -71,3 +71,11 @@ func (mc multiCore) Write(ent Entry, fields []Field) error {
 	}
 	return errs.AsError()
 }
+
+func (mc multiCore) Sync() error {
+	var errs multierror.Error
+	for i := range mc {
+		errs = errs.Append(mc[i].Sync())
+	}
+	return errs.AsError()
+}

zapcore/tee_test.go

@@ -21,9 +21,11 @@
 package zapcore_test
 
 import (
+	"errors"
 	"testing"
 
 	"go.uber.org/zap/internal/observer"
+	"go.uber.org/zap/testutils"
 	. "go.uber.org/zap/zapcore"
 
 	"github.com/stretchr/testify/assert"
@@ -133,3 +135,23 @@ func TestTeeEnabled(t *testing.T) {
 		assert.Equal(t, tt.enabled, tee.Enabled(tt.lvl), "Unexpected Enabled result for level %s.", tt.lvl)
 	}
 }
+
+func TestTeeSync(t *testing.T) {
+	tee := NewTee(
+		observer.New(InfoLevel, nil, false),
+		observer.New(WarnLevel, nil, false),
+	)
+	assert.NoError(t, tee.Sync(), "Unexpected error from Syncing a tee.")
+
+	sink := &testutils.Discarder{}
+	err := errors.New("failed")
+	sink.SetError(err)
+
+	noSync := NewCore(
+		NewJSONEncoder(testEncoderConfig()),
+		sink,
+		DebugLevel,
+	)
+	tee = NewTee(tee, noSync)
+	assert.Equal(t, err, tee.Sync(), "Expected an error when part of tee can't Sync.")
+}