diff --git a/README.md b/README.md index f7527f5fb..6d00f55c1 100644 --- a/README.md +++ b/README.md @@ -141,27 +141,22 @@ downstream systems. The tracing information is added to each implementations hea The log package is designed to be a leveled logger with field support. -The log package defines two interfaces (Logger and Factory) that have to be implemented in order to set up the logging in this framework. -After implementing the two interfaces you can setup logging by doing the following: +The log package defines the logger interface and a factory function type that needs to be implemented in order to set up the logging in this framework. ```go - // instantiate the implemented factory and fields (map[string]interface{}) + // instantiate the implemented factory func type and fields (map[string]interface{}) err := log.Setup(factory, fields) // handle error ``` `If the setup is omitted the package will not setup any logging!` -In order to get a logger use the the following: +From there logging is as simple as ```go - logger := log.Create() + log.Info("Hello world!") ``` -which returns a logger with all fields appended along with the source code file of the class where the logger has been created. - -`It is advisable to create one logger in every type/package once and use it to get information about the log origin!` - The implementations should support following log levels: - Debug, which should log the message with debug level @@ -175,7 +170,7 @@ The first four (Debug, Info, Warn and Error) give the opportunity to differentia The package supports fields, which are logged along with the message, to augment the information further to ease querying in the log management system. -The following implementations are provided as sub-package: +The following implementations are provided as sub-package and are by default wired up in the framework: - zerolog, which supports the excellent [zerolog](https://github.com/rs/zerolog) library and is set up by default @@ -185,8 +180,6 @@ The logger interface defines the actual logger. ```go type Logger interface { - Level() Level - Fields() map[string]interface{} Fatal(...interface{}) Fatalf(string, ...interface{}) Panic(...interface{}) @@ -206,14 +199,8 @@ In order to be consistent with the design the implementation of the `Fatal(f)` h ### Factory -The factory interface defines a factory for creating a logger. +The factory function type defines a factory for creating a logger. ```go -type Factory interface { - Create(map[string]interface{}) Logger -} -``` - -Two methods are supported: - -- Create, which creates a logger with the specified fields (or nil) +type FactoryFunc func(map[string]interface{}) Logger +``` \ No newline at end of file diff --git a/async/amqp/amqp.go b/async/amqp/amqp.go index 80a64262d..93f86bb3a 100644 --- a/async/amqp/amqp.go +++ b/async/amqp/amqp.go @@ -65,7 +65,6 @@ type Consumer struct { cfg amqp.Config ch *amqp.Channel conn *amqp.Connection - log log.Logger } // New creates a new AMQP consumer with some defaults. Use option to change. @@ -109,7 +108,6 @@ func New(name, url, queue, exchange string, oo ...OptionFunc) (*Consumer, error) // Consume starts of consuming a AMQP queue. func (c *Consumer) Consume(ctx context.Context) (<-chan async.Message, <-chan error, error) { - c.log = log.Create() deliveries, err := c.consumer() if err != nil { return nil, nil, errors.Wrap(err, "failed initialize consumer") @@ -121,10 +119,10 @@ func (c *Consumer) Consume(ctx context.Context) (<-chan async.Message, <-chan er go func() { select { case <-ctx.Done(): - c.log.Info("canceling consuming messages requested") + log.Info("canceling consuming messages requested") return case d := <-deliveries: - c.log.Debugf("processing message %s", d.MessageId) + log.Debugf("processing message %s", d.MessageId) go func(d *amqp.Delivery) { sp, chCtx := trace.ConsumerSpan(ctx, c.name, trace.AMQPConsumerComponent, mapHeader(d.Headers)) @@ -186,7 +184,7 @@ func (c *Consumer) consumer() (<-chan amqp.Delivery, error) { c.ch = ch c.tag = uuid.New().String() - c.log.Infof("consuming messages for tag %s", c.tag) + log.Infof("consuming messages for tag %s", c.tag) q, err := ch.QueueDeclare(c.queue, true, false, false, false, nil) if err != nil { diff --git a/async/component.go b/async/component.go index bcbeedb0e..0a161a205 100644 --- a/async/component.go +++ b/async/component.go @@ -15,7 +15,6 @@ type Component struct { sync.Mutex cns Consumer cnl context.CancelFunc - log log.Logger } // New returns a new async component. @@ -39,7 +38,6 @@ func (c *Component) Run(ctx context.Context) error { c.Lock() ctx, cnl := context.WithCancel(ctx) c.cnl = cnl - c.log = log.Create() c.Unlock() chMsg, chErr, err := c.cns.Consume(ctx) @@ -52,11 +50,11 @@ func (c *Component) Run(ctx context.Context) error { for { select { case <-ctx.Done(): - c.log.Info("canceling consuming messages requested") + log.Info("canceling consuming messages requested") failCh <- nil return case msg := <-chMsg: - c.log.Debug("New message from consumer arrived") + log.Debug("New message from consumer arrived") go func() { err = c.proc(ctx, msg) if err != nil { diff --git a/async/kafka/kafka.go b/async/kafka/kafka.go index 6ab03e5c5..ac2c71563 100644 --- a/async/kafka/kafka.go +++ b/async/kafka/kafka.go @@ -64,7 +64,6 @@ type Consumer struct { contentType string cnl context.CancelFunc ms sarama.Consumer - log log.Logger } // New creates a ew Kafka consumer with defaults. To override those default you should provide a option. @@ -117,7 +116,6 @@ func New(name, ct, topic string, brokers []string, oo ...OptionFunc) (*Consumer, // Consume starts consuming messages from a Kafka topic. func (c *Consumer) Consume(ctx context.Context) (<-chan async.Message, <-chan error, error) { - c.log = log.Create() ctx, cnl := context.WithCancel(ctx) c.cnl = cnl @@ -134,12 +132,12 @@ func (c *Consumer) Consume(ctx context.Context) (<-chan async.Message, <-chan er for { select { case <-ctx.Done(): - c.log.Info("canceling consuming messages requested") + log.Info("canceling consuming messages requested") return case consumerError := <-consumer.Errors(): chErr <- consumerError case msg := <-consumer.Messages(): - c.log.Debugf("data received from topic %s", msg.Topic) + log.Debugf("data received from topic %s", msg.Topic) topicPartitionOffsetDiffGaugeSet(msg.Topic, msg.Partition, consumer.HighWaterMarkOffset(), msg.Offset) go func() { sp, chCtx := trace.ConsumerSpan(ctx, c.name, trace.KafkaConsumerComponent, mapHeader(msg.Headers)) diff --git a/examples/amqp.go b/examples/amqp.go index e1f01767d..f72040662 100644 --- a/examples/amqp.go +++ b/examples/amqp.go @@ -12,7 +12,6 @@ import ( type amqpComponent struct { cmp patron.Component - log log.Logger } func newAmqpComponent(name, url, queue, exchange string) (*amqpComponent, error) { @@ -33,7 +32,6 @@ func newAmqpComponent(name, url, queue, exchange string) (*amqpComponent, error) } func (ac *amqpComponent) Process(ctx context.Context, msg async.Message) error { - ac.log = log.Create() var ads Audits err := msg.Decode(&ads) @@ -44,7 +42,7 @@ func (ac *amqpComponent) Process(ctx context.Context, msg async.Message) error { ads.append(Audit{Name: "AMQP consumer", Started: time.Now()}) for _, a := range ads { - ac.log.Infof("%s@ took %s", a.Name, a.Duration) + log.Infof("%s@ took %s", a.Name, a.Duration) } return nil diff --git a/examples/docker-compose.yml b/examples/docker-compose.yml index 7cc2dbd86..cef670ae3 100644 --- a/examples/docker-compose.yml +++ b/examples/docker-compose.yml @@ -45,9 +45,6 @@ services: ports: - "15672:15672" - "5672:5672" - environment: - - RABBITMQ_DEFAULT_USER=admin - - RABBITMQ_DEFAULT_PASS=admin kafka: image: spotify/kafka hostname: "kafka" diff --git a/examples/kafka.go b/examples/kafka.go index cc68bc164..1758caea9 100644 --- a/examples/kafka.go +++ b/examples/kafka.go @@ -15,7 +15,6 @@ import ( type kafkaComponent struct { cmp patron.Component pub amqp.Publisher - log log.Logger } func newKafkaComponent(name, broker, topic, amqpURL, amqpExc string) (*kafkaComponent, error) { @@ -42,7 +41,6 @@ func newKafkaComponent(name, broker, topic, amqpURL, amqpExc string) (*kafkaComp } func (kc *kafkaComponent) Process(ctx context.Context, msg async.Message) error { - kc.log = log.Create() var ads Audits err := msg.Decode(&ads) @@ -63,7 +61,7 @@ func (kc *kafkaComponent) Process(ctx context.Context, msg async.Message) error } for _, a := range ads { - kc.log.Infof("%s@ took %s", a.Name, a.Duration) + log.Infof("%s@ took %s", a.Name, a.Duration) } return nil diff --git a/examples/main.go b/examples/main.go index b768e90f4..909e73844 100644 --- a/examples/main.go +++ b/examples/main.go @@ -30,7 +30,7 @@ func (a *Audits) append(aud Audit) { } const ( - amqpURL = "amqp://admin:admin@localhost:5672/" + amqpURL = "amqp://guest:guest@localhost:5672/" amqpExchange = "patron" amqpQueue = "patron" kafkaTopic = "patron-topic" @@ -62,17 +62,17 @@ func main() { amqpCmp, err := newAmqpComponent(name, amqpURL, amqpQueue, amqpExchange) if err != nil { - log.Create().Fatalf("failed to create processor %v", err) + log.Fatalf("failed to create processor %v", err) } kafkaCmp, err := newKafkaComponent(name, kafkaBroker, kafkaTopic, amqpURL, amqpExchange) if err != nil { - log.Create().Fatalf("failed to create processor %v", err) + log.Fatalf("failed to create processor %v", err) } httpCmp, err := newHTTPComponent(kafkaBroker, kafkaTopic) if err != nil { - log.Create().Fatalf("failed to create processor %v", err) + log.Fatalf("failed to create processor %v", err) } // Set up routes @@ -82,11 +82,11 @@ func main() { srv, err := patron.New(name, version, patron.Routes(routes), patron.Components(kafkaCmp.cmp, amqpCmp.cmp)) if err != nil { - log.Create().Fatalf("failed to create service %v", err) + log.Fatalf("failed to create service %v", err) } err = srv.Run() if err != nil { - log.Create().Fatalf("failed to create service %v", err) + log.Fatalf("failed to create service %v", err) } } diff --git a/log/log.go b/log/log.go index cf273aa30..b0b64e99e 100644 --- a/log/log.go +++ b/log/log.go @@ -2,10 +2,6 @@ package log import ( "errors" - "fmt" - "path" - "path/filepath" - "runtime" ) // The Level type definition. @@ -30,8 +26,6 @@ const ( // Logger interface definition of a logger. type Logger interface { - Level() Level - Fields() map[string]interface{} Fatal(...interface{}) Fatalf(string, ...interface{}) Panic(...interface{}) @@ -46,18 +40,13 @@ type Logger interface { Debugf(string, ...interface{}) } -// Factory interface for creating loggers. -type Factory interface { - Create(map[string]interface{}) Logger -} +// FactoryFunc function type for creating loggers. +type FactoryFunc func(map[string]interface{}) Logger -var ( - factory Factory = nilFactory{} - fields = make(map[string]interface{}) -) +var logger Logger = nilLogger{} // Setup logging by providing a logger factory. -func Setup(f Factory, fls map[string]interface{}) error { +func Setup(f FactoryFunc, fls map[string]interface{}) error { if f == nil { return errors.New("factory is nil") } @@ -66,73 +55,71 @@ func Setup(f Factory, fls map[string]interface{}) error { fls = make(map[string]interface{}) } - factory = f - fields = fls + logger = f(fls) return nil } -// Create returns a new logger with all fields inherited and with source file mapping. -func Create() Logger { - if factory == nil { - return nil - } +// Panic logging. +func Panic(args ...interface{}) { + logger.Panic(args...) +} - fls := make(map[string]interface{}) +// Panicf logging. +func Panicf(msg string, args ...interface{}) { + logger.Panicf(msg, args...) +} - for k, v := range fields { - fls[k] = v - } +// Fatal logging. +func Fatal(args ...interface{}) { + logger.Fatal(args...) +} - if key, val, ok := sourceFields(); ok { - fls[key] = val - } +// Fatalf logging. +func Fatalf(msg string, args ...interface{}) { + logger.Fatalf(msg, args...) +} - return factory.Create(fls) +// Error logging. +func Error(args ...interface{}) { + logger.Error(args...) } -func sourceFields() (key string, src string, ok bool) { - _, file, _, ok := runtime.Caller(2) - if !ok { - return - } +// Errorf logging. +func Errorf(msg string, args ...interface{}) { + logger.Errorf(msg, args...) +} - src = getSource(file) - key = "src" - ok = true - return +// Warn logging. +func Warn(args ...interface{}) { + logger.Warn(args...) } -func getSource(file string) (src string) { - if file == "" { - return - } - d, f := filepath.Split(file) - d = path.Base(d) - if d == "." || d == "" { - src = f - } else { - src = fmt.Sprintf("%s/%s", d, f) - } - return +// Warnf logging. +func Warnf(msg string, args ...interface{}) { + logger.Warnf(msg, args...) } -type nilFactory struct { +// Info logging. +func Info(args ...interface{}) { + logger.Info(args...) } -func (nf nilFactory) Create(fields map[string]interface{}) Logger { - return &nilLogger{fls: fields} +// Infof logging. +func Infof(msg string, args ...interface{}) { + logger.Infof(msg, args...) } -type nilLogger struct { - fls map[string]interface{} +// Debug logging. +func Debug(args ...interface{}) { + logger.Debug(args...) } -func (nl nilLogger) Level() Level { - return DebugLevel +// Debugf logging. +func Debugf(msg string, args ...interface{}) { + logger.Debugf(msg, args...) } -func (nl nilLogger) Fields() map[string]interface{} { - return nl.fls +type nilLogger struct { } func (nl nilLogger) Panic(args ...interface{}) { diff --git a/log/log_test.go b/log/log_test.go index d8586dc9f..4f0579bff 100644 --- a/log/log_test.go +++ b/log/log_test.go @@ -10,11 +10,11 @@ func TestSetup(t *testing.T) { assert := assert.New(t) tests := []struct { name string - f Factory + f FactoryFunc wantErr bool }{ {"failure with nil factory", nil, true}, - {"success", &nilFactory{}, false}, + {"success", func(map[string]interface{}) Logger { return nil }, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -28,35 +28,143 @@ func TestSetup(t *testing.T) { } } -func TestCreate(t *testing.T) { - assert := assert.New(t) - t.Run("success", func(t *testing.T) { - fields = map[string]interface{}{"key": "val"} - expected := map[string]interface{}{"key": "val", "src": "log/log_test.go"} - l := Create() - assert.NotNil(l) - assert.Equal(DebugLevel, l.Level()) - assert.Equal(expected, l.Fields()) - }) - t.Run("factory nil", func(t *testing.T) { - factory = nil - l := Create() - assert.Nil(l) - }) -} - -func Test_getSource(t *testing.T) { - assert := assert.New(t) - tests := []struct { - name string - file string - wantSrc string - }{ - {name: "empty", file: "", wantSrc: ""}, - {name: "no parent folder", file: "main.go", wantSrc: "main.go"}, - {name: "with parent folder", file: "/home/patron/main.go", wantSrc: "patron/main.go"}, - } - for _, tt := range tests { - assert.Equal(tt.wantSrc, getSource(tt.file)) - } +func TestLog_Panic(t *testing.T) { + l := testLogger{} + logger = &l + Panic("panic") + assert.Equal(t, 1, l.panicCount) +} + +func TestLog_Panicf(t *testing.T) { + l := testLogger{} + logger = &l + Panicf("panic %s", "1") + assert.Equal(t, 1, l.panicCount) +} + +func TestLog_Fatal(t *testing.T) { + l := testLogger{} + logger = &l + Fatal("fatal") + assert.Equal(t, 1, l.fatalCount) +} + +func TestLog_Fatalf(t *testing.T) { + l := testLogger{} + logger = &l + Fatalf("fatal %s", "1") + assert.Equal(t, 1, l.fatalCount) +} + +func TestLog_Error(t *testing.T) { + l := testLogger{} + logger = &l + Error("error") + assert.Equal(t, 1, l.errorCount) +} + +func TestLog_Errorf(t *testing.T) { + l := testLogger{} + logger = &l + Errorf("error %s", "1") + assert.Equal(t, 1, l.errorCount) +} + +func TestLog_Warn(t *testing.T) { + l := testLogger{} + logger = &l + Warn("warn") + assert.Equal(t, 1, l.warnCount) +} + +func TestLog_Warnf(t *testing.T) { + l := testLogger{} + logger = &l + Warnf("warn %s", "1") + assert.Equal(t, 1, l.warnCount) +} + +func TestLog_Info(t *testing.T) { + l := testLogger{} + logger = &l + Info("info") + assert.Equal(t, 1, l.infoCount) +} + +func TestLog_Infof(t *testing.T) { + l := testLogger{} + logger = &l + Infof("info %s", "1") + assert.Equal(t, 1, l.infoCount) +} + +func TestLog_Debug(t *testing.T) { + l := testLogger{} + logger = &l + Debug("debug") + assert.Equal(t, 1, l.debugCount) +} + +func TestLog_Debugf(t *testing.T) { + l := testLogger{} + logger = &l + Debugf("debug %s", "1") + assert.Equal(t, 1, l.debugCount) +} + +type testLogger struct { + debugCount int + infoCount int + warnCount int + errorCount int + fatalCount int + panicCount int +} + +func (t *testLogger) Panic(args ...interface{}) { + t.panicCount++ +} + +func (t *testLogger) Panicf(msg string, args ...interface{}) { + t.panicCount++ +} + +func (t *testLogger) Fatal(args ...interface{}) { + t.fatalCount++ +} + +func (t *testLogger) Fatalf(msg string, args ...interface{}) { + t.fatalCount++ +} + +func (t *testLogger) Error(args ...interface{}) { + t.errorCount++ +} + +func (t *testLogger) Errorf(msg string, args ...interface{}) { + t.errorCount++ +} + +func (t *testLogger) Warn(args ...interface{}) { + t.warnCount++ +} + +func (t *testLogger) Warnf(msg string, args ...interface{}) { + t.warnCount++ +} + +func (t *testLogger) Info(args ...interface{}) { + t.infoCount++ +} + +func (t *testLogger) Infof(msg string, args ...interface{}) { + t.infoCount++ +} + +func (t *testLogger) Debug(args ...interface{}) { + t.debugCount++ +} + +func (t *testLogger) Debugf(msg string, args ...interface{}) { + t.debugCount++ } diff --git a/log/zerolog/factory.go b/log/zerolog/factory.go index b653c39c4..2721fa060 100644 --- a/log/zerolog/factory.go +++ b/log/zerolog/factory.go @@ -1,34 +1,61 @@ package zerolog import ( + "fmt" "os" + "path" + "path/filepath" + "runtime" "time" "github.com/mantzas/patron/log" "github.com/rs/zerolog" ) -// Factory implementation of zerolog. -type Factory struct { - logger *zerolog.Logger - lvl log.Level +// Create creates a zerolog factory with default settings. +func Create(lvl log.Level) log.FactoryFunc { + zerolog.LevelFieldName = "lvl" + zerolog.MessageFieldName = "msg" + zerolog.TimeFieldFormat = time.RFC3339Nano + zl := zerolog.New(os.Stdout).With().Timestamp().Logger().Hook(sourceHook{skip: 7}) + return func(f map[string]interface{}) log.Logger { + return NewLogger(&zl, lvl, f) + } } -// NewFactory creates a new zerolog factory. -func NewFactory(l *zerolog.Logger, lvl log.Level) log.Factory { - return &Factory{logger: l, lvl: lvl} +type sourceHook struct { + skip int } -// DefaultFactory creates a zerolog factory with default settings. -func DefaultFactory(lvl log.Level) log.Factory { - zerolog.LevelFieldName = "lvl" - zerolog.MessageFieldName = "msg" - zerolog.TimeFieldFormat = time.RFC3339Nano - zl := zerolog.New(os.Stdout).With().Timestamp().Logger() - return NewFactory(&zl, lvl) +func (sh sourceHook) Run(e *zerolog.Event, level zerolog.Level, msg string) { + k, v, ok := sourceFields(sh.skip) + if !ok { + return + } + e.Str(k, v) +} + +func sourceFields(skip int) (key string, src string, ok bool) { + _, file, line, ok := runtime.Caller(skip) + if !ok { + return + } + src = getSource(file, line) + key = "src" + ok = true + return } -// Create a new logger. -func (zf *Factory) Create(f map[string]interface{}) log.Logger { - return NewLogger(zf.logger, zf.lvl, f) +func getSource(file string, line int) (src string) { + if file == "" { + return + } + d, f := filepath.Split(file) + d = path.Base(d) + if d == "." || d == "" { + src = fmt.Sprintf("%s:%d", f, line) + } else { + src = fmt.Sprintf("%s/%s:%d", d, f, line) + } + return } diff --git a/log/zerolog/factory_test.go b/log/zerolog/factory_test.go index 3003e0e74..e15b6418f 100644 --- a/log/zerolog/factory_test.go +++ b/log/zerolog/factory_test.go @@ -1,38 +1,38 @@ package zerolog import ( - "bytes" - "io" "testing" "github.com/mantzas/patron/log" - "github.com/rs/zerolog" "github.com/stretchr/testify/assert" ) -func TestNewFactory(t *testing.T) { - assert := assert.New(t) - var b bytes.Buffer - f := createFactory(&b) - assert.NotNil(f) -} - func TestDefaultFactory(t *testing.T) { assert := assert.New(t) - f := DefaultFactory(log.InfoLevel) + f := Create(log.InfoLevel) assert.NotNil(f) } -func TestFactory_Create(t *testing.T) { +func Test_getSource(t *testing.T) { assert := assert.New(t) - var b bytes.Buffer - f := createFactory(&b) - l := f.Create(nil) - assert.NotNil(l) - assert.Len(l.Fields(), 0) + tests := []struct { + name string + file string + wantSrc string + }{ + {name: "empty", file: "", wantSrc: ""}, + {name: "no parent folder", file: "main.go", wantSrc: "main.go:10"}, + {name: "with parent folder", file: "/home/patron/main.go", wantSrc: "patron/main.go:10"}, + } + for _, tt := range tests { + assert.Equal(tt.wantSrc, getSource(tt.file, 10)) + } } -func createFactory(wr io.Writer) log.Factory { - l := zerolog.New(wr) - return NewFactory(&l, log.DebugLevel) +func Test_sourceFields(t *testing.T) { + assert := assert.New(t) + key, src, ok := sourceFields(1) + assert.True(ok) + assert.Equal("src", key) + assert.Equal("zerolog/factory_test.go:34", src) } diff --git a/log/zerolog/logger.go b/log/zerolog/logger.go index f4e0c46a5..81a88fecf 100644 --- a/log/zerolog/logger.go +++ b/log/zerolog/logger.go @@ -24,8 +24,6 @@ func init() { // Logger abstraction based on zerolog. type Logger struct { logger *zerolog.Logger - fields map[string]interface{} - lvl log.Level } // NewLogger creates a new logger. @@ -34,17 +32,7 @@ func NewLogger(l *zerolog.Logger, lvl log.Level, f map[string]interface{}) log.L f = make(map[string]interface{}) } zl := l.Level(levelMap[lvl]).With().Fields(f).Logger() - return &Logger{logger: &zl, fields: f, lvl: lvl} -} - -// Level returns the minimum level. -func (zl Logger) Level() log.Level { - return zl.lvl -} - -// Fields returns the fields associated with this logger. -func (zl Logger) Fields() map[string]interface{} { - return zl.fields + return &Logger{logger: &zl} } // Panic logging. diff --git a/log/zerolog/logger_test.go b/log/zerolog/logger_test.go index ff4c24e5b..de7cdcc6c 100644 --- a/log/zerolog/logger_test.go +++ b/log/zerolog/logger_test.go @@ -27,19 +27,10 @@ func TestNewLogger(t *testing.T) { l := NewLogger(&zerolog.Logger{}, tt.lvl, tt.f) assert.NotNil(l) - assert.Equal(l.Level(), tt.lvl) - assert.Len(l.Fields(), tt.expectedFields) }) } } -func TestLogger_Fields(t *testing.T) { - assert := assert.New(t) - l := NewLogger(&zerolog.Logger{}, log.DebugLevel, f) - assert.NotNil(l) - assert.Equal(f, l.Fields()) -} - func TestLogger_Panic(t *testing.T) { assert := assert.New(t) var b bytes.Buffer diff --git a/option.go b/option.go index c2df7c26d..317676e9c 100644 --- a/option.go +++ b/option.go @@ -3,6 +3,7 @@ package patron import ( "errors" + "github.com/mantzas/patron/log" "github.com/mantzas/patron/sync/http" ) @@ -16,7 +17,7 @@ func Routes(rr []http.Route) OptionFunc { return errors.New("routes are required") } s.routes = rr - s.log.Info("routes options are set") + log.Info("routes options are set") return nil } } @@ -28,7 +29,7 @@ func HealthCheck(hcf http.HealthCheckFunc) OptionFunc { return errors.New("health check func is required") } s.hcf = hcf - s.log.Info("health check func is set") + log.Info("health check func is set") return nil } } @@ -40,7 +41,7 @@ func Components(cc ...Component) OptionFunc { return errors.New("components are required") } s.cps = append(s.cps, cc...) - s.log.Info("component options are set") + log.Info("component options are set") return nil } } diff --git a/service.go b/service.go index 9e92b57ce..23274b3df 100644 --- a/service.go +++ b/service.go @@ -36,7 +36,6 @@ type Service struct { hcf http.HealthCheckFunc ctx context.Context cancel context.CancelFunc - log log.Logger } // New creates a new named service and allows for customization through functional options. @@ -51,9 +50,9 @@ func New(name, version string, oo ...OptionFunc) (*Service, error) { } ctx, cancel := context.WithCancel(context.Background()) - s := Service{cps: []Component{}, hcf: http.DefaultHealthCheck, ctx: ctx, cancel: cancel, log: log.Create()} + s := Service{cps: []Component{}, hcf: http.DefaultHealthCheck, ctx: ctx, cancel: cancel} - err := s.setupLogging(name, version) + err := SetupLogging(name, version) if err != nil { return nil, err } @@ -85,7 +84,7 @@ func (s *Service) setupTermSignal() { stop := make(chan os.Signal, 1) signal.Notify(stop, os.Interrupt, syscall.SIGTERM) <-stop - s.log.Info("term signal received, cancelling") + log.Info("term signal received, cancelling") s.cancel() }() } @@ -105,14 +104,14 @@ func (s *Service) Run() error { select { case err := <-errCh: - s.log.Error("component returned a error") + log.Error("component returned a error") err1 := s.Shutdown() if err1 != nil { return errors.Wrapf(err, "failed to shutdown %v", err1) } return err case <-s.ctx.Done(): - s.log.Info("stop signal received") + log.Info("stop signal received") return s.Shutdown() } } @@ -124,10 +123,10 @@ func (s *Service) Shutdown() error { defer func() { err := trace.Close() if err != nil { - s.log.Errorf("failed to close trace %v", err) + log.Errorf("failed to close trace %v", err) } }() - s.log.Info("shutting down components") + log.Info("shutting down components") wg := sync.WaitGroup{} agr := agr_errors.New() @@ -165,7 +164,7 @@ func SetupLogging(name, version string) error { "host": hostname, } - err = log.Setup(zerolog.DefaultFactory(log.Level(lvl)), f) + err = log.Setup(zerolog.Create(log.Level(lvl)), f) if err != nil { return errors.Wrap(err, "failed to setup logging") } @@ -173,16 +172,6 @@ func SetupLogging(name, version string) error { return nil } -func (s *Service) setupLogging(name, version string) error { - - err := SetupLogging(name, version) - if err != nil { - return err - } - s.log = log.Create() - return nil -} - func (s *Service) setupDefaultTracing(name, version string) error { agent, ok := os.LookupEnv("PATRON_JAEGER_AGENT") if !ok { @@ -204,7 +193,7 @@ func (s *Service) setupDefaultTracing(name, version string) error { return errors.Wrap(err, "env var for jaeger sampler param is not valid") } } - s.log.Infof("setting up default tracing to %s, %s with param %s", agent, tp, prm) + log.Infof("setting up default tracing to %s, %s with param %s", agent, tp, prm) return trace.Setup(name, version, agent, tp, prmVal) } @@ -221,7 +210,7 @@ func (s *Service) createHTTPComponent() (Component, error) { } } - s.log.Infof("creating default HTTP component at port %s", strconv.FormatInt(portVal, 10)) + log.Infof("creating default HTTP component at port %s", strconv.FormatInt(portVal, 10)) options := []http.OptionFunc{ http.Port(int(portVal)), diff --git a/sync/http/component.go b/sync/http/component.go index c85e03e11..9208c82c3 100644 --- a/sync/http/component.go +++ b/sync/http/component.go @@ -29,7 +29,6 @@ type Component struct { srv *http.Server certFile string keyFile string - log log.Logger } // New returns a new component. @@ -53,8 +52,7 @@ func New(oo ...OptionFunc) (*Component, error) { // Run starts the HTTP server. func (s *Component) Run(ctx context.Context) error { s.m.Lock() - s.log = log.Create() - s.log.Debug("applying tracing to routes") + log.Debug("applying tracing to routes") for i := 0; i < len(s.routes); i++ { if s.routes[i].Trace { s.routes[i].Handler = DefaultMiddleware(s.routes[i].Pattern, s.routes[i].Handler) @@ -62,15 +60,15 @@ func (s *Component) Run(ctx context.Context) error { s.routes[i].Handler = RecoveryMiddleware(s.routes[i].Handler) } } - s.srv = createHTTPServer(s.port, createHandler(s.routes, s.log.Debugf)) + s.srv = createHTTPServer(s.port, createHandler(s.routes)) s.m.Unlock() if s.certFile != "" && s.keyFile != "" { - s.log.Infof("HTTPS component listening on port %d", s.port) + log.Infof("HTTPS component listening on port %d", s.port) return s.srv.ListenAndServeTLS(s.certFile, s.keyFile) } - s.log.Infof("HTTP component listening on port %d", s.port) + log.Infof("HTTP component listening on port %d", s.port) return s.srv.ListenAndServe() } @@ -78,7 +76,7 @@ func (s *Component) Run(ctx context.Context) error { func (s *Component) Shutdown(ctx context.Context) error { s.m.Lock() defer s.m.Unlock() - s.log.Info("shutting down component") + log.Info("shutting down component") if s.srv == nil { return nil } @@ -95,12 +93,12 @@ func createHTTPServer(port int, sm http.Handler) *http.Server { } } -func createHandler(routes []Route, logf func(msg string, args ...interface{})) http.Handler { - logf("adding %d routes", len(routes)) +func createHandler(routes []Route) http.Handler { + log.Debugf("adding %d routes", len(routes)) router := httprouter.New() for _, route := range routes { router.HandlerFunc(route.Method, route.Pattern, route.Handler) - logf("added route %s %s", route.Method, route.Pattern) + log.Debugf("added route %s %s", route.Method, route.Pattern) } return router } diff --git a/sync/http/component_test.go b/sync/http/component_test.go index bca3ca97d..181c9fc1e 100644 --- a/sync/http/component_test.go +++ b/sync/http/component_test.go @@ -82,7 +82,6 @@ func Test_createHTTPServer(t *testing.T) { func TestCreateHandler(t *testing.T) { assert := assert.New(t) - infof := func(msg string, args ...interface{}) {} - h := createHandler([]Route{NewRoute("/", "GET", nil, false)}, infof) + h := createHandler([]Route{NewRoute("/", "GET", nil, false)}) assert.NotNil(h) } diff --git a/sync/http/handler_test.go b/sync/http/handler_test.go index 1e7868e16..7a69190f2 100644 --- a/sync/http/handler_test.go +++ b/sync/http/handler_test.go @@ -218,8 +218,7 @@ func Test_extractParams(t *testing.T) { fields = req.Fields return nil, nil } - infof := func(msg string, args ...interface{}) {} - h := createHandler([]Route{NewRoute("/users/:id/status", "GET", proc, false)}, infof) + h := createHandler([]Route{NewRoute("/users/:id/status", "GET", proc, false)}) h.ServeHTTP(httptest.NewRecorder(), req) assert.Equal("1", fields["id"]) } diff --git a/trace/sql/sql.go b/trace/sql/sql.go index ff5066088..21f542aee 100644 --- a/trace/sql/sql.go +++ b/trace/sql/sql.go @@ -17,7 +17,6 @@ type connInfo struct { func (c *connInfo) startSpan( ctx context.Context, opName, stmt string, - tags ...opentracing.Tag, ) (opentracing.Span, context.Context) { return trace.SQLSpan(ctx, opName, "sql", "rdbms", c.instance, c.user, stmt) } diff --git a/trace/trace.go b/trace/trace.go index 0de73a691..9fdfca1cb 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -31,9 +31,8 @@ const ( ) var ( - cls io.Closer - innerLog log.Logger - version string + cls io.Closer + version string ) func init() { @@ -60,7 +59,7 @@ func Setup(name, ver, agentAddress, samplerType string, samplerParam float64) er time.Sleep(100 * time.Millisecond) metricsFactory := prometheus.New() tr, clsTemp, err := cfg.NewTracer( - config.Logger(jaegerLoggerAdapter{log: log.Create()}), + config.Logger(jaegerLoggerAdapter{}), config.Observer(rpcmetrics.NewObserver(metricsFactory.Namespace(name, nil), rpcmetrics.DefaultNameNormalizer)), ) if err != nil { @@ -69,13 +68,12 @@ func Setup(name, ver, agentAddress, samplerType string, samplerParam float64) er cls = clsTemp opentracing.SetGlobalTracer(tr) version = ver - innerLog = log.Create() return nil } // Close the tracer. func Close() error { - innerLog.Debug("closing tracer") + log.Debug("closing tracer") return cls.Close() } @@ -87,7 +85,7 @@ func ConsumerSpan( ) (opentracing.Span, context.Context) { spCtx, err := opentracing.GlobalTracer().Extract(opentracing.HTTPHeaders, opentracing.TextMapCarrier(hdr)) if err != nil && err != opentracing.ErrSpanContextNotFound { - innerLog.Errorf("failed to extract consumer span: %v", err) + log.Errorf("failed to extract consumer span: %v", err) } sp := opentracing.StartSpan(name, consumerOption{ctx: spCtx}) ext.Component.Set(sp, cmp) @@ -111,7 +109,7 @@ func SpanError(sp opentracing.Span) { func HTTPSpan(path string, r *http.Request) (opentracing.Span, *http.Request) { ctx, err := opentracing.GlobalTracer().Extract(opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(r.Header)) if err != nil && err != opentracing.ErrSpanContextNotFound { - innerLog.Errorf("failed to extract HTTP span: %v", err) + log.Errorf("failed to extract HTTP span: %v", err) } sp := opentracing.StartSpan(HTTPOpName(r.Method, path), ext.RPCServerOption(ctx)) ext.HTTPMethod.Set(sp, r.Method) @@ -167,15 +165,14 @@ func HTTPOpName(method, path string) string { } type jaegerLoggerAdapter struct { - log log.Logger } func (l jaegerLoggerAdapter) Error(msg string) { - l.log.Error(msg) + log.Error(msg) } func (l jaegerLoggerAdapter) Infof(msg string, args ...interface{}) { - l.log.Infof(msg, args...) + log.Infof(msg, args...) } type consumerOption struct { diff --git a/trace/trace_test.go b/trace/trace_test.go index 019d6099d..bb0e2a57f 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -5,8 +5,6 @@ import ( "net/http" "testing" - "github.com/mantzas/patron/log" - "github.com/mantzas/patron/log/zerolog" "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" @@ -25,7 +23,6 @@ func TestSetup_Tracer_Close(t *testing.T) { func TestStartFinishConsumerSpan(t *testing.T) { assert := assert.New(t) - innerLog = log.Create() mtr := mocktracer.New() opentracing.SetGlobalTracer(mtr) hdr := map[string]string{"key": "val"} @@ -49,9 +46,6 @@ func TestStartFinishConsumerSpan(t *testing.T) { func TestStartFinishChildSpan(t *testing.T) { assert := assert.New(t) - err := log.Setup(zerolog.DefaultFactory(log.DebugLevel), nil) - assert.NoError(err) - innerLog = log.Create() mtr := mocktracer.New() opentracing.SetGlobalTracer(mtr) sp, ctx := ConsumerSpan(context.Background(), "test", AMQPConsumerComponent, nil) @@ -86,9 +80,6 @@ func TestStartFinishChildSpan(t *testing.T) { func TestHTTPStartFinishSpan(t *testing.T) { assert := assert.New(t) - err := log.Setup(zerolog.DefaultFactory(log.DebugLevel), nil) - assert.NoError(err) - innerLog = log.Create() mtr := mocktracer.New() opentracing.SetGlobalTracer(mtr) req, err := http.NewRequest("GET", "/", nil)