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

[configcompression] Rename CompressionType to Type #9416

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/configcompression-rename-enum.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: deprecation

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: configcompression

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate `CompressionType`, use `Type` instead.

# One or more tracking issues or pull requests related to the change
issues: [9416]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
61 changes: 43 additions & 18 deletions config/configcompression/compressiontype.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,41 @@ package configcompression // import "go.opentelemetry.io/collector/config/config

import "fmt"

type CompressionType string
// CompressionType represents a compression method
// Deprecated [0.94.0]: use Type instead.
type CompressionType = Type

// Type represents a compression method
type Type string
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved

const (
Gzip CompressionType = "gzip"
Zlib CompressionType = "zlib"
TypeGzip Type = "gzip"
TypeZlib Type = "zlib"
TypeDeflate Type = "deflate"
TypeSnappy Type = "snappy"
TypeZstd Type = "zstd"
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
typeNone Type = "none"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should "none" be a valid type? By valid I mean public to be used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the original PR's conversation there wasn't a public use case for none: #4651 (comment). Until the last 2 weeks there haven't been any material changes to the package so I'm guessing that there still isn't any public use case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something we could do after 1.0, I think we can keep it as is and expose in the future if we have a specific need

typeEmpty Type = ""

// Gzip
// Deprecated [0.94.0]: use TypeGzip instead.
Gzip CompressionType = "gzip"

// Zlib
// Deprecated [0.94.0]: use TypeZlib instead.
Zlib CompressionType = "zlib"

// Deflate
// Deprecated [0.94.0]: use TypeDeflate instead.
Deflate CompressionType = "deflate"
Snappy CompressionType = "snappy"
Zstd CompressionType = "zstd"
none CompressionType = "none"
empty CompressionType = ""

// Snappy
// Deprecated [0.94.0]: use TypeSnappy instead.
Snappy CompressionType = "snappy"

// Zstd
// Deprecated [0.94.0]: use TypeZstd instead.
Zstd CompressionType = "zstd"
)

// IsCompressed returns false if CompressionType is nil, none, or empty. Otherwise it returns true.
Expand All @@ -26,19 +51,19 @@ func IsCompressed(compressionType CompressionType) bool {

// IsCompressed returns false if CompressionType is nil, none, or empty.
// Otherwise, returns true.
func (ct *CompressionType) IsCompressed() bool {
return ct != nil && *ct != empty && *ct != none
func (ct *Type) IsCompressed() bool {
return *ct != typeEmpty && *ct != typeNone
}

func (ct *CompressionType) UnmarshalText(in []byte) error {
switch typ := CompressionType(in); typ {
case Gzip,
Zlib,
Deflate,
Snappy,
Zstd,
none,
empty:
func (ct *Type) UnmarshalText(in []byte) error {
switch typ := Type(in); typ {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Sorry, but my mind goes crazy when I see a switch used like an if :)) Can we fix this probably in a separate PR :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you objecting to typ := Type(in); typ or the switch statement in general?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I don't like switch statements with only 2 branches, the are if's no? :)) But is a nit and not a blocker for sure.

case TypeGzip,
TypeZlib,
TypeDeflate,
TypeSnappy,
TypeZstd,
typeNone,
typeEmpty:
*ct = typ
return nil
default:
Expand Down
4 changes: 2 additions & 2 deletions config/configcompression/compressiontype_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ func TestUnmarshalText(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
temp := none
temp := typeNone
err := temp.UnmarshalText(tt.compressionName)
if tt.shouldError {
assert.Error(t, err)
return
}
require.NoError(t, err)
ct := CompressionType(tt.compressionName)
ct := Type(tt.compressionName)
assert.Equal(t, temp, ct)
assert.Equal(t, tt.isCompressed, ct.IsCompressed())
})
Expand Down
10 changes: 5 additions & 5 deletions config/configgrpc/configgrpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type ClientConfig struct {
Endpoint string `mapstructure:"endpoint"`

// The compression key for supported compression types within collector.
Compression configcompression.CompressionType `mapstructure:"compression"`
Compression configcompression.Type `mapstructure:"compression"`

// TLSSetting struct exposes TLS client configuration.
TLSSetting configtls.TLSClientSetting `mapstructure:"tls"`
Expand Down Expand Up @@ -390,13 +390,13 @@ func (gss *ServerConfig) toServerOption(host component.Host, settings component.
}

// getGRPCCompressionName returns compression name registered in grpc.
func getGRPCCompressionName(compressionType configcompression.CompressionType) (string, error) {
func getGRPCCompressionName(compressionType configcompression.Type) (string, error) {
switch compressionType {
case configcompression.Gzip:
case configcompression.TypeGzip:
return gzip.Name, nil
case configcompression.Snappy:
case configcompression.TypeSnappy:
return snappy.Name, nil
case configcompression.Zstd:
case configcompression.TypeZstd:
return zstd.Name, nil
default:
return "", fmt.Errorf("unsupported compression type %q", compressionType)
Expand Down
6 changes: 3 additions & 3 deletions config/configgrpc/configgrpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
"test": "test",
},
Endpoint: "localhost:1234",
Compression: configcompression.Gzip,
Compression: configcompression.TypeGzip,
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
},
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
"test": "test",
},
Endpoint: "localhost:1234",
Compression: configcompression.Snappy,
Compression: configcompression.TypeSnappy,
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
},
Expand Down Expand Up @@ -147,7 +147,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
"test": "test",
},
Endpoint: "localhost:1234",
Compression: configcompression.Zstd,
Compression: configcompression.TypeZstd,
TLSSetting: configtls.TLSClientSetting{
Insecure: false,
},
Expand Down
4 changes: 2 additions & 2 deletions config/confighttp/compression.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import (

type compressRoundTripper struct {
rt http.RoundTripper
compressionType configcompression.CompressionType
compressionType configcompression.Type
compressor *compressor
}

func newCompressRoundTripper(rt http.RoundTripper, compressionType configcompression.CompressionType) (*compressRoundTripper, error) {
func newCompressRoundTripper(rt http.RoundTripper, compressionType configcompression.Type) (*compressRoundTripper, error) {
encoder, err := newCompressor(compressionType)
if err != nil {
return nil, err
Expand Down
18 changes: 9 additions & 9 deletions config/confighttp/compression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestHTTPClientCompression(t *testing.T) {

tests := []struct {
name string
encoding configcompression.CompressionType
encoding configcompression.Type
reqBody []byte
shouldError bool
}{
Expand All @@ -51,31 +51,31 @@ func TestHTTPClientCompression(t *testing.T) {
},
{
name: "ValidGzip",
encoding: configcompression.Gzip,
encoding: configcompression.TypeGzip,
reqBody: compressedGzipBody.Bytes(),
shouldError: false,
},
{
name: "ValidZlib",
encoding: configcompression.Zlib,
encoding: configcompression.TypeZlib,
reqBody: compressedZlibBody.Bytes(),
shouldError: false,
},
{
name: "ValidDeflate",
encoding: configcompression.Deflate,
encoding: configcompression.TypeDeflate,
reqBody: compressedDeflateBody.Bytes(),
shouldError: false,
},
{
name: "ValidSnappy",
encoding: configcompression.Snappy,
encoding: configcompression.TypeSnappy,
reqBody: compressedSnappyBody.Bytes(),
shouldError: false,
},
{
name: "ValidZstd",
encoding: configcompression.Zstd,
encoding: configcompression.TypeZstd,
reqBody: compressedZstdBody.Bytes(),
shouldError: false,
},
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestHTTPContentCompressionRequestWithNilBody(t *testing.T) {
require.NoError(t, err, "failed to create request to test handler")

client := http.Client{}
client.Transport, err = newCompressRoundTripper(http.DefaultTransport, configcompression.Gzip)
client.Transport, err = newCompressRoundTripper(http.DefaultTransport, configcompression.TypeGzip)
require.NoError(t, err)
res, err := client.Do(req)
require.NoError(t, err)
Expand Down Expand Up @@ -305,7 +305,7 @@ func TestHTTPContentCompressionCopyError(t *testing.T) {
require.NoError(t, err)

client := http.Client{}
client.Transport, err = newCompressRoundTripper(http.DefaultTransport, configcompression.Gzip)
client.Transport, err = newCompressRoundTripper(http.DefaultTransport, configcompression.TypeGzip)
require.NoError(t, err)
_, err = client.Do(req)
require.Error(t, err)
Expand All @@ -329,7 +329,7 @@ func TestHTTPContentCompressionRequestBodyCloseError(t *testing.T) {
require.NoError(t, err)

client := http.Client{}
client.Transport, err = newCompressRoundTripper(http.DefaultTransport, configcompression.Gzip)
client.Transport, err = newCompressRoundTripper(http.DefaultTransport, configcompression.TypeGzip)
require.NoError(t, err)
_, err = client.Do(req)
require.Error(t, err)
Expand Down
10 changes: 5 additions & 5 deletions config/confighttp/compressor.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ type compressor struct {

// writerFactory defines writer field in CompressRoundTripper.
// The validity of input is already checked when NewCompressRoundTripper was called in confighttp,
func newCompressor(compressionType configcompression.CompressionType) (*compressor, error) {
func newCompressor(compressionType configcompression.Type) (*compressor, error) {
switch compressionType {
case configcompression.Gzip:
case configcompression.TypeGzip:
return gZipPool, nil
case configcompression.Snappy:
case configcompression.TypeSnappy:
return snappyPool, nil
case configcompression.Zstd:
case configcompression.TypeZstd:
return zStdPool, nil
case configcompression.Zlib, configcompression.Deflate:
case configcompression.TypeZlib, configcompression.TypeDeflate:
return zLibPool, nil
}
return nil, errors.New("unsupported compression type, ")
Expand Down
2 changes: 1 addition & 1 deletion config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type ClientConfig struct {
Auth *configauth.Authentication `mapstructure:"auth"`

// The compression key for supported compression types within collector.
Compression configcompression.CompressionType `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.
// There's an already set value, and we want to override it only if an explicit value provided
Expand Down
4 changes: 2 additions & 2 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) {
name: "with_auth_configuration_has_extension_and_compression",
settings: ClientConfig{
Endpoint: "localhost:1234",
Auth: &configauth.Authentication{AuthenticatorID: mockID},
Compression: configcompression.Gzip,
Auth: &configauth.Authentication{AuthenticatorID: component.NewID("mock")},
Compression: configcompression.TypeGzip,
},
shouldErr: false,
host: &mockHost{
Expand Down
2 changes: 1 addition & 1 deletion exporter/otlpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func createDefaultConfig() component.Config {
ClientConfig: configgrpc.ClientConfig{
Headers: map[string]configopaque.String{},
// Default to gzip compression
Compression: configcompression.Gzip,
Compression: configcompression.TypeGzip,
// We almost read 0 bytes, so no need to tune ReadBufferSize.
WriteBufferSize: 512 * 1024,
},
Expand Down
8 changes: 4 additions & 4 deletions exporter/otlpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestCreateDefaultConfig(t *testing.T) {
assert.Equal(t, ocfg.RetryConfig, configretry.NewDefaultBackOffConfig())
assert.Equal(t, ocfg.QueueConfig, exporterhelper.NewDefaultQueueSettings())
assert.Equal(t, ocfg.TimeoutSettings, exporterhelper.NewDefaultTimeoutSettings())
assert.Equal(t, ocfg.Compression, configcompression.Gzip)
assert.Equal(t, ocfg.Compression, configcompression.TypeGzip)
}

func TestCreateMetricsExporter(t *testing.T) {
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: &Config{
ClientConfig: configgrpc.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.Gzip,
Compression: configcompression.TypeGzip,
},
},
},
Expand All @@ -111,7 +111,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: &Config{
ClientConfig: configgrpc.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.Snappy,
Compression: configcompression.TypeSnappy,
},
},
},
Expand All @@ -120,7 +120,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: &Config{
ClientConfig: configgrpc.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.Zstd,
Compression: configcompression.TypeZstd,
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion exporter/otlphttpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func createDefaultConfig() component.Config {
Timeout: 30 * time.Second,
Headers: map[string]configopaque.String{},
// Default to gzip compression
Compression: configcompression.Gzip,
Compression: configcompression.TypeGzip,
// We almost read 0 bytes, so no need to tune ReadBufferSize.
WriteBufferSize: 512 * 1024,
},
Expand Down
8 changes: 4 additions & 4 deletions exporter/otlphttpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestCreateDefaultConfig(t *testing.T) {
assert.Equal(t, ocfg.RetryConfig.InitialInterval, 5*time.Second, "default retry InitialInterval")
assert.Equal(t, ocfg.RetryConfig.MaxInterval, 30*time.Second, "default retry MaxInterval")
assert.Equal(t, ocfg.QueueConfig.Enabled, true, "default sending queue is enabled")
assert.Equal(t, ocfg.Compression, configcompression.Gzip)
assert.Equal(t, ocfg.Compression, configcompression.TypeGzip)
}

func TestCreateMetricsExporter(t *testing.T) {
Expand Down Expand Up @@ -132,7 +132,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.Gzip,
Compression: configcompression.TypeGzip,
},
},
},
Expand All @@ -141,7 +141,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.Snappy,
Compression: configcompression.TypeSnappy,
},
},
},
Expand All @@ -150,7 +150,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: endpoint,
Compression: configcompression.Zstd,
Compression: configcompression.TypeZstd,
},
},
},
Expand Down