[breaking] Remove support for SAMPLING_TYPE env var and 'static' value (

#4735) These were deprecated in 2021 (#3302) --------- Signed-off-by: Yuri Shkuro <[email protected]>
jaegertracing · Sep 9, 2023 · bc8711b · bc8711b
1 parent 2ac6aac
commit bc8711b
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 101 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,22 @@ next release (yyyy-mm-dd)
 
 #### ⛔ Breaking Changes
 
+#### New Features
+
+#### Bug fixes, Minor Improvements
+
+### UI Changes
+
+1.50.0 (yyyy-mm-dd)
+-------------------
+
+### Backend Changes
+
+#### ⛔ Breaking Changes
+
+* [sampling] Remove support for SAMPLING_TYPE env var and 'static' value ([@yurishkuro](https://github.com/yurishkuro) in [#4735](https://github.com/jaegertracing/jaeger/pull/4735))
+
+
 #### New Features
 
 #### Bug fixes, Minor Improvements

diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go
@@ -69,7 +69,7 @@ func main() {
 	if err != nil {
 		log.Fatalf("Cannot initialize storage factory: %v", err)
 	}
-	strategyStoreFactoryConfig, err := ss.FactoryConfigFromEnv(os.Stderr)
+	strategyStoreFactoryConfig, err := ss.FactoryConfigFromEnv()
 	if err != nil {
 		log.Fatalf("Cannot initialize sampling strategy store factory config: %v", err)
 	}

diff --git a/cmd/collector/main.go b/cmd/collector/main.go
@@ -52,7 +52,7 @@ func main() {
 	if err != nil {
 		log.Fatalf("Cannot initialize storage factory: %v", err)
 	}
-	strategyStoreFactoryConfig, err := ss.FactoryConfigFromEnv(os.Stderr)
+	strategyStoreFactoryConfig, err := ss.FactoryConfigFromEnv()
 	if err != nil {
 		log.Fatalf("Cannot initialize sampling strategy store factory config: %v", err)
 	}

diff --git a/plugin/sampling/strategystore/factory_config.go b/plugin/sampling/strategystore/factory_config.go
@@ -16,18 +16,12 @@ package strategystore
 
 import (
 	"fmt"
-	"io"
 	"os"
 )
 
 const (
 	// SamplingTypeEnvVar is the name of the env var that defines the type of sampling strategy store used.
 	SamplingTypeEnvVar = "SAMPLING_CONFIG_TYPE"
-
-	// previously the SAMPLING_TYPE env var was used for configuration, continue to support this old env var with warnings
-	deprecatedSamplingTypeEnvVar = "SAMPLING_TYPE"
-	// static is the old name for "file". we will translate from the deprecated to current name here. all other code will expect "file"
-	deprecatedSamplingTypeStatic = "static"
 )
 
 // FactoryConfig tells the Factory what sampling type it needs to create.
@@ -38,8 +32,8 @@ type FactoryConfig struct {
 // FactoryConfigFromEnv reads the desired sampling type from the SAMPLING_CONFIG_TYPE environment variable. Allowed values:
 // * `file` - built-in
 // * `adaptive` - built-in
-func FactoryConfigFromEnv(log io.Writer) (*FactoryConfig, error) {
-	strategyStoreType := getStrategyStoreTypeFromEnv(log)
+func FactoryConfigFromEnv() (*FactoryConfig, error) {
+	strategyStoreType := getStrategyStoreTypeFromEnv()
 	if strategyStoreType != samplingTypeAdaptive &&
 		strategyStoreType != samplingTypeFile {
 		return nil, fmt.Errorf("invalid sampling type: %s. Valid types are %v", strategyStoreType, AllSamplingTypes)
@@ -50,24 +44,13 @@ func FactoryConfigFromEnv(log io.Writer) (*FactoryConfig, error) {
 	}, nil
 }
 
-func getStrategyStoreTypeFromEnv(log io.Writer) string {
+func getStrategyStoreTypeFromEnv() string {
 	// check the new env var
 	strategyStoreType := os.Getenv(SamplingTypeEnvVar)
 	if strategyStoreType != "" {
 		return strategyStoreType
 	}
 
-	// accept the old env var and value but warn
-	strategyStoreType = os.Getenv(deprecatedSamplingTypeEnvVar)
-	if strategyStoreType != "" {
-		fmt.Fprintf(log, "WARNING: Using deprecated '%s' env var. Please switch to '%s'.\n", deprecatedSamplingTypeEnvVar, SamplingTypeEnvVar)
-		if strategyStoreType == deprecatedSamplingTypeStatic {
-			fmt.Fprintf(log, "WARNING: Using deprecated '%s' value for %s. Please switch to '%s'.\n", strategyStoreType, SamplingTypeEnvVar, samplingTypeFile)
-			strategyStoreType = samplingTypeFile
-		}
-		return strategyStoreType
-	}
-
 	// default
 	return samplingTypeFile
 }
diff --git a/plugin/sampling/strategystore/factory_config_test.go b/plugin/sampling/strategystore/factory_config_test.go
@@ -16,7 +16,6 @@
 package strategystore
 
 import (
-	"io"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -27,71 +26,42 @@ func TestFactoryConfigFromEnv(t *testing.T) {
 	tests := []struct {
 		name         string
 		env          string
-		envVar       string
 		expectedType Kind
 		expectsError bool
 	}{
 		{
 			name:         "default",
 			expectedType: Kind("file"),
 		},
-		{
-			name:         "file on deprecatedSamplingTypeEnvVar",
-			env:          "file",
-			envVar:       deprecatedSamplingTypeEnvVar,
-			expectedType: Kind("file"),
-		},
 		{
 			name:         "file on SamplingTypeEnvVar",
 			env:          "file",
-			envVar:       SamplingTypeEnvVar,
-			expectedType: Kind("file"),
-		},
-		{
-			name:         "static works on the deprecatedSamplingTypeEnvVar",
-			env:          "static",
-			envVar:       deprecatedSamplingTypeEnvVar,
 			expectedType: Kind("file"),
 		},
 		{
-			name:         "static fails on the SamplingTypeEnvVar",
+			name:         "old value 'static' fails on the SamplingTypeEnvVar",
 			env:          "static",
-			envVar:       SamplingTypeEnvVar,
 			expectsError: true,
 		},
-		{
-			name:         "adaptive on deprecatedSamplingTypeEnvVar",
-			env:          "adaptive",
-			envVar:       deprecatedSamplingTypeEnvVar,
-			expectedType: Kind("adaptive"),
-		},
 		{
 			name:         "adaptive on SamplingTypeEnvVar",
 			env:          "adaptive",
-			envVar:       SamplingTypeEnvVar,
 			expectedType: Kind("adaptive"),
 		},
-		{
-			name:         "unexpected string on deprecatedSamplingTypeEnvVar",
-			env:          "??",
-			envVar:       deprecatedSamplingTypeEnvVar,
-			expectsError: true,
-		},
 		{
 			name:         "unexpected string on SamplingTypeEnvVar",
 			env:          "??",
-			envVar:       SamplingTypeEnvVar,
 			expectsError: true,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			if len(tc.envVar) != 0 {
-				t.Setenv(tc.envVar, tc.env)
+			if tc.env != "" {
+				t.Setenv(SamplingTypeEnvVar, tc.env)
 			}
 
-			f, err := FactoryConfigFromEnv(io.Discard)
+			f, err := FactoryConfigFromEnv()
 			if tc.expectsError {
 				assert.Error(t, err)
 				return
@@ -102,48 +72,3 @@ func TestFactoryConfigFromEnv(t *testing.T) {
 		})
 	}
 }
-
-func TestGetStrategyStoreTypeFromEnv(t *testing.T) {
-	tests := []struct {
-		name               string
-		deprecatedEnvValue string
-		currentEnvValue    string
-		expected           string
-	}{
-		{
-			name:     "default to file",
-			expected: "file",
-		},
-		{
-			name:            "current env var works",
-			currentEnvValue: "foo",
-			expected:        "foo",
-		},
-		{
-			name:               "current overrides deprecated",
-			currentEnvValue:    "foo",
-			deprecatedEnvValue: "blerg",
-			expected:           "foo",
-		},
-		{
-			name:               "deprecated accepted",
-			deprecatedEnvValue: "blerg",
-			expected:           "blerg",
-		},
-		{
-			name:               "static is switched to file",
-			deprecatedEnvValue: "static",
-			expected:           "file",
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Setenv(SamplingTypeEnvVar, tc.currentEnvValue)
-			t.Setenv(deprecatedSamplingTypeEnvVar, tc.deprecatedEnvValue)
-
-			actual := getStrategyStoreTypeFromEnv(io.Discard)
-			assert.Equal(t, actual, tc.expected)
-		})
-	}
-}