From 45752f5810806d2f6943f49ae05679804f590338 Mon Sep 17 00:00:00 2001 From: Joshua T Corbin Date: Tue, 6 Dec 2016 15:31:47 -0800 Subject: [PATCH] Drop CheckedMessage --- checked_message.go | 145 ------------------------ checked_message_bench_test.go | 99 ----------------- checked_message_test.go | 200 ---------------------------------- 3 files changed, 444 deletions(-) delete mode 100644 checked_message.go delete mode 100644 checked_message_bench_test.go delete mode 100644 checked_message_test.go diff --git a/checked_message.go b/checked_message.go deleted file mode 100644 index 1d6623926..000000000 --- a/checked_message.go +++ /dev/null @@ -1,145 +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 "sync" - -var _cmPool = sync.Pool{ - New: func() interface{} { - return &CheckedMessage{ - lvl: invalidLevel, - } - }, -} - -// A CheckedMessage is the result of a call to Logger.Check, which allows -// especially performance-sensitive applications to avoid allocations for disabled -// or heavily sampled log levels. -type CheckedMessage struct { - logger Logger - safeToWrite bool - lvl Level - msg string - - // singly linked list built by Chain - next *CheckedMessage // carried by each part of Chain-ed list - tail *CheckedMessage // non-nil only in the head of a Chain-ed list - // NOTE: suffixes are NOT valid lists, since they lack a tail pointer; tail - // pointer exists solely as an optimization for calling head.Chain. -} - -// NewCheckedMessage constructs a CheckedMessage. It's only intended for use by -// wrapper libraries, and shouldn't be necessary in application code. -func NewCheckedMessage(logger Logger, lvl Level, msg string) *CheckedMessage { - m := _cmPool.Get().(*CheckedMessage) - m.safeToWrite, m.logger, m.lvl, m.msg = true, logger, lvl, msg - return m -} - -// Write logs the pre-checked message with the supplied fields. It will call -// the underlying level method (Debug, Info, Warn, Error, DPanic, Panic, and -// Fatal) for the defined levels; the Log method is only called for unknown -// logging levels. -// -// It MUST be called at most once, since Write will return the *CheckedMessage -// to an internal pool for potentially immediate re-use; re-using a -// *CheckedMessage after calling Write() will result in data races or other -// undefined behavior. An attempt is made to detect and DPanic log any re-use, -// but such detection is not guaranteed due to race conditions. -func (m *CheckedMessage) Write(fields ...Field) { - if m == nil { - return - } - - if !m.safeToWrite { - // we're living in racy times, so copy what we can out of the pointer - // that we have, and at least tell the user something - if logger := m.logger; logger != nil { - lvl, msg := m.lvl, m.msg - logger.DPanic( - "Must not call zap.(*CheckedMessage).Write() more than once", - Nest("prior", Stringer("level", lvl), String("msg", msg)), - ) - } - return - } - m.safeToWrite = false - - switch m.lvl { - case DebugLevel: - m.logger.Debug(m.msg, fields...) - case InfoLevel: - m.logger.Info(m.msg, fields...) - case WarnLevel: - m.logger.Warn(m.msg, fields...) - case ErrorLevel: - m.logger.Error(m.msg, fields...) - case PanicLevel: - m.logger.Panic(m.msg, fields...) - case FatalLevel: - m.logger.Fatal(m.msg, fields...) - default: - m.logger.Log(m.lvl, m.msg, fields...) - } - - m.next.Write(fields...) - m.next, m.tail = nil, nil - _cmPool.Put(m) -} - -// Chain combines two or more CheckedMessages. If the receiver message is not -// OK(), the passed message is returned. Otherwise if the passed message is -// OK(), then it is retained such that its Write() will be called after the -// receiver's Write(), and any previously Chain()'ed messages already so -// retained. -func (m *CheckedMessage) Chain(ms ...*CheckedMessage) *CheckedMessage { - for _, m2 := range ms { - if !m2.OK() { - continue - } - if m.OK() { - m.push(m2) - } else { - m = m2 - } - } - return m -} - -func (m *CheckedMessage) push(next *CheckedMessage) { - if m.tail != nil { - m.tail.next = next - } else if m.next != nil { - m.logger.DPanic("invalid CheckedMessage linked list; did we lose our head?", String("original", m.msg)) - } else { - m.next = next - } - if next.tail != nil { - m.tail, next.tail = next.tail, nil - } else { - m.tail = next - } -} - -// OK checks whether it's safe to call Write. -func (m *CheckedMessage) OK() bool { - return m != nil -} diff --git a/checked_message_bench_test.go b/checked_message_bench_test.go deleted file mode 100644 index 296beee57..000000000 --- a/checked_message_bench_test.go +++ /dev/null @@ -1,99 +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" - - "github.com/uber-go/atomic" -) - -func benchmarkLoggers(levels []Level, options ...Option) []Logger { - logs := make([]Logger, len(levels)) - for i, lvl := range levels { - logs[i] = New(NullEncoder(), append([]Option{lvl}, options...)...) - } - return logs -} - -func runIndexedPara(b *testing.B, f func(pb *testing.PB, j int)) { - p := atomic.NewInt32(0) - b.ResetTimer() - b.RunParallel(func(pb *testing.PB) { - f(pb, int(p.Inc())) - }) -} - -func BenchmarkCheckedMessage_Chain(b *testing.B) { - logs := benchmarkLoggers([]Level{InfoLevel, ErrorLevel}, DiscardOutput) - data := []struct { - lvl Level - msg string - }{ - {DebugLevel, "meh"}, - {InfoLevel, "fwiw"}, - {ErrorLevel, "hey!"}, - } - runIndexedPara(b, func(pb *testing.PB, j int) { - myInfoLog := logs[0].With(Int("p", j)) - myErrorLog := logs[1].With(Int("p", j)) - i := 0 - for pb.Next() { - d := data[i%len(data)] - cm := myInfoLog.Check(d.lvl, d.msg) - cm = cm.Chain(myErrorLog.Check(d.lvl, d.msg)) - if cm.OK() { - cm.Write(Int("i", i)) - } - i++ - } - }) -} - -func BenchmarkCheckedMessage_Chain_sliceLoggers(b *testing.B) { - logs := benchmarkLoggers([]Level{InfoLevel, ErrorLevel}, DiscardOutput) - data := []struct { - lvl Level - msg string - }{ - {DebugLevel, "meh"}, - {InfoLevel, "fwiw"}, - {ErrorLevel, "hey!"}, - } - runIndexedPara(b, func(pb *testing.PB, j int) { - myLogs := make([]Logger, len(logs)) - for i, log := range logs { - myLogs[i] = log.With(Int("p", j)) - } - i := 0 - for pb.Next() { - d := data[i%len(data)] - var cm *CheckedMessage - for _, log := range myLogs { - cm = cm.Chain(log.Check(d.lvl, d.msg)) - } - if cm.OK() { - cm.Write(Int("i", i)) - } - i++ - } - }) -} diff --git a/checked_message_test.go b/checked_message_test.go deleted file mode 100644 index ac24e4efc..000000000 --- a/checked_message_test.go +++ /dev/null @@ -1,200 +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" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestJSONLoggerCheck(t *testing.T) { - withJSONLogger(t, opts(InfoLevel), func(logger Logger, buf *testBuffer) { - assert.False( - t, - logger.Check(DebugLevel, "Debug.").OK(), - "Expected CheckedMessage to be not OK at disabled levels.", - ) - - cm := logger.Check(InfoLevel, "Info.") - require.True(t, cm.OK(), "Expected CheckedMessage to be OK at enabled levels.") - cm.Write(Int("magic", 42)) - assert.Equal( - t, - `{"level":"info","msg":"Info.","magic":42}`, - buf.Stripped(), - "Unexpected output after writing a CheckedMessage.", - ) - }) -} - -func TestCheckedMessageUnsafeWrite(t *testing.T) { - withJSONLogger(t, opts(InfoLevel), func(logger Logger, buf *testBuffer) { - cm := logger.Check(InfoLevel, "bob lob law blog") - cm.Write() - cm.Write() - assert.Equal(t, []string{ - `{"level":"info","msg":"bob lob law blog"}`, - `{"level":"dpanic","msg":"Must not call zap.(*CheckedMessage).Write() more than once","prior":{"level":"info","msg":"bob lob law blog"}}`, - }, buf.Lines(), "Expected one lob log, and a fatal") - }) -} - -func TestCheckedMessage_Chain(t *testing.T) { - withJSONLogger(t, opts(InfoLevel), func(logger Logger, buf *testBuffer) { - loga := logger.With(String("name", "A")) - logb := logger.With(String("name", "B")) - logc := logger.With(String("name", "C")) - - for i, tt := range []struct { - cms []*CheckedMessage - expectedOK bool - desc string - expected []string - }{ - // not-ok base cases - { - cms: []*CheckedMessage{nil}, - expectedOK: false, - desc: "nil", - expected: []string{}, - }, - { - cms: []*CheckedMessage{ - nil, - loga.Check(DebugLevel, "nope"), - }, - expectedOK: false, - desc: "nil, A decline", - expected: []string{}, - }, - { - cms: []*CheckedMessage{ - nil, - loga.Check(DebugLevel, "nope"), - logb.Check(DebugLevel, "nope"), - }, - expectedOK: false, - desc: "nil, A and B decline", - expected: []string{}, - }, - - // singleton ok cases - { - cms: []*CheckedMessage{ - nil, - loga.Check(InfoLevel, "apple"), - }, - expectedOK: true, - desc: "nil, A OK", - expected: []string{ - `{"level":"info","msg":"apple","name":"A","i":3}`, - }, - }, - { - cms: []*CheckedMessage{ - loga.Check(InfoLevel, "banana"), - logb.Check(DebugLevel, "banana"), - }, - expectedOK: true, - desc: "A OK, B decline", - expected: []string{ - `{"level":"info","msg":"banana","name":"A","i":4}`, - }, - }, - - // compound ok cases - { - cms: []*CheckedMessage{ - loga.Check(InfoLevel, "cherry"), - logb.Check(InfoLevel, "cherry"), - }, - expectedOK: true, - desc: "A and B OK", - expected: []string{ - `{"level":"info","msg":"cherry","name":"A","i":5}`, - `{"level":"info","msg":"cherry","name":"B","i":5}`, - }, - }, - { - cms: []*CheckedMessage{ - loga.Check(InfoLevel, "durian"), - nil, - logb.Check(InfoLevel, "durian"), - }, - expectedOK: true, - desc: "A OK, nil, B OK", - expected: []string{ - `{"level":"info","msg":"durian","name":"A","i":6}`, - `{"level":"info","msg":"durian","name":"B","i":6}`, - }, - }, - { - cms: []*CheckedMessage{ - loga.Check(InfoLevel, "elderberry"), - logb.Check(InfoLevel, "elderberry"), - nil, - }, - expectedOK: true, - desc: "A OK, B OK, nil", - expected: []string{ - `{"level":"info","msg":"elderberry","name":"A","i":7}`, - `{"level":"info","msg":"elderberry","name":"B","i":7}`, - }, - }, - - // trees - { - cms: []*CheckedMessage{ - loga.Check(InfoLevel, "fig"), - logb.Check(InfoLevel, "fig").Chain(logc.Check(InfoLevel, "fig")), - }, - expectedOK: true, - desc: "A OK, ( B OK, C OK )", - expected: []string{ - `{"level":"info","msg":"fig","name":"A","i":8}`, - `{"level":"info","msg":"fig","name":"B","i":8}`, - `{"level":"info","msg":"fig","name":"C","i":8}`, - }, - }, - { - cms: []*CheckedMessage{ - logb.Check(InfoLevel, "guava").Chain(logc.Check(InfoLevel, "guava")), - loga.Check(InfoLevel, "guava"), - }, - expectedOK: true, - desc: "( B OK, C OK ), A OK", - expected: []string{ - `{"level":"info","msg":"guava","name":"B","i":9}`, - `{"level":"info","msg":"guava","name":"C","i":9}`, - `{"level":"info","msg":"guava","name":"A","i":9}`, - }, - }, - } { - cm := tt.cms[0].Chain(tt.cms[1:]...) - assert.Equal(t, cm.OK(), tt.expectedOK, "%s: expected cm.OK()", tt.desc) - cm.Write(Int("i", i)) - assert.Equal(t, tt.expected, buf.Lines(), "expected output from MultiCheckedMessage") - buf.Reset() - } - }) -}