From 5d55ea12d9c84fc01f527d3b38f780d5a08afc3a Mon Sep 17 00:00:00 2001
From: Jan Provaznik <jprovaznik@gitlab.com>
Date: Tue, 12 Mar 2024 09:44:44 +0100
Subject: [PATCH] Introduce default client for send-url injector

The http client has been always reused for send-url injector.

However, it changed recently when an ability to customize the
timeouts has been introduced. This MR caches http client and re-use it
for requests with same params.
---
 workhorse/internal/sendurl/sendurl.go      | 29 ++++++++++++++++++----
 workhorse/internal/sendurl/sendurl_test.go | 19 ++++++++++++++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/workhorse/internal/sendurl/sendurl.go b/workhorse/internal/sendurl/sendurl.go
index 116c68ecba91..e95ee5780b0c 100644
--- a/workhorse/internal/sendurl/sendurl.go
+++ b/workhorse/internal/sendurl/sendurl.go
@@ -6,6 +6,8 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"sync"
+	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -33,6 +35,14 @@ type entryParams struct {
 	Method                string
 }
 
+type cacheKey struct {
+	requestTimeout  time.Duration
+	responseTimeout time.Duration
+	allowRedirects  bool
+}
+
+var httpClients sync.Map
+
 var SendURL = &entry{"send-url:"}
 
 var rangeHeaderKeys = []string{
@@ -129,9 +139,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 	}
 
 	// execute new request
-	var resp *http.Response
-	resp, err = newClient(params).Do(newReq)
-
+	resp, err := cachedClient(params).Do(newReq)
 	if err != nil {
 		status := http.StatusInternalServerError
 
@@ -174,7 +182,17 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 	sendURLRequestsSucceeded.Inc()
 }
 
-func newClient(params entryParams) *http.Client {
+func cachedClient(params entryParams) *http.Client {
+	key := cacheKey{
+		requestTimeout:  params.DialTimeout.Duration,
+		responseTimeout: params.ResponseHeaderTimeout.Duration,
+		allowRedirects:  params.AllowRedirects,
+	}
+	cachedClient, found := httpClients.Load(key)
+	if found {
+		return cachedClient.(*http.Client)
+	}
+
 	var options []transport.Option
 
 	if params.DialTimeout.Duration != 0 {
@@ -187,11 +205,12 @@ func newClient(params entryParams) *http.Client {
 	client := &http.Client{
 		Transport: transport.NewRestrictedTransport(options...),
 	}
-
 	if !params.AllowRedirects {
 		client.CheckRedirect = httpClientNoRedirect
 	}
 
+	httpClients.Store(key, client)
+
 	return client
 }
 
diff --git a/workhorse/internal/sendurl/sendurl_test.go b/workhorse/internal/sendurl/sendurl_test.go
index 4bebe43a649b..008ea58ac369 100644
--- a/workhorse/internal/sendurl/sendurl_test.go
+++ b/workhorse/internal/sendurl/sendurl_test.go
@@ -292,3 +292,22 @@ func TestErrorWithCustomStatusCode(t *testing.T) {
 
 	require.Equal(t, http.StatusTeapot, response.Code)
 }
+
+func TestHttpClientReuse(t *testing.T) {
+	expectedKey := cacheKey{
+		requestTimeout:  0,
+		responseTimeout: 0,
+		allowRedirects:  false,
+	}
+	httpClients.Delete(expectedKey)
+
+	response := testEntryServer(t, "/get/request", nil, false)
+	require.Equal(t, http.StatusOK, response.Code)
+	_, found := httpClients.Load(expectedKey)
+	require.Equal(t, true, found)
+
+	storedClient := &http.Client{}
+	httpClients.Store(expectedKey, storedClient)
+	require.Equal(t, cachedClient(entryParams{}), storedClient)
+	require.NotEqual(t, cachedClient(entryParams{AllowRedirects: true}), storedClient)
+}
-- 
GitLab