fix(metricprovider): reuse http.Transport for http.Client (#3780)

The current code creates a new http.Transport for each http.Client that is created, which leads to a leak in TCP connections due to keep-alive. Instead, reuse the same http.Transport between requests. According to the http.Transport docs, this is safe for concurrent use. Signed-off-by: Kevin Ji <[email protected]>
argoproj · Aug 12, 2024 · 6b2ee92 · 6b2ee92
1 parent 8f74229
commit 6b2ee92
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 13 deletions.
diff --git a/metricproviders/prometheus/prometheus.go b/metricproviders/prometheus/prometheus.go
@@ -209,6 +209,21 @@ func NewPrometheusProvider(api v1.API, logCtx log.Entry, metric v1alpha1.Metric)
 	return provider, nil
 }
 
+func newHTTPTransport(insecureSkipVerify bool) *http.Transport {
+	return &http.Transport{
+		Proxy: http.ProxyFromEnvironment,
+		DialContext: (&net.Dialer{
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+		}).DialContext,
+		TLSHandshakeTimeout: 10 * time.Second,
+		TLSClientConfig:     &tls.Config{InsecureSkipVerify: insecureSkipVerify},
+	}
+}
+
+var secureTransport *http.Transport = newHTTPTransport(false)
+var insecureTransport *http.Transport = newHTTPTransport(true)
+
 // NewPrometheusAPI generates a prometheus API from the metric configuration
 func NewPrometheusAPI(metric v1alpha1.Metric) (v1.API, error) {
 	envValuesByKey := make(map[string]string)
@@ -232,15 +247,10 @@ func NewPrometheusAPI(metric v1alpha1.Metric) (v1.API, error) {
 	}
 
 	var roundTripper http.RoundTripper
-
-	roundTripper = &http.Transport{
-		Proxy: http.ProxyFromEnvironment,
-		DialContext: (&net.Dialer{
-			Timeout:   30 * time.Second,
-			KeepAlive: 30 * time.Second,
-		}).DialContext,
-		TLSHandshakeTimeout: 10 * time.Second,
-		TLSClientConfig:     &tls.Config{InsecureSkipVerify: metric.Provider.Prometheus.Insecure},
+	if metric.Provider.Prometheus.Insecure {
+		roundTripper = insecureTransport
+	} else {
+		roundTripper = secureTransport
 	}
 
 	// attach custom headers to api requests, if specified

diff --git a/metricproviders/webmetric/webmetric.go b/metricproviders/webmetric/webmetric.go
@@ -176,6 +176,10 @@ func (p *Provider) GarbageCollect(run *v1alpha1.AnalysisRun, metric v1alpha1.Met
 	return nil
 }
 
+var insecureTransport *http.Transport = &http.Transport{
+	TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+}
+
 func NewWebMetricHttpClient(metric v1alpha1.Metric) (*http.Client, error) {
 	var timeout time.Duration
 	var oauthCfg clientcredentials.Config
@@ -191,10 +195,7 @@ func NewWebMetricHttpClient(metric v1alpha1.Metric) (*http.Client, error) {
 		Timeout: timeout,
 	}
 	if metric.Provider.Web.Insecure {
-		tr := &http.Transport{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		}
-		c.Transport = tr
+		c.Transport = insecureTransport
 	}
 	if metric.Provider.Web.Authentication.OAuth2.TokenURL != "" {
 		if metric.Provider.Web.Authentication.OAuth2.ClientID == "" || metric.Provider.Web.Authentication.OAuth2.ClientSecret == "" {