Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce confighttp.HTTPTransportSettings #2470

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/confighttp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ README](../configtls/README.md).
- `headers`: name/value pairs added to the HTTP request headers
- [`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)
- [`response_header_timeout`](https://golang.org/pkg/net/http/#Transport)
[`write_buffer_size`](https://golang.org/pkg/net/http/#Transport)

Example:

Expand Down
32 changes: 21 additions & 11 deletions config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ type HTTPClientSettings struct {
// The target URL to send data to (e.g.: http://some.url:9411/v1/traces).
Endpoint string `mapstructure:"endpoint"`

HTTPTransportSettings `mapstructure:",squash"`
}

type HTTPTransportSettings struct {
// TLSSetting struct exposes TLS client configuration.
TLSSetting configtls.TLSClientSetting `mapstructure:",squash"`

Expand All @@ -42,6 +46,9 @@ type HTTPClientSettings struct {
// Timeout parameter configures `http.Client.Timeout`.
Timeout time.Duration `mapstructure:"timeout,omitempty"`

// ResponseHeaderTimeout parameter configures `http.Client.ResponseHeaderTimeout`
ResponseHeaderTimeout time.Duration `mapstructure:"response_header_timeout,omitempty"`

// Additional headers attached to each HTTP request sent by the client.
// Existing header values are overwritten if collision happens.
Headers map[string]string `mapstructure:"headers,omitempty"`
Expand All @@ -50,40 +57,43 @@ type HTTPClientSettings struct {
CustomRoundTripper func(next http.RoundTripper) (http.RoundTripper, error)
}

func (hcs *HTTPClientSettings) ToClient() (*http.Client, error) {
tlsCfg, err := hcs.TLSSetting.LoadTLSConfig()
func (hts *HTTPTransportSettings) ToClient() (*http.Client, error) {
tlsCfg, err := hts.TLSSetting.LoadTLSConfig()
if err != nil {
return nil, err
}
transport := http.DefaultTransport.(*http.Transport).Clone()
if tlsCfg != nil {
transport.TLSClientConfig = tlsCfg
}
if hcs.ReadBufferSize > 0 {
transport.ReadBufferSize = hcs.ReadBufferSize
if hts.ReadBufferSize > 0 {
transport.ReadBufferSize = hts.ReadBufferSize
}
if hts.WriteBufferSize > 0 {
transport.WriteBufferSize = hts.WriteBufferSize
}
if hcs.WriteBufferSize > 0 {
transport.WriteBufferSize = hcs.WriteBufferSize
if hts.ResponseHeaderTimeout > 0 {
transport.ResponseHeaderTimeout = hts.ResponseHeaderTimeout
}

clientTransport := (http.RoundTripper)(transport)
if len(hcs.Headers) > 0 {
if len(hts.Headers) > 0 {
clientTransport = &headerRoundTripper{
transport: transport,
headers: hcs.Headers,
headers: hts.Headers,
}
}

if hcs.CustomRoundTripper != nil {
clientTransport, err = hcs.CustomRoundTripper(clientTransport)
if hts.CustomRoundTripper != nil {
clientTransport, err = hts.CustomRoundTripper(clientTransport)
if err != nil {
return nil, err
}
}

return &http.Client{
Transport: clientTransport,
Timeout: hcs.Timeout,
Timeout: hts.Timeout,
}, nil
}

Expand Down
73 changes: 36 additions & 37 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,26 @@ import (
"go.opentelemetry.io/collector/config/configtls"
)

func TestAllHTTPClientSettings(t *testing.T) {
tests := []struct {
name string
settings HTTPClientSettings
func TestAllHTTPTransportSettings_ToClient(t *testing.T) {
tests := map[string]struct {
settings HTTPTransportSettings
shouldError bool
}{
{
name: "all_valid_settings",
settings: HTTPClientSettings{
Endpoint: "localhost:1234",
"all_valid_settings": {
settings: HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
},
ReadBufferSize: 1024,
WriteBufferSize: 512,
CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil },
ReadBufferSize: 1024,
WriteBufferSize: 512,
CustomRoundTripper: func(next http.RoundTripper) (http.RoundTripper, error) { return next, nil },
Timeout: 30 * time.Second,
ResponseHeaderTimeout: 10 * time.Second,
},
shouldError: false,
},
{
name: "error_round_tripper_returned",
settings: HTTPClientSettings{
Endpoint: "localhost:1234",
"error_round_tripper_returned": {
settings: HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
},
Expand All @@ -65,8 +62,8 @@ func TestAllHTTPClientSettings(t *testing.T) {
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
client, err := test.settings.ToClient()
if test.shouldError {
assert.Error(t, err)
Expand All @@ -80,15 +77,14 @@ func TestAllHTTPClientSettings(t *testing.T) {
}
}

func TestHTTPClientSettingsError(t *testing.T) {
tests := []struct {
settings HTTPClientSettings
func TestAllHTTPTransportSettings_ToClientError(t *testing.T) {
tests := map[string]struct {
settings HTTPTransportSettings
err string
}{
{
"can not load unknown CA file": {
err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
settings: HTTPClientSettings{
Endpoint: "",
settings: HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/doesnt/exist",
Expand All @@ -98,10 +94,9 @@ func TestHTTPClientSettingsError(t *testing.T) {
},
},
},
{
"missing key file for tls client authentication": {
err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
settings: HTTPClientSettings{
Endpoint: "",
settings: HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "/doesnt/exist",
Expand All @@ -112,8 +107,8 @@ func TestHTTPClientSettingsError(t *testing.T) {
},
},
}
for _, test := range tests {
t.Run(test.err, func(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
_, err := test.settings.ToClient()
assert.Regexp(t, test.err, err)
})
Expand Down Expand Up @@ -296,8 +291,10 @@ func TestHttpReception(t *testing.T) {
}

hcs := &HTTPClientSettings{
Endpoint: prefix + ln.Addr().String(),
TLSSetting: *tt.tlsClientCreds,
Endpoint: prefix + ln.Addr().String(),
HTTPTransportSettings: HTTPTransportSettings{
TLSSetting: *tt.tlsClientCreds,
},
}
client, errClient := hcs.ToClient()
assert.NoError(t, errClient)
Expand Down Expand Up @@ -409,13 +406,15 @@ func TestHttpHeaders(t *testing.T) {
defer server.Close()
serverURL, _ := url.Parse(server.URL)
setting := HTTPClientSettings{
Endpoint: serverURL.String(),
TLSSetting: configtls.TLSClientSetting{},
ReadBufferSize: 0,
WriteBufferSize: 0,
Timeout: 0,
Headers: map[string]string{
"header1": "value1",
Endpoint: serverURL.String(),
HTTPTransportSettings: HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{},
ReadBufferSize: 0,
WriteBufferSize: 0,
Timeout: 0,
Headers: map[string]string{
"header1": "value1",
},
},
}
client, _ := setting.ToClient()
Expand Down
30 changes: 16 additions & 14 deletions exporter/otlphttpexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,25 @@ func TestLoadConfig(t *testing.T) {
QueueSize: 10,
},
HTTPClientSettings: confighttp.HTTPClientSettings{
Headers: map[string]string{
"can you have a . here?": "F0000000-0000-0000-0000-000000000000",
"header1": "234",
"another": "somevalue",
},
Endpoint: "https://1.2.3.4:1234",
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/var/lib/mycert.pem",
CertFile: "certfile",
KeyFile: "keyfile",
HTTPTransportSettings: confighttp.HTTPTransportSettings{
Headers: map[string]string{
"can you have a . here?": "F0000000-0000-0000-0000-000000000000",
"header1": "234",
"another": "somevalue",
},
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/var/lib/mycert.pem",
CertFile: "certfile",
KeyFile: "keyfile",
},
Insecure: true,
},
Insecure: true,
ReadBufferSize: 123,
WriteBufferSize: 345,
Timeout: time.Second * 10,
},
ReadBufferSize: 123,
WriteBufferSize: 345,
Timeout: time.Second * 10,
},
})
}
10 changes: 6 additions & 4 deletions exporter/otlphttpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ func createDefaultConfig() configmodels.Exporter {
QueueSettings: exporterhelper.DefaultQueueSettings(),
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "",
Timeout: 30 * time.Second,
Headers: map[string]string{},
// We almost read 0 bytes, so no need to tune ReadBufferSize.
WriteBufferSize: 512 * 1024,
HTTPTransportSettings: confighttp.HTTPTransportSettings{
Timeout: 30 * time.Second,
Headers: map[string]string{},
// We almost read 0 bytes, so no need to tune ReadBufferSize.
WriteBufferSize: 512 * 1024,
},
},
}
}
Expand Down
30 changes: 19 additions & 11 deletions exporter/otlphttpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ func TestCreateTraceExporter(t *testing.T) {
config: Config{
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: endpoint,
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
HTTPTransportSettings: confighttp.HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
},
},
},
},
Expand All @@ -90,9 +92,11 @@ func TestCreateTraceExporter(t *testing.T) {
config: Config{
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: endpoint,
Headers: map[string]string{
"hdr1": "val1",
"hdr2": "val2",
HTTPTransportSettings: confighttp.HTTPTransportSettings{
Headers: map[string]string{
"hdr1": "val1",
"hdr2": "val2",
},
},
},
},
Expand All @@ -102,9 +106,11 @@ func TestCreateTraceExporter(t *testing.T) {
config: Config{
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: endpoint,
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "testdata/test_cert.pem",
HTTPTransportSettings: confighttp.HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "testdata/test_cert.pem",
},
},
},
},
Expand All @@ -115,9 +121,11 @@ func TestCreateTraceExporter(t *testing.T) {
config: Config{
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: endpoint,
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "nosuchfile",
HTTPTransportSettings: confighttp.HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "nosuchfile",
},
},
},
},
Expand Down
22 changes: 12 additions & 10 deletions exporter/prometheusremotewriteexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,20 @@ func Test_loadConfig(t *testing.T) {
ExternalLabels: map[string]string{"key1": "value1", "key2": "value2"},
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "localhost:8888",
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/var/lib/mycert.pem", // This is subject to change, but currently I have no idea what else to put here lol
HTTPTransportSettings: confighttp.HTTPTransportSettings{
TLSSetting: configtls.TLSClientSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/var/lib/mycert.pem", // This is subject to change, but currently I have no idea what else to put here lol
},
Insecure: false,
},
Insecure: false,
ReadBufferSize: 0,
WriteBufferSize: 512 * 1024,
Timeout: 5 * time.Second,
Headers: map[string]string{
"prometheus-remote-write-version": "0.1.0",
"x-scope-orgid": "234"},
},
ReadBufferSize: 0,
WriteBufferSize: 512 * 1024,
Timeout: 5 * time.Second,
Headers: map[string]string{
"prometheus-remote-write-version": "0.1.0",
"x-scope-orgid": "234"},
},
})
}
8 changes: 5 additions & 3 deletions exporter/prometheusremotewriteexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,11 @@ func Test_PushMetrics(t *testing.T) {
Namespace: "",
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "http://some.url:9411/api/prom/push",
// We almost read 0 bytes, so no need to tune ReadBufferSize.
ReadBufferSize: 0,
WriteBufferSize: 512 * 1024,
HTTPTransportSettings: confighttp.HTTPTransportSettings{
// We almost read 0 bytes, so no need to tune ReadBufferSize.
ReadBufferSize: 0,
WriteBufferSize: 512 * 1024,
},
},
}
assert.NotNil(t, config)
Expand Down
12 changes: 7 additions & 5 deletions exporter/prometheusremotewriteexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ func createDefaultConfig() configmodels.Exporter {
QueueSettings: exporterhelper.DefaultQueueSettings(),
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "http://some.url:9411/api/prom/push",
// We almost read 0 bytes, so no need to tune ReadBufferSize.
ReadBufferSize: 0,
WriteBufferSize: 512 * 1024,
Timeout: exporterhelper.DefaultTimeoutSettings().Timeout,
Headers: map[string]string{},
HTTPTransportSettings: confighttp.HTTPTransportSettings{
// We almost read 0 bytes, so no need to tune ReadBufferSize.
ReadBufferSize: 0,
WriteBufferSize: 512 * 1024,
Timeout: exporterhelper.DefaultTimeoutSettings().Timeout,
Headers: map[string]string{},
},
},
}
}
Loading