From 8982b1578393f1888c8eccb1d6364b14aab4681f Mon Sep 17 00:00:00 2001 From: johnabass Date: Wed, 30 Aug 2017 19:18:39 -0700 Subject: [PATCH 1/2] Updated webpa-common --- src/glide.lock | 45 ++++++++++++++++++++------------------------- src/glide.yaml | 11 +---------- 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/glide.lock b/src/glide.lock index 031d0b03..18795549 100644 --- a/src/glide.lock +++ b/src/glide.lock @@ -1,5 +1,5 @@ -hash: 9d972e38411bdc71b9d6eb05d4a9d606230d28a67ab8d5e5bedda799249ce70d -updated: 2017-08-24T15:30:24.893161003-07:00 +hash: 239eddb48eed656cce1c7b0f957b9d4a89399be7bdf97409e8993c11eb99e914 +updated: 2017-08-30T19:18:12.858965759-07:00 imports: - name: github.com/billhathaway/consistentHash version: addea16d2229dba874111898b45be7f4a78af631 @@ -8,7 +8,7 @@ imports: subpackages: - linux - name: github.com/Comcast/webpa-common - version: 7696a5eab9625e508963a5a2e9343214060f9718 + version: 9f0810dec1058c339282960f3e34d9d94f8dd6be subpackages: - concurrent - device @@ -16,12 +16,20 @@ imports: - health - httperror - logging - - logging/golog - server - service - wrp - name: github.com/fsnotify/fsnotify version: a904159b9206978bb6d53fcc7a769e5cd726c737 +- name: github.com/go-kit/kit + version: a9ca6725cbbea455e61c6bc8a1ed28e81eb3493b + subpackages: + - log + - log/level +- name: github.com/go-logfmt/logfmt + version: 390ab7935ee28ec6b286364bba9b4dd6410cb3d5 +- name: github.com/go-stack/stack + version: 817915b46b97fd7bb80e8ab6b69f01a53ac3eebf - name: github.com/gorilla/context version: 08b5f424b9271eedf6f9f0ce86cb9396ed337a42 - name: github.com/gorilla/mux @@ -39,13 +47,8 @@ imports: - json/parser - json/scanner - json/token -- name: github.com/ian-kent/go-log - version: 5731446c36ab9f716106ce0731f484c50fdf1ad1 - subpackages: - - appenders - - layout - - levels - - logger +- name: github.com/kr/logfmt + version: b84e30acd515aadc4b783ad4ff83aff3299bdfe0 - name: github.com/magiconair/properties version: b3b15ef068fd0b17ddf408a23669f20811d194d2 - name: github.com/mitchellh/mapstructure @@ -54,8 +57,6 @@ imports: version: df1e16fde7fc330a0ca68167c23bf7ed6ac31d6d - name: github.com/pelletier/go-toml version: d1fa2118c12c44e4f5004da216d1efad10cb4924 -- name: github.com/philhofer/fwd - version: 98c11a7a6ec829d672b03833c3d69a7fae1ca972 - name: github.com/samuel/go-zookeeper version: e6b59f6144beb8570562539c1898a0b1fea34b41 subpackages: @@ -71,19 +72,11 @@ imports: - name: github.com/spf13/jwalterweatherman version: fa7ca7e836cf3a8bb4ebf799f472c12d7e903d66 - name: github.com/spf13/pflag - version: 9ff6c6923cfffbcd502984b8e0c80539a94968b7 + version: e57e3eeb33f795204c1ca35f56c44f83227c6e66 - name: github.com/spf13/viper - version: 5ed0fc31f7f453625df314d8e66b9791e8d13003 + version: 25b30aa063fc18e48662b86996252eabdcf2f0c7 - name: github.com/strava/go.serversets version: 28ab685d470f2bfb6d8abe4077cc1ab19c33e24b -- name: github.com/t-k/fluent-logger-golang - version: 0f8ec08f2057a61574b6943e75045fffbeae894e - subpackages: - - fluent -- name: github.com/tinylib/msgp - version: 38a6f61a768dc24552dad94611923841b53acc4f - subpackages: - - msgp - name: github.com/ugorji/go version: d23841a297e5489e787e72fceffabf9d2994b52a subpackages: @@ -97,11 +90,13 @@ imports: subpackages: - transform - unicode/norm +- name: gopkg.in/natefinch/lumberjack.v2 + version: a96e63847dc3c67d17befa69c303767e2f84e54f - name: gopkg.in/yaml.v2 version: 4c78c975fe7c825c6d1466c42be594d1d6f3aba6 testImports: - name: github.com/davecgh/go-spew - version: 6d212800a42e8ab5c146b8ace3490ee17e5225f9 + version: 04cdfd42973bb9c8589fd6a731800cf222fde1a9 subpackages: - spew - name: github.com/pmezard/go-difflib @@ -111,7 +106,7 @@ testImports: - name: github.com/stretchr/objx version: cbeaeb16a013161a98496fad62933b1d21786672 - name: github.com/stretchr/testify - version: 69483b4bd14f5845b5a1e55bca19e954e827f1d0 + version: 890a5c3458b43e6104ff5da8dfa139d013d77544 subpackages: - assert - mock diff --git a/src/glide.yaml b/src/glide.yaml index 9f2f7fa1..4d15c96a 100644 --- a/src/glide.yaml +++ b/src/glide.yaml @@ -2,14 +2,5 @@ package: . homepage: https://github.com/Comcast/talaria import: - package: github.com/Comcast/webpa-common - version: 7696a5eab9625e508963a5a2e9343214060f9718 - subpackages: - - device - - logging - - wrp - - concurrent - - server - - service -- package: github.com/gorilla/mux - version: v1.3.0 + version: 9f0810dec1058c339282960f3e34d9d94f8dd6be From 0b5ed42b8129f70c1b88d8bc58f99d9046b21d62 Mon Sep 17 00:00:00 2001 From: johnabass Date: Wed, 30 Aug 2017 19:31:17 -0700 Subject: [PATCH 2/2] Updated logging; updated sample configuration --- src/talaria/dispatcher.go | 12 +++++++----- src/talaria/outbounder.go | 9 +++++---- src/talaria/primaryHandler.go | 4 ++-- src/talaria/talaria.go | 14 +++++++++----- src/talaria/talaria.json | 5 +++-- src/talaria/workerPool.go | 17 +++++++++++------ 6 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/talaria/dispatcher.go b/src/talaria/dispatcher.go index 5b948e27..2622e71f 100644 --- a/src/talaria/dispatcher.go +++ b/src/talaria/dispatcher.go @@ -12,6 +12,7 @@ import ( "github.com/Comcast/webpa-common/device" "github.com/Comcast/webpa-common/logging" "github.com/Comcast/webpa-common/wrp" + "github.com/go-kit/kit/log" ) // outboundEnvelope is a tuple of information related to handling an asynchronous HTTP request @@ -30,7 +31,7 @@ type Dispatcher interface { // dispatcher is the internal Dispatcher implementation type dispatcher struct { - logger logging.Logger + errorLog log.Logger urlFilter URLFilter method string timeout time.Duration @@ -51,8 +52,9 @@ func NewDispatcher(o *Outbounder, urlFilter URLFilter) (Dispatcher, <-chan *outb } outbounds := make(chan *outboundEnvelope, o.outboundQueueSize()) + logger := o.logger() return &dispatcher{ - logger: o.logger(), + errorLog: logging.Error(logger), urlFilter: urlFilter, method: o.method(), timeout: o.requestTimeout(), @@ -150,15 +152,15 @@ func (d *dispatcher) OnDeviceEvent(event *device.Event) { if strings.HasPrefix(destination, EventPrefix) { eventType := destination[len(EventPrefix):] if err := d.dispatchEvent(eventType, contentType, event.Contents); err != nil { - d.logger.Error("Error dispatching event [%s]: %s", destination, err) + d.errorLog.Log(logging.MessageKey(), "Error dispatching event", "destination", destination, logging.ErrorKey(), err) } } else if strings.HasPrefix(destination, DNSPrefix) { unfilteredURL := destination[len(DNSPrefix):] if err := d.dispatchTo(unfilteredURL, contentType, event.Contents); err != nil { - d.logger.Error("Error dispatching to [%s]: %s", destination, err) + d.errorLog.Log(logging.MessageKey(), "Error dispatching to endpoint", "destination", destination, logging.ErrorKey(), err) } } else { - d.logger.Error("Unable to route to [%s]", destination) + d.errorLog.Log(logging.MessageKey(), "Unroutable destination", "destination", destination) } } } diff --git a/src/talaria/outbounder.go b/src/talaria/outbounder.go index 00506289..3563460a 100644 --- a/src/talaria/outbounder.go +++ b/src/talaria/outbounder.go @@ -6,6 +6,7 @@ import ( "github.com/Comcast/webpa-common/device" "github.com/Comcast/webpa-common/logging" + "github.com/go-kit/kit/log" "github.com/spf13/viper" ) @@ -48,13 +49,13 @@ type Outbounder struct { MaxIdleConnsPerHost int `json:"maxIdleConnsPerHost"` IdleConnTimeout time.Duration `json:"idleConnTimeout"` AuthKey []string `json:"authKey"` - Logger logging.Logger `json:"-"` + Logger log.Logger `json:"-"` } // NewOutbounder returns an Outbounder unmarshalled from a Viper environment. // This function allows the Viper instance to be nil, in which case a default // Outbounder is returned. -func NewOutbounder(logger logging.Logger, v *viper.Viper) (o *Outbounder, err error) { +func NewOutbounder(logger log.Logger, v *viper.Viper) (o *Outbounder, err error) { o = &Outbounder{ Method: DefaultMethod, RequestTimeout: DefaultRequestTimeout, @@ -85,7 +86,7 @@ func (o *Outbounder) String() string { } } -func (o *Outbounder) logger() logging.Logger { +func (o *Outbounder) logger() log.Logger { if o != nil && o.Logger != nil { return o.Logger } @@ -198,7 +199,7 @@ func (o *Outbounder) clientTimeout() time.Duration { // Start spawns all necessary goroutines and returns a device.Listener func (o *Outbounder) Start() (device.Listener, error) { - o.logger().Info("Starting outbounder: %s", o) + logging.Info(o.logger()).Log(logging.MessageKey(), "Starting outbounder", "outbounder", o) dispatcher, outbounds, err := NewDispatcher(o, nil) if err != nil { return nil, err diff --git a/src/talaria/primaryHandler.go b/src/talaria/primaryHandler.go index 25838558..9d66be87 100644 --- a/src/talaria/primaryHandler.go +++ b/src/talaria/primaryHandler.go @@ -4,8 +4,8 @@ import ( "net/http" "github.com/Comcast/webpa-common/device" - "github.com/Comcast/webpa-common/logging" "github.com/Comcast/webpa-common/wrp" + "github.com/go-kit/kit/log" "github.com/gorilla/mux" "github.com/spf13/viper" ) @@ -17,7 +17,7 @@ const ( version = "v2" ) -func NewPrimaryHandler(logger logging.Logger, connectedUpdates <-chan []byte, manager device.Manager, v *viper.Viper) (http.Handler, error) { +func NewPrimaryHandler(logger log.Logger, connectedUpdates <-chan []byte, manager device.Manager, v *viper.Viper) (http.Handler, error) { poolFactory, err := wrp.NewPoolFactory(v.Sub(wrp.ViperKey)) if err != nil { return nil, err diff --git a/src/talaria/talaria.go b/src/talaria/talaria.go index 5cd01579..9ace27ad 100644 --- a/src/talaria/talaria.go +++ b/src/talaria/talaria.go @@ -15,6 +15,7 @@ import ( "github.com/Comcast/webpa-common/logging" "github.com/Comcast/webpa-common/server" "github.com/Comcast/webpa-common/service" + "github.com/go-kit/kit/log" "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -28,7 +29,7 @@ const ( // startDeviceManagement handles the configuration and initialization of the device management subsystem // for talaria. The returned HTTP handler can be used for device connections and messages, while the returned // Manager can be used to route and administer the set of connected devices. -func startDeviceManagement(logger logging.Logger, h *health.Health, v *viper.Viper) (http.Handler, device.Manager, error) { +func startDeviceManagement(logger log.Logger, h *health.Health, v *viper.Viper) (http.Handler, device.Manager, error) { deviceOptions, err := device.NewOptions(logger, v.Sub(device.DeviceManagerKey)) if err != nil { return nil, nil, err @@ -80,7 +81,10 @@ func talaria(arguments []string) int { return 1 } - logger.Info("Using configuration file: %s", v.ConfigFileUsed()) + var ( + infoLog = logging.Info(logger) + errorLog = logging.Error(logger) + ) // // Initialize the manager first, as if it fails we don't want to advertise this service @@ -110,7 +114,7 @@ func talaria(arguments []string) int { return 2 } - logger.Info("Service options: %#v", serviceOptions) + infoLog.Log("configurationFile", v.ConfigFileUsed(), "serviceOptions", serviceOptions) var ( accessor = service.NewUpdatableAccessor(serviceOptions, nil) @@ -122,14 +126,14 @@ func talaria(arguments []string) int { manager.DisconnectIf(func(candidate device.ID) bool { hashedEndpoint, err := accessor.Get(candidate.Bytes()) if err != nil { - logger.Error("Error while attempting to rehash device id %s: %s", candidate, err) + errorLog.Log(logging.MessageKey(), "Error while attempting to rehash device", "deviceID", candidate, logging.ErrorKey(), err) return true } // disconnect if hashedEndpoint was NOT found in the endpoints that this talaria instance registered under disconnect := !registeredEndpoints.Has(hashedEndpoint) if disconnect { - logger.Info("Disconnecting %s due to service discovery rehash", candidate) + infoLog.Log(logging.MessageKey(), "Service discovery rehash", "deviceID", candidate) } return disconnect diff --git a/src/talaria/talaria.json b/src/talaria/talaria.json index 56379b4c..8f3dc44e 100644 --- a/src/talaria/talaria.json +++ b/src/talaria/talaria.json @@ -26,7 +26,8 @@ }, "log" : { - "file" : "console", - "level" : "DEBUG" + "file" : "stdout", + "level" : "DEBUG", + "json": true } } diff --git a/src/talaria/workerPool.go b/src/talaria/workerPool.go index 95e502a9..f4c39624 100644 --- a/src/talaria/workerPool.go +++ b/src/talaria/workerPool.go @@ -1,11 +1,13 @@ package main import ( - "github.com/Comcast/webpa-common/logging" "io" "io/ioutil" "net/http" "sync" + + "github.com/Comcast/webpa-common/logging" + "github.com/go-kit/kit/log" ) // NewTransactor returns a closure which can handle HTTP transactions. @@ -25,7 +27,8 @@ func NewTransactor(o *Outbounder) func(*http.Request) (*http.Response, error) { // WorkerPool describes a pool of goroutines that dispatch http.Request objects to // a transactor function type WorkerPool struct { - logger logging.Logger + errorLog log.Logger + debugLog log.Logger outbounds <-chan *outboundEnvelope workerPoolSize uint transactor func(*http.Request) (*http.Response, error) @@ -34,8 +37,10 @@ type WorkerPool struct { } func NewWorkerPool(o *Outbounder, outbounds <-chan *outboundEnvelope) *WorkerPool { + logger := o.logger() return &WorkerPool{ - logger: o.logger(), + errorLog: logging.Error(logger), + debugLog: logging.Debug(logger), outbounds: outbounds, workerPoolSize: o.workerPoolSize(), transactor: NewTransactor(o), @@ -59,14 +64,14 @@ func (wp *WorkerPool) transact(e *outboundEnvelope) { response, err := wp.transactor(e.request) if err != nil { - wp.logger.Error("HTTP error: %s", err) + wp.errorLog.Log(logging.MessageKey(), "HTTP transaction error", logging.ErrorKey(), err) return } if response.StatusCode < 400 { - wp.logger.Debug("HTTP response status: %s, target: %s", response.Status, e.request.URL) + wp.debugLog.Log(logging.MessageKey(), "HTTP response", "status", response.Status, "url", e.request.URL) } else { - wp.logger.Error("HTTP response status: %s, target: %s", response.Status, e.request.URL) + wp.errorLog.Log(logging.MessageKey(), "HTTP response", "status", response.Status, "url", e.request.URL) } io.Copy(ioutil.Discard, response.Body)