From 6a9c79c4f3287dac697d90e3acd129858e180914 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Mon, 19 Apr 2021 14:04:09 +0200 Subject: [PATCH] Move HTTP keepalive and HTTP2 options to functional options Signed-off-by: Marco Pracucci --- config/http_config.go | 44 +++++++++++++++++++++++++++++--------- config/http_config_test.go | 18 ++++++++-------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/config/http_config.go b/config/http_config.go index 17a9f2eb..b3726da9 100644 --- a/config/http_config.go +++ b/config/http_config.go @@ -40,6 +40,12 @@ var DefaultHTTPClientConfig = HTTPClientConfig{ FollowRedirects: true, } +// defaultHTTPClientOptions holds the default HTTP client options. +var defaultHTTPClientOptions = httpClientOptions{ + keepAlivesEnabled: true, + http2Enabled: true, +} + type closeIdler interface { CloseIdleConnections() } @@ -201,7 +207,9 @@ func (a *BasicAuth) UnmarshalYAML(unmarshal func(interface{}) error) error { type DialContextFunc func(context.Context, string, string) (net.Conn, error) type httpClientOptions struct { - dialContextFunc DialContextFunc + dialContextFunc DialContextFunc + keepAlivesEnabled bool + http2Enabled bool } // HTTPClientOption defines an option that can be applied to the HTTP client. @@ -214,15 +222,30 @@ func WithDialContextFunc(fn DialContextFunc) HTTPClientOption { } } +// WithKeepAlivesDisabled allows to disable HTTP keepalive. +func WithKeepAlivesDisabled() HTTPClientOption { + return func(opts *httpClientOptions) { + opts.keepAlivesEnabled = false + } +} + +// WithHTTP2Disabled allows to disable HTTP2. +func WithHTTP2Disabled() HTTPClientOption { + return func(opts *httpClientOptions) { + opts.http2Enabled = false + } +} + // NewClient returns a http.Client using the specified http.RoundTripper. func newClient(rt http.RoundTripper) *http.Client { return &http.Client{Transport: rt} } // NewClientFromConfig returns a new HTTP client configured for the -// given config.HTTPClientConfig. The name is used as go-conntrack metric label. -func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, enableHTTP2 bool, optFuncs ...HTTPClientOption) (*http.Client, error) { - rt, err := NewRoundTripperFromConfig(cfg, name, disableKeepAlives, enableHTTP2, optFuncs...) +// given config.HTTPClientConfig and config.HTTPClientOption. +// The name is used as go-conntrack metric label. +func NewClientFromConfig(cfg HTTPClientConfig, name string, optFuncs ...HTTPClientOption) (*http.Client, error) { + rt, err := NewRoundTripperFromConfig(cfg, name, optFuncs...) if err != nil { return nil, err } @@ -236,11 +259,12 @@ func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, e } // NewRoundTripperFromConfig returns a new HTTP RoundTripper configured for the -// given config.HTTPClientConfig. The name is used as go-conntrack metric label. -func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, enableHTTP2 bool, optFuncs ...HTTPClientOption) (http.RoundTripper, error) { - opts := &httpClientOptions{} +// given config.HTTPClientConfig and config.HTTPClientOption. +// The name is used as go-conntrack metric label. +func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, optFuncs ...HTTPClientOption) (http.RoundTripper, error) { + opts := defaultHTTPClientOptions for _, f := range optFuncs { - f(opts) + f(&opts) } var dialContext func(ctx context.Context, network, addr string) (net.Conn, error) @@ -263,7 +287,7 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAli Proxy: http.ProxyURL(cfg.ProxyURL.URL), MaxIdleConns: 20000, MaxIdleConnsPerHost: 1000, // see https://github.com/golang/go/issues/13801 - DisableKeepAlives: disableKeepAlives, + DisableKeepAlives: !opts.keepAlivesEnabled, TLSClientConfig: tlsConfig, DisableCompression: true, // 5 minutes is typically above the maximum sane scrape interval. So we can @@ -273,7 +297,7 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, disableKeepAli ExpectContinueTimeout: 1 * time.Second, DialContext: dialContext, } - if enableHTTP2 { + if opts.http2Enabled { // HTTP/2 support is golang has many problematic cornercases where // dead connections would be kept and used in connection pools. // https://github.com/golang/go/issues/32388 diff --git a/config/http_config_test.go b/config/http_config_test.go index 61406c77..a0511dd2 100644 --- a/config/http_config_test.go +++ b/config/http_config_test.go @@ -354,7 +354,7 @@ func TestNewClientFromConfig(t *testing.T) { if err != nil { t.Fatal(err.Error()) } - client, err := NewClientFromConfig(validConfig.clientConfig, "test", false, true) + client, err := NewClientFromConfig(validConfig.clientConfig, "test") if err != nil { t.Errorf("Can't create a client from this config: %+v", validConfig.clientConfig) continue @@ -404,7 +404,7 @@ func TestNewClientFromInvalidConfig(t *testing.T) { } for _, invalidConfig := range newClientInvalidConfig { - client, err := NewClientFromConfig(invalidConfig.clientConfig, "test", false, true) + client, err := NewClientFromConfig(invalidConfig.clientConfig, "test") if client != nil { t.Errorf("A client instance was returned instead of nil using this config: %+v", invalidConfig.clientConfig) } @@ -423,7 +423,7 @@ func TestCustomDialContextFunc(t *testing.T) { } cfg := HTTPClientConfig{} - client, err := NewClientFromConfig(cfg, "test", false, true, WithDialContextFunc(dialFn)) + client, err := NewClientFromConfig(cfg, "test", WithDialContextFunc(dialFn)) if err != nil { t.Fatalf("Can't create a client from this config: %+v", cfg) } @@ -460,7 +460,7 @@ func TestMissingBearerAuthFile(t *testing.T) { } defer testServer.Close() - client, err := NewClientFromConfig(cfg, "test", false, true) + client, err := NewClientFromConfig(cfg, "test") if err != nil { t.Fatal(err) } @@ -658,7 +658,7 @@ func TestBasicAuthNoPassword(t *testing.T) { if err != nil { t.Fatalf("Error loading HTTP client config: %v", err) } - client, err := NewClientFromConfig(*cfg, "test", false, true) + client, err := NewClientFromConfig(*cfg, "test") if err != nil { t.Fatalf("Error creating HTTP Client: %v", err) } @@ -684,7 +684,7 @@ func TestBasicAuthNoUsername(t *testing.T) { if err != nil { t.Fatalf("Error loading HTTP client config: %v", err) } - client, err := NewClientFromConfig(*cfg, "test", false, true) + client, err := NewClientFromConfig(*cfg, "test") if err != nil { t.Fatalf("Error creating HTTP Client: %v", err) } @@ -710,7 +710,7 @@ func TestBasicAuthPasswordFile(t *testing.T) { if err != nil { t.Fatalf("Error loading HTTP client config: %v", err) } - client, err := NewClientFromConfig(*cfg, "test", false, true) + client, err := NewClientFromConfig(*cfg, "test") if err != nil { t.Fatalf("Error creating HTTP Client: %v", err) } @@ -861,7 +861,7 @@ func TestTLSRoundTripper(t *testing.T) { writeCertificate(bs, tc.cert, cert) writeCertificate(bs, tc.key, key) if c == nil { - c, err = NewClientFromConfig(cfg, "test", false, true) + c, err = NewClientFromConfig(cfg, "test") if err != nil { t.Fatalf("Error creating HTTP Client: %v", err) } @@ -933,7 +933,7 @@ func TestTLSRoundTripperRaces(t *testing.T) { writeCertificate(bs, TLSCAChainPath, ca) writeCertificate(bs, ClientCertificatePath, cert) writeCertificate(bs, ClientKeyNoPassPath, key) - c, err = NewClientFromConfig(cfg, "test", false, true) + c, err = NewClientFromConfig(cfg, "test") if err != nil { t.Fatalf("Error creating HTTP Client: %v", err) }