Skip to content

Commit c2af75d

Browse files
authored
[confighttp] Remove pointers from integer fields (#12323)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description I was looking through our defaults for a few `confighttp.ClientConfig` fields and found that `0` disables these fields in the underlying `http.Transport` object, which means we don't need them to be pointers. Docs for the four fields we set in `http.Transport` here: https://pkg.go.dev/net/http#Transport.MaxIdleConns I also stopped assigning defaults to any fields we take from `http.DefaultTransport` where setting them is a no-op since they aren't set in `http.DefaultTransport`. <!-- Issue number if applicable --> #### Link to tracking issue Helps unblock confighttp 1.0 from depending on #9478
1 parent dae5d19 commit c2af75d

File tree

4 files changed

+99
-55
lines changed

4 files changed

+99
-55
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: breaking
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: confighttp
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Make the client config options `max_idle_conns`, `max_idle_conns_per_host`, `max_conns_per_host`, and `idle_conn_timeout` integers
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [9478]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: All four options can be set to `0` where they were previously set to `null`
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [user]

config/confighttp/confighttp.go

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -77,21 +77,20 @@ type ClientConfig struct {
7777
CompressionParams configcompression.CompressionParams `mapstructure:"compression_params"`
7878

7979
// MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open.
80-
// By default, it is set to 100.
81-
MaxIdleConns *int `mapstructure:"max_idle_conns"`
80+
// By default, it is set to 100. Zero means no limit.
81+
MaxIdleConns int `mapstructure:"max_idle_conns"`
8282

8383
// MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connections the host can keep open.
84-
// By default, it is set to [http.DefaultTransport.MaxIdleConnsPerHost].
85-
MaxIdleConnsPerHost *int `mapstructure:"max_idle_conns_per_host"`
84+
// Default is 0 (unlimited).
85+
MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host"`
8686

8787
// MaxConnsPerHost limits the total number of connections per host, including connections in the dialing,
88-
// active, and idle states.
89-
// By default, it is set to [http.DefaultTransport.MaxConnsPerHost].
90-
MaxConnsPerHost *int `mapstructure:"max_conns_per_host"`
88+
// active, and idle states. Default is 0 (unlimited).
89+
MaxConnsPerHost int `mapstructure:"max_conns_per_host"`
9190

9291
// IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself.
93-
// By default, it is set to [http.DefaultTransport.IdleConnTimeout]
94-
IdleConnTimeout *time.Duration `mapstructure:"idle_conn_timeout"`
92+
// By default, it is set to 90 seconds.
93+
IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"`
9594

9695
// DisableKeepAlives, if true, disables HTTP keep-alives and will only use the connection to the server
9796
// for a single HTTP request.
@@ -129,13 +128,9 @@ func NewDefaultClientConfig() ClientConfig {
129128
defaultTransport := http.DefaultTransport.(*http.Transport)
130129

131130
return ClientConfig{
132-
ReadBufferSize: defaultTransport.ReadBufferSize,
133-
WriteBufferSize: defaultTransport.WriteBufferSize,
134-
Headers: map[string]configopaque.String{},
135-
MaxIdleConns: &defaultTransport.MaxIdleConns,
136-
MaxIdleConnsPerHost: &defaultTransport.MaxIdleConnsPerHost,
137-
MaxConnsPerHost: &defaultTransport.MaxConnsPerHost,
138-
IdleConnTimeout: &defaultTransport.IdleConnTimeout,
131+
Headers: map[string]configopaque.String{},
132+
MaxIdleConns: defaultTransport.MaxIdleConns,
133+
IdleConnTimeout: defaultTransport.IdleConnTimeout,
139134
}
140135
}
141136

@@ -172,21 +167,10 @@ func (hcs *ClientConfig) ToClient(ctx context.Context, host component.Host, sett
172167
transport.WriteBufferSize = hcs.WriteBufferSize
173168
}
174169

175-
if hcs.MaxIdleConns != nil {
176-
transport.MaxIdleConns = *hcs.MaxIdleConns
177-
}
178-
179-
if hcs.MaxIdleConnsPerHost != nil {
180-
transport.MaxIdleConnsPerHost = *hcs.MaxIdleConnsPerHost
181-
}
182-
183-
if hcs.MaxConnsPerHost != nil {
184-
transport.MaxConnsPerHost = *hcs.MaxConnsPerHost
185-
}
186-
187-
if hcs.IdleConnTimeout != nil {
188-
transport.IdleConnTimeout = *hcs.IdleConnTimeout
189-
}
170+
transport.MaxIdleConns = hcs.MaxIdleConns
171+
transport.MaxIdleConnsPerHost = hcs.MaxIdleConnsPerHost
172+
transport.MaxConnsPerHost = hcs.MaxConnsPerHost
173+
transport.IdleConnTimeout = hcs.IdleConnTimeout
190174

191175
// Setting the Proxy URL
192176
if hcs.ProxyURL != "" {

config/confighttp/confighttp_test.go

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ func TestAllHTTPClientSettings(t *testing.T) {
7777
},
7878
ReadBufferSize: 1024,
7979
WriteBufferSize: 512,
80-
MaxIdleConns: &maxIdleConns,
81-
MaxIdleConnsPerHost: &maxIdleConnsPerHost,
82-
MaxConnsPerHost: &maxConnsPerHost,
83-
IdleConnTimeout: &idleConnTimeout,
80+
MaxIdleConns: maxIdleConns,
81+
MaxIdleConnsPerHost: maxIdleConnsPerHost,
82+
MaxConnsPerHost: maxConnsPerHost,
83+
IdleConnTimeout: idleConnTimeout,
8484
Compression: "",
8585
DisableKeepAlives: true,
8686
Cookies: &CookiesConfig{Enabled: true},
@@ -98,10 +98,10 @@ func TestAllHTTPClientSettings(t *testing.T) {
9898
},
9999
ReadBufferSize: 1024,
100100
WriteBufferSize: 512,
101-
MaxIdleConns: &maxIdleConns,
102-
MaxIdleConnsPerHost: &maxIdleConnsPerHost,
103-
MaxConnsPerHost: &maxConnsPerHost,
104-
IdleConnTimeout: &idleConnTimeout,
101+
MaxIdleConns: maxIdleConns,
102+
MaxIdleConnsPerHost: maxIdleConnsPerHost,
103+
MaxConnsPerHost: maxConnsPerHost,
104+
IdleConnTimeout: idleConnTimeout,
105105
Compression: "none",
106106
DisableKeepAlives: true,
107107
HTTP2ReadIdleTimeout: idleConnTimeout,
@@ -118,10 +118,10 @@ func TestAllHTTPClientSettings(t *testing.T) {
118118
},
119119
ReadBufferSize: 1024,
120120
WriteBufferSize: 512,
121-
MaxIdleConns: &maxIdleConns,
122-
MaxIdleConnsPerHost: &maxIdleConnsPerHost,
123-
MaxConnsPerHost: &maxConnsPerHost,
124-
IdleConnTimeout: &idleConnTimeout,
121+
MaxIdleConns: maxIdleConns,
122+
MaxIdleConnsPerHost: maxIdleConnsPerHost,
123+
MaxConnsPerHost: maxConnsPerHost,
124+
IdleConnTimeout: idleConnTimeout,
125125
Compression: "gzip",
126126
DisableKeepAlives: true,
127127
HTTP2ReadIdleTimeout: idleConnTimeout,
@@ -138,10 +138,10 @@ func TestAllHTTPClientSettings(t *testing.T) {
138138
},
139139
ReadBufferSize: 1024,
140140
WriteBufferSize: 512,
141-
MaxIdleConns: &maxIdleConns,
142-
MaxIdleConnsPerHost: &maxIdleConnsPerHost,
143-
MaxConnsPerHost: &maxConnsPerHost,
144-
IdleConnTimeout: &idleConnTimeout,
141+
MaxIdleConns: maxIdleConns,
142+
MaxIdleConnsPerHost: maxIdleConnsPerHost,
143+
MaxConnsPerHost: maxConnsPerHost,
144+
IdleConnTimeout: idleConnTimeout,
145145
Compression: "gzip",
146146
DisableKeepAlives: true,
147147
HTTP2ReadIdleTimeout: idleConnTimeout,
@@ -212,19 +212,19 @@ func TestPartialHTTPClientSettings(t *testing.T) {
212212
transport := client.Transport.(*http.Transport)
213213
assert.EqualValues(t, 1024, transport.ReadBufferSize)
214214
assert.EqualValues(t, 512, transport.WriteBufferSize)
215-
assert.EqualValues(t, 100, transport.MaxIdleConns)
215+
assert.EqualValues(t, 0, transport.MaxIdleConns)
216216
assert.EqualValues(t, 0, transport.MaxIdleConnsPerHost)
217217
assert.EqualValues(t, 0, transport.MaxConnsPerHost)
218-
assert.EqualValues(t, 90*time.Second, transport.IdleConnTimeout)
218+
assert.EqualValues(t, 0, transport.IdleConnTimeout)
219219
assert.False(t, transport.DisableKeepAlives)
220220
})
221221
}
222222
}
223223

224224
func TestDefaultHTTPClientSettings(t *testing.T) {
225225
httpClientSettings := NewDefaultClientConfig()
226-
assert.EqualValues(t, 100, *httpClientSettings.MaxIdleConns)
227-
assert.EqualValues(t, 90*time.Second, *httpClientSettings.IdleConnTimeout)
226+
assert.EqualValues(t, 100, httpClientSettings.MaxIdleConns)
227+
assert.EqualValues(t, 90*time.Second, httpClientSettings.IdleConnTimeout)
228228
}
229229

230230
func TestProxyURL(t *testing.T) {
@@ -1027,6 +1027,41 @@ func TestHttpClientHostHeader(t *testing.T) {
10271027
})
10281028
}
10291029

1030+
func TestHttpTransportOptions(t *testing.T) {
1031+
settings := componenttest.NewNopTelemetrySettings()
1032+
// Disable OTel instrumentation so the *http.Transport object is directly accessible
1033+
settings.MeterProvider = nil
1034+
settings.TracerProvider = nil
1035+
1036+
clientConfig := NewDefaultClientConfig()
1037+
clientConfig.MaxIdleConns = 100
1038+
clientConfig.IdleConnTimeout = time.Duration(100)
1039+
clientConfig.MaxConnsPerHost = 100
1040+
clientConfig.MaxIdleConnsPerHost = 100
1041+
client, err := clientConfig.ToClient(context.Background(), &mockHost{}, settings)
1042+
require.NoError(t, err)
1043+
transport, ok := client.Transport.(*http.Transport)
1044+
require.True(t, ok, "client.Transport is not an *http.Transport")
1045+
require.Equal(t, 100, transport.MaxIdleConns)
1046+
require.Equal(t, time.Duration(100), transport.IdleConnTimeout)
1047+
require.Equal(t, 100, transport.MaxConnsPerHost)
1048+
require.Equal(t, 100, transport.MaxIdleConnsPerHost)
1049+
1050+
clientConfig = NewDefaultClientConfig()
1051+
clientConfig.MaxIdleConns = 0
1052+
clientConfig.IdleConnTimeout = 0
1053+
clientConfig.MaxConnsPerHost = 0
1054+
clientConfig.IdleConnTimeout = time.Duration(0)
1055+
client, err = clientConfig.ToClient(context.Background(), &mockHost{}, settings)
1056+
require.NoError(t, err)
1057+
transport, ok = client.Transport.(*http.Transport)
1058+
require.True(t, ok, "client.Transport is not an *http.Transport")
1059+
require.Equal(t, 0, transport.MaxIdleConns)
1060+
require.Equal(t, time.Duration(0), transport.IdleConnTimeout)
1061+
require.Equal(t, 0, transport.MaxConnsPerHost)
1062+
require.Equal(t, 0, transport.MaxIdleConnsPerHost)
1063+
}
1064+
10301065
func TestContextWithClient(t *testing.T) {
10311066
testCases := []struct {
10321067
name string

exporter/otlphttpexporter/config_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ func TestUnmarshalConfig(t *testing.T) {
7777
WriteBufferSize: 345,
7878
Timeout: time.Second * 10,
7979
Compression: "gzip",
80-
MaxIdleConns: &defaultMaxIdleConns,
81-
MaxIdleConnsPerHost: &defaultMaxIdleConnsPerHost,
82-
MaxConnsPerHost: &defaultMaxConnsPerHost,
83-
IdleConnTimeout: &defaultIdleConnTimeout,
80+
MaxIdleConns: defaultMaxIdleConns,
81+
MaxIdleConnsPerHost: defaultMaxIdleConnsPerHost,
82+
MaxConnsPerHost: defaultMaxConnsPerHost,
83+
IdleConnTimeout: defaultIdleConnTimeout,
8484
},
8585
}, cfg)
8686
}

0 commit comments

Comments
 (0)