Replace logrus with zap (#43)

.travis.yml

@@ -15,10 +15,8 @@ before_install:
 install: true
 before_script:
   - make
-  - make clean_string
 script:
   - make race
-  - make lint
   - "$HOME/gopath/bin/goveralls -v -service=travis-ci -package=."
 notifications:
   email:

Makefile

@@ -18,9 +18,9 @@ deps:
 	gometalinter.v1 --install
 
 lint:
-	gometalinter.v1 ./... --vendor --deadline=10000s --dupl-threshold=100 --disable=interfacer --disable=gas
+	gometalinter.v1 ./... --vendor --deadline=1h --dupl-threshold=100 --disable=interfacer --disable=gas
 
-test: 
+test:
 	go test -v ./...
 
 race:

client.go

@@ -8,6 +8,8 @@ import (
 	"sync"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/performancecopilot/speed/bytewriter"
 )
 
@@ -34,8 +36,6 @@ const MaxDataValueSize = 16
 // EraseFileOnStop if set to true, will also delete the memory mapped file
 var EraseFileOnStop = false
 
-var clientlogger = log.WithField("prefix", "client")
-
 // Client defines the interface for a type that can talk to an instrumentation agent
 type Client interface {
 	// a client must contain a registry of metrics
@@ -132,7 +132,10 @@ func NewPCPClientWithRegistry(name string, registry *PCPRegistry) (*PCPClient, e
 	}
 
 	if logging {
-		clientlogger.WithField("location", fileLocation).Info("deduced location to write the MMV file")
+		logger.Info("deduced location to write the MMV file",
+			zap.String("module", "client"),
+			zap.String("location", fileLocation),
+		)
 	}
 
 	return &PCPClient{
@@ -206,15 +209,20 @@ func (c *PCPClient) Start() error {
 	writer, err := bytewriter.NewMemoryMappedWriter(c.loc, l)
 	if err != nil {
 		if logging {
-			clientlogger.WithField("error", err).Error("cannot create MemoryMappedBuffer")
+			logger.Info("cannot create MemoryMappedBuffer",
+				zap.String("module", "client"),
+				zap.Error(err),
+			)
 		}
 		return err
 	}
 	c.writer = writer
 
 	c.start()
 	if logging {
-		clientlogger.Info("written the different components, the registered metrics should be visible now")
+		logger.Info("written the different components, the registered metrics should be visible now",
+			zap.String("module", "client"),
+		)
 	}
 
 	c.r.mapped = true
@@ -648,7 +656,9 @@ func (c *PCPClient) Stop() error {
 	}
 
 	if logging {
-		clientlogger.Info("stopping the client")
+		logger.Info("stopping the client",
+			zap.String("module", "client"),
+		)
 	}
 
 	c.stop()
@@ -659,13 +669,18 @@ func (c *PCPClient) Stop() error {
 	c.writer = nil
 	if err != nil {
 		if logging {
-			clientlogger.WithField("error", err).Error("error unmapping MemoryMappedBuffer")
+			logger.Info("error unmapping MemoryMappedBuffer",
+				zap.String("module", "client"),
+				zap.Error(err),
+			)
 		}
 		return err
 	}
 
 	if logging {
-		clientlogger.Info("unmapped the memory mapped file")
+		logger.Info("unmapped the memory mapped file",
+			zap.String("module", "client"),
+		)
 	}
 
 	return nil

config.go

@@ -6,7 +6,7 @@ import (
 	"path/filepath"
 	"regexp"
 
-	"github.com/Sirupsen/logrus"
+	"go.uber.org/zap"
 )
 
 // rootPath stores path to the pcp root installation
@@ -32,10 +32,10 @@ func initConfig() error {
 	rootPath = r
 
 	if logging {
-		log.WithFields(logrus.Fields{
-			"prefix":   "config",
-			"rootPath": rootPath,
-		}).Info("detected root directory for PCP")
+		logger.Info("detected root directory for PCP",
+			zap.String("module", "config"),
+			zap.String("rootPath", rootPath),
+		)
 	}
 
 	c, ok := os.LookupEnv("PCP_CONF")
@@ -45,10 +45,10 @@ func initConfig() error {
 	confPath = c
 
 	if logging {
-		log.WithFields(logrus.Fields{
-			"prefix":   "config",
-			"confPath": confPath,
-		}).Info("detected directory for PCP config file")
+		logger.Info("detected directory for PCP config file",
+			zap.String("module", "config"),
+			zap.String("confPath", confPath),
+		)
 	}
 
 	f, err := os.Open(confPath)
@@ -69,7 +69,9 @@ func initConfig() error {
 	}
 
 	if logging {
-		log.WithFields(logrus.Fields{"prefix": "config"}).Info("successfully read PCP config file")
+		logger.Info("successfully read PCP config file",
+			zap.String("module", "config"),
+		)
 	}
 
 	return nil

metrics.go

@@ -130,6 +130,8 @@ func (m MetricType) resolve(val interface{}) interface{} {
 
 // MetricUnit defines the interface for a unit type for speed.
 type MetricUnit interface {
+	fmt.Stringer
+
 	// return 32 bit PMAPI representation for the unit
 	// see: https://github.com/performancecopilot/pcp/blob/master/src/include/pcp/pmapi.h#L61-L101
 	PMAPI() uint32

registry.go

@@ -6,7 +6,7 @@ import (
 	"regexp"
 	"sync"
 
-	"github.com/Sirupsen/logrus"
+	"go.uber.org/zap"
 )
 
 // Registry defines a valid set of instance domains and metrics
@@ -157,11 +157,11 @@ func (r *PCPRegistry) AddInstanceDomain(indom InstanceDomain) error {
 	}
 
 	if logging {
-		log.WithFields(logrus.Fields{
-			"prefix":        "registry",
-			"name":          indom.Name(),
-			"instanceCount": indom.InstanceCount(),
-		}).Info("added new instance domain")
+		logger.Info("added new instance domain",
+			zap.String("module", "registry"),
+			zap.String("name", indom.Name()),
+			zap.Int("instanceCount", indom.InstanceCount()),
+		)
 	}
 
 	if indom.(*PCPInstanceDomain).shortDescription != "" {
@@ -227,13 +227,13 @@ func (r *PCPRegistry) AddMetric(m Metric) error {
 	r.addMetric(pcpm)
 
 	if logging {
-		log.WithFields(logrus.Fields{
-			"prefix":    "registry",
-			"name":      m.Name(),
-			"type":      m.Type(),
-			"unit":      m.Unit(),
-			"semantics": m.Semantics(),
-		}).Info("added new metric")
+		logger.Info("added new metric",
+			zap.String("module", "registry"),
+			zap.String("name", m.Name()),
+			zap.String("type", m.Type().String()),
+			zap.String("semantics", m.Semantics().String()),
+			zap.String("unit", m.Unit().String()),
+		)
 	}
 
 	return nil

speed.go

@@ -10,22 +10,34 @@ package speed
 
 import (
 	"hash/fnv"
+	"io"
+	"os"
 
-	"github.com/Sirupsen/logrus"
-	prefixed "github.com/x-cray/logrus-prefixed-formatter"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 )
 
 // Version is the last tagged version of the package
 const Version = "1.0.0"
 
-var log = logrus.New()
-
 var logging bool
+var logWriters = []zapcore.WriteSyncer{os.Stdout}
+var logger *zap.Logger
+var zapEncoderConfig = zapcore.EncoderConfig{
+	TimeKey:        "ts",
+	LevelKey:       "level",
+	NameKey:        "logger",
+	CallerKey:      "caller",
+	MessageKey:     "msg",
+	StacktraceKey:  "stacktrace",
+	EncodeLevel:    zapcore.LowercaseLevelEncoder,
+	EncodeTime:     zapcore.ISO8601TimeEncoder,
+	EncodeDuration: zapcore.SecondsDurationEncoder,
+}
 
 func initLogging() {
-	log.Formatter = new(prefixed.TextFormatter)
-	log.Level = logrus.InfoLevel
 	logging = false
+	initializeLogger()
 }
 
 // EnableLogging logging enables logging for logrus if true is passed
@@ -34,17 +46,44 @@ func EnableLogging(enable bool) {
 	logging = enable
 }
 
+// AddLogWriter adds a new io.Writer as a target for writing
+// logs.
+func AddLogWriter(writer io.Writer) {
+	logWriters = append(logWriters, zapcore.AddSync(writer))
+	initializeLogger()
+}
+
+// SetLogWriters will set the passed io.Writer instances as targets for
+// writing logs.
+func SetLogWriters(writers ...io.Writer) {
+	writesyncers := make([]zapcore.WriteSyncer, 0, len(writers))
+	for _, w := range writers {
+		writesyncers = append(writesyncers, zapcore.AddSync(w))
+	}
+
+	logWriters = writesyncers
+	initializeLogger()
+}
+
+func initializeLogger() {
+	ws := zap.CombineWriteSyncers(logWriters...)
+	logger = zap.New(zapcore.NewCore(
+		zapcore.NewConsoleEncoder(zapEncoderConfig),
+		ws, zapcore.InfoLevel,
+	))
+}
+
 // init maintains a central location of all things that happen when the package is initialized
 // instead of everything being scattered in multiple source files
 func init() {
 	initLogging()
 
 	err := initConfig()
 	if err != nil && logging {
-		log.WithFields(logrus.Fields{
-			"prefix": "config",
-			"error":  err,
-		}).Error("error initializing config. maybe PCP isn't installed properly")
+		logger.Error("error initializing config. maybe PCP isn't installed properly",
+			zap.String("module", "config"),
+			zap.Error(err),
+		)
 	}
 }
 

speed_test.go

@@ -1,6 +1,9 @@
 package speed
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func BenchmarkGetHash(b *testing.B) {
 	strings := []string{
@@ -16,3 +19,56 @@ func BenchmarkGetHash(b *testing.B) {
 		_ = hash(strings[i%l], 0)
 	}
 }
+
+type testWriter struct {
+	message string
+	t       testing.TB
+}
+
+func (w *testWriter) Write(b []byte) (int, error) {
+	s := string(b)
+	if !strings.Contains(s, w.message) {
+		w.t.Error("expected log'", string(b), "' to contain", w.message)
+	}
+
+	return len(b), nil
+}
+
+func TestSetLogWriters(t *testing.T) {
+	cases := []string{
+		"a",
+		"abcdefghijklmnopqrstuvwxyz",
+		"aaaaaaaaaaaaaaaaaaaaaaaaaa",
+		"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+		"abcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxy",
+	}
+
+	for _, s := range cases {
+		w := &testWriter{s, t}
+		SetLogWriters(w)
+
+		if len(logWriters) != 1 {
+			t.Error("expected the length of logWriters to be 1")
+		}
+
+		logger.Info(s)
+	}
+}
+
+func TestAddLogWriters(t *testing.T) {
+	if len(logWriters) != 1 {
+		t.Error("expected the length of logWriters to be 1")
+	}
+
+	AddLogWriter(&testWriter{"a", t})
+
+	if len(logWriters) != 2 {
+		t.Error("expected the length of logWriters to be 2")
+	}
+
+	AddLogWriter(&testWriter{"b", t})
+
+	if len(logWriters) != 3 {
+		t.Error("expected the length of logWriters to be 3")
+	}
+}