From eea00110148680b35f4ca1af86ffd98973b0ef89 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Fri, 8 Oct 2021 19:17:53 +0530 Subject: [PATCH 1/9] Added `MaxIdleConns`, `MaxIdleConnsPerHost` and `IdleConnTimeout` in HTTPClientSettings --- config/confighttp/README.md | 3 ++ config/confighttp/confighttp.go | 31 +++++++++++++ config/confighttp/confighttp_test.go | 65 ++++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/config/confighttp/README.md b/config/confighttp/README.md index 460951f2fcd..9f74d21ef3e 100644 --- a/config/confighttp/README.md +++ b/config/confighttp/README.md @@ -20,6 +20,9 @@ README](../configtls/README.md). - [`read_buffer_size`](https://golang.org/pkg/net/http/#Transport) - [`timeout`](https://golang.org/pkg/net/http/#Client) - [`write_buffer_size`](https://golang.org/pkg/net/http/#Transport) +- [`max_idle_conns`](https://golang.org/pkg/net/http/#Transport) +- [`max_idle_conns_per_host`](https://golang.org/pkg/net/http/#Transport) +- [`idle_conn_timeout`](https://golang.org/pkg/net/http/#Transport) Example: diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 423e70027a7..f1e971cdcbf 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -16,6 +16,7 @@ package confighttp // import "go.opentelemetry.io/collector/config/confighttp" import ( "crypto/tls" + "errors" "fmt" "net" "net/http" @@ -58,6 +59,15 @@ type HTTPClientSettings struct { // Auth configuration for outgoing HTTP calls. Auth *configauth.Authentication `mapstructure:"auth,omitempty"` + + // MaxIdleConns is used to set a limit to the maximum idle HTTP connection the client can keep open. + MaxIdleConns int `mapstructure:"max_idle_conns"` + + // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connection the host can keep open. + MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host"` + + // IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself. + IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"` } // ToClient creates an HTTP client. @@ -77,6 +87,27 @@ func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Ext transport.WriteBufferSize = hcs.WriteBufferSize } + if hcs.MaxIdleConns < 0 { + return nil, errors.New(`cannot have a negative "max_idle_conns"`) + } else if hcs.MaxIdleConns > 0 { + transport.MaxIdleConns = hcs.MaxIdleConns + } + + if hcs.MaxIdleConnsPerHost < 0 { + return nil, errors.New(`cannot have a negative "max_idle_conns_per_host"`) + } else if hcs.MaxIdleConnsPerHost > 0 { + if hcs.MaxIdleConnsPerHost > hcs.MaxIdleConns { + return nil, errors.New(`"max_idle_conns_per_host" cannot be greater than "max_idle_conns"`) + } + transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost + } + + if hcs.IdleConnTimeout < 0 { + return nil, errors.New(`cannot have a negative "idle_conn_timeout"`) + } else if hcs.IdleConnTimeout > 0 { + transport.IdleConnTimeout = hcs.IdleConnTimeout + } + clientTransport := (http.RoundTripper)(transport) if len(hcs.Headers) > 0 { clientTransport = &headerRoundTripper{ diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 6afe9d66234..748b95c1df5 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -60,9 +60,12 @@ func TestAllHTTPClientSettings(t *testing.T) { TLSSetting: configtls.TLSClientSetting{ Insecure: false, }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxIdleConns: 50, + MaxIdleConnsPerHost: 40, + IdleConnTimeout: 30 * time.Second, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, }, shouldError: false, }, @@ -92,6 +95,9 @@ func TestAllHTTPClientSettings(t *testing.T) { transport := client.Transport.(*http.Transport) assert.EqualValues(t, 1024, transport.ReadBufferSize) assert.EqualValues(t, 512, transport.WriteBufferSize) + assert.EqualValues(t, 50, transport.MaxIdleConns) + assert.EqualValues(t, 40, transport.MaxIdleConnsPerHost) + assert.EqualValues(t, 30*time.Second, transport.IdleConnTimeout) }) } } @@ -134,6 +140,59 @@ func TestHTTPClientSettingsError(t *testing.T) { Auth: &configauth.Authentication{AuthenticatorID: config.NewComponentID("dummy")}, }, }, + { + err: "cannot have a negative \"max_idle_conns\"", + settings: HTTPClientSettings{ + Endpoint: "localhost:1234", + TLSSetting: &configtls.TLSClientSetting{ + Insecure: false, + }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxIdleConns: -10, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + }, + }, + { + err: "cannot have a negative \"max_idle_conns_per_host\"", + settings: HTTPClientSettings{ + Endpoint: "localhost:1234", + TLSSetting: &configtls.TLSClientSetting{ + Insecure: false, + }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxIdleConnsPerHost: -10, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + }, + }, + { + err: "\"max_idle_conns_per_host\" cannot be greater than \"max_idle_conns\"", + settings: HTTPClientSettings{ + Endpoint: "localhost:1234", + TLSSetting: &configtls.TLSClientSetting{ + Insecure: false, + }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxIdleConns: 40, + MaxIdleConnsPerHost: 50, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + }, + }, + { + err: "cannot have a negative \"idle_conn_timeout\"", + settings: HTTPClientSettings{ + Endpoint: "localhost:1234", + TLSSetting: &configtls.TLSClientSetting{ + Insecure: false, + }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + IdleConnTimeout: -2 * time.Second, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + }, + }, } for _, test := range tests { t.Run(test.err, func(t *testing.T) { From c6d78deedca0ea7a72c57d6817d1424851bec0d3 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Wed, 13 Oct 2021 18:09:06 +0530 Subject: [PATCH 2/9] Removed unnecessary validatioins form confighttp --- config/confighttp/confighttp.go | 18 ++-------- config/confighttp/confighttp_test.go | 53 ---------------------------- 2 files changed, 3 insertions(+), 68 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index f1e971cdcbf..b528e13aa28 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -16,7 +16,6 @@ package confighttp // import "go.opentelemetry.io/collector/config/confighttp" import ( "crypto/tls" - "errors" "fmt" "net" "net/http" @@ -87,24 +86,13 @@ func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Ext transport.WriteBufferSize = hcs.WriteBufferSize } - if hcs.MaxIdleConns < 0 { - return nil, errors.New(`cannot have a negative "max_idle_conns"`) - } else if hcs.MaxIdleConns > 0 { + if hcs.MaxIdleConns != 0 { transport.MaxIdleConns = hcs.MaxIdleConns } - if hcs.MaxIdleConnsPerHost < 0 { - return nil, errors.New(`cannot have a negative "max_idle_conns_per_host"`) - } else if hcs.MaxIdleConnsPerHost > 0 { - if hcs.MaxIdleConnsPerHost > hcs.MaxIdleConns { - return nil, errors.New(`"max_idle_conns_per_host" cannot be greater than "max_idle_conns"`) - } - transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost - } + transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost - if hcs.IdleConnTimeout < 0 { - return nil, errors.New(`cannot have a negative "idle_conn_timeout"`) - } else if hcs.IdleConnTimeout > 0 { + if hcs.IdleConnTimeout != 0 { transport.IdleConnTimeout = hcs.IdleConnTimeout } diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 748b95c1df5..e6ed8f3a5a0 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -140,59 +140,6 @@ func TestHTTPClientSettingsError(t *testing.T) { Auth: &configauth.Authentication{AuthenticatorID: config.NewComponentID("dummy")}, }, }, - { - err: "cannot have a negative \"max_idle_conns\"", - settings: HTTPClientSettings{ - Endpoint: "localhost:1234", - TLSSetting: &configtls.TLSClientSetting{ - Insecure: false, - }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: -10, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, - }, - }, - { - err: "cannot have a negative \"max_idle_conns_per_host\"", - settings: HTTPClientSettings{ - Endpoint: "localhost:1234", - TLSSetting: &configtls.TLSClientSetting{ - Insecure: false, - }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConnsPerHost: -10, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, - }, - }, - { - err: "\"max_idle_conns_per_host\" cannot be greater than \"max_idle_conns\"", - settings: HTTPClientSettings{ - Endpoint: "localhost:1234", - TLSSetting: &configtls.TLSClientSetting{ - Insecure: false, - }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: 40, - MaxIdleConnsPerHost: 50, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, - }, - }, - { - err: "cannot have a negative \"idle_conn_timeout\"", - settings: HTTPClientSettings{ - Endpoint: "localhost:1234", - TLSSetting: &configtls.TLSClientSetting{ - Insecure: false, - }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - IdleConnTimeout: -2 * time.Second, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, - }, - }, } for _, test := range tests { t.Run(test.err, func(t *testing.T) { From 95e158c377cafadeda9221a18306c7493531f379 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Tue, 19 Oct 2021 18:17:25 +0530 Subject: [PATCH 3/9] Added MaxConnsPerHost parameter in confighttp --- config/confighttp/README.md | 1 + config/confighttp/confighttp.go | 5 +++++ config/confighttp/confighttp_test.go | 2 ++ 3 files changed, 8 insertions(+) diff --git a/config/confighttp/README.md b/config/confighttp/README.md index 9f74d21ef3e..5e443ac7054 100644 --- a/config/confighttp/README.md +++ b/config/confighttp/README.md @@ -22,6 +22,7 @@ README](../configtls/README.md). - [`write_buffer_size`](https://golang.org/pkg/net/http/#Transport) - [`max_idle_conns`](https://golang.org/pkg/net/http/#Transport) - [`max_idle_conns_per_host`](https://golang.org/pkg/net/http/#Transport) +- [`max_conns_per_host`](https://golang.org/pkg/net/http/#Transport) - [`idle_conn_timeout`](https://golang.org/pkg/net/http/#Transport) Example: diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index b528e13aa28..009c7531612 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -65,6 +65,10 @@ type HTTPClientSettings struct { // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connection the host can keep open. MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host"` + // MaxConnsPerHost limits the total number of connections per host, including connections in the dialing, + // active, and idle states. + MaxConnsPerHost int `mapstructure:"max_conns_per_host"` + // IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself. IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"` } @@ -91,6 +95,7 @@ func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Ext } transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost + transport.MaxConnsPerHost = hcs.MaxConnsPerHost if hcs.IdleConnTimeout != 0 { transport.IdleConnTimeout = hcs.IdleConnTimeout diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index e6ed8f3a5a0..5655b7de4ec 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -64,6 +64,7 @@ func TestAllHTTPClientSettings(t *testing.T) { WriteBufferSize: 512, MaxIdleConns: 50, MaxIdleConnsPerHost: 40, + MaxConnsPerHost: 45, IdleConnTimeout: 30 * time.Second, CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, }, @@ -97,6 +98,7 @@ func TestAllHTTPClientSettings(t *testing.T) { assert.EqualValues(t, 512, transport.WriteBufferSize) assert.EqualValues(t, 50, transport.MaxIdleConns) assert.EqualValues(t, 40, transport.MaxIdleConnsPerHost) + assert.EqualValues(t, 45, transport.MaxConnsPerHost) assert.EqualValues(t, 30*time.Second, transport.IdleConnTimeout) }) } From 4ec275fa68814db3b157c8c6301ad59df8c0ca20 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Thu, 21 Oct 2021 13:46:33 +0530 Subject: [PATCH 4/9] Changed MaxIdleConns and IdleConnTimeout parameter type to pointer variable --- config/confighttp/confighttp.go | 12 +++++----- config/confighttp/confighttp_test.go | 34 +++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 009c7531612..fbf5f7f3b7d 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -60,7 +60,7 @@ type HTTPClientSettings struct { Auth *configauth.Authentication `mapstructure:"auth,omitempty"` // MaxIdleConns is used to set a limit to the maximum idle HTTP connection the client can keep open. - MaxIdleConns int `mapstructure:"max_idle_conns"` + MaxIdleConns *int `mapstructure:"max_idle_conns"` // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connection the host can keep open. MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host"` @@ -70,7 +70,7 @@ type HTTPClientSettings struct { MaxConnsPerHost int `mapstructure:"max_conns_per_host"` // IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself. - IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"` + IdleConnTimeout *time.Duration `mapstructure:"idle_conn_timeout"` } // ToClient creates an HTTP client. @@ -90,15 +90,15 @@ func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Ext transport.WriteBufferSize = hcs.WriteBufferSize } - if hcs.MaxIdleConns != 0 { - transport.MaxIdleConns = hcs.MaxIdleConns + if hcs.MaxIdleConns != nil { + transport.MaxIdleConns = *hcs.MaxIdleConns } transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost transport.MaxConnsPerHost = hcs.MaxConnsPerHost - if hcs.IdleConnTimeout != 0 { - transport.IdleConnTimeout = hcs.IdleConnTimeout + if hcs.IdleConnTimeout != nil { + transport.IdleConnTimeout = *hcs.IdleConnTimeout } clientTransport := (http.RoundTripper)(transport) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 5655b7de4ec..ad385db41a7 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -48,6 +48,8 @@ func TestAllHTTPClientSettings(t *testing.T) { ext := map[config.ComponentID]component.Extension{ config.NewComponentID("testauth"): &configauth.MockClientAuthenticator{ResultRoundTripper: &customRoundTripper{}}, } + max_idle_conns := 50 + idle_conn_timeout := 30 * time.Second tests := []struct { name string settings HTTPClientSettings @@ -62,14 +64,27 @@ func TestAllHTTPClientSettings(t *testing.T) { }, ReadBufferSize: 1024, WriteBufferSize: 512, - MaxIdleConns: 50, + MaxIdleConns: &max_idle_conns, MaxIdleConnsPerHost: 40, MaxConnsPerHost: 45, - IdleConnTimeout: 30 * time.Second, + IdleConnTimeout: &idle_conn_timeout, CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, }, shouldError: false, }, + { + name: "valid_partial_settings", + settings: HTTPClientSettings{ + Endpoint: "localhost:1234", + TLSSetting: &configtls.TLSClientSetting{ + Insecure: false, + }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + }, + shouldError: false, + }, { name: "error_round_tripper_returned", settings: HTTPClientSettings{ @@ -96,10 +111,17 @@ func TestAllHTTPClientSettings(t *testing.T) { transport := client.Transport.(*http.Transport) assert.EqualValues(t, 1024, transport.ReadBufferSize) assert.EqualValues(t, 512, transport.WriteBufferSize) - assert.EqualValues(t, 50, transport.MaxIdleConns) - assert.EqualValues(t, 40, transport.MaxIdleConnsPerHost) - assert.EqualValues(t, 45, transport.MaxConnsPerHost) - assert.EqualValues(t, 30*time.Second, transport.IdleConnTimeout) + if test.name == "valid_partial_settings" { + assert.EqualValues(t, 100, transport.MaxIdleConns) + assert.EqualValues(t, 0, transport.MaxIdleConnsPerHost) + assert.EqualValues(t, 0, transport.MaxConnsPerHost) + assert.EqualValues(t, 90*time.Second, transport.IdleConnTimeout) + } else { + assert.EqualValues(t, 50, transport.MaxIdleConns) + assert.EqualValues(t, 40, transport.MaxIdleConnsPerHost) + assert.EqualValues(t, 45, transport.MaxConnsPerHost) + assert.EqualValues(t, 30*time.Second, transport.IdleConnTimeout) + } }) } } From aa8a18bcf7c3b82550b57be030b1dd366136c271 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Thu, 21 Oct 2021 18:36:24 +0530 Subject: [PATCH 5/9] Modified the test case --- config/confighttp/confighttp.go | 4 +- config/confighttp/confighttp_test.go | 71 ++++++++++++++++++---------- 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index fbf5f7f3b7d..84d413ff43c 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -59,10 +59,10 @@ type HTTPClientSettings struct { // Auth configuration for outgoing HTTP calls. Auth *configauth.Authentication `mapstructure:"auth,omitempty"` - // MaxIdleConns is used to set a limit to the maximum idle HTTP connection the client can keep open. + // MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open. MaxIdleConns *int `mapstructure:"max_idle_conns"` - // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connection the host can keep open. + // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connections the host can keep open. MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host"` // MaxConnsPerHost limits the total number of connections per host, including connections in the dialing, diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index ad385db41a7..f288ac1db69 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -48,8 +48,8 @@ func TestAllHTTPClientSettings(t *testing.T) { ext := map[config.ComponentID]component.Extension{ config.NewComponentID("testauth"): &configauth.MockClientAuthenticator{ResultRoundTripper: &customRoundTripper{}}, } - max_idle_conns := 50 - idle_conn_timeout := 30 * time.Second + maxIdleConns := 50 + idleConnTimeout := 30 * time.Second tests := []struct { name string settings HTTPClientSettings @@ -64,16 +64,16 @@ func TestAllHTTPClientSettings(t *testing.T) { }, ReadBufferSize: 1024, WriteBufferSize: 512, - MaxIdleConns: &max_idle_conns, + MaxIdleConns: &maxIdleConns, MaxIdleConnsPerHost: 40, MaxConnsPerHost: 45, - IdleConnTimeout: &idle_conn_timeout, + IdleConnTimeout: &idleConnTimeout, CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, }, shouldError: false, }, { - name: "valid_partial_settings", + name: "error_round_tripper_returned", settings: HTTPClientSettings{ Endpoint: "localhost:1234", TLSSetting: &configtls.TLSClientSetting{ @@ -81,12 +81,43 @@ func TestAllHTTPClientSettings(t *testing.T) { }, ReadBufferSize: 1024, WriteBufferSize: 512, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return nil, errors.New("error") }, }, - shouldError: false, + shouldError: true, }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + client, err := test.settings.ToClient(ext) + if test.shouldError { + assert.Error(t, err) + return + } + assert.NoError(t, err) + transport := client.Transport.(*http.Transport) + assert.EqualValues(t, 1024, transport.ReadBufferSize) + assert.EqualValues(t, 512, transport.WriteBufferSize) + assert.EqualValues(t, 50, transport.MaxIdleConns) + assert.EqualValues(t, 40, transport.MaxIdleConnsPerHost) + assert.EqualValues(t, 45, transport.MaxConnsPerHost) + assert.EqualValues(t, 30*time.Second, transport.IdleConnTimeout) + + }) + } +} + +func TestPartialHTTPClientSettings(t *testing.T) { + ext := map[config.ComponentID]component.Extension{ + config.NewComponentID("testauth"): &configauth.MockClientAuthenticator{ResultRoundTripper: &customRoundTripper{}}, + } + tests := []struct { + name string + settings HTTPClientSettings + shouldError bool + }{ { - name: "error_round_tripper_returned", + name: "valid_partial_settings", settings: HTTPClientSettings{ Endpoint: "localhost:1234", TLSSetting: configtls.TLSClientSetting{ @@ -94,34 +125,24 @@ func TestAllHTTPClientSettings(t *testing.T) { }, ReadBufferSize: 1024, WriteBufferSize: 512, - CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return nil, errors.New("error") }, + CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, }, - shouldError: true, + shouldError: false, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { client, err := test.settings.ToClient(ext) - if test.shouldError { - assert.Error(t, err) - return - } assert.NoError(t, err) transport := client.Transport.(*http.Transport) assert.EqualValues(t, 1024, transport.ReadBufferSize) assert.EqualValues(t, 512, transport.WriteBufferSize) - if test.name == "valid_partial_settings" { - assert.EqualValues(t, 100, transport.MaxIdleConns) - assert.EqualValues(t, 0, transport.MaxIdleConnsPerHost) - assert.EqualValues(t, 0, transport.MaxConnsPerHost) - assert.EqualValues(t, 90*time.Second, transport.IdleConnTimeout) - } else { - assert.EqualValues(t, 50, transport.MaxIdleConns) - assert.EqualValues(t, 40, transport.MaxIdleConnsPerHost) - assert.EqualValues(t, 45, transport.MaxConnsPerHost) - assert.EqualValues(t, 30*time.Second, transport.IdleConnTimeout) - } + assert.EqualValues(t, 100, transport.MaxIdleConns) + assert.EqualValues(t, 0, transport.MaxIdleConnsPerHost) + assert.EqualValues(t, 0, transport.MaxConnsPerHost) + assert.EqualValues(t, 90*time.Second, transport.IdleConnTimeout) + }) } } From 8dd15aa3ae641c768e5fb2222411faf851d34e8d Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Thu, 28 Oct 2021 10:45:25 +0530 Subject: [PATCH 6/9] Updated confighttp --- config/confighttp/confighttp.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 84d413ff43c..1daa7d611a2 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -60,6 +60,7 @@ type HTTPClientSettings struct { Auth *configauth.Authentication `mapstructure:"auth,omitempty"` // MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open. + // Here pointer is used to differentiate `no input` from `zero value input` MaxIdleConns *int `mapstructure:"max_idle_conns"` // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connections the host can keep open. @@ -70,6 +71,7 @@ type HTTPClientSettings struct { MaxConnsPerHost int `mapstructure:"max_conns_per_host"` // IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself. + // Here pointer is used to differentiate `no input` from `zero value input` IdleConnTimeout *time.Duration `mapstructure:"idle_conn_timeout"` } From c1e98a7cf69c0688075eb90cf0f423524ea70c9d Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Fri, 29 Oct 2021 12:13:23 +0530 Subject: [PATCH 7/9] Fixed linting --- config/confighttp/confighttp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index f288ac1db69..247add0d3b2 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -76,7 +76,7 @@ func TestAllHTTPClientSettings(t *testing.T) { name: "error_round_tripper_returned", settings: HTTPClientSettings{ Endpoint: "localhost:1234", - TLSSetting: &configtls.TLSClientSetting{ + TLSSetting: configtls.TLSClientSetting{ Insecure: false, }, ReadBufferSize: 1024, From 0904856896453fee5ffe0a19068d4f4a04470945 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Fri, 12 Nov 2021 12:37:13 +0530 Subject: [PATCH 8/9] Make all new parameter type to pointer --- config/confighttp/confighttp.go | 19 +++++++++++++------ config/confighttp/confighttp_test.go | 6 ++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 1daa7d611a2..d7b25d67306 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -60,18 +60,20 @@ type HTTPClientSettings struct { Auth *configauth.Authentication `mapstructure:"auth,omitempty"` // MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open. - // Here pointer is used to differentiate `no input` from `zero value input` + // There's an already set value, and we want to override it only if an explicit value provided MaxIdleConns *int `mapstructure:"max_idle_conns"` // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connections the host can keep open. - MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host"` + // There's an already set value, and we want to override it only if an explicit value provided + MaxIdleConnsPerHost *int `mapstructure:"max_idle_conns_per_host"` // MaxConnsPerHost limits the total number of connections per host, including connections in the dialing, // active, and idle states. - MaxConnsPerHost int `mapstructure:"max_conns_per_host"` + // There's an already set value, and we want to override it only if an explicit value provided + MaxConnsPerHost *int `mapstructure:"max_conns_per_host"` // IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself. - // Here pointer is used to differentiate `no input` from `zero value input` + // There's an already set value, and we want to override it only if an explicit value provided IdleConnTimeout *time.Duration `mapstructure:"idle_conn_timeout"` } @@ -96,8 +98,13 @@ func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Ext transport.MaxIdleConns = *hcs.MaxIdleConns } - transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost - transport.MaxConnsPerHost = hcs.MaxConnsPerHost + if hcs.MaxIdleConnsPerHost != nil { + transport.MaxIdleConnsPerHost = *hcs.MaxIdleConnsPerHost + } + + if hcs.MaxConnsPerHost != nil { + transport.MaxConnsPerHost = *hcs.MaxConnsPerHost + } if hcs.IdleConnTimeout != nil { transport.IdleConnTimeout = *hcs.IdleConnTimeout diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 247add0d3b2..98f2febd183 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -49,6 +49,8 @@ func TestAllHTTPClientSettings(t *testing.T) { config.NewComponentID("testauth"): &configauth.MockClientAuthenticator{ResultRoundTripper: &customRoundTripper{}}, } maxIdleConns := 50 + maxIdleConnsPerHost := 40 + maxConnsPerHost := 45 idleConnTimeout := 30 * time.Second tests := []struct { name string @@ -65,8 +67,8 @@ func TestAllHTTPClientSettings(t *testing.T) { ReadBufferSize: 1024, WriteBufferSize: 512, MaxIdleConns: &maxIdleConns, - MaxIdleConnsPerHost: 40, - MaxConnsPerHost: 45, + MaxIdleConnsPerHost: &maxIdleConnsPerHost, + MaxConnsPerHost: &maxConnsPerHost, IdleConnTimeout: &idleConnTimeout, CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil }, }, From c27c673a5cd4928b34b71f784c0fb5cee477fa10 Mon Sep 17 00:00:00 2001 From: Ravi Rupapara Date: Mon, 6 Dec 2021 17:21:19 +0530 Subject: [PATCH 9/9] Add DefaultHTTPClientSettings function in confighttp --- config/confighttp/confighttp.go | 15 +++++++++++++++ config/confighttp/confighttp_test.go | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index d7b25d67306..7e19bdb7741 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -77,6 +77,21 @@ type HTTPClientSettings struct { IdleConnTimeout *time.Duration `mapstructure:"idle_conn_timeout"` } +// DefaultHTTPClientSettings returns HTTPClientSettings type object with +// the default values of 'MaxIdleConns' and 'IdleConnTimeout'. +// Other config options are not added as they are initialized with 'zero value' by GoLang as default. +// We encourage to use this function to create an object of HTTPClientSettings. +func DefaultHTTPClientSettings() HTTPClientSettings { + // The default values are taken from the values of 'DefaultTransport' of 'http' package. + maxIdleConns := 100 + idleConnTimeout := 90 * time.Second + + return HTTPClientSettings{ + MaxIdleConns: &maxIdleConns, + IdleConnTimeout: &idleConnTimeout, + } +} + // ToClient creates an HTTP client. func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Extension) (*http.Client, error) { tlsCfg, err := hcs.TLSSetting.LoadTLSConfig() diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 98f2febd183..834708779db 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -149,6 +149,12 @@ func TestPartialHTTPClientSettings(t *testing.T) { } } +func TestDefaultHTTPClientSettings(t *testing.T) { + httpClientSettings := DefaultHTTPClientSettings() + assert.EqualValues(t, 100, *httpClientSettings.MaxIdleConns) + assert.EqualValues(t, 90*time.Second, *httpClientSettings.IdleConnTimeout) +} + func TestHTTPClientSettingsError(t *testing.T) { tests := []struct { settings HTTPClientSettings