Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add APIs to support buffering #364

Merged
merged 2 commits into from
Mar 10, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions internal/observer/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
5 changes: 5 additions & 0 deletions logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package zap

import (
"errors"
"sync"
"testing"

Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions sugar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 8 additions & 1 deletion zapcore/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 22 additions & 0 deletions zapcore/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package zapcore_test

import (
"errors"
"io/ioutil"
"os"
"testing"
Expand Down Expand Up @@ -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.")
}
}

Expand All @@ -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))
Expand All @@ -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
Expand Down
10 changes: 3 additions & 7 deletions zapcore/sampler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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 (
Expand Down
8 changes: 8 additions & 0 deletions zapcore/tee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
22 changes: 22 additions & 0 deletions zapcore/tee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.")
}