Refactor DefaultSinks() into RegisterSink

Akin to RegisterEncoder

config.go

@@ -21,8 +21,6 @@
 package zap
 
 import (
- "io"
- "os"
  "sort"
  "time"
 
@@ -166,20 +164,12 @@ func NewDevelopmentConfig() Config {
 
 // Build constructs a logger from the Config and Options.
 func (cfg Config) Build(opts ...Option) (*Logger, error) {
- return cfg.BuildWithSinks(DefaultSinks(), opts...)
-}
-
-// BuildWithSinks uses the map of sinks to construct a logger from the
-// passed options. For example, the option "stdout" will construct a logger
-// that routes output to the stdout sink, but "/filepath" will fallback
-// to writing to file.
-func (cfg Config) BuildWithSinks(sm map[string]Sink, opts ...Option) (*Logger, error) {
  enc, err := cfg.buildEncoder()
  if err != nil {
  return nil, err
  }
 
- sink, errSink, err := cfg.openSinks(sm)
+ sink, errSink, err := cfg.openSinks()
  if err != nil {
  return nil, err
  }
@@ -235,12 +225,12 @@ func (cfg Config) buildOptions(errSink zapcore.WriteSyncer) []Option {
  return opts
 }
 
-func (cfg Config) openSinks(sm map[string]Sink) (zapcore.WriteSyncer, zapcore.WriteSyncer, error) {
- sink, closeOut, err := OpenWithSinks(sm, cfg.OutputPaths...)
+func (cfg Config) openSinks() (zapcore.WriteSyncer, zapcore.WriteSyncer, error) {
+ sink, closeOut, err := Open(cfg.OutputPaths...)
  if err != nil {
  return nil, nil, err
  }
- errSink, _, err := OpenWithSinks(sm, cfg.ErrorOutputPaths...)
+ errSink, _, err := Open(cfg.ErrorOutputPaths...)
  if err != nil {
  closeOut()
  return nil, nil, err
@@ -251,24 +241,3 @@ func (cfg Config) openSinks(sm map[string]Sink) (zapcore.WriteSyncer, zapcore.Wr
 func (cfg Config) buildEncoder() (zapcore.Encoder, error) {
  return newEncoder(cfg.Encoding, cfg.EncoderConfig)
 }
-
-// Sink defines the interface to write to and close logger destinations.
-type Sink interface {
- zapcore.WriteSyncer
- io.Closer
-}
-
-// DefaultSinks generates a map of regularly used writeSyncers, coupled
-// with the appropriate Closer.
-func DefaultSinks() map[string]Sink {
- return map[string]Sink{
- "stdout": NopCloserSink{os.Stdout},
- "stderr": NopCloserSink{os.Stderr},
- }
-}
-
-// NopCloserSink wraps a WriteSyncer with a no-op Close() method.
-type NopCloserSink struct{ zapcore.WriteSyncer }
-
-// Close does nothing (no-op).
-func (NopCloserSink) Close() error { return nil }

sink.go

@@ -0,0 +1,59 @@
+package zap
+
+import (
+ "errors"
+ "fmt"
+ "io"
+ "os"
+ "sync"
+
+ "go.uber.org/zap/zapcore"
+)
+
+var (
+ _sinkMutex sync.RWMutex
+ errSinkNotFound = errors.New("sink for the given key not found")
+ _sinkFactories = map[string]func() (Sink, error){
+ "stdout": func() (Sink, error) { return NopCloserSink{os.Stdout}, nil },
+ "stderr": func() (Sink, error) { return NopCloserSink{os.Stderr}, nil },
+ }
+)
+
+// 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
+ }
+ return sinkFactory()
+}
+
+// NopCloserSink wraps a WriteSyncer with a no-op Close() method.
+type NopCloserSink struct{ zapcore.WriteSyncer }
+
+// Close does nothing (no-op).
+func (NopCloserSink) Close() error { return nil }

sink_test.go

@@ -0,0 +1,55 @@
+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) {
+ 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},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.key, func(t *testing.T) {
+ _, err := NewSink(tt.key)
+ assert.Equal(t, tt.err, err)
+ })
+ }
+}

writer.go

@@ -38,20 +38,7 @@ import (
 // Passing no paths returns a no-op WriteSyncer. The special paths "stdout" and
 // "stderr" are interpreted as os.Stdout and os.Stderr, respectively.
 func Open(paths ...string) (zapcore.WriteSyncer, func(), error) {
- return OpenWithSinks(DefaultSinks(), paths...)
-}
-
-// OpenWithSinks is a high-level wrapper that takes a map of sinks and a
-// variadic number of paths. The map customizes how to open and close a
-// particular path, with the default being the creation of a file
-// at said path. e.g. "stdout", "/file/destination".
-// It then combines all of the writers into a locked WriteSyncer and
-// returns any error encountered and a function to close any opened files.
-//
-// Passing no paths returns a no-op WriteSyncer. The special paths "stdout" and
-// "stderr" are interpreted as os.Stdout and os.Stderr, respectively.
-func OpenWithSinks(sm map[string]Sink, paths ...string) (zapcore.WriteSyncer, func(), error) {
- writers, close, err := open(sm, paths)
+ writers, close, err := open(paths)
  if err != nil {
  return nil, nil, err
  }
@@ -60,7 +47,7 @@ func OpenWithSinks(sm map[string]Sink, paths ...string) (zapcore.WriteSyncer, fu
  return writer, close, nil
 }
 
-func open(sm map[string]Sink, paths []string) ([]zapcore.WriteSyncer, func(), error) {
+func open(paths []string) ([]zapcore.WriteSyncer, func(), error) {
  var openErr error
  writers := make([]zapcore.WriteSyncer, 0, len(paths))
  closers := make([]io.Closer, 0, len(paths))
@@ -70,16 +57,19 @@ func open(sm map[string]Sink, paths []string) ([]zapcore.WriteSyncer, func(), er
  }
  }
  for _, path := range paths {
- if sink, ok := sm[path]; ok {
+ if sink, err := NewSink(path); err == nil {
  writers = append(writers, sink)
  closers = append(closers, sink)
  continue
- }
- f, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
- openErr = multierr.Append(openErr, err)
- if err == nil {
- writers = append(writers, f)
- closers = append(closers, f)
+ } else if err == errSinkNotFound {
+ f, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
+ openErr = multierr.Append(openErr, err)
+ if err == nil {
+ writers = append(writers, f)
+ closers = append(closers, f)
+ }
+ } else {
+ openErr = multierr.Append(openErr, err)
  }
  }
 

writer_test.go

@@ -114,10 +114,11 @@ func (w *testWriter) Sync() error {
  return nil
 }
 
-func TestOpenWithSinksCustomSink(t *testing.T) {
+func TestOpenWithCustomSink(t *testing.T) {
  tw := &testWriter{"test", t}
- sinks := map[string]Sink{"customsink": NopCloserSink{tw}}
- w, cleanup, err := OpenWithSinks(sinks, "customsink")
+ ctr := func() (Sink, error) { return NopCloserSink{tw}, nil }
+ assert.Nil(t, RegisterSink("customsink", ctr))
+ w, cleanup, err := Open("customsink")
  assert.Nil(t, err)
  defer cleanup()
  w.Write([]byte("test"))