From 35cd7966cc81adc0449c91b3f6b1939812be73aa Mon Sep 17 00:00:00 2001 From: Joshua T Corbin Date: Tue, 6 Dec 2016 15:09:40 -0800 Subject: [PATCH] Simplify Entry and co (#198) * Shift hook running into Meta * Firm up a convenience Meta.Encode method * Unify time.Now calling, drop test stub --- entry.go | 25 ++----------------------- entry_test.go | 43 ------------------------------------------- hook.go | 2 +- logger.go | 16 +++------------- meta.go | 35 ++++++++++++++++++++++++++++++++++- 5 files changed, 40 insertions(+), 81 deletions(-) delete mode 100644 entry_test.go diff --git a/entry.go b/entry.go index e3cd3e9fc..2f85457f6 100644 --- a/entry.go +++ b/entry.go @@ -20,15 +20,7 @@ package zap -import ( - "sync" - "time" -) - -var ( - _timeNow = time.Now // for tests - _entryPool = sync.Pool{New: func() interface{} { return &Entry{} }} -) +import "time" // An Entry represents a complete log message. The entry's structured context // is already serialized, but the log level, time, and message are available @@ -43,20 +35,7 @@ type Entry struct { enc Encoder } -func newEntry(lvl Level, msg string, enc Encoder) *Entry { - e := _entryPool.Get().(*Entry) - e.Level = lvl - e.Message = msg - e.Time = _timeNow().UTC() - e.enc = enc - return e -} - // Fields returns a mutable reference to the entry's accumulated context. -func (e *Entry) Fields() KeyValue { +func (e Entry) Fields() KeyValue { return e.enc } - -func (e *Entry) free() { - _entryPool.Put(e) -} diff --git a/entry_test.go b/entry_test.go deleted file mode 100644 index cc1720be2..000000000 --- a/entry_test.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (c) 2016 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -package zap - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" -) - -func stubNow(afterEpoch time.Duration) func() { - prev := _timeNow - t := time.Unix(0, int64(afterEpoch)) - _timeNow = func() time.Time { return t } - return func() { _timeNow = prev } -} - -func TestNewEntry(t *testing.T) { - defer stubNow(0)() - e := newEntry(DebugLevel, "hello", nil) - assert.Equal(t, DebugLevel, e.Level, "Unexpected log level.") - assert.Equal(t, time.Unix(0, 0).UTC(), e.Time, "Unexpected time.") - assert.Nil(t, e.Fields(), "Unexpected fields.") -} diff --git a/hook.go b/hook.go index e1b65a94c..dad2605d5 100644 --- a/hook.go +++ b/hook.go @@ -32,7 +32,7 @@ var ( errCaller = errors.New("failed to get caller") // Skip Caller, Logger.log, and the leveled Logger method when using // runtime.Caller. - _callerSkip = 3 + _callerSkip = 4 ) // A Hook is executed each time the logger writes an Entry. It can modify the diff --git a/logger.go b/logger.go index d513979b0..776ec9546 100644 --- a/logger.go +++ b/logger.go @@ -22,6 +22,7 @@ package zap import ( "os" + "time" ) // For tests. @@ -125,21 +126,10 @@ func (log *logger) log(lvl Level, msg string, fields []Field) { return } - temp := log.Encoder.Clone() - addFields(temp, fields) - - entry := newEntry(lvl, msg, temp) - for _, hook := range log.Hooks { - if err := hook(entry); err != nil { - log.InternalError("hook", err) - } - } - - if err := temp.WriteEntry(log.Output, entry.Message, entry.Level, entry.Time); err != nil { + t := time.Now().UTC() + if err := log.Encode(log.Output, t, lvl, msg, fields); err != nil { log.InternalError("encoder", err) } - temp.Free() - entry.free() if lvl > ErrorLevel { // Sync on Panic and Fatal, since they may crash the program. diff --git a/meta.go b/meta.go index 520cea131..41c2e73e5 100644 --- a/meta.go +++ b/meta.go @@ -22,9 +22,18 @@ package zap import ( "fmt" + "io" "os" + "sync" + "time" ) +var _entryPool = sync.Pool{ + New: func() interface{} { + return &Entry{} + }, +} + // Meta is implementation-agnostic state management for Loggers. Most Logger // implementations can reduce the required boilerplate by embedding a Meta. // @@ -83,6 +92,30 @@ func (m Meta) Check(log Logger, lvl Level, msg string) *CheckedMessage { // ErrorOutput. This method should only be used to report internal logger // problems and should not be used to report user-caused problems. func (m Meta) InternalError(cause string, err error) { - fmt.Fprintf(m.ErrorOutput, "%v %s error: %v\n", _timeNow().UTC(), cause, err) + fmt.Fprintf(m.ErrorOutput, "%v %s error: %v\n", time.Now().UTC(), cause, err) m.ErrorOutput.Sync() } + +// Encode runs any Hook functions and then writes an encoded log entry to the +// given io.Writer, returning any error. +func (m Meta) Encode(w io.Writer, t time.Time, lvl Level, msg string, fields []Field) error { + enc := m.Encoder.Clone() + addFields(enc, fields) + if len(m.Hooks) >= 0 { + entry := _entryPool.Get().(*Entry) + entry.Level = lvl + entry.Message = msg + entry.Time = t + entry.enc = enc + for _, hook := range m.Hooks { + if err := hook(entry); err != nil { + m.InternalError("hook", err) + } + } + msg, enc = entry.Message, entry.enc + _entryPool.Put(entry) + } + err := enc.WriteEntry(w, msg, lvl, t) + enc.Free() + return err +}