Add logger to factory

Signed-off-by: Davit Yeghshatyan <davo@uber.com>

Author: Davit Yeghshatyan

cmd/collector/main.go

@@ -58,7 +58,11 @@ func main() {
 	var signalsChannel = make(chan os.Signal, 0)
 	signal.Notify(signalsChannel, os.Interrupt, syscall.SIGTERM)
 
-	storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr))
+	logger, err := zap.NewProduction()
+	if err != nil {
+		log.Fatalf("Cannot initialize logger: %v", err)
+	}
+	storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, logger))
 	if err != nil {
 		log.Fatalf("Cannot initialize storage factory: %v", err)
 	}

cmd/query/main.go

@@ -45,7 +45,11 @@ func main() {
 	var serverChannel = make(chan os.Signal, 0)
 	signal.Notify(serverChannel, os.Interrupt, syscall.SIGTERM)
 
-	storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr))
+	logger, err := zap.NewProduction()
+	if err != nil {
+		log.Fatalf("Cannot initialize logger: %v", err)
+	}
+	storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, logger))
 	if err != nil {
 		log.Fatalf("Cannot initialize storage factory: %v", err)
 	}

cmd/standalone/main.go

@@ -66,7 +66,11 @@ func main() {
 	if os.Getenv(storage.SpanStorageTypeEnvVar) == "" {
 		os.Setenv(storage.SpanStorageTypeEnvVar, "memory") // other storage types default to SpanStorage
 	}
-	storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr))
+	logger, err := zap.NewProduction()
+	if err != nil {
+		log.Fatalf("Cannot initialize logger: %v", err)
+	}
+	storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, logger))
 	if err != nil {
 		log.Fatalf("Cannot initialize storage factory: %v", err)
 	}

plugin/storage/factory.go

@@ -163,8 +163,8 @@ func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) {
 // CreateArchiveSpanWriter implements storage.ArchiveFactory
 func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) {
 	if len(f.SpanWriterTypes) > 1 {
-		fmt.Fprintf(f.log, "WARNING: Since there are multiple span storage types,"+
-			" the first (%s) will be used for archving\n", f.SpanWriterTypes[0])
+		f.logger.Warn("since there are multiple span storage types," +
+			" the first (" + f.SpanWriterTypes[0] + ") will be used for archiving\n")
 	}
 	factory, ok := f.factories[f.SpanWriterTypes[0]]
 	if !ok {

plugin/storage/factory_config.go

@@ -15,10 +15,10 @@
 package storage
 
 import (
-	"fmt"
-	"io"
 	"os"
 	"strings"
+
+	"go.uber.org/zap"
 )
 
 const (
@@ -39,7 +39,7 @@ type FactoryConfig struct {
 	SpanWriterTypes         []string
 	SpanReaderType          string
 	DependenciesStorageType string
-	log                     io.Writer
+	logger                  *zap.Logger
 }
 
 // FactoryConfigFromEnvAndCLI reads the desired types of storage backends from SPAN_STORAGE_TYPE,
@@ -52,11 +52,11 @@ type FactoryConfig struct {
 //
 // For backwards compatibility it also parses the args looking for deprecated --span-storage.type flag.
 // If found, it writes a deprecation warning to the log.
-func FactoryConfigFromEnvAndCLI(args []string, log io.Writer) FactoryConfig {
+func FactoryConfigFromEnvAndCLI(args []string, logger *zap.Logger) FactoryConfig {
 	spanStorageType := os.Getenv(SpanStorageTypeEnvVar)
 	if spanStorageType == "" {
 		// for backwards compatibility check command line for --span-storage.type flag
-		spanStorageType = spanStorageTypeFromArgs(args, log)
+		spanStorageType = spanStorageTypeFromArgs(args, logger)
 	}
 	if spanStorageType == "" {
 		spanStorageType = cassandraStorageType
@@ -66,9 +66,8 @@ func FactoryConfigFromEnvAndCLI(args []string, log io.Writer) FactoryConfig {
 	spanReaderType := os.Getenv(SpanReaderTypeEnvVar)
 	if spanReaderType == "" {
 		if len(spanWriterTypes) > 1 {
-			fmt.Fprintf(log, "WARNING: The first span storage type listed (%s) will be used for reading. "+
-				"Please use environment variable %s to specify which storage type to read from\n",
-				spanWriterTypes[0], SpanReaderTypeEnvVar)
+			logger.Warn("the first span storage type listed (" + spanWriterTypes[0] + ") will be used for reading. " +
+				"Please use environment variable " + SpanReaderTypeEnvVar + " to specify which storage type to read from")
 		}
 		spanReaderType = spanWriterTypes[0]
 	}
@@ -81,24 +80,20 @@ func FactoryConfigFromEnvAndCLI(args []string, log io.Writer) FactoryConfig {
 		SpanWriterTypes:         spanWriterTypes,
 		SpanReaderType:          spanReaderType,
 		DependenciesStorageType: depStorageType,
-		log: log,
+		logger:                  logger,
 	}
 }
 
-func spanStorageTypeFromArgs(args []string, log io.Writer) string {
+func spanStorageTypeFromArgs(args []string, logger *zap.Logger) string {
 	for i, token := range args {
 		if i == 0 {
 			continue // skip app name; easier than dealing with +-1 offset
 		}
 		if !strings.HasPrefix(token, spanStorageFlag) {
 			continue
 		}
-		fmt.Fprintf(
-			log,
-			"WARNING: found deprecated command line option %s, please use environment variable %s instead\n",
-			token,
-			SpanStorageTypeEnvVar,
-		)
+		logger.Warn("found deprecated command line option " + token +
+			", please use environment variable " + SpanStorageTypeEnvVar + " instead")
 		if token == spanStorageFlag && i < len(args)-1 {
 			return args[i+1]
 		}

plugin/storage/factory_config_test.go

@@ -15,11 +15,13 @@
 package storage
 
 import (
-	"bytes"
 	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+
+	"github.com/jaegertracing/jaeger/pkg/testutils"
 )
 
 func clearEnv() {
@@ -49,7 +51,7 @@ func TestFactoryConfigFromEnv(t *testing.T) {
 
 	os.Setenv(SpanStorageTypeEnvVar, elasticsearchStorageType+","+kafkaStorageType)
 
-	f = FactoryConfigFromEnvAndCLI(nil, &bytes.Buffer{})
+	f = FactoryConfigFromEnvAndCLI(nil, zap.NewNop())
 	assert.Equal(t, 2, len(f.SpanWriterTypes))
 	assert.Equal(t, []string{elasticsearchStorageType, kafkaStorageType}, f.SpanWriterTypes)
 	assert.Equal(t, elasticsearchStorageType, f.SpanReaderType)
@@ -75,15 +77,15 @@ func TestFactoryConfigFromEnvDeprecated(t *testing.T) {
 		{args: []string{"appname", "-x", "y"}, log: false, value: "cassandra"},
 	}
 	for _, testCase := range testCases {
-		log := new(bytes.Buffer)
-		f := FactoryConfigFromEnvAndCLI(testCase.args, log)
+		logger, buffer := testutils.NewLogger()
+		f := FactoryConfigFromEnvAndCLI(testCase.args, logger)
 		assert.Equal(t, 1, len(f.SpanWriterTypes))
 		assert.Equal(t, testCase.value, f.SpanWriterTypes[0])
 		assert.Equal(t, testCase.value, f.SpanReaderType)
 		assert.Equal(t, testCase.value, f.DependenciesStorageType)
 		if testCase.log {
-			expectedLog := "WARNING: found deprecated command line option"
-			assert.Equal(t, expectedLog, log.String()[0:len(expectedLog)])
+			expectedLog := "{\"level\":\"warn\",\"msg\":\"found deprecated command line option"
+			assert.Equal(t, expectedLog, buffer.String()[0:len(expectedLog)])
 		}
 	}
 }

plugin/storage/factory_test.go

@@ -40,6 +40,7 @@ func defaultCfg() FactoryConfig {
 		SpanWriterTypes:         []string{cassandraStorageType},
 		SpanReaderType:          cassandraStorageType,
 		DependenciesStorageType: cassandraStorageType,
+		logger:                  zap.NewNop(),
 	}
 }
 
@@ -135,7 +136,12 @@ func TestCreate(t *testing.T) {
 }
 
 func TestCreateArchive(t *testing.T) {
-	f, err := NewFactory(defaultCfg())
+	f, err := NewFactory(FactoryConfig{
+		SpanWriterTypes:         []string{cassandraStorageType, elasticsearchStorageType},
+		SpanReaderType:          cassandraStorageType,
+		DependenciesStorageType: cassandraStorageType,
+		logger:                  zap.NewNop(),
+	})
 	require.NoError(t, err)
 	assert.NotEmpty(t, f.factories[cassandraStorageType])