Unmarshal compression type/level from configuration and extending tes…

…t coverage
open-telemetry · Oct 1, 2024 · 800a4c2 · 800a4c2
1 parent aca0f3d
commit 800a4c2
Show file tree

Hide file tree

Showing 7 changed files with 140 additions and 78 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,
 		},

diff --git 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