diff --git a/.chloggen/configure-compression-levels.yaml b/.chloggen/configure-compression-levels.yaml index 67470c154f2..7b5b6da8e48 100644 --- a/.chloggen/configure-compression-levels.yaml +++ b/.chloggen/configure-compression-levels.yaml @@ -1,7 +1,7 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: breaking +change_type: 'enhancement' # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) component: confighttp diff --git a/config/configcompression/compressiontype.go b/config/configcompression/compressiontype.go index 073eca3ce1f..4403ca08e92 100644 --- a/config/configcompression/compressiontype.go +++ b/config/configcompression/compressiontype.go @@ -5,6 +5,8 @@ package configcompression // import "go.opentelemetry.io/collector/config/config import ( "fmt" + "strconv" + "strings" "github.com/klauspost/compress/zlib" ) @@ -14,8 +16,8 @@ type Type string type Level int type TypeWithLevel struct { - Type Type `mapstructure:"type"` - Level Level `mapstructure:"level"` + Type Type + Level Level } const ( @@ -34,19 +36,39 @@ func (ct *Type) IsCompressed() bool { return *ct != typeEmpty && *ct != typeNone } -func (ct *TypeWithLevel) UnmarshalText() (TypeWithLevel, error) { - typ := ct.Type - if (typ == TypeGzip && isValidLevel(int(ct.Level))) || - (typ == TypeZlib && isValidLevel(int(ct.Level))) || - (typ == TypeDeflate && isValidLevel(int(ct.Level))) || - typ == TypeSnappy || - typ == TypeZstd || - typ == typeNone || - typ == typeEmpty { - return TypeWithLevel{Type: typ, Level: ct.Level}, nil +func (ct *TypeWithLevel) UnmarshalText(in []byte) error { + var compressionTyp Type + var level int + var err error + parts := strings.Split(string(in), "/") + compressionTyp = Type(parts[0]) + level = zlib.DefaultCompression + if len(parts) > 1 { + level, err = strconv.Atoi(parts[1]) + if err != nil { + return fmt.Errorf("invalid compression level: %q", parts[1]) + } + if compressionTyp == TypeSnappy || + compressionTyp == typeNone || + compressionTyp == typeEmpty { + return fmt.Errorf("compression level is not supported for %q", compressionTyp) + } } + ct.Level = Level(level) + if (compressionTyp == TypeGzip && isValidLevel(level)) || + (compressionTyp == TypeZlib && isValidLevel(level)) || + (compressionTyp == TypeDeflate && isValidLevel(level)) || + compressionTyp == TypeSnappy || + compressionTyp == TypeZstd || + compressionTyp == typeNone || + compressionTyp == typeEmpty { + ct.Level = Level(level) + ct.Type = compressionTyp + return nil + } + + return fmt.Errorf("unsupported compression type/level %s/%d", compressionTyp, ct.Level) - return TypeWithLevel{Type: typ, Level: ct.Level}, fmt.Errorf("unsupported compression type/level %q/%q", typ, ct.Level) } func isValidLevel(level int) bool { diff --git a/config/configcompression/compressiontype_test.go b/config/configcompression/compressiontype_test.go index fbc2227112a..42d7b26c344 100644 --- a/config/configcompression/compressiontype_test.go +++ b/config/configcompression/compressiontype_test.go @@ -4,6 +4,7 @@ package configcompression import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -47,6 +48,12 @@ func TestUnmarshalText(t *testing.T) { shouldError: false, isCompressed: true, }, + { + name: "ValidZstdLevel", + compressionName: []byte("zstd/11"), + shouldError: false, + isCompressed: true, + }, { name: "ValidEmpty", compressionName: []byte(""), @@ -62,19 +69,47 @@ func TestUnmarshalText(t *testing.T) { compressionName: []byte("ggip"), shouldError: true, }, + { + name: "InvalidSnappy", + compressionName: []byte("snappy/1"), + shouldError: true, + }, + { + name: "InvalidNone", + compressionName: []byte("none/1"), + shouldError: true, + }, + { + name: "InvalidGzip", + compressionName: []byte("gzip/10"), + shouldError: true, + isCompressed: true, + }, + { + name: "InvalidZlib", + compressionName: []byte("zlib/10"), + shouldError: true, + isCompressed: true, + }, + { + name: "InvalidZstdLevel", + compressionName: []byte("zstd/ten"), + shouldError: true, + isCompressed: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - temp := TypeWithLevel{Type(tt.compressionName), 0} - ct, err := temp.UnmarshalText() + var temp TypeWithLevel + err := temp.UnmarshalText(tt.compressionName) if tt.shouldError { assert.Error(t, err) return } require.NoError(t, err) - // ct := Type(tt.compressionName) - assert.Equal(t, temp, ct) - assert.Equal(t, tt.isCompressed, ct.Type.IsCompressed()) + ct := Type(strings.Split(string(tt.compressionName), "/")[0]) + assert.Equal(t, temp.Type, ct) + assert.Equal(t, tt.isCompressed, ct.IsCompressed()) }) } } diff --git a/config/confighttp/README.md b/config/confighttp/README.md index 48dc53dcc57..23dbe845375 100644 --- a/config/confighttp/README.md +++ b/config/confighttp/README.md @@ -28,27 +28,27 @@ README](../configtls/README.md). - `none` will be treated as uncompressed, and any other inputs will cause an error. - Compression levels can now be configured as part of the compression type like below. Not specifying any compression level will result in the default. - `gzip` - - NoCompression: `0` - - BestSpeed: `1` - - BestCompression: `9` - - DefaultCompression: `-1` + - NoCompression: `gzip/0` + - BestSpeed: `gzip/1` + - BestCompression: `gzip/9` + - DefaultCompression: `gzip/-1` - `zlib` - - NoCompression: `0` - - BestSpeed: `1` - - BestCompression: `9` - - DefaultCompression: `-1` + - NoCompression: `zlib/0` + - BestSpeed: `zlib/1` + - BestCompression: `zlib/9` + - DefaultCompression: `zlib/-1` - `deflate` - - NoCompression: `0` - - BestSpeed: `1` - - BestCompression: `9` - - DefaultCompression: `-1` + - NoCompression: `deflate/0` + - BestSpeed: `deflate/1` + - BestCompression: `deflate/9` + - DefaultCompression: `deflate/-1` - `zstd` - - SpeedFastest: `1` - - SpeedDefault: `3` - - SpeedBetterCompression: `6` - - SpeedBestCompression: `11` + - SpeedFastest: `zstd/1` + - SpeedDefault: `zstd/3` + - SpeedBetterCompression: `zstd/6` + - SpeedBestCompression: `zstd/11` - `snappy` - No compression levels supported + No compression levels supported yet - [`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) @@ -75,9 +75,7 @@ exporter: headers: test1: "value1" "test 2": "value 2" - compression: - type: zstd - level: 11 + compression: zstd cookies: enabled: true ``` diff --git a/config/confighttp/compression_test.go b/config/confighttp/compression_test.go index 67cedce22a3..229d64da0f7 100644 --- a/config/confighttp/compression_test.go +++ b/config/confighttp/compression_test.go @@ -34,51 +34,58 @@ func TestHTTPClientCompression(t *testing.T) { compressedSnappyBody := compressSnappy(t, testBody) compressedZstdBody := compressZstd(t, testBody) + const ( + GzipLevel configcompression.Type = "gzip/1" + ZlibLevel configcompression.Type = "zlib/1" + DeflateLevel configcompression.Type = "deflate/1" + ZstdLevel configcompression.Type = "zstd/11" + ) + tests := []struct { name string - encoding configcompression.TypeWithLevel + encoding configcompression.Type reqBody []byte shouldError bool }{ { name: "ValidEmpty", - encoding: configcompression.TypeWithLevel{Type: configcompression.Type(""), Level: 0}, + encoding: "", reqBody: testBody, shouldError: false, }, { name: "ValidNone", - encoding: configcompression.TypeWithLevel{Type: configcompression.Type("none"), Level: 0}, + encoding: "none", reqBody: testBody, shouldError: false, }, { name: "ValidGzip", - encoding: configcompression.TypeWithLevel{Type: configcompression.TypeGzip, Level: gzip.BestSpeed}, + encoding: GzipLevel, reqBody: compressedGzipBody.Bytes(), shouldError: false, }, { name: "ValidZlib", - encoding: configcompression.TypeWithLevel{Type: configcompression.TypeZlib, Level: zlib.BestSpeed}, + encoding: ZlibLevel, reqBody: compressedZlibBody.Bytes(), shouldError: false, }, { name: "ValidDeflate", - encoding: configcompression.TypeWithLevel{Type: configcompression.TypeDeflate, Level: zlib.BestSpeed}, + encoding: DeflateLevel, reqBody: compressedDeflateBody.Bytes(), shouldError: false, }, { name: "ValidSnappy", - encoding: configcompression.TypeWithLevel{Type: configcompression.TypeSnappy, Level: 0}, + encoding: configcompression.TypeSnappy, reqBody: compressedSnappyBody.Bytes(), shouldError: false, }, { name: "ValidZstd", - encoding: configcompression.TypeWithLevel{Type: configcompression.TypeZstd, Level: 11}, + encoding: ZstdLevel, reqBody: compressedZstdBody.Bytes(), shouldError: false, }, @@ -290,7 +297,7 @@ func TestHTTPContentCompressionRequestWithNilBody(t *testing.T) { require.NoError(t, err, "failed to create request to test handler") client := http.Client{} - compression := configcompression.TypeWithLevel{configcompression.TypeGzip, gzip.BestSpeed} + compression := configcompression.TypeWithLevel{Type: configcompression.TypeGzip, Level: gzip.BestSpeed} client.Transport, err = newCompressRoundTripper(http.DefaultTransport, compression) require.NoError(t, err) res, err := client.Do(req) @@ -311,7 +318,7 @@ func TestHTTPContentCompressionCopyError(t *testing.T) { require.NoError(t, err) client := srv.Client() - compression := configcompression.TypeWithLevel{configcompression.TypeGzip, zlib.DefaultCompression} + compression := configcompression.TypeWithLevel{Type: configcompression.TypeGzip, Level: zlib.DefaultCompression} client.Transport, err = newCompressRoundTripper(http.DefaultTransport, compression) require.NoError(t, err) _, err = client.Do(req) @@ -336,7 +343,7 @@ func TestHTTPContentCompressionRequestBodyCloseError(t *testing.T) { require.NoError(t, err) client := srv.Client() - compression := configcompression.TypeWithLevel{configcompression.TypeGzip, zlib.DefaultCompression} + compression := configcompression.TypeWithLevel{Type: configcompression.TypeGzip, Level: zlib.DefaultCompression} client.Transport, err = newCompressRoundTripper(http.DefaultTransport, compression) require.NoError(t, err) _, err = client.Do(req) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 218e1f98543..ca1432c32a1 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -4,7 +4,6 @@ package confighttp // import "go.opentelemetry.io/collector/config/confighttp" import ( - "compress/zlib" "context" "crypto/tls" "errors" @@ -14,8 +13,6 @@ import ( "net/http" "net/http/cookiejar" "net/url" - "strconv" - "strings" "time" "github.com/rs/cors" @@ -70,7 +67,7 @@ type ClientConfig struct { Auth *configauth.Authentication `mapstructure:"auth"` // The compression key for supported compression types within collector. - Compression configcompression.TypeWithLevel `mapstructure:"compression"` + Compression configcompression.Type `mapstructure:"compression"` // MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open. // By default, it is set to 100. @@ -135,21 +132,6 @@ func NewDefaultClientConfig() ClientConfig { } } -// Gets the compression type and level from the configuration. -func getCompression(compressionField configcompression.Type) (compressionType configcompression.Type, compressionLevel configcompression.Level) { - parts := strings.Split(string(compressionField), "/") - - compressionLevel = zlib.DefaultCompression - compressionType = configcompression.Type(parts[0]) - if len(parts) > 1 { - levelStr := parts[1] - if level, err := strconv.Atoi(levelStr); err == nil { - compressionLevel = configcompression.Level(level) - } - } - return compressionType, compressionLevel -} - // ToClient creates an HTTP client. func (hcs *ClientConfig) ToClient(ctx context.Context, host component.Host, settings component.TelemetrySettings) (*http.Client, error) { tlsCfg, err := hcs.TLSSetting.LoadTLSConfig(ctx) @@ -234,13 +216,12 @@ func (hcs *ClientConfig) ToClient(ctx context.Context, host component.Host, sett // Compress the body using specified compression methods if non-empty string is provided. // Supporting gzip, zlib, deflate, snappy, and zstd; none is treated as uncompressed. - // compressionType, compressionLevel := getCompression(hcs.Compression.Type) - // compressionTypeWithLevel := configcompression.TypeWithLevel{compressionType, compressionLevel} - compressionTypeWithLevel, err := hcs.Compression.UnmarshalText() + var compressionTypeWithLevel configcompression.TypeWithLevel + err = compressionTypeWithLevel.UnmarshalText([]byte(hcs.Compression)) if err != nil { return nil, err } - if hcs.Compression.Type.IsCompressed() { + if hcs.Compression.IsCompressed() { clientTransport, err = newCompressRoundTripper(clientTransport, compressionTypeWithLevel) if err != nil { return nil, err diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 66ee74911ac..f0cd7f6fcd1 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -5,7 +5,6 @@ package confighttp import ( "bytes" - "compress/zlib" "context" "errors" "fmt" @@ -85,7 +84,7 @@ func TestAllHTTPClientSettings(t *testing.T) { MaxIdleConnsPerHost: &maxIdleConnsPerHost, MaxConnsPerHost: &maxConnsPerHost, IdleConnTimeout: &idleConnTimeout, - Compression: configcompression.TypeWithLevel{"", zlib.DefaultCompression}, + Compression: "", DisableKeepAlives: true, Cookies: &CookiesConfig{Enabled: true}, HTTP2ReadIdleTimeout: idleConnTimeout, @@ -106,7 +105,7 @@ func TestAllHTTPClientSettings(t *testing.T) { MaxIdleConnsPerHost: &maxIdleConnsPerHost, MaxConnsPerHost: &maxConnsPerHost, IdleConnTimeout: &idleConnTimeout, - Compression: configcompression.TypeWithLevel{"none", zlib.DefaultCompression}, + Compression: "none", DisableKeepAlives: true, HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, @@ -126,7 +125,7 @@ func TestAllHTTPClientSettings(t *testing.T) { MaxIdleConnsPerHost: &maxIdleConnsPerHost, MaxConnsPerHost: &maxConnsPerHost, IdleConnTimeout: &idleConnTimeout, - Compression: configcompression.TypeWithLevel{"gzip", zlib.DefaultCompression}, + Compression: "gzip", DisableKeepAlives: true, HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, @@ -146,13 +145,33 @@ func TestAllHTTPClientSettings(t *testing.T) { MaxIdleConnsPerHost: &maxIdleConnsPerHost, MaxConnsPerHost: &maxConnsPerHost, IdleConnTimeout: &idleConnTimeout, - Compression: configcompression.TypeWithLevel{"gzip", zlib.DefaultCompression}, + Compression: "gzip", DisableKeepAlives: true, HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, }, shouldError: false, }, + { + name: "invalid_settings_http2_health_check", + settings: ClientConfig{ + Endpoint: "localhost:1234", + TLSSetting: configtls.ClientConfig{ + Insecure: false, + }, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxIdleConns: &maxIdleConns, + MaxIdleConnsPerHost: &maxIdleConnsPerHost, + MaxConnsPerHost: &maxConnsPerHost, + IdleConnTimeout: &idleConnTimeout, + Compression: "gzip/ten", + DisableKeepAlives: true, + HTTP2ReadIdleTimeout: idleConnTimeout, + HTTP2PingTimeout: http2PingTimeout, + }, + shouldError: true, + }, } for _, tt := range tests { @@ -413,7 +432,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { settings: ClientConfig{ Endpoint: "localhost:1234", Auth: &configauth.Authentication{AuthenticatorID: component.MustNewID("mock")}, - Compression: configcompression.TypeWithLevel{configcompression.TypeGzip, zlib.DefaultCompression}, + Compression: configcompression.TypeGzip, }, shouldErr: false, host: &mockHost{ @@ -450,10 +469,10 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { transport := client.Transport // Compression should wrap Auth, unwrap it - if tt.settings.Compression.Type.IsCompressed() { + if tt.settings.Compression.IsCompressed() { ct, ok := transport.(*compressRoundTripper) assert.True(t, ok) - assert.Equal(t, tt.settings.Compression, ct.compressionType) + assert.Equal(t, tt.settings.Compression, ct.compressionType.Type) transport = ct.rt }