Remove two deprecated fields [chore] (#129)

Simply remove two deprecated fields. The v0.13 release includes support
for the old and new fields to support migration.

Author: jmacd

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+- Remove two deprecated fields, both concurrent batch processor `max_in_flight_bytes`
+  and otelarrow receiver `memory_limit` fields have corresponding `_mib` field names
+  for consistency.
+
 ## [0.13.0](https://github.com/open-telemetry/otel-arrow/releases/tag/v0.13.0) - 2023-12-20
 
 - Concurrent batch processor: Fail fast for large batch sizes. [#126](https://github.com/open-telemetry/otel-arrow/pull/126)

collector/processor/concurrentbatchprocessor/config.go

@@ -48,9 +48,6 @@ type Config struct {
 	// MaxInFlightSizeMiB limits the number of bytes in queue waiting to be
 	// processed by the senders.
 	MaxInFlightSizeMiB uint32 `mapstructure:"max_in_flight_size_mib"`
-
-	// Deprecated: Use MaxInFlightSizeMiB instead.
-	MaxInFlightBytes uint32 `mapstructure:"max_in_flight_bytes"`
 }
 
 var _ component.Config = (*Config)(nil)
@@ -71,19 +68,8 @@ func (cfg *Config) Validate() error {
 	if cfg.Timeout < 0 {
 		return errors.New("timeout must be greater or equal to 0")
 	}
-
-	if cfg.MaxInFlightBytes != 0 && cfg.MaxInFlightSizeMiB != 0 {
-		return errors.New("max_in_flight_bytes is deprecated, use only max_in_flight_size_mib instead")
-	}
-
-	if cfg.MaxInFlightBytes > 0 {
-		// Round up
-		cfg.MaxInFlightSizeMiB = (cfg.MaxInFlightBytes - 1 + 1<<20) >> 20
-		cfg.MaxInFlightBytes = 0
-	}
-
-	if cfg.MaxInFlightSizeMiB < 0 {
-		return errors.New("max_in_flight_size_mib must be greater than or equal to 0")
+	if cfg.MaxInFlightSizeMiB <= 0 {
+		return errors.New("max_in_flight_size_mib must be greater than 0")
 	}
 	return nil
 }

collector/processor/concurrentbatchprocessor/config_test.go

@@ -41,37 +41,41 @@ func TestUnmarshalConfig(t *testing.T) {
 
 func TestValidateConfig_DefaultBatchMaxSize(t *testing.T) {
 	cfg := &Config{
-		SendBatchSize:    100,
-		SendBatchMaxSize: 0,
+		SendBatchSize:      100,
+		SendBatchMaxSize:   0,
+		MaxInFlightSizeMiB: 1,
 	}
 	assert.NoError(t, cfg.Validate())
 }
 
 func TestValidateConfig_ValidBatchSizes(t *testing.T) {
 	cfg := &Config{
-		SendBatchSize:    100,
-		SendBatchMaxSize: 1000,
+		SendBatchSize:      100,
+		SendBatchMaxSize:   1000,
+		MaxInFlightSizeMiB: 1,
 	}
 	assert.NoError(t, cfg.Validate())
 
 }
 
 func TestValidateConfig_InvalidBatchSize(t *testing.T) {
 	cfg := &Config{
-		SendBatchSize:    1000,
-		SendBatchMaxSize: 100,
+		SendBatchSize:      1000,
+		SendBatchMaxSize:   100,
+		MaxInFlightSizeMiB: 1,
 	}
 	assert.Error(t, cfg.Validate())
 }
 
 func TestValidateConfig_InvalidTimeout(t *testing.T) {
 	cfg := &Config{
-		Timeout: -time.Second,
+		Timeout:            -time.Second,
+		MaxInFlightSizeMiB: 1,
 	}
 	assert.Error(t, cfg.Validate())
 }
 
-func TestValidateConfig_ValidZero(t *testing.T) {
+func TestValidateConfig_InvalidZero(t *testing.T) {
 	cfg := &Config{}
-	assert.NoError(t, cfg.Validate())
+	assert.Error(t, cfg.Validate())
 }

collector/receiver/otelarrowreceiver/config.go

@@ -4,7 +4,6 @@
 package otelarrowreceiver // import "github.com/open-telemetry/otel-arrow/collector/receiver/otelarrowreceiver"
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/open-telemetry/otel-arrow/collector/compression/zstd"
@@ -16,7 +15,6 @@ import (
 const (
 	// Confmap values.
 	protoGRPC                = "protocols::grpc"
-	protoArrowOldMemoryLimit = "protocols::arrow::memory_limit"
 	protoArrowMemoryLimitMiB = "protocols::arrow::memory_limit_mib"
 )
 
@@ -28,9 +26,6 @@ type Protocols struct {
 
 // ArrowSettings support configuring the Arrow receiver.
 type ArrowSettings struct {
-	// DeprecatedMemoryLimit is deprecated, use MemoryLimitMiB.
-	DeprecatedMemoryLimit uint64 `mapstructure:"memory_limit"`
-
 	// MemoryLimitMiB is the size of a shared memory region used
 	// by all Arrow streams, in MiB.  When too much load is
 	// passing through, they will see ResourceExhausted errors.
@@ -58,14 +53,6 @@ func (cfg *Config) Validate() error {
 }
 
 func (cfg *ArrowSettings) Validate() error {
-	if cfg.DeprecatedMemoryLimit != 0 && cfg.MemoryLimitMiB != 0 {
-		return errors.New("memory_limit is deprecated, use only memory_limit_mib")
-	}
-	if cfg.DeprecatedMemoryLimit != 0 {
-		// Round up
-		cfg.MemoryLimitMiB = (cfg.DeprecatedMemoryLimit - 1 + 1<<20) >> 20
-		cfg.DeprecatedMemoryLimit = 0
-	}
 	if err := cfg.Zstd.Validate(); err != nil {
 		return fmt.Errorf("zstd decoder: invalid configuration: %w", err)
 	}
@@ -80,11 +67,5 @@ func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
 		return err
 	}
 
-	// Allow the deprecated field, when explicitly set, to unset
-	// the new default value.
-	if conf.IsSet(protoArrowOldMemoryLimit) && !conf.IsSet(protoArrowMemoryLimitMiB) {
-		cfg.Arrow.MemoryLimitMiB = 0
-	}
-
 	return nil
 }

collector/receiver/otelarrowreceiver/config_test.go

@@ -39,28 +39,6 @@ func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
 	assert.Equal(t, defaultOnlyGRPC, cfg)
 }
 
-func TestUnmarshalOldMemoryLimitConfig(t *testing.T) {
-	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "oldmemlimit.yaml"))
-	require.NoError(t, err)
-	factory := NewFactory()
-	cfg := factory.CreateDefaultConfig()
-	assert.NoError(t, component.UnmarshalConfig(cm, cfg))
-	expectCfg := factory.CreateDefaultConfig().(*Config)
-	// The number in config is <1MB, so Validate() rounds up.
-	expectCfg.Arrow.MemoryLimitMiB = 1
-	assert.NoError(t, cfg.(*Config).Validate())
-	assert.Equal(t, expectCfg, cfg)
-}
-
-func TestUnmarshalBothMemoryLimitConfig(t *testing.T) {
-	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "bothmemlimit.yaml"))
-	require.NoError(t, err)
-	factory := NewFactory()
-	cfg := factory.CreateDefaultConfig()
-	assert.NoError(t, component.UnmarshalConfig(cm, cfg))
-	assert.Error(t, cfg.(*Config).Validate())
-}
-
 func TestUnmarshalConfig(t *testing.T) {
 	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml"))
 	require.NoError(t, err)