Decouple logger from configuration struct

cmd/dns-inventory/dns-inventory.go

@@ -40,11 +40,8 @@ func main() {
 		log.Fatal(err)
 	}
 
-	// Pass the global logger to the inventory library.
-	cfg.Logger = log
-
 	// Initialize a new inventory.
-	dnsInventory, err := inventory.New(cfg)
+	dnsInventory, err := inventory.New(cfg, log)
 	if err != nil {
 		log.Fatal(err)
 	}

internal/logger/logger.go

@@ -1,39 +1,38 @@
 package logger
 
 import (
-	"encoding/json"
 	"os"
 
 	"github.com/mattn/go-isatty"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 )
 
 func New(level string) (*zap.SugaredLogger, error) {
-	var cfg zap.Config
-	enc := "json"
+	zapLevel, err := zap.ParseAtomicLevel(level)
+	if err != nil {
+		return nil, errors.Wrap(err, "log level parsing error")
+	}
 
+	encoding := "json"
 	if isatty.IsTerminal(os.Stdout.Fd()) {
-		enc = "console"
+		encoding = "console"
 	}
 
-	cfgJSON := []byte(`{
-		"development": false,
-	  "level": "` + level + `",
-	  "encoding": "` + enc + `",
-	  "outputPaths": ["stderr"],
-	  "errorOutputPaths": ["stderr"],
-	  "encoderConfig": {
-			"timeKey": "timestamp",
-			"timeEncoder": "iso8601",
-	    "messageKey": "message",
-	    "levelKey": "level",
-	    "levelEncoder": "capital"
-	  }
-	}`)
-
-	if err := json.Unmarshal(cfgJSON, &cfg); err != nil {
-		return nil, errors.Wrap(err, "json unmarshalling error")
+	cfg := zap.Config{
+		Development:      false,
+		Level:            zapLevel,
+		Encoding:         encoding,
+		OutputPaths:      []string{"stderr"},
+		ErrorOutputPaths: []string{"stderr"},
+		EncoderConfig: zapcore.EncoderConfig{
+			TimeKey:     "timestamp",
+			EncodeTime:  zapcore.ISO8601TimeEncoder,
+			MessageKey:  "message",
+			LevelKey:    "level",
+			EncodeLevel: zapcore.CapitalLevelEncoder,
+		},
 	}
 
 	logger, err := cfg.Build()

pkg/inventory/datasource.go

@@ -5,13 +5,13 @@ import (
 )
 
 // NewDatasource creates a datasource based on the inventory configuration.
-func NewDatasource(cfg *Config) (Datasource, error) {
+func NewDatasource(cfg *Config, log Logger) (Datasource, error) {
 	// Select datasource implementation.
 	switch cfg.Datasource {
 	case DNSDatasourceType:
-		return NewDNSDatasource(cfg)
+		return NewDNSDatasource(cfg, log)
 	case EtcdDatasourceType:
-		return NewEtcdDatasource(cfg)
+		return NewEtcdDatasource(cfg, log)
 	default:
 		return nil, errors.Errorf("unknown datasource type: %s", cfg.Datasource)
 	}

pkg/inventory/dns.go

@@ -216,10 +216,10 @@ func (d *DNSDatasource) PublishRecords(records []*DatasourceRecord) error {
 func (d *DNSDatasource) Close() {}
 
 // NewDNSDatasource creates a DNS datasource.
-func NewDNSDatasource(cfg *Config) (*DNSDatasource, error) {
+func NewDNSDatasource(cfg *Config, log Logger) (*DNSDatasource, error) {
 	return &DNSDatasource{
 		Config: cfg,
-		Logger: cfg.Logger,
+		Logger: log,
 		Client: &dns.Client{
 			Timeout: cfg.DNS.Timeout,
 		},

pkg/inventory/etcd.go

@@ -213,7 +213,7 @@ func makeEtcdTLSConfig(cfg *Config) (*tls.Config, error) {
 }
 
 // NewEtcdDatasource creates an etcd datasource.
-func NewEtcdDatasource(cfg *Config) (*EtcdDatasource, error) {
+func NewEtcdDatasource(cfg *Config, log Logger) (*EtcdDatasource, error) {
 	// Etcd client configuration
 	clientCfg := etcdv3.Config{
 		Endpoints:   cfg.Etcd.Endpoints,
@@ -245,7 +245,7 @@ func NewEtcdDatasource(cfg *Config) (*EtcdDatasource, error) {
 
 	return &EtcdDatasource{
 		Config: cfg,
-		Logger: cfg.Logger,
+		Logger: log,
 		Client: client,
 	}, nil
 }

pkg/inventory/inventory.go

@@ -224,7 +224,7 @@ func (i *Inventory) PublishHosts(hosts map[string][]*HostAttributes) error {
 }
 
 // New creates an instance of the DNS inventory with user-supplied configuration.
-func New(cfg *Config) (*Inventory, error) {
+func New(cfg *Config, log Logger) (*Inventory, error) {
 	// Setup package global state
 	adiHostAttributeNames = make(map[string]string)
 	adiHostAttributeNames["OS"] = cfg.Txt.Keys.Os
@@ -234,36 +234,37 @@ func New(cfg *Config) (*Inventory, error) {
 	adiHostAttributeNames["VARS"] = cfg.Txt.Keys.Vars
 
 	// Initialize logger.
-	if cfg.Logger == nil {
-		l, err := logger.New("info")
-		if err != nil {
+	if log == nil {
+		var err error
+		if log, err = logger.New("info"); err != nil {
 			return nil, errors.Wrap(err, "logger initialization failure")
 		}
-		cfg.Logger = l
+
+		log.Warn("no custom logger passed to inventory.New(), using defaults")
 	}
 
 	// Initialize datasource.
-	ds, err := NewDatasource(cfg)
+	ds, err := NewDatasource(cfg, log)
 	if err != nil {
 		return nil, errors.Wrap(err, "datasource initialization failure")
 	}
 
 	// Initialize struct validator.
-	validator := validator.New()
-	validator.RegisterValidation("notblank", validators.NotBlank)
-	validator.RegisterValidation("safelist", isSafeList)
-	validator.RegisterValidation("safelistsep", isSafeListWithSeparator)
+	val := validator.New()
+	val.RegisterValidation("notblank", validators.NotBlank)
+	val.RegisterValidation("safelist", isSafeList)
+	val.RegisterValidation("safelistsep", isSafeListWithSeparator)
 
-	i := &Inventory{
+	inventory := &Inventory{
 		Config:    cfg,
-		Logger:    cfg.Logger,
-		Validator: validator,
+		Logger:    log,
+		Validator: val,
 
 		Datasource: ds,
 		Tree:       NewTree(),
 	}
 
-	return i, nil
+	return inventory, nil
 }
 
 // NewDefault creates an instance of the DNS inventory with the default configuration.
@@ -274,5 +275,5 @@ func NewDefault() (*Inventory, error) {
 		return nil, errors.Wrap(err, "defaults initialization failure")
 	}
 
-	return New(cfg)
+	return New(cfg, nil)
 }

pkg/inventory/types.go

@@ -23,9 +23,6 @@ type (
 
 	// Config represents the main inventory configuration.
 	Config struct {
-		// A logger for the inventory.
-		// By default, the global zap.SugaredLogger is used.
-		Logger Logger
 		// Datasource type.
 		// Currently supported: dns, etcd.
 		Datasource string `mapstructure:"datasource" default:"dns"`