From 4b424a2a739141cb0d477b4805a56c824b70cf74 Mon Sep 17 00:00:00 2001 From: ncohensm Date: Tue, 12 Jul 2016 09:35:04 -0700 Subject: [PATCH] address further code review comments --- plugins/inputs/http_listener/README.md | 6 +-- plugins/inputs/http_listener/http_listener.go | 37 ++++++++----------- .../http_listener/http_listener_test.go | 5 ++- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/plugins/inputs/http_listener/README.md b/plugins/inputs/http_listener/README.md index 8066fa00e..5f0c5970e 100644 --- a/plugins/inputs/http_listener/README.md +++ b/plugins/inputs/http_listener/README.md @@ -17,7 +17,7 @@ This is a sample configuration for the plugin. ## Address and port to host HTTP listener on service_address = ":8186" - ## timeouts in seconds - read_timeout = "10" - write_timeout = "10" + ## timeouts + read_timeout = "10s" + write_timeout = "10s" ``` diff --git a/plugins/inputs/http_listener/http_listener.go b/plugins/inputs/http_listener/http_listener.go index 0474bf082..30f7411aa 100644 --- a/plugins/inputs/http_listener/http_listener.go +++ b/plugins/inputs/http_listener/http_listener.go @@ -5,11 +5,11 @@ import ( "log" "net" "net/http" - "strconv" "sync" "time" "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/plugins/inputs" "github.com/influxdata/telegraf/plugins/inputs/http_listener/stoppableListener" "github.com/influxdata/telegraf/plugins/parsers" @@ -17,8 +17,8 @@ import ( type HttpListener struct { ServiceAddress string - ReadTimeout string - WriteTimeout string + ReadTimeout internal.Duration + WriteTimeout internal.Duration sync.Mutex @@ -32,9 +32,9 @@ const sampleConfig = ` ## Address and port to host HTTP listener on service_address = ":8186" - ## timeouts in seconds - read_timeout = "10" - write_timeout = "10" + ## timeouts + read_timeout = "10s" + write_timeout = "10s" ` func (t *HttpListener) SampleConfig() string { @@ -90,19 +90,17 @@ func (t *HttpListener) Stop() { // httpListen listens for HTTP requests. func (t *HttpListener) httpListen() error { - readTimeout, err := strconv.ParseInt(t.ReadTimeout, 10, 32) - if err != nil { - return err + if t.ReadTimeout.Duration < time.Second { + t.ReadTimeout.Duration = time.Second * 10 } - writeTimeout, err := strconv.ParseInt(t.WriteTimeout, 10, 32) - if err != nil { - return err + if t.WriteTimeout.Duration < time.Second { + t.WriteTimeout.Duration = time.Second * 10 } var server = http.Server{ Handler: t, - ReadTimeout: time.Duration(readTimeout) * time.Second, - WriteTimeout: time.Duration(writeTimeout) * time.Second, + ReadTimeout: t.ReadTimeout.Duration, + WriteTimeout: t.WriteTimeout.Duration, } return server.Serve(t.listener) @@ -113,8 +111,8 @@ func (t *HttpListener) ServeHTTP(res http.ResponseWriter, req *http.Request) { if err != nil { log.Printf("Problem reading request: [%s], Error: %s\n", string(body), err) - res.WriteHeader(http.StatusInternalServerError) - res.Write([]byte("ERROR reading request")) + http.Error(res, "ERROR reading request", http.StatusInternalServerError) + return } var path = req.URL.Path[1:] @@ -125,11 +123,9 @@ func (t *HttpListener) ServeHTTP(res http.ResponseWriter, req *http.Request) { if err == nil { t.storeMetrics(metrics) res.WriteHeader(http.StatusNoContent) - res.Write([]byte("")) } else { log.Printf("Problem parsing body: [%s], Error: %s\n", string(body), err) - res.WriteHeader(http.StatusInternalServerError) - res.Write([]byte("ERROR parsing metrics")) + http.Error(res, "ERROR parsing metrics", http.StatusInternalServerError) } } else if path == "query" { // Deliver a dummy response to the query endpoint, as some InfluxDB clients test endpoint availability with a query @@ -139,8 +135,7 @@ func (t *HttpListener) ServeHTTP(res http.ResponseWriter, req *http.Request) { res.Write([]byte("{\"results\":[]}")) } else { // Don't know how to respond to calls to other endpoints - res.WriteHeader(http.StatusNotFound) - res.Write([]byte("Not Found")) + http.NotFound(res, req) } } diff --git a/plugins/inputs/http_listener/http_listener_test.go b/plugins/inputs/http_listener/http_listener_test.go index d8b543d7e..27452e1d6 100644 --- a/plugins/inputs/http_listener/http_listener_test.go +++ b/plugins/inputs/http_listener/http_listener_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/plugins/parsers" "github.com/influxdata/telegraf/testutil" @@ -29,8 +30,8 @@ cpu_load_short,host=server06 value=12.0 1422568543702900257 func newTestHttpListener() *HttpListener { listener := &HttpListener{ ServiceAddress: ":8186", - ReadTimeout: "10", - WriteTimeout: "10", + ReadTimeout: internal.Duration{Duration: time.Second * 10}, + WriteTimeout: internal.Duration{Duration: time.Second * 10}, } return listener }