Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce logging level configuration #514

Merged
merged 5 commits into from
Nov 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions cmd/agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package main

import (
"fmt"
"os"
"runtime"

"github.com/pkg/errors"
Expand All @@ -30,14 +32,22 @@ import (
)

func main() {
logger, _ := zap.NewProduction()
v := viper.New()
var command = &cobra.Command{
Use: "jaeger-agent",
Short: "Jaeger agent is a local daemon program which collects tracing data.",
Long: `Jaeger agent is a daemon program that runs on every host and receives tracing data submitted by Jaeger client libraries.`,
RunE: func(cmd *cobra.Command, args []string) error {
flags.TryLoadConfigFile(v, logger)
err := flags.TryLoadConfigFile(v)
if err != nil {
return err
}

sFlags := new(flags.SharedFlags).InitFromViper(v)
logger, err := sFlags.NewLogger(zap.NewProductionConfig())
if err != nil {
return err
}

builder := &app.Builder{}
builder.InitFromViper(v)
Expand Down Expand Up @@ -70,6 +80,7 @@ func main() {
)

if err := command.Execute(); err != nil {
logger.Fatal("agent command failed", zap.Error(err))
fmt.Println(err.Error())
os.Exit(1)
}
}
18 changes: 14 additions & 4 deletions cmd/collector/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package main

import (
"fmt"
"net"
"net/http"
"os"
Expand Down Expand Up @@ -50,7 +51,6 @@ func main() {
var signalsChannel = make(chan os.Signal, 0)
signal.Notify(signalsChannel, os.Interrupt, syscall.SIGTERM)

logger, _ := zap.NewProduction()
serviceName := "jaeger-collector"
casOptions := casFlags.NewOptions("cassandra")
esOptions := esFlags.NewOptions("es")
Expand All @@ -61,10 +61,18 @@ func main() {
Short: "Jaeger collector receives and processes traces from Jaeger agents and clients",
Long: `Jaeger collector receives traces from Jaeger agents and agent and runs them through
a processing pipeline.`,
Run: func(cmd *cobra.Command, args []string) {
flags.TryLoadConfigFile(v, logger)
RunE: func(cmd *cobra.Command, args []string) error {
err := flags.TryLoadConfigFile(v)
if err != nil {
return err
}

sFlags := new(flags.SharedFlags).InitFromViper(v)
logger, err := sFlags.NewLogger(zap.NewProductionConfig())
if err != nil {
return err
}

casOptions.InitFromViper(v)
esOptions.InitFromViper(v)

Expand Down Expand Up @@ -127,6 +135,7 @@ func main() {
case <-signalsChannel:
logger.Info("Jaeger Collector is finishing")
}
return nil
},
}

Expand All @@ -143,7 +152,8 @@ func main() {
)

if error := command.Execute(); error != nil {
logger.Fatal(error.Error())
fmt.Println(error.Error())
os.Exit(1)
}
}

Expand Down
39 changes: 20 additions & 19 deletions cmd/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
package flags

import (
"errors"
"flag"
"fmt"
"strings"
"time"

"github.com/pkg/errors"
"github.com/spf13/viper"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)

const (
Expand All @@ -44,14 +44,26 @@ func AddConfigFileFlag(flagSet *flag.FlagSet) {
}

// TryLoadConfigFile initializes viper with config file specified as flag
func TryLoadConfigFile(v *viper.Viper, logger *zap.Logger) {
func TryLoadConfigFile(v *viper.Viper) error {
if file := v.GetString(configFile); file != "" {
v.SetConfigFile(file)
err := v.ReadInConfig()
if err != nil {
logger.Fatal("Error loading config file", zap.Error(err), zap.String(configFile, file))
return errors.Wrapf(err, "Error loading config file %s", file)
}
}
return nil
}

// NewLogger returns logger based on configuration in SharedFlags
func (flags *SharedFlags) NewLogger(conf zap.Config, options ...zap.Option) (*zap.Logger, error) {
var level zapcore.Level
err := (&level).UnmarshalText([]byte(flags.Logging.Level))
if err != nil {
return nil, err
}
conf.Level = zap.NewAtomicLevelAt(level)
return conf.Build(options...)
}

// SharedFlags holds flags configuration
Expand All @@ -60,19 +72,22 @@ type SharedFlags struct {
SpanStorage spanStorage
// DependencyStorage defines common settings for Dependency Storage.
DependencyStorage dependencyStorage
// Logging holds logging configuration
Logging logging
}

// InitFromViper initializes SharedFlags with properties from viper
func (flags *SharedFlags) InitFromViper(v *viper.Viper) *SharedFlags {
flags.SpanStorage.Type = v.GetString(spanStorageType)
flags.DependencyStorage.DataFrequency = v.GetDuration(dependencyStorageDataFrequency)
flags.Logging.Level = v.GetString(logLevel)
return flags
}

// AddFlags adds flags for SharedFlags
func AddFlags(flagSet *flag.FlagSet) {
flagSet.String(spanStorageType, CassandraStorageType, fmt.Sprintf("The type of span storage backend to use, options are currently [%v,%v,%v]", CassandraStorageType, ESStorageType, MemoryStorageType))
flagSet.String(logLevel, "info", "Minimal allowed log level")
flagSet.String(logLevel, "info", "Minimal allowed log Level. For more levels see https://github.com/uber-go/zap")
flagSet.Duration(dependencyStorageDataFrequency, time.Hour*24, "Frequency of service dependency calculations")
}

Expand All @@ -88,19 +103,5 @@ type spanStorage struct {
}

type dependencyStorage struct {
Type string
DataFrequency time.Duration
}

type cassandraOptions struct {
ConnectionsPerHost int
MaxRetryAttempts int
QueryTimeout time.Duration
Servers string
Port int
Keyspace string
}

func (co cassandraOptions) ServerList() []string {
return strings.Split(co.Servers, ",")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

18 changes: 14 additions & 4 deletions cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package main

import (
"fmt"
"net/http"
"os"
"os/signal"
Expand Down Expand Up @@ -45,7 +46,6 @@ func main() {
var serverChannel = make(chan os.Signal, 0)
signal.Notify(serverChannel, os.Interrupt, syscall.SIGTERM)

logger, _ := zap.NewProduction()
casOptions := casFlags.NewOptions("cassandra", "cassandra.archive")
esOptions := esFlags.NewOptions("es", "es.archive")
v := viper.New()
Expand All @@ -54,10 +54,18 @@ func main() {
Use: "jaeger-query",
Short: "Jaeger query is a service to access tracing data",
Long: `Jaeger query is a service to access tracing data and host UI.`,
Run: func(cmd *cobra.Command, args []string) {
flags.TryLoadConfigFile(v, logger)
RunE: func(cmd *cobra.Command, args []string) error {
err := flags.TryLoadConfigFile(v)
if err != nil {
return err
}

sFlags := new(flags.SharedFlags).InitFromViper(v)
logger, err := sFlags.NewLogger(zap.NewProductionConfig())
if err != nil {
return err
}

casOptions.InitFromViper(v)
esOptions.InitFromViper(v)
queryOpts := new(builder.QueryOptions).InitFromViper(v)
Expand Down Expand Up @@ -119,6 +127,7 @@ func main() {
case <-serverChannel:
logger.Info("Jaeger Query is finishing")
}
return nil
},
}

Expand All @@ -135,7 +144,8 @@ func main() {
)

if error := command.Execute(); error != nil {
logger.Fatal(error.Error())
fmt.Println(error.Error())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unfortunate, because a logger may print the error quite differently, e.g. with a stack trace, write to file, report to Sentry, etc. If we guaranteed that the command only returned errors when it can't init the logger, then it wouldn't be too bad, but I don't think this is generally the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement is invoked when:

  • an error is propagated from runE -> in all cases except logger creation and config file parsing zap.Fatal is called which directly calls os.exit.
  • cobra fails to parse flags e.g. a wrong/undefined flag

Other solution might be to use a default logger here.

os.Exit(1)
}
}

Expand Down
18 changes: 13 additions & 5 deletions cmd/standalone/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"net"
"net/http"
"os"
"runtime"
"strconv"

Expand Down Expand Up @@ -51,19 +52,25 @@ import (

// standalone/main is a standalone full-stack jaeger backend, backed by a memory store
func main() {
logger, _ := zap.NewProduction()
v := viper.New()

command := &cobra.Command{
Use: "jaeger-standalone",
Short: "Jaeger all-in-one distribution with agent, collector and query in one process.",
Long: `Jaeger all-in-one distribution with agent, collector and query. Use with caution this version
uses only in-memory database.`,
RunE: func(cmd *cobra.Command, args []string) error {
flags.TryLoadConfigFile(v, logger)
err := flags.TryLoadConfigFile(v)
if err != nil {
return err
}

runtime.GOMAXPROCS(runtime.NumCPU())
sFlags := new(flags.SharedFlags).InitFromViper(v)
logger, err := sFlags.NewLogger(zap.NewProductionConfig())
if err != nil {
return err
}

runtime.GOMAXPROCS(runtime.NumCPU())
cOpts := new(collector.CollectorOptions).InitFromViper(v)
qOpts := new(query.QueryOptions).InitFromViper(v)

Expand Down Expand Up @@ -93,7 +100,8 @@ func main() {
)

if err := command.Execute(); err != nil {
logger.Fatal("standalone command failed", zap.Error(err))
fmt.Println(err.Error())
os.Exit(1)
}
}

Expand Down