Prevent double-wrapping in zbark (#17)

As users and packages start using these wrappers, we're likely to start
double- and triple-wrapping loggers. Without special care, this will get
quite inefficient.

This change alters the `Zapify` and `Barkify` functions to detect
double-wrapping and recover the original logger, rather than adding
another layer of indirection.

zbark/barkify.go

@@ -30,6 +30,9 @@ import (
 // the bark.Logger interface. Note that the wrapper always returns nil from
 // the Fields method, since zap doesn't support this functionality.
 func Barkify(l *zap.Logger) bark.Logger {
+	if z, ok := l.Core().(*zapper); ok {
+		return z.l
+	}
 	return barker{l.Sugar()}
 }
 

zbark/double_wrap_test.go

@@ -0,0 +1,39 @@
+// Copyright (c) 2017 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 zbark
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/uber-common/bark"
+	"go.uber.org/zap"
+)
+
+func TestZapifyDoubleWrap(t *testing.T) {
+	core := Zapify(Barkify(zap.NewNop())).Core()
+	_, ok := core.(*zapper)
+	assert.False(t, ok, "Using both Zapify and Barkify should recover original logger.")
+}
+
+func TestBarkifyDoubleWrap(t *testing.T) {
+	b := Barkify(Zapify(bark.NewNopLogger()))
+	_, ok := b.(barker)
+	assert.False(t, ok, "Using both Barkify and Zapify should recover original logger.")
+}

zbark/zapify.go

@@ -30,6 +30,9 @@ import (
 // *zap.Logger. Note that the wrapper always treats zap's DPanicLevel as an
 // error (even in development).
 func Zapify(l bark.Logger) *zap.Logger {
+	if b, ok := l.(barker); ok {
+		return b.SugaredLogger.Desugar()
+	}
 	return zap.New(&zapper{l})
 }