From 8d4533cde09712d093906e2892cd99c51c2a78f1 Mon Sep 17 00:00:00 2001 From: Dimitri Roche Date: Wed, 27 Jun 2018 19:43:35 -0400 Subject: [PATCH] Ability to build logger with custom Sink (#572) Using the same global-registry pattern that we're using for encoders, let users register new sinks (like the built-in `stdout` and `stderr`). --- sink.go | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++ sink_test.go | 76 ++++++++++++++++++++++++++++++++++++++++ writer.go | 34 ++++++++++-------- writer_test.go | 72 +++++++++++++++++++++++++++----------- 4 files changed, 240 insertions(+), 36 deletions(-) create mode 100644 sink.go create mode 100644 sink_test.go diff --git a/sink.go b/sink.go new file mode 100644 index 000000000..8f3670d3a --- /dev/null +++ b/sink.go @@ -0,0 +1,94 @@ +// 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 ( + "errors" + "fmt" + "io" + "os" + "sync" + + "go.uber.org/zap/zapcore" +) + +var ( + _sinkMutex sync.RWMutex + _sinkFactories map[string]func() (Sink, error) +) + +func init() { + resetSinkRegistry() +} + +func resetSinkRegistry() { + _sinkMutex.Lock() + defer _sinkMutex.Unlock() + _sinkFactories = map[string]func() (Sink, error){ + "stdout": func() (Sink, error) { return nopCloserSink{os.Stdout}, nil }, + "stderr": func() (Sink, error) { return nopCloserSink{os.Stderr}, nil }, + } +} + +type errSinkNotFound struct { + key string +} + +func (e *errSinkNotFound) Error() string { + return fmt.Sprintf("no sink found for %q", e.key) +} + +// Sink defines the interface to write to and close logger destinations. +type Sink interface { + zapcore.WriteSyncer + io.Closer +} + +// RegisterSink adds a Sink at the given key so it can be referenced +// in config OutputPaths. +func RegisterSink(key string, sinkFactory func() (Sink, error)) error { + _sinkMutex.Lock() + defer _sinkMutex.Unlock() + if key == "" { + return errors.New("sink key cannot be blank") + } + if _, ok := _sinkFactories[key]; ok { + return fmt.Errorf("sink already registered for key %q", key) + } + _sinkFactories[key] = sinkFactory + return nil +} + +// newSink invokes the registered sink factory to create and return the +// sink for the given key. Returns errSinkNotFound if the key cannot be found. +func newSink(key string) (Sink, error) { + _sinkMutex.RLock() + defer _sinkMutex.RUnlock() + sinkFactory, ok := _sinkFactories[key] + if !ok { + return nil, &errSinkNotFound{key} + } + return sinkFactory() +} + +type nopCloserSink struct{ zapcore.WriteSyncer } + +func (nopCloserSink) Close() error { return nil } diff --git a/sink_test.go b/sink_test.go new file mode 100644 index 000000000..c3169b2de --- /dev/null +++ b/sink_test.go @@ -0,0 +1,76 @@ +// 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 ( + "errors" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRegisterSink(t *testing.T) { + tests := []struct { + name string + key string + factory func() (Sink, error) + wantError bool + }{ + {"valid", "valid", func() (Sink, error) { return nopCloserSink{os.Stdout}, nil }, false}, + {"empty", "", func() (Sink, error) { return nopCloserSink{os.Stdout}, nil }, true}, + {"stdout", "stdout", func() (Sink, error) { return nopCloserSink{os.Stdout}, nil }, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := RegisterSink(tt.key, tt.factory) + if tt.wantError { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + assert.NotNil(t, _sinkFactories[tt.key], "expected the factory to be present") + } + }) + } +} + +func TestNewSink(t *testing.T) { + defer resetSinkRegistry() + errTestSink := errors.New("test erroring") + err := RegisterSink("errors", func() (Sink, error) { return nil, errTestSink }) + assert.Nil(t, err) + tests := []struct { + key string + err error + }{ + {"stdout", nil}, + {"errors", errTestSink}, + {"nonexistent", &errSinkNotFound{"nonexistent"}}, + } + + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + _, err := newSink(tt.key) + assert.Equal(t, tt.err, err) + }) + } +} diff --git a/writer.go b/writer.go index 16f55ce48..559d070f5 100644 --- a/writer.go +++ b/writer.go @@ -21,6 +21,7 @@ package zap import ( + "io" "io/ioutil" "os" @@ -49,29 +50,32 @@ func Open(paths ...string) (zapcore.WriteSyncer, func(), error) { func open(paths []string) ([]zapcore.WriteSyncer, func(), error) { var openErr error writers := make([]zapcore.WriteSyncer, 0, len(paths)) - files := make([]*os.File, 0, len(paths)) + closers := make([]io.Closer, 0, len(paths)) close := func() { - for _, f := range files { - f.Close() + for _, c := range closers { + c.Close() } } for _, path := range paths { - switch path { - case "stdout": - writers = append(writers, os.Stdout) - // Don't close standard out. + sink, err := newSink(path) + if err == nil { + // Using a registered sink constructor. + writers = append(writers, sink) + closers = append(closers, sink) continue - case "stderr": - writers = append(writers, os.Stderr) - // Don't close standard error. + } + if _, ok := err.(*errSinkNotFound); ok { + // No named sink constructor, use key as path to log file. + f, e := os.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) + openErr = multierr.Append(openErr, e) + if e == nil { + writers = append(writers, f) + closers = append(closers, f) + } continue } - f, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) + // Sink constructor failed. openErr = multierr.Append(openErr, err) - if err == nil { - writers = append(writers, f) - files = append(files, f) - } } if openErr != nil { diff --git a/writer_test.go b/writer_test.go index 1293832ab..6efd60965 100644 --- a/writer_test.go +++ b/writer_test.go @@ -21,8 +21,12 @@ package zap import ( + "encoding/hex" + "errors" "io/ioutil" + "math/rand" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -44,28 +48,25 @@ func TestOpenNoPaths(t *testing.T) { } func TestOpen(t *testing.T) { - temp, err := ioutil.TempFile("", "zap-open-test") - require.NoError(t, err, "Couldn't create a temporary file for test.") - defer os.Remove(temp.Name()) + tempName := tempFileName("", "zap-open-test") + assert.False(t, fileExists(tempName)) tests := []struct { - paths []string - filenames []string - error string + paths []string + error string }{ - {[]string{"stdout"}, []string{os.Stdout.Name()}, ""}, - {[]string{"stderr"}, []string{os.Stderr.Name()}, ""}, - {[]string{temp.Name()}, []string{temp.Name()}, ""}, - {[]string{"/foo/bar/baz"}, []string{}, "open /foo/bar/baz: no such file or directory"}, + {[]string{"stdout"}, ""}, + {[]string{"stderr"}, ""}, + {[]string{tempName}, ""}, + {[]string{"/foo/bar/baz"}, "open /foo/bar/baz: no such file or directory"}, { - paths: []string{"stdout", "/foo/bar/baz", temp.Name(), "/baz/quux"}, - filenames: []string{os.Stdout.Name(), temp.Name()}, - error: "open /foo/bar/baz: no such file or directory; open /baz/quux: no such file or directory", + paths: []string{"stdout", "/foo/bar/baz", tempName, "/baz/quux"}, + error: "open /foo/bar/baz: no such file or directory; open /baz/quux: no such file or directory", }, } for _, tt := range tests { - wss, cleanup, err := open(tt.paths) + _, cleanup, err := Open(tt.paths...) if err == nil { defer cleanup() } @@ -75,14 +76,10 @@ func TestOpen(t *testing.T) { } else { assert.Equal(t, tt.error, err.Error(), "Unexpected error opening paths %v.", tt.paths) } - names := make([]string, len(wss)) - for i, ws := range wss { - f, ok := ws.(*os.File) - require.True(t, ok, "Expected all WriteSyncers returned from open() to be files.") - names[i] = f.Name() - } - assert.Equal(t, tt.filenames, names, "Opened unexpected files given paths %v.", tt.paths) } + + assert.True(t, fileExists(tempName)) + os.Remove(tempName) } func TestOpenFails(t *testing.T) { @@ -118,8 +115,41 @@ func (w *testWriter) Sync() error { return nil } +func TestOpenWithCustomSink(t *testing.T) { + defer resetSinkRegistry() + tw := &testWriter{"test", t} + ctr := func() (Sink, error) { return nopCloserSink{tw}, nil } + assert.Nil(t, RegisterSink("TestOpenWithCustomSink", ctr)) + w, cleanup, err := Open("TestOpenWithCustomSink") + assert.Nil(t, err) + defer cleanup() + w.Write([]byte("test")) +} + +func TestOpenWithErroringSinkFactory(t *testing.T) { + defer resetSinkRegistry() + expectedErr := errors.New("expected factory error") + ctr := func() (Sink, error) { return nil, expectedErr } + assert.Nil(t, RegisterSink("TestOpenWithErroringSinkFactory", ctr)) + _, _, err := Open("TestOpenWithErroringSinkFactory") + assert.Equal(t, expectedErr, err) +} + func TestCombineWriteSyncers(t *testing.T) { tw := &testWriter{"test", t} w := CombineWriteSyncers(tw) w.Write([]byte("test")) } + +func tempFileName(prefix, suffix string) string { + randBytes := make([]byte, 16) + rand.Read(randBytes) + return filepath.Join(os.TempDir(), prefix+hex.EncodeToString(randBytes)+suffix) +} + +func fileExists(name string) bool { + if _, err := os.Stat(name); os.IsNotExist(err) { + return false + } + return true +}