Merge pull request #153 from doyshinda/custom_timeouts

Add support for custom timeouts

README.md

@@ -166,6 +166,14 @@ will create a URL like this:
 http://<podIP>:9090/metrics?foo=bar&baz=bop
 ```
 
+There are also configuration options for custom (connect and request) timeouts when querying pods for metrics:
+```yaml
+metric-config.pods.requests-per-second.json-path/request-timeout: 2s
+metric-config.pods.requests-per-second.json-path/connect-timeout: 500ms
+```
+
+The default for both of the above values is 15 seconds.
+
 ## Prometheus collector
 
 The Prometheus collector is a generic collector which can map Prometheus

go.sum

@@ -357,6 +357,7 @@ github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7z
 github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
 github.com/prometheus/procfs v0.0.8 h1:+fpWZdT24pJBiqJdAwYBjPSk+5YmQzYNPYzQsdzLkt8=
 github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A=
+github.com/prometheus/procfs v0.0.11 h1:DhHlBtkHWPYi8O2y31JkK0TF+DGM+51OopZjH/Ia5qI=
 github.com/prometheus/procfs v0.0.11/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU=
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
 github.com/remyoudompheng/bigfft v0.0.0-20170806203942-52369c62f446/go.mod h1:uYEyJGbgTkfkS4+E/PavXkNJcbFIpEtjt2B0KDQ5+9M=

pkg/collector/httpmetrics/json_path.go

@@ -26,21 +26,28 @@ func NewJSONPathMetricsGetter(httpClient *http.Client, aggregatorFunc Aggregator
 	return &JSONPathMetricsGetter{client: httpClient, aggregator: aggregatorFunc, jsonPath: compiledPath}
 }
 
-func DefaultMetricsHTTPClient() *http.Client {
+var DefaultRequestTimeout = 15 * time.Second
+var DefaultConnectTimeout = 15 * time.Second
+
+func CustomMetricsHTTPClient(requestTimeout time.Duration, connectTimeout time.Duration) *http.Client {
 	client := &http.Client{
 		Transport: &http.Transport{
 			DialContext: (&net.Dialer{
-				Timeout: 15 * time.Second,
+				Timeout: connectTimeout,
 			}).DialContext,
 			MaxIdleConns:          50,
 			IdleConnTimeout:       90 * time.Second,
 			ExpectContinueTimeout: 1 * time.Second,
 		},
-		Timeout: 15 * time.Second,
+		Timeout: requestTimeout,
 	}
 	return client
 }
 
+func DefaultMetricsHTTPClient() *http.Client {
+	return CustomMetricsHTTPClient(DefaultRequestTimeout, DefaultConnectTimeout)
+}
+
 // GetMetric gets metric from pod by fetching json metrics from the pods metric
 // endpoint and extracting the desired value using the specified json path
 // query.

pkg/collector/httpmetrics/pod_metrics.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/url"
 	"strconv"
+	"time"
 
 	"github.com/oliveagle/jsonpath"
 	v1 "k8s.io/api/core/v1"
@@ -72,7 +73,33 @@ func NewPodMetricsJSONPathGetter(config map[string]string) (*PodMetricsJSONPathG
 			return nil, err
 		}
 	}
-	getter.metricGetter = NewJSONPathMetricsGetter(DefaultMetricsHTTPClient(), aggregator, jsonPath)
+
+	requestTimeout := DefaultRequestTimeout
+	connectTimeout := DefaultConnectTimeout
+
+	if v, ok := config["request-timeout"]; ok {
+		d, err := time.ParseDuration(v)
+		if err != nil {
+			return nil, err
+		}
+		if d < 0 {
+			return nil, fmt.Errorf("Invalid request-timeout config value: %s", v)
+		}
+		requestTimeout = d
+	}
+
+	if v, ok := config["connect-timeout"]; ok {
+		d, err := time.ParseDuration(v)
+		if err != nil {
+			return nil, err
+		}
+		if d < 0 {
+			return nil, fmt.Errorf("Invalid connect-timeout config value: %s", v)
+		}
+		connectTimeout = d
+	}
+
+	getter.metricGetter = NewJSONPathMetricsGetter(CustomMetricsHTTPClient(requestTimeout, connectTimeout), aggregator, jsonPath)
 	return &getter, nil
 }
 

pkg/collector/httpmetrics/pod_metrics_test.go

@@ -3,6 +3,7 @@ package httpmetrics
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/oliveagle/jsonpath"
 	"github.com/stretchr/testify/require"
@@ -131,3 +132,67 @@ func TestBuildMetricsURL(t *testing.T) {
 	receivedURLNoQuery := getterWithNoQuery.buildMetricsURL(ip)
 	require.Equal(t, receivedURLNoQuery.String(), expectedURLNoQuery)
 }
+
+func TestCustomTimeouts(t *testing.T) {
+	scheme := "http"
+	port := "9090"
+	path := "/v1/test/"
+
+	// Test no custom options results in default timeouts
+	defaultConfig := map[string]string{
+		"json-key":  "$.value",
+		"scheme":    scheme,
+		"path":      path,
+		"port":      port,
+	}
+	defaultTime := time.Duration(15000) * time.Millisecond
+
+	defaultGetter, err1 := NewPodMetricsJSONPathGetter(defaultConfig)
+	require.NoError(t, err1)
+	require.Equal(t, defaultGetter.metricGetter.client.Timeout, defaultTime)
+
+	// Test with custom request timeout
+	configWithRequestTimeout := map[string]string{
+		"json-key":  "$.value",
+		"scheme":    scheme,
+		"path":      path,
+		"port":      port,
+		"request-timeout": "978ms",
+	}
+	exectedTimeout := time.Duration(978) * time.Millisecond
+	customRequestGetter, err2 := NewPodMetricsJSONPathGetter(configWithRequestTimeout)
+	require.NoError(t, err2)
+	require.Equal(t, customRequestGetter.metricGetter.client.Timeout, exectedTimeout)
+
+	// Test with custom connect timeout. Unfortunately, it seems there's no way to access the
+	// connect timeout of the client struct to actually verify it's set :/
+	configWithConnectTimeout := map[string]string{
+		"json-key":  "$.value",
+		"scheme":    scheme,
+		"path":      path,
+		"port":      port,
+		"connect-timeout": "512ms",
+	}
+	customRequestGetter, err3 := NewPodMetricsJSONPathGetter(configWithConnectTimeout)
+	require.NoError(t, err3)
+
+	configWithInvalidTimeout := map[string]string{
+		"json-key":  "$.value",
+		"scheme":    scheme,
+		"path":      path,
+		"port":      port,
+		"request-timeout": "-256ms",
+	}
+	_, err4 := NewPodMetricsJSONPathGetter(configWithInvalidTimeout)
+	require.Error(t, err4)
+
+	configWithInvalidTimeout = map[string]string{
+		"json-key":  "$.value",
+		"scheme":    scheme,
+		"path":      path,
+		"port":      port,
+		"connect-timeout": "-256ms",
+	}
+	_, err5 := NewPodMetricsJSONPathGetter(configWithInvalidTimeout)
+	require.Error(t, err5)
+}