Repository: kedacore/http-add-on
Pull request #184 (2021-06-17 17:09:53)

Factoring the HTTP Transport out of the forwarder
This commit reuses the `http.RoundTripper` so that we pool connections. Before, we were creating a new round tripper -- and hence, a new connection pool -- every time we forwarded a request. The result was that we were not reusing connection, and were attempting to create _new_ connections every time we forwarded a request.


### Checklist

- [x] Commits are signed with Developer Certificate of Origin (DCO)
- [x] Any necessary documentation is added, such as:
  - [`README.md`](/README.md)
  - [`docs/`](./docs)
  - [The docs repo](https://github.com/kedacore/keda-docs)

Fixes #

@ajanth97 and I found this when auditing the codebase for performance problems


arschles: Thanks @ajanth97 !
khaosdoctor: Looks nice! Took a look at the performance measures too, it's amazing!

Patch:
diff --git a/interceptor/proxy_handlers.go b/interceptor/proxy_handlers.go
index d002793..47293e2 100644
--- a/interceptor/proxy_handlers.go
+++ b/interceptor/proxy_handlers.go
@@ -27,8 +27,18 @@ func newForwardingHandler(
 	dialCtxFunc kedanet.DialContextFunc,
 	waitFunc forwardWaitFunc,
 	waitTimeout time.Duration,
-	respTimeout time.Duration,
+	respHeaderTimeout time.Duration,
 ) http.Handler {
+	roundTripper := &http.Transport{
+		Proxy:                 http.ProxyFromEnvironment,
+		DialContext:           dialCtxFunc,
+		ForceAttemptHTTP2:     true,
+		MaxIdleConns:          100,
+		IdleConnTimeout:       90 * time.Second,
+		TLSHandshakeTimeout:   10 * time.Second,
+		ExpectContinueTimeout: 1 * time.Second,
+		ResponseHeaderTimeout: respHeaderTimeout,
+	}
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		ctx, done := context.WithTimeout(r.Context(), waitTimeout)
 		defer done()
@@ -42,6 +52,6 @@ func newForwardingHandler(
 			return
 		}
 
-		forwardRequest(w, r, dialCtxFunc, respTimeout, fwdSvcURL)
+		forwardRequest(w, r, roundTripper, fwdSvcURL)
 	})
 }
diff --git a/interceptor/request_forwarder.go b/interceptor/request_forwarder.go
index 66eb9ea..adfdb11 100644
--- a/interceptor/request_forwarder.go
+++ b/interceptor/request_forwarder.go
@@ -5,28 +5,14 @@ import (
 	"net/http"
 	"net/http/httputil"
 	"net/url"
-	"time"
-
-	kedanet "github.com/kedacore/http-add-on/pkg/net"
 )
 
 func forwardRequest(
 	w http.ResponseWriter,
 	r *http.Request,
-	dialCtxFunc kedanet.DialContextFunc,
-	respHeaderTimeout time.Duration,
+	roundTripper http.RoundTripper,
 	fwdSvcURL *url.URL,
 ) {
-	roundTripper := &http.Transport{
-		Proxy:                 http.ProxyFromEnvironment,
-		DialContext:           dialCtxFunc,
-		ForceAttemptHTTP2:     true,
-		MaxIdleConns:          100,
-		IdleConnTimeout:       90 * time.Second,
-		TLSHandshakeTimeout:   10 * time.Second,
-		ExpectContinueTimeout: 1 * time.Second,
-		ResponseHeaderTimeout: respHeaderTimeout,
-	}
 	proxy := httputil.NewSingleHostReverseProxy(fwdSvcURL)
 	proxy.Transport = roundTripper
 	proxy.Director = func(req *http.Request) {


Test patch:
diff --git a/interceptor/request_forwarder_test.go b/interceptor/request_forwarder_test.go
index ff7ace4..e2888f9 100644
--- a/interceptor/request_forwarder_test.go
+++ b/interceptor/request_forwarder_test.go
@@ -14,6 +14,16 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
+func newRoundTripper(
+	dialCtxFunc kedanet.DialContextFunc,
+	httpRespHeaderTimeout time.Duration,
+) http.RoundTripper {
+	return &http.Transport{
+		DialContext:           dialCtxFunc,
+		ResponseHeaderTimeout: httpRespHeaderTimeout,
+	}
+}
+
 func defaultTimeouts() config.Timeouts {
 	return config.Timeouts{
 		Connect:            100 * time.Millisecond,
@@ -69,8 +79,7 @@ func TestForwarderSuccess(t *testing.T) {
 	forwardRequest(
 		res,
 		req,
-		dialCtxFunc,
-		timeouts.ResponseHeader,
+		newRoundTripper(dialCtxFunc, timeouts.ResponseHeader),
 		forwardURL,
 	)
 
@@ -112,8 +121,7 @@ func TestForwarderHeaderTimeout(t *testing.T) {
 	forwardRequest(
 		res,
 		req,
-		dialCtxFunc,
-		timeouts.ResponseHeader,
+		newRoundTripper(dialCtxFunc, timeouts.ResponseHeader),
 		originURL,
 	)
 
@@ -163,8 +171,7 @@ func TestForwarderWaitsForSlowOrigin(t *testing.T) {
 	forwardRequest(
 		res,
 		req,
-		dialCtxFunc,
-		timeouts.ResponseHeader,
+		newRoundTripper(dialCtxFunc, timeouts.ResponseHeader),
 		originURL,
 	)
 	// wait for the goroutine above to finish, with a little cusion
@@ -192,8 +199,7 @@ func TestForwarderConnectionRetryAndTimeout(t *testing.T) {
 	forwardRequest(
 		res,
 		req,
-		dialCtxFunc,
-		timeouts.ResponseHeader,
+		newRoundTripper(dialCtxFunc, timeouts.ResponseHeader),
 		noSuchURL,
 	)
 	elapsed := time.Since(start)