Skip to content

Commit

Permalink
[exporter/otlphttpexporter] Remove unnecessary nil assignment in defa…
Browse files Browse the repository at this point in the history
…ult client config

This is a follow up to open-telemetry#11273. Although I set fields MaxIdleConns, MaxIdleConnsPerHost, MaxConnsPerHost, IdleConnTimeout to nil manually to keep backwards compatibility, it turns out that in the call to [ToClient](https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/confighttp/confighttp.go#L141-L166) the http.Transport defaults are used. Thus, not setting to nil will maintain the same behaviour and is not necessary.
  • Loading branch information
mackjmr committed Sep 30, 2024
1 parent 431fd11 commit c4c0e1a
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 51 deletions.
4 changes: 0 additions & 4 deletions exporter/otlphttpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ func createDefaultConfig() component.Config {
clientConfig.Compression = configcompression.TypeGzip
// We almost read 0 bytes, so no need to tune ReadBufferSize.
clientConfig.WriteBufferSize = 512 * 1024
clientConfig.MaxIdleConns = nil
clientConfig.MaxIdleConnsPerHost = nil
clientConfig.MaxConnsPerHost = nil
clientConfig.IdleConnTimeout = nil

return &Config{
RetryConfig: configretry.NewDefaultBackOffConfig(),
Expand Down
83 changes: 36 additions & 47 deletions exporter/otlphttpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,21 @@ func TestCreateMetricsExporter(t *testing.T) {
require.NotNil(t, oexp)
}

func clientConfig(endpoint string, headers map[string]configopaque.String, tlsSetting configtls.ClientConfig, compression configcompression.Type) confighttp.ClientConfig {
clientConfig := confighttp.NewDefaultClientConfig()
clientConfig.TLSSetting = tlsSetting
clientConfig.Compression = compression
if endpoint != "" {
clientConfig.Endpoint = endpoint
}
if headers != nil {
clientConfig.Headers = headers
}
return clientConfig
}

func TestCreateTracesExporter(t *testing.T) {
var configCompression configcompression.Type
endpoint := "http://" + testutil.GetAvailableLocalAddress(t)

tests := []struct {
Expand All @@ -62,111 +76,86 @@ func TestCreateTracesExporter(t *testing.T) {
{
name: "NoEndpoint",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: "",
},
ClientConfig: clientConfig("", nil, configtls.ClientConfig{}, configCompression),
},
mustFailOnCreate: true,
},
{
name: "UseSecure",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
TLSSetting: configtls.ClientConfig{
Insecure: false,
},
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{
Insecure: false,
}, configCompression),
},
},
{
name: "Headers",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Headers: map[string]configopaque.String{
"hdr1": "val1",
"hdr2": "val2",
},
},
ClientConfig: clientConfig(endpoint, map[string]configopaque.String{
"hdr1": "val1",
"hdr2": "val2",
}, configtls.ClientConfig{}, configCompression),
},
},
{
name: "CaCert",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
TLSSetting: configtls.ClientConfig{
Config: configtls.Config{
CAFile: filepath.Join("testdata", "test_cert.pem"),
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{
Config: configtls.Config{
CAFile: filepath.Join("testdata", "test_cert.pem"),
},
},
}, configCompression),
},
},
{
name: "CertPemFileError",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
TLSSetting: configtls.ClientConfig{
Config: configtls.Config{
CAFile: "nosuchfile",
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{
Config: configtls.Config{
CAFile: "nosuchfile",
},
},
configCompression),
},
mustFailOnCreate: false,
mustFailOnStart: true,
},
{
name: "NoneCompression",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: "none",
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{}, "none"),
},
},
{
name: "GzipCompression",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.TypeGzip,
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{}, configcompression.TypeGzip),
},
},
{
name: "SnappyCompression",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.TypeSnappy,
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{}, configcompression.TypeSnappy),
},
},
{
name: "ZstdCompression",
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.TypeZstd,
},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{}, configcompression.TypeZstd),
},
},
{
name: "ProtoEncoding",
config: &Config{
Encoding: EncodingProto,
ClientConfig: confighttp.ClientConfig{Endpoint: endpoint},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{}, configCompression),
},
},
{
name: "JSONEncoding",
config: &Config{
Encoding: EncodingJSON,
ClientConfig: confighttp.ClientConfig{Endpoint: endpoint},
ClientConfig: clientConfig(endpoint, nil, configtls.ClientConfig{}, configCompression),
},
},
}
Expand Down

0 comments on commit c4c0e1a

Please sign in to comment.