From 1258f5a1d28b87f1028d6eb42a06a417fe611abf Mon Sep 17 00:00:00 2001 From: Tim Raymond Date: Mon, 26 Sep 2016 12:12:13 -0400 Subject: [PATCH] Add logrus for structured logging This adds the logrus package to facilitate structured logging. Logrus is encapsulated in the log/ subpackage and currently used in the influx/ subpackage. --- Godeps | 1 + influx/influx.go | 21 +++++++++++++++++---- influx/influx_test.go | 29 +++++++++++++++++++++++++++-- log/log.go | 26 ++++++++++++++++++++++++++ restapi/configure_mr_fusion.go | 1 + 5 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 log/log.go diff --git a/Godeps b/Godeps index 8e91935e04..6b9be89140 100644 --- a/Godeps +++ b/Godeps @@ -1,5 +1,6 @@ github.com/PuerkitoBio/purell 8a290539e2e8629dbc4e6bad948158f790ec31f4 github.com/PuerkitoBio/urlesc 5bd2802263f21d8788851d5305584c82a5c75d7e +github.com/Sirupsen/logrus 3ec0642a7fb6488f65b06f9040adc67e3990296a github.com/asaskevich/govalidator 593d64559f7600f29581a3ee42177f5dbded27a9 github.com/boltdb/bolt 5cc10bbbc5c141029940133bb33c9e969512a698 github.com/elazarl/go-bindata-assetfs 9a6736ed45b44bf3835afeebb3034b57ed329f3e diff --git a/influx/influx.go b/influx/influx.go index 90dba90083..36aad2a69a 100644 --- a/influx/influx.go +++ b/influx/influx.go @@ -7,6 +7,7 @@ import ( "net/url" "github.com/influxdata/mrfusion" + "github.com/influxdata/mrfusion/log" "golang.org/x/net/context" ) @@ -14,18 +15,23 @@ import ( // Client is a device for retrieving time series data from an InfluxDB instance type Client struct { URL *url.URL + + lg log.Logger } // NewClient initializes an HTTP Client for InfluxDB. UDP, although supported // for querying InfluxDB, is not supported here to remove the need to // explicitly Close the client. -func NewClient(host string) (*Client, error) { +func NewClient(host string, lg log.Logger) (*Client, error) { + l := lg.WithField("host", host) u, err := url.Parse(host) if err != nil { + l.Error("Error initialize influx client: err:", err) return nil, err } return &Client{ URL: u, + lg: l, }, nil } @@ -38,7 +44,7 @@ func (r Response) MarshalJSON() ([]byte, error) { return r.Results, nil } -func query(u *url.URL, q mrfusion.Query) (mrfusion.Response, error) { +func (c *Client) query(u *url.URL, q mrfusion.Query) (mrfusion.Response, error) { u.Path = "query" req, err := http.NewRequest("POST", u.String(), nil) @@ -74,12 +80,19 @@ func query(u *url.URL, q mrfusion.Query) (mrfusion.Response, error) { // If we got a valid decode error, send that back if decErr != nil { - return nil, fmt.Errorf("unable to decode json: received status code %d err: %s", resp.StatusCode, decErr) + c.lg. + WithField("influx_status", resp.StatusCode). + Error("Error parsing results from influxdb: err:", decErr) + return nil, decErr } // If we don't have an error in our json response, and didn't get statusOK // then send back an error if resp.StatusCode != http.StatusOK && response.Err != "" { + c.lg. + WithField("influx_status", resp.StatusCode). + Error("Received non-200 response from influxdb") + return &response, fmt.Errorf("received status code %d from server", resp.StatusCode) } @@ -98,7 +111,7 @@ type result struct { func (c *Client) Query(ctx context.Context, q mrfusion.Query) (mrfusion.Response, error) { resps := make(chan (result)) go func() { - resp, err := query(c.URL, q) + resp, err := c.query(c.URL, q) resps <- result{resp, err} }() diff --git a/influx/influx_test.go b/influx/influx_test.go index 1a8ec03d84..84247e3034 100644 --- a/influx/influx_test.go +++ b/influx/influx_test.go @@ -8,6 +8,7 @@ import ( "github.com/influxdata/mrfusion" "github.com/influxdata/mrfusion/influx" + "github.com/influxdata/mrfusion/log" "golang.org/x/net/context" ) @@ -25,7 +26,7 @@ func Test_Influx_MakesRequestsToQueryEndpoint(t *testing.T) { defer ts.Close() var series mrfusion.TimeSeries - series, err := influx.NewClient(ts.URL) + series, err := influx.NewClient(ts.URL, log.New()) if err != nil { t.Fatal("Unexpected error initializing client: err:", err) } @@ -58,7 +59,7 @@ func Test_Influx_CancelsInFlightRequests(t *testing.T) { ts.Close() }() - series, _ := influx.NewClient(ts.URL) + series, _ := influx.NewClient(ts.URL, log.New()) ctx, cancel := context.WithCancel(context.Background()) errs := make(chan (error)) @@ -99,3 +100,27 @@ func Test_Influx_CancelsInFlightRequests(t *testing.T) { t.Error("Expected timeout error but wasn't. err was", err) } } + +func Test_Influx_RejectsInvalidHosts(t *testing.T) { + _, err := influx.NewClient(":", log.New()) + if err == nil { + t.Fatal("Expected err but was nil") + } +} + +func Test_Influx_ReportsInfluxErrs(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + cl, err := influx.NewClient(ts.URL, log.New()) + if err != nil { + t.Fatal("Encountered unexpected error while initializing influx client: err:", err) + } + + _, err = cl.Query(context.Background(), mrfusion.Query{"show shards", "_internal", "autogen"}) + if err == nil { + t.Fatal("Expected an error but received none") + } +} diff --git a/log/log.go b/log/log.go new file mode 100644 index 0000000000..377a6d9eea --- /dev/null +++ b/log/log.go @@ -0,0 +1,26 @@ +package log + +import ( + "os" + + "github.com/Sirupsen/logrus" +) + +type Logger interface { + Info(...interface{}) + Warn(...interface{}) + Debug(...interface{}) + Panic(...interface{}) + Error(...interface{}) + + WithField(string, interface{}) *logrus.Entry +} + +func New() Logger { + return &logrus.Logger{ + Out: os.Stderr, + Formatter: new(logrus.TextFormatter), + Hooks: make(logrus.LevelHooks), + Level: logrus.DebugLevel, + } +} diff --git a/restapi/configure_mr_fusion.go b/restapi/configure_mr_fusion.go index 2ea0132a90..f82987b0bc 100644 --- a/restapi/configure_mr_fusion.go +++ b/restapi/configure_mr_fusion.go @@ -17,6 +17,7 @@ import ( "github.com/influxdata/mrfusion/dist" "github.com/influxdata/mrfusion/handlers" "github.com/influxdata/mrfusion/influx" + fusionlog "github.com/influxdata/mrfusion/log" "github.com/influxdata/mrfusion/mock" op "github.com/influxdata/mrfusion/restapi/operations" )