diff --git a/plugin/sampling/strategystore/factory_config.go b/plugin/sampling/strategystore/factory_config.go index d7ea7509dde..824d1a7ac98 100644 --- a/plugin/sampling/strategystore/factory_config.go +++ b/plugin/sampling/strategystore/factory_config.go @@ -41,16 +41,10 @@ type FactoryConfig struct { func FactoryConfigFromEnv(log io.Writer) (*FactoryConfig, error) { strategyStoreType := getStrategyStoreTypeFromEnv(log) if strategyStoreType != samplingTypeAdaptive && - strategyStoreType != samplingTypeFile && - strategyStoreType != deprecatedSamplingTypeStatic { + strategyStoreType != samplingTypeFile { return nil, fmt.Errorf("invalid sampling type: %s. Valid types are %v", strategyStoreType, AllSamplingTypes) } - if strategyStoreType == deprecatedSamplingTypeStatic { - fmt.Fprintf(log, "WARNING: Using deprecated '%s' value for %s. Please switch to '%s'.\n", strategyStoreType, SamplingTypeEnvVar, samplingTypeFile) - strategyStoreType = samplingTypeFile - } - return &FactoryConfig{ StrategyStoreType: Kind(strategyStoreType), }, nil @@ -63,10 +57,14 @@ func getStrategyStoreTypeFromEnv(log io.Writer) string { return strategyStoreType } - // accept the old env var but warn + // 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 } diff --git a/plugin/sampling/strategystore/factory_config_test.go b/plugin/sampling/strategystore/factory_config_test.go index 4a4de33ea82..035b144e66d 100644 --- a/plugin/sampling/strategystore/factory_config_test.go +++ b/plugin/sampling/strategystore/factory_config_test.go @@ -27,50 +27,78 @@ import ( func TestFactoryConfigFromEnv(t *testing.T) { tests := []struct { env string + envVar string expectedType Kind expectsError bool }{ + // default { expectedType: Kind("file"), }, + // file on both env vars { env: "file", + envVar: deprecatedSamplingTypeEnvVar, expectedType: Kind("file"), }, { - // static is deprecated and maps to file. functionality has not changed + env: "file", + envVar: SamplingTypeEnvVar, + expectedType: Kind("file"), + }, + // static works on the deprecated env var, but fails on the new + { env: "static", + envVar: deprecatedSamplingTypeEnvVar, expectedType: Kind("file"), }, + { + env: "static", + envVar: SamplingTypeEnvVar, + expectsError: true, + }, + // adaptive on both env vars + { + env: "adaptive", + envVar: deprecatedSamplingTypeEnvVar, + expectedType: Kind("adaptive"), + }, { env: "adaptive", + envVar: SamplingTypeEnvVar, expectedType: Kind("adaptive"), }, + // unexpected string on both env vars { env: "??", + envVar: deprecatedSamplingTypeEnvVar, + expectsError: true, + }, + { + env: "??", + envVar: SamplingTypeEnvVar, expectsError: true, }, } for _, tc := range tests { - // for each test case test both the old and new env vars - for _, envVar := range []string{SamplingTypeEnvVar, deprecatedSamplingTypeEnvVar} { - // clear env - os.Setenv(SamplingTypeEnvVar, "") - os.Setenv(deprecatedSamplingTypeEnvVar, "") + // clear env + os.Setenv(SamplingTypeEnvVar, "") + os.Setenv(deprecatedSamplingTypeEnvVar, "") - err := os.Setenv(envVar, tc.env) + if len(tc.envVar) != 0 { + err := os.Setenv(tc.envVar, tc.env) require.NoError(t, err) + } - f, err := FactoryConfigFromEnv(io.Discard) - if tc.expectsError { - assert.Error(t, err) - continue - } - - require.NoError(t, err) - assert.Equal(t, tc.expectedType, f.StrategyStoreType) + f, err := FactoryConfigFromEnv(io.Discard) + if tc.expectsError { + assert.Error(t, err) + continue } + + require.NoError(t, err) + assert.Equal(t, tc.expectedType, f.StrategyStoreType) } } @@ -100,6 +128,11 @@ func TestGetStrategyStoreTypeFromEnv(t *testing.T) { deprecatedEnvValue: "blerg", expected: "blerg", }, + // static is switched to file + { + deprecatedEnvValue: "static", + expected: "file", + }, } for _, tc := range tests {