Fix http_response input creation of transport on every gather

plugins/inputs/http_response/http_response.go

@@ -25,7 +25,6 @@ type HTTPResponse struct {
 	Headers             map[string]string
 	FollowRedirects     bool
 	ResponseStringMatch string
-	compiledStringMatch *regexp.Regexp
 
 	// Path to CA file
 	SSLCA string `toml:"ssl_ca"`
@@ -35,6 +34,9 @@ type HTTPResponse struct {
 	SSLKey string `toml:"ssl_key"`
 	// Use SSL but skip chain & host verification
 	InsecureSkipVerify bool
+
+	compiledStringMatch *regexp.Regexp
+	client              *http.Client
 }
 
 // Description returns the plugin Description
@@ -88,13 +90,11 @@ func (h *HTTPResponse) createHttpClient() (*http.Client, error) {
 	if err != nil {
 		return nil, err
 	}
-	tr := &http.Transport{
-		ResponseHeaderTimeout: h.ResponseTimeout.Duration,
-		TLSClientConfig:       tlsCfg,
-	}
 	client := &http.Client{
-		Transport: tr,
+		Transport: &http.Transport{
-		Timeout:   h.ResponseTimeout.Duration,
+			TLSClientConfig: tlsCfg,
+		},
+		Timeout: h.ResponseTimeout.Duration,
 	}
 
 	if h.FollowRedirects == false {
@@ -106,15 +106,10 @@ func (h *HTTPResponse) createHttpClient() (*http.Client, error) {
 }
 
 // HTTPGather gathers all fields and returns any errors it encounters
-func (h *HTTPResponse) HTTPGather() (map[string]interface{}, error) {
+func (h *HTTPResponse) httpGather() (map[string]interface{}, error) {
 	// Prepare fields
 	fields := make(map[string]interface{})
 
-	client, err := h.createHttpClient()
-	if err != nil {
-		return nil, err
-	}
-
 	var body io.Reader
 	if h.Body != "" {
 		body = strings.NewReader(h.Body)
@@ -133,7 +128,7 @@ func (h *HTTPResponse) HTTPGather() (map[string]interface{}, error) {
 
 	// Start Timer
 	start := time.Now()
-	resp, err := client.Do(request)
+	resp, err := h.client.Do(request)
 	if err != nil {
 		if h.FollowRedirects {
 			return nil, err
@@ -202,8 +197,17 @@ func (h *HTTPResponse) Gather(acc telegraf.Accumulator) error {
 	// Prepare data
 	tags := map[string]string{"server": h.Address, "method": h.Method}
 	var fields map[string]interface{}
+
+	if h.client == nil {
+		client, err := h.createHttpClient()
+		if err != nil {
+			return err
+		}
+		h.client = client
+	}
+
 	// Gather data
-	fields, err = h.HTTPGather()
+	fields, err = h.httpGather()
 	if err != nil {
 		return err
 	}

plugins/inputs/http_response/http_response_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/influxdata/telegraf/internal"
+	"github.com/influxdata/telegraf/testutil"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -73,13 +74,13 @@ func TestHeaders(t *testing.T) {
 			"Host":         "Hello",
 		},
 	}
-	fields, err := h.HTTPGather()
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok := acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusOK, value)
-	assert.NotNil(t, fields["response_time"])
 }
 
 func TestFields(t *testing.T) {
@@ -97,13 +98,14 @@ func TestFields(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
+
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok := acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusOK, value)
-	assert.NotNil(t, fields["response_time"])
 }
 
 func TestRedirects(t *testing.T) {
@@ -121,12 +123,13 @@ func TestRedirects(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok := acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusOK, value)
 
 	h = &HTTPResponse{
 		Address:         ts.URL + "/badredirect",
@@ -138,8 +141,12 @@ func TestRedirects(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err = h.HTTPGather()
+	acc = testutil.Accumulator{}
+	err = h.Gather(&acc)
 	require.Error(t, err)
+
+	value, ok = acc.IntField("http_response", "http_response_code")
+	require.False(t, ok)
 }
 
 func TestMethod(t *testing.T) {
@@ -157,12 +164,13 @@ func TestMethod(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok := acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusOK, value)
 
 	h = &HTTPResponse{
 		Address:         ts.URL + "/mustbepostmethod",
@@ -174,12 +182,13 @@ func TestMethod(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err = h.HTTPGather()
+	acc = testutil.Accumulator{}
+	err = h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok = acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusMethodNotAllowed, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusMethodNotAllowed, value)
 
 	//check that lowercase methods work correctly
 	h = &HTTPResponse{
@@ -192,12 +201,13 @@ func TestMethod(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err = h.HTTPGather()
+	acc = testutil.Accumulator{}
+	err = h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok = acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusMethodNotAllowed, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusMethodNotAllowed, value)
 }
 
 func TestBody(t *testing.T) {
@@ -215,12 +225,13 @@ func TestBody(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok := acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusOK, value)
 
 	h = &HTTPResponse{
 		Address:         ts.URL + "/musthaveabody",
@@ -231,12 +242,13 @@ func TestBody(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err = h.HTTPGather()
+	acc = testutil.Accumulator{}
+	err = h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
+
-	if assert.NotNil(t, fields["http_response_code"]) {
+	value, ok = acc.IntField("http_response", "http_response_code")
-		assert.Equal(t, http.StatusBadRequest, fields["http_response_code"])
+	require.True(t, ok)
-	}
+	require.Equal(t, http.StatusBadRequest, value)
 }
 
 func TestStringMatch(t *testing.T) {
@@ -255,15 +267,18 @@ func TestStringMatch(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
-	if assert.NotNil(t, fields["http_response_code"]) {
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
-	}
-	assert.Equal(t, 1, fields["response_string_match"])
-	assert.NotNil(t, fields["response_time"])
 
+	value, ok := acc.IntField("http_response", "http_response_code")
+	require.True(t, ok)
+	require.Equal(t, http.StatusOK, value)
+	value, ok = acc.IntField("http_response", "response_string_match")
+	require.True(t, ok)
+	require.Equal(t, 1, value)
+	_, ok = acc.FloatField("http_response", "response_time")
+	require.True(t, ok)
 }
 
 func TestStringMatchJson(t *testing.T) {
@@ -282,15 +297,18 @@ func TestStringMatchJson(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
 	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
-	if assert.NotNil(t, fields["http_response_code"]) {
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
-	}
-	assert.Equal(t, 1, fields["response_string_match"])
-	assert.NotNil(t, fields["response_time"])
 
+	value, ok := acc.IntField("http_response", "http_response_code")
+	require.True(t, ok)
+	require.Equal(t, http.StatusOK, value)
+	value, ok = acc.IntField("http_response", "response_string_match")
+	require.True(t, ok)
+	require.Equal(t, 1, value)
+	_, ok = acc.FloatField("http_response", "response_time")
+	require.True(t, ok)
 }
 
 func TestStringMatchFail(t *testing.T) {
@@ -309,18 +327,26 @@ func TestStringMatchFail(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	fields, err := h.HTTPGather()
-	require.NoError(t, err)
-	assert.NotEmpty(t, fields)
-	if assert.NotNil(t, fields["http_response_code"]) {
-		assert.Equal(t, http.StatusOK, fields["http_response_code"])
-	}
-	assert.Equal(t, 0, fields["response_string_match"])
-	assert.NotNil(t, fields["response_time"])
 
+	var acc testutil.Accumulator
+	err := h.Gather(&acc)
+	require.NoError(t, err)
+
+	value, ok := acc.IntField("http_response", "http_response_code")
+	require.True(t, ok)
+	require.Equal(t, http.StatusOK, value)
+	value, ok = acc.IntField("http_response", "response_string_match")
+	require.True(t, ok)
+	require.Equal(t, 0, value)
+	_, ok = acc.FloatField("http_response", "response_time")
+	require.True(t, ok)
 }
 
 func TestTimeout(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping test with sleep in short mode.")
+	}
+
 	mux := setUpTestMux()
 	ts := httptest.NewServer(mux)
 	defer ts.Close()
@@ -335,6 +361,10 @@ func TestTimeout(t *testing.T) {
 		},
 		FollowRedirects: true,
 	}
-	_, err := h.HTTPGather()
+	var acc testutil.Accumulator
-	require.Error(t, err)
+	err := h.Gather(&acc)
+	require.NoError(t, err)
+
+	ok := acc.HasIntField("http_response", "http_response_code")
+	require.False(t, ok)
 }

testutil/accumulator.go

@@ -417,3 +417,53 @@ func (a *Accumulator) HasMeasurement(measurement string) bool {
 	}
 	return false
 }
+
+func (a *Accumulator) IntField(measurement string, field string) (int, bool) {
+	a.Lock()
+	defer a.Unlock()
+	for _, p := range a.Metrics {
+		if p.Measurement == measurement {
+			for fieldname, value := range p.Fields {
+				if fieldname == field {
+					v, ok := value.(int)
+					return v, ok
+				}
+			}
+		}
+	}
+
+	return 0, false
+}
+
+func (a *Accumulator) FloatField(measurement string, field string) (float64, bool) {
+	a.Lock()
+	defer a.Unlock()
+	for _, p := range a.Metrics {
+		if p.Measurement == measurement {
+			for fieldname, value := range p.Fields {
+				if fieldname == field {
+					v, ok := value.(float64)
+					return v, ok
+				}
+			}
+		}
+	}
+
+	return 0.0, false
+}
+
+func (a *Accumulator) StringField(measurement string, field string) (string, bool) {
+	a.Lock()
+	defer a.Unlock()
+	for _, p := range a.Metrics {
+		if p.Measurement == measurement {
+			for fieldname, value := range p.Fields {
+				if fieldname == field {
+					v, ok := value.(string)
+					return v, ok
+				}
+			}
+		}
+	}
+	return "", false
+}