diff --git a/config/configparser/config.go b/config/configparser/config.go index e344742bcd0..33874784d4c 100644 --- a/config/configparser/config.go +++ b/config/configparser/config.go @@ -28,7 +28,7 @@ import ( "github.com/spf13/viper" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configload" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configmodels" ) @@ -38,7 +38,6 @@ type configErrorCode int const ( _ configErrorCode = iota // skip 0, start errors codes from 1. - errInvalidSubConfig errInvalidTypeAndNameKey errUnknownType errDuplicateName @@ -94,12 +93,9 @@ type pipelineSettings struct { // typeAndNameSeparator is the separator that is used between type and name in type/name composite keys. const typeAndNameSeparator = "/" -// Load loads a Config from Viper. +// Load loads a Config from Parser. // After loading the config, need to check if it is valid by calling `ValidateConfig`. -func Load( - v *viper.Viper, - factories component.Factories, -) (*configmodels.Config, error) { +func Load(v *config.Parser, factories component.Factories) (*configmodels.Config, error) { var config configmodels.Config @@ -120,7 +116,7 @@ func Load( // Start with the service extensions. - extensions, err := loadExtensions(v.GetStringMap(extensionsKeyName), factories.Extensions) + extensions, err := loadExtensions(cast.ToStringMap(v.Get(extensionsKeyName)), factories.Extensions) if err != nil { return nil, err } @@ -128,19 +124,19 @@ func Load( // Load data components (receivers, exporters, and processors). - receivers, err := loadReceivers(v.GetStringMap(receiversKeyName), factories.Receivers) + receivers, err := loadReceivers(cast.ToStringMap(v.Get(receiversKeyName)), factories.Receivers) if err != nil { return nil, err } config.Receivers = receivers - exporters, err := loadExporters(v.GetStringMap(exportersKeyName), factories.Exporters) + exporters, err := loadExporters(cast.ToStringMap(v.Get(exportersKeyName)), factories.Exporters) if err != nil { return nil, err } config.Exporters = exporters - processors, err := loadProcessors(v.GetStringMap(processorsKeyName), factories.Processors) + processors, err := loadProcessors(cast.ToStringMap(v.Get(processorsKeyName)), factories.Processors) if err != nil { return nil, err } @@ -550,29 +546,8 @@ func defaultUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) return componentViperSection.UnmarshalExact(intoCfg) } -// Copied from the Viper but changed to use the same delimiter -// and return error if the sub is not a map. -// See https://github.com/spf13/viper/issues/871 -func ViperSubExact(v *viper.Viper, key string) (*viper.Viper, error) { - data := v.Get(key) - if data == nil { - return configload.NewViper(), nil - } - - if reflect.TypeOf(data).Kind() == reflect.Map { - subv := configload.NewViper() - // Cannot return error because the subv is empty. - _ = subv.MergeConfigMap(cast.ToStringMap(data)) - return subv, nil - } - return nil, &configError{ - code: errInvalidSubConfig, - msg: fmt.Sprintf("unexpected sub-config value kind for key:%s value:%v kind:%v)", key, data, reflect.TypeOf(data).Kind()), - } -} - func viperFromStringMap(data map[string]interface{}) *viper.Viper { - v := configload.NewViper() + v := config.NewViper() // Cannot return error because the subv is empty. _ = v.MergeConfigMap(cast.ToStringMap(data)) return v diff --git a/config/configparser/config_test.go b/config/configparser/config_test.go index b7551f8999b..5d8ca33d6c9 100644 --- a/config/configparser/config_test.go +++ b/config/configparser/config_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configload" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/internal/testcomponents" @@ -202,11 +202,11 @@ func TestSimpleConfig(t *testing.T) { assert.NoError(t, err) // Load the config - config, err := loadConfigFile(t, path.Join(".", "testdata", test.name+".yaml"), factories) + cfg, err := loadConfigFile(t, path.Join(".", "testdata", test.name+".yaml"), factories) require.NoError(t, err, "Unable to load config") // Verify extensions. - assert.Equalf(t, 1, len(config.Extensions), "TEST[%s]", test.name) + assert.Equalf(t, 1, len(cfg.Extensions), "TEST[%s]", test.name) assert.Equalf(t, &testcomponents.ExampleExtensionCfg{ ExtensionSettings: configmodels.ExtensionSettings{ @@ -217,15 +217,15 @@ func TestSimpleConfig(t *testing.T) { ExtraMapSetting: map[string]string{"ext-1": extensionExtraMapValue + "_1", "ext-2": extensionExtraMapValue + "_2"}, ExtraListSetting: []string{extensionExtraListElement + "_1", extensionExtraListElement + "_2"}, }, - config.Extensions["exampleextension"], + cfg.Extensions["exampleextension"], "TEST[%s] Did not load extension config correctly", test.name) // Verify service. - assert.Equalf(t, 1, len(config.Service.Extensions), "TEST[%s]", test.name) - assert.Equalf(t, "exampleextension", config.Service.Extensions[0], "TEST[%s]", test.name) + assert.Equalf(t, 1, len(cfg.Service.Extensions), "TEST[%s]", test.name) + assert.Equalf(t, "exampleextension", cfg.Service.Extensions[0], "TEST[%s]", test.name) // Verify receivers - assert.Equalf(t, 1, len(config.Receivers), "TEST[%s]", test.name) + assert.Equalf(t, 1, len(cfg.Receivers), "TEST[%s]", test.name) assert.Equalf(t, &testcomponents.ExampleReceiver{ @@ -240,11 +240,11 @@ func TestSimpleConfig(t *testing.T) { ExtraMapSetting: map[string]string{"recv.1": receiverExtraMapValue + "_1", "recv.2": receiverExtraMapValue + "_2"}, ExtraListSetting: []string{receiverExtraListElement + "_1", receiverExtraListElement + "_2"}, }, - config.Receivers["examplereceiver"], + cfg.Receivers["examplereceiver"], "TEST[%s] Did not load receiver config correctly", test.name) // Verify exporters - assert.Equalf(t, 1, len(config.Exporters), "TEST[%s]", test.name) + assert.Equalf(t, 1, len(cfg.Exporters), "TEST[%s]", test.name) assert.Equalf(t, &testcomponents.ExampleExporter{ @@ -257,11 +257,11 @@ func TestSimpleConfig(t *testing.T) { ExtraMapSetting: map[string]string{"exp_1": exporterExtraMapValue + "_1", "exp_2": exporterExtraMapValue + "_2"}, ExtraListSetting: []string{exporterExtraListElement + "_1", exporterExtraListElement + "_2"}, }, - config.Exporters["exampleexporter"], + cfg.Exporters["exampleexporter"], "TEST[%s] Did not load exporter config correctly", test.name) // Verify Processors - assert.Equalf(t, 1, len(config.Processors), "TEST[%s]", test.name) + assert.Equalf(t, 1, len(cfg.Processors), "TEST[%s]", test.name) assert.Equalf(t, &testcomponents.ExampleProcessorCfg{ @@ -273,11 +273,11 @@ func TestSimpleConfig(t *testing.T) { ExtraMapSetting: map[string]string{"proc_1": processorExtraMapValue + "_1", "proc_2": processorExtraMapValue + "_2"}, ExtraListSetting: []string{processorExtraListElement + "_1", processorExtraListElement + "_2"}, }, - config.Processors["exampleprocessor"], + cfg.Processors["exampleprocessor"], "TEST[%s] Did not load processor config correctly", test.name) // Verify Pipelines - assert.Equalf(t, 1, len(config.Service.Pipelines), "TEST[%s]", test.name) + assert.Equalf(t, 1, len(cfg.Service.Pipelines), "TEST[%s]", test.name) assert.Equalf(t, &configmodels.Pipeline{ @@ -287,7 +287,7 @@ func TestSimpleConfig(t *testing.T) { Processors: []string{"exampleprocessor"}, Exporters: []string{"exampleexporter"}, }, - config.Service.Pipelines["traces"], + cfg.Service.Pipelines["traces"], "TEST[%s] Did not load pipeline config correctly", test.name) }) } @@ -304,11 +304,11 @@ func TestEscapedEnvVars(t *testing.T) { assert.NoError(t, err) // Load the config - config, err := loadConfigFile(t, path.Join(".", "testdata", "simple-config-with-escaped-env.yaml"), factories) + cfg, err := loadConfigFile(t, path.Join(".", "testdata", "simple-config-with-escaped-env.yaml"), factories) require.NoError(t, err, "Unable to load config") // Verify extensions. - assert.Equal(t, 1, len(config.Extensions)) + assert.Equal(t, 1, len(cfg.Extensions)) assert.Equal(t, &testcomponents.ExampleExtensionCfg{ ExtensionSettings: configmodels.ExtensionSettings{ @@ -319,15 +319,15 @@ func TestEscapedEnvVars(t *testing.T) { ExtraMapSetting: map[string]string{"ext-1": "${EXTENSIONS_EXAMPLEEXTENSION_EXTRA_MAP_EXT_VALUE_1}", "ext-2": "${EXTENSIONS_EXAMPLEEXTENSION_EXTRA_MAP_EXT_VALUE_2}"}, ExtraListSetting: []string{"${EXTENSIONS_EXAMPLEEXTENSION_EXTRA_LIST_VALUE_1}", "${EXTENSIONS_EXAMPLEEXTENSION_EXTRA_LIST_VALUE_2}"}, }, - config.Extensions["exampleextension"], + cfg.Extensions["exampleextension"], "Did not load extension config correctly") // Verify service. - assert.Equal(t, 1, len(config.Service.Extensions)) - assert.Equal(t, "exampleextension", config.Service.Extensions[0]) + assert.Equal(t, 1, len(cfg.Service.Extensions)) + assert.Equal(t, "exampleextension", cfg.Service.Extensions[0]) // Verify receivers - assert.Equal(t, 1, len(config.Receivers)) + assert.Equal(t, 1, len(cfg.Receivers)) assert.Equal(t, &testcomponents.ExampleReceiver{ @@ -357,11 +357,11 @@ func TestEscapedEnvVars(t *testing.T) { }, ExtraListSetting: []string{"$RECEIVERS_EXAMPLERECEIVER_EXTRA_LIST_VALUE_1", "$RECEIVERS_EXAMPLERECEIVER_EXTRA_LIST_VALUE_2"}, }, - config.Receivers["examplereceiver"], + cfg.Receivers["examplereceiver"], "Did not load receiver config correctly") // Verify exporters - assert.Equal(t, 1, len(config.Exporters)) + assert.Equal(t, 1, len(cfg.Exporters)) assert.Equal(t, &testcomponents.ExampleExporter{ @@ -373,11 +373,11 @@ func TestEscapedEnvVars(t *testing.T) { ExtraMapSetting: map[string]string{"exp_1": "${EXPORTERS_EXAMPLEEXPORTER_EXTRA_MAP_EXP_VALUE_1}", "exp_2": "${EXPORTERS_EXAMPLEEXPORTER_EXTRA_MAP_EXP_VALUE_2}"}, ExtraListSetting: []string{"${EXPORTERS_EXAMPLEEXPORTER_EXTRA_LIST_VALUE_1}", "${EXPORTERS_EXAMPLEEXPORTER_EXTRA_LIST_VALUE_2}"}, }, - config.Exporters["exampleexporter"], + cfg.Exporters["exampleexporter"], "Did not load exporter config correctly") // Verify Processors - assert.Equal(t, 1, len(config.Processors)) + assert.Equal(t, 1, len(cfg.Processors)) assert.Equal(t, &testcomponents.ExampleProcessorCfg{ @@ -389,11 +389,11 @@ func TestEscapedEnvVars(t *testing.T) { ExtraMapSetting: map[string]string{"proc_1": "$PROCESSORS_EXAMPLEPROCESSOR_EXTRA_MAP_PROC_VALUE_1", "proc_2": "$PROCESSORS_EXAMPLEPROCESSOR_EXTRA_MAP_PROC_VALUE_2"}, ExtraListSetting: []string{"$PROCESSORS_EXAMPLEPROCESSOR_EXTRA_LIST_VALUE_1", "$PROCESSORS_EXAMPLEPROCESSOR_EXTRA_LIST_VALUE_2"}, }, - config.Processors["exampleprocessor"], + cfg.Processors["exampleprocessor"], "Did not load processor config correctly") // Verify Pipelines - assert.Equal(t, 1, len(config.Service.Pipelines)) + assert.Equal(t, 1, len(cfg.Service.Pipelines)) assert.Equal(t, &configmodels.Pipeline{ @@ -403,7 +403,7 @@ func TestEscapedEnvVars(t *testing.T) { Processors: []string{"exampleprocessor"}, Exporters: []string{"exampleexporter"}, }, - config.Service.Pipelines["traces"], + cfg.Service.Pipelines["traces"], "Did not load pipeline config correctly") } @@ -495,10 +495,8 @@ func TestLoadEmptyAllSections(t *testing.T) { } func loadConfigFile(t *testing.T, fileName string, factories component.Factories) (*configmodels.Config, error) { - // Read yaml config from file - v := configload.NewViper() - v.SetConfigFile(fileName) - require.NoErrorf(t, v.ReadInConfig(), "unable to read the file %v", fileName) + v, err := config.NewParserFromFile(fileName) + require.NoError(t, err) // Load the config from viper using the given factories. return Load(v, factories) @@ -532,7 +530,7 @@ func TestExpandEnvLoadedConfig(t *testing.T) { testString := "$PTR_VALUE" - config := &testConfig{ + cfg := &testConfig{ ExporterSettings: configmodels.ExporterSettings{ TypeVal: configmodels.Type("test"), NameVal: "test", @@ -550,7 +548,7 @@ func TestExpandEnvLoadedConfig(t *testing.T) { IntValue: 3, } - expandEnvLoadedConfig(config) + expandEnvLoadedConfig(cfg) replacedTestString := "replaced_ptr_value" @@ -570,7 +568,7 @@ func TestExpandEnvLoadedConfig(t *testing.T) { StringValue: "replaced_value", StringPtrValue: &replacedTestString, IntValue: 3, - }, config) + }, cfg) } func TestExpandEnvLoadedConfigEscapedEnv(t *testing.T) { @@ -586,7 +584,7 @@ func TestExpandEnvLoadedConfigEscapedEnv(t *testing.T) { testString := "$$ESCAPED_PTR_VALUE" - config := &testConfig{ + cfg := &testConfig{ ExporterSettings: configmodels.ExporterSettings{ TypeVal: configmodels.Type("test"), NameVal: "test", @@ -604,7 +602,7 @@ func TestExpandEnvLoadedConfigEscapedEnv(t *testing.T) { IntValue: 3, } - expandEnvLoadedConfig(config) + expandEnvLoadedConfig(cfg) replacedTestString := "$ESCAPED_PTR_VALUE" @@ -624,7 +622,7 @@ func TestExpandEnvLoadedConfigEscapedEnv(t *testing.T) { StringValue: "$ESCAPED_VALUE", StringPtrValue: &replacedTestString, IntValue: 3, - }, config) + }, cfg) } func TestExpandEnvLoadedConfigMissingEnv(t *testing.T) { @@ -636,7 +634,7 @@ func TestExpandEnvLoadedConfigMissingEnv(t *testing.T) { testString := "$PTR_VALUE" - config := &testConfig{ + cfg := &testConfig{ ExporterSettings: configmodels.ExporterSettings{ TypeVal: configmodels.Type("test"), NameVal: "test", @@ -654,7 +652,7 @@ func TestExpandEnvLoadedConfigMissingEnv(t *testing.T) { IntValue: 3, } - expandEnvLoadedConfig(config) + expandEnvLoadedConfig(cfg) replacedTestString := "" @@ -674,31 +672,29 @@ func TestExpandEnvLoadedConfigMissingEnv(t *testing.T) { StringValue: "", StringPtrValue: &replacedTestString, IntValue: 3, - }, config) + }, cfg) } func TestExpandEnvLoadedConfigNil(t *testing.T) { - var config *testConfig + var cfg *testConfig // This should safely do nothing - expandEnvLoadedConfig(config) + expandEnvLoadedConfig(cfg) - assert.Equal(t, (*testConfig)(nil), config) + assert.Equal(t, (*testConfig)(nil), cfg) } func TestExpandEnvLoadedConfigNoPointer(t *testing.T) { assert.NoError(t, os.Setenv("VALUE", "replaced_value")) - config := testConfig{ + cfg := testConfig{ StringValue: "$VALUE", } - // This should do nothing as config is not a pointer - expandEnvLoadedConfig(config) + // This should do nothing as cfg is not a pointer + expandEnvLoadedConfig(cfg) - assert.Equal(t, testConfig{ - StringValue: "$VALUE", - }, config) + assert.Equal(t, testConfig{StringValue: "$VALUE"}, cfg) } type testUnexportedConfig struct { @@ -715,15 +711,15 @@ func TestExpandEnvLoadedConfigUnexportedField(t *testing.T) { assert.NoError(t, os.Unsetenv("VALUE")) }() - config := &testUnexportedConfig{ + cfg := &testUnexportedConfig{ unexportedStringValue: "$VALUE", ExportedStringValue: "$VALUE", } - expandEnvLoadedConfig(config) + expandEnvLoadedConfig(cfg) assert.Equal(t, &testUnexportedConfig{ unexportedStringValue: "$VALUE", ExportedStringValue: "replaced_value", - }, config) + }, cfg) } diff --git a/config/configtest/configtest.go b/config/configtest/configtest.go index 444de671445..2729adb6f48 100644 --- a/config/configtest/configtest.go +++ b/config/configtest/configtest.go @@ -17,33 +17,21 @@ package configtest import ( "testing" - "github.com/spf13/viper" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configload" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/config/configparser" ) -// NewViperFromYamlFile creates a viper instance that reads the given fileName as yaml config -// and can then be used to unmarshal the file contents to objects. -// Example usage for testing can be found in configtest_test.go -func NewViperFromYamlFile(t *testing.T, fileName string) *viper.Viper { - // Read yaml config from file - v := configload.NewViper() - v.SetConfigFile(fileName) - require.NoErrorf(t, v.ReadInConfig(), "unable to read the file %v", fileName) - - return v -} - // LoadConfigFile loads a config from file. func LoadConfigFile(t *testing.T, fileName string, factories component.Factories) (*configmodels.Config, error) { - v := NewViperFromYamlFile(t, fileName) - + // Read yaml config from file + cp, err := config.NewParserFromFile(fileName) + require.NoError(t, err) // Load the config from viper using the given factories. - cfg, err := configparser.Load(v, factories) + cfg, err := configparser.Load(cp, factories) if err != nil { return nil, err } diff --git a/config/configload/configload.go b/config/parser.go similarity index 61% rename from config/configload/configload.go rename to config/parser.go index 7ecc3882e14..cd2d88b53d0 100644 --- a/config/configload/configload.go +++ b/config/parser.go @@ -12,12 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -// package configload implements the configuration Parser. -package configload +// package config implements the configuration Parser. +package config import ( + "fmt" + "reflect" "strings" + "github.com/spf13/cast" "github.com/spf13/viper" ) @@ -39,8 +42,19 @@ func NewParser() *Parser { } } -// FromViper creates a Parser from a Viper instance. -func FromViper(v *viper.Viper) *Parser { +// NewParserFromFile creates a new Parser by reading the given file. +func NewParserFromFile(fileName string) (*Parser, error) { + // Read yaml config from file + v := NewViper() + v.SetConfigFile(fileName) + if err := v.ReadInConfig(); err != nil { + return nil, fmt.Errorf("unable to read the file %v: %w", fileName, err) + } + return ParserFromViper(v), nil +} + +// ParserFromViper creates a Parser from a Viper instance. +func ParserFromViper(v *viper.Viper) *Parser { return &Parser{v: v} } @@ -49,16 +63,54 @@ type Parser struct { v *viper.Viper } -// Viper returns the underlying Viper instance. -func (l *Parser) Viper() *viper.Viper { - return l.v +// AllKeys returns all keys holding a value, regardless of where they are set. +// Nested keys are returned with a KeyDelimiter separator +func (l *Parser) AllKeys() []string { + return l.v.AllKeys() +} + +// Unmarshal unmarshals the config into a struct. Make sure that the tags +// on the fields of the structure are properly set. +func (l *Parser) Unmarshal(rawVal interface{}) error { + return l.v.Unmarshal(rawVal) } // UnmarshalExact unmarshals the config into a struct, erroring if a field is nonexistent. func (l *Parser) UnmarshalExact(intoCfg interface{}) error { + l.v.AllKeys() return l.v.UnmarshalExact(intoCfg) } +// Get can retrieve any value given the key to use. +func (l *Parser) Get(key string) interface{} { + return l.v.Get(key) +} + +// Set sets the value for the key. +func (l *Parser) Set(key string, value interface{}) { + l.v.Set(key, value) +} + +// Sub returns new Parser instance representing a sub tree of this instance. +func (l *Parser) Sub(key string) (*Parser, error) { + // Copied from the Viper but changed to use the same delimiter + // and return error if the sub is not a map. + // See https://github.com/spf13/viper/issues/871 + data := l.Get(key) + if data == nil { + return NewParser(), nil + } + + if reflect.TypeOf(data).Kind() == reflect.Map { + subv := NewViper() + // Cannot return error because the subv is empty. + _ = subv.MergeConfigMap(cast.ToStringMap(data)) + return ParserFromViper(subv), nil + } + + return nil, fmt.Errorf("unexpected sub-config value kind for key:%s value:%v kind:%v)", key, data, reflect.TypeOf(data).Kind()) +} + // deepSearch scans deep maps, following the key indexes listed in the // sequence "path". // The last value is expected to be another map, and is returned. diff --git a/config/configload/configload_test.go b/config/parser_test.go similarity index 98% rename from config/configload/configload_test.go rename to config/parser_test.go index 1308be875f5..7ff8fd3914e 100644 --- a/config/configload/configload_test.go +++ b/config/parser_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package configload +package config import ( "testing" @@ -81,7 +81,7 @@ func TestToStringMap(t *testing.T) { v := NewViper() v.SetConfigFile(test.fileName) require.NoError(t, v.ReadInConfig(), "Unable to read configuration file '%s'", test.fileName) - parser := FromViper(v) + parser := ParserFromViper(v) assert.Equal(t, test.stringMap, parser.ToStringMap()) }) } diff --git a/config/configload/testdata/basic_types.yaml b/config/testdata/basic_types.yaml similarity index 100% rename from config/configload/testdata/basic_types.yaml rename to config/testdata/basic_types.yaml diff --git a/config/configload/testdata/config.yaml b/config/testdata/config.yaml similarity index 100% rename from config/configload/testdata/config.yaml rename to config/testdata/config.yaml diff --git a/internal/processor/filtermetric/config_test.go b/internal/processor/filtermetric/config_test.go index 2c28d5d5753..274ae27bdc6 100644 --- a/internal/processor/filtermetric/config_test.go +++ b/internal/processor/filtermetric/config_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/config/configtest" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/internal/processor/filterset" "go.opentelemetry.io/collector/internal/processor/filterset/regexp" ) @@ -48,8 +48,8 @@ func createConfigWithRegexpOptions(filters []string, rCfg *regexp.Config) *Match func TestConfig(t *testing.T) { testFile := path.Join(".", "testdata", "config.yaml") - v := configtest.NewViperFromYamlFile(t, testFile) - + v, err := config.NewParserFromFile(testFile) + require.NoError(t, err) testYamls := map[string]MatchProperties{} require.NoErrorf(t, v.UnmarshalExact(&testYamls), "unable to unmarshal yaml from file %v", testFile) diff --git a/internal/processor/filterset/config_test.go b/internal/processor/filterset/config_test.go index 0ce9f7c43ed..90b4647d3f5 100644 --- a/internal/processor/filterset/config_test.go +++ b/internal/processor/filterset/config_test.go @@ -21,13 +21,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/config/configtest" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/internal/processor/filterset/regexp" ) func readTestdataConfigYamls(t *testing.T, filename string) map[string]*Config { testFile := path.Join(".", "testdata", filename) - v := configtest.NewViperFromYamlFile(t, testFile) + v, err := config.NewParserFromFile(testFile) + require.NoError(t, err) cfgs := map[string]*Config{} require.NoErrorf(t, v.UnmarshalExact(&cfgs), "unable to unmarshal yaml from file %v", testFile) diff --git a/internal/processor/filterset/regexp/config_test.go b/internal/processor/filterset/regexp/config_test.go index 00465d742b5..23fd9a9121f 100644 --- a/internal/processor/filterset/regexp/config_test.go +++ b/internal/processor/filterset/regexp/config_test.go @@ -21,12 +21,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/config/configtest" + "go.opentelemetry.io/collector/config" ) func TestConfig(t *testing.T) { testFile := path.Join(".", "testdata", "config.yaml") - v := configtest.NewViperFromYamlFile(t, testFile) + v, err := config.NewParserFromFile(testFile) + require.NoError(t, err) actualConfigs := map[string]*Config{} require.NoErrorf(t, v.UnmarshalExact(&actualConfigs), "unable to unmarshal yaml from file %v", testFile) diff --git a/receiver/hostmetricsreceiver/factory.go b/receiver/hostmetricsreceiver/factory.go index 2e43ba4abe1..b3aa3d428b4 100644 --- a/receiver/hostmetricsreceiver/factory.go +++ b/receiver/hostmetricsreceiver/factory.go @@ -23,8 +23,8 @@ import ( "go.uber.org/zap" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configmodels" - "go.opentelemetry.io/collector/config/configparser" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal" "go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/cpuscraper" @@ -78,8 +78,9 @@ func NewFactory() component.ReceiverFactory { func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error { // load the non-dynamic config normally + cp := config.ParserFromViper(componentViperSection) - err := componentViperSection.Unmarshal(intoCfg) + err := cp.Unmarshal(intoCfg) if err != nil { return err } @@ -93,11 +94,11 @@ func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) cfg.Scrapers = map[string]internal.Config{} - scrapersViperSection, err := configparser.ViperSubExact(componentViperSection, scrapersKey) + scrapersSection, err := cp.Sub(scrapersKey) if err != nil { return err } - if len(scrapersViperSection.AllKeys()) == 0 { + if len(scrapersSection.AllKeys()) == 0 { return errors.New("must specify at least one scraper when using hostmetrics receiver") } @@ -108,7 +109,7 @@ func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) } collectorCfg := factory.CreateDefaultConfig() - collectorViperSection, err := configparser.ViperSubExact(scrapersViperSection, key) + collectorViperSection, err := scrapersSection.Sub(key) if err != nil { return err } diff --git a/receiver/jaegerreceiver/factory_test.go b/receiver/jaegerreceiver/factory_test.go index 2bedac16081..5dee435e299 100644 --- a/receiver/jaegerreceiver/factory_test.go +++ b/receiver/jaegerreceiver/factory_test.go @@ -26,11 +26,11 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componenttest" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configcheck" "go.opentelemetry.io/collector/config/configerror" "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/confighttp" - "go.opentelemetry.io/collector/config/configload" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configtest" "go.opentelemetry.io/collector/config/configtls" @@ -344,9 +344,9 @@ func TestCustomUnmarshalErrors(t *testing.T) { fu, ok := factory.(component.ConfigUnmarshaler) assert.True(t, ok) - err := fu.Unmarshal(configload.NewViper(), nil) + err := fu.Unmarshal(config.NewViper(), nil) assert.Error(t, err, "should not have been able to marshal to a nil config") - err = fu.Unmarshal(configload.NewViper(), &RemoteSamplingConfig{}) + err = fu.Unmarshal(config.NewViper(), &RemoteSamplingConfig{}) assert.Error(t, err, "should not have been able to marshal to a non-jaegerreceiver config") } diff --git a/service/application.go b/service/application.go index 8afa389785d..bba0c501e94 100644 --- a/service/application.go +++ b/service/application.go @@ -27,12 +27,11 @@ import ( "syscall" "github.com/spf13/cobra" - "github.com/spf13/viper" "go.uber.org/zap" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configcheck" - "go.opentelemetry.io/collector/config/configload" "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/config/configparser" "go.opentelemetry.io/collector/config/configtelemetry" @@ -97,27 +96,26 @@ type Parameters struct { // The ConfigFactory implementation should call AddSetFlagProperties to enable configuration passed via `--set` flag. // Viper and command instances are passed from the Application. // The factories also belong to the Application and are equal to the factories passed via Parameters. -type ConfigFactory func(v *viper.Viper, cmd *cobra.Command, factories component.Factories) (*configmodels.Config, error) +type ConfigFactory func(cmd *cobra.Command, factories component.Factories) (*configmodels.Config, error) // FileLoaderConfigFactory implements ConfigFactory and it creates configuration from file // and from --set command line flag (if the flag is present). -func FileLoaderConfigFactory(v *viper.Viper, cmd *cobra.Command, factories component.Factories) (*configmodels.Config, error) { +func FileLoaderConfigFactory(cmd *cobra.Command, factories component.Factories) (*configmodels.Config, error) { file := builder.GetConfigFile() if file == "" { return nil, errors.New("config file not specified") } - // first load the config file - v.SetConfigFile(file) - err := v.ReadInConfig() + + cp, err := config.NewParserFromFile(file) if err != nil { return nil, fmt.Errorf("error loading config file %q: %v", file, err) } // next overlay the config file with --set flags - if err := AddSetFlagProperties(v, cmd); err != nil { + if err := AddSetFlagProperties(cp, cmd); err != nil { return nil, fmt.Errorf("failed to process set flag: %v", err) } - return configparser.Load(v, factories) + return configparser.Load(cp, factories) } // New creates and returns a new instance of Application. @@ -237,7 +235,7 @@ func (app *Application) setupConfigurationComponents(ctx context.Context, factor app.logger.Info("Loading configuration...") - cfg, err := factory(configload.NewViper(), app.rootCmd, app.factories) + cfg, err := factory(app.rootCmd, app.factories) if err != nil { return fmt.Errorf("cannot load configuration: %w", err) } diff --git a/service/application_test.go b/service/application_test.go index 24664cf9ac8..98e5771b013 100644 --- a/service/application_test.go +++ b/service/application_test.go @@ -30,14 +30,13 @@ import ( "github.com/prometheus/common/expfmt" "github.com/spf13/cobra" - "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configload" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/config/configparser" "go.opentelemetry.io/collector/processor/attributesprocessor" @@ -145,7 +144,7 @@ func TestApplication_StartAsGoRoutine(t *testing.T) { params := Parameters{ ApplicationStartInfo: component.DefaultApplicationStartInfo(), - ConfigFactory: func(_ *viper.Viper, _ *cobra.Command, factories component.Factories) (*configmodels.Config, error) { + ConfigFactory: func(_ *cobra.Command, factories component.Factories) (*configmodels.Config, error) { return constructMimumalOpConfig(t, factories), nil }, Factories: factories, @@ -235,7 +234,7 @@ func TestSetFlag(t *testing.T) { "--set=processors.doesnotexist.timeout=2s", }) require.NoError(t, err) - cfg, err := FileLoaderConfigFactory(configload.NewViper(), app.rootCmd, factories) + cfg, err := FileLoaderConfigFactory(app.rootCmd, factories) require.Error(t, err) require.Nil(t, cfg) @@ -248,7 +247,7 @@ func TestSetFlag(t *testing.T) { "--set=processors.batch/foo.timeout=2s", }) require.NoError(t, err) - cfg, err := FileLoaderConfigFactory(configload.NewViper(), app.rootCmd, factories) + cfg, err := FileLoaderConfigFactory(app.rootCmd, factories) require.NoError(t, err) assert.NotNil(t, cfg) err = cfg.Validate() @@ -278,7 +277,7 @@ func TestSetFlag(t *testing.T) { "--set=extensions.health_check.port=8080", }) require.NoError(t, err) - cfg, err := FileLoaderConfigFactory(configload.NewViper(), app.rootCmd, factories) + cfg, err := FileLoaderConfigFactory(app.rootCmd, factories) require.NoError(t, err) require.NotNil(t, cfg) err = cfg.Validate() @@ -300,7 +299,6 @@ func TestSetFlag_component_does_not_exist(t *testing.T) { factories, err := defaultcomponents.Components() require.NoError(t, err) - v := configload.NewViper() cmd := &cobra.Command{} addSetFlag(cmd.Flags()) fs := &flag.FlagSet{} @@ -315,7 +313,7 @@ func TestSetFlag_component_does_not_exist(t *testing.T) { "--set=processors.attributes.actions.value=bar", "--set=receivers.jaeger.protocols.grpc.endpoint=localhost:12345", }) - cfg, err := FileLoaderConfigFactory(v, cmd, factories) + cfg, err := FileLoaderConfigFactory(cmd, factories) require.NoError(t, err) require.NotNil(t, cfg) } @@ -341,10 +339,10 @@ service: processors: [batch] exporters: [logging] ` - v := configload.NewViper() + v := config.NewViper() v.SetConfigType("yaml") v.ReadConfig(strings.NewReader(configStr)) - cfg, err := configparser.Load(v, factories) + cfg, err := configparser.Load(config.ParserFromViper(v), factories) assert.NoError(t, err) err = cfg.Validate() assert.NoError(t, err) diff --git a/service/set_flag.go b/service/set_flag.go index c6cc3efd22d..19610e64007 100644 --- a/service/set_flag.go +++ b/service/set_flag.go @@ -21,9 +21,8 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/spf13/viper" - "go.opentelemetry.io/collector/config/configload" + "go.opentelemetry.io/collector/config" ) const ( @@ -38,7 +37,7 @@ func addSetFlag(flagSet *pflag.FlagSet) { // AddSetFlagProperties overrides properties from set flag(s) in supplied viper instance. // The implementation reads set flag(s) from the cmd and passes the content to a new viper instance as .properties file. // Then the properties from new viper instance are read and set to the supplied viper. -func AddSetFlagProperties(v *viper.Viper, cmd *cobra.Command) error { +func AddSetFlagProperties(cp *config.Parser, cmd *cobra.Command) error { flagProperties, err := cmd.Flags().GetStringArray(setFlagName) if err != nil { return err @@ -53,7 +52,7 @@ func AddSetFlagProperties(v *viper.Viper, cmd *cobra.Command) error { return err } } - viperFlags := configload.NewViper() + viperFlags := config.NewViper() viperFlags.SetConfigType(setFlagFileType) if err := viperFlags.ReadConfig(b); err != nil { return fmt.Errorf("failed to read set flag config: %v", err) @@ -78,20 +77,20 @@ func AddSetFlagProperties(v *viper.Viper, cmd *cobra.Command) error { rootKeys := map[string]struct{}{} for _, k := range viperFlags.AllKeys() { - keys := strings.Split(k, configload.KeyDelimiter) + keys := strings.Split(k, config.KeyDelimiter) if len(keys) > 0 { rootKeys[keys[0]] = struct{}{} } } for k := range rootKeys { - v.Set(k, v.Get(k)) + cp.Set(k, cp.Get(k)) } // now that we've copied the config into the viper "overrides" copy the --set flags // as well for _, k := range viperFlags.AllKeys() { - v.Set(k, viperFlags.Get(k)) + cp.Set(k, viperFlags.Get(k)) } return nil diff --git a/service/set_flag_test.go b/service/set_flag_test.go index f6f0c63ffcc..e8d05d7c51f 100644 --- a/service/set_flag_test.go +++ b/service/set_flag_test.go @@ -18,9 +18,10 @@ import ( "testing" "github.com/spf13/cobra" - "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "go.opentelemetry.io/collector/config" ) func TestSetFlags(t *testing.T) { @@ -35,30 +36,28 @@ func TestSetFlags(t *testing.T) { }) require.NoError(t, err) - v := viper.New() - err = AddSetFlagProperties(v, cmd) + cp := config.NewParser() + err = AddSetFlagProperties(cp, cmd) require.NoError(t, err) - settings := v.AllSettings() - assert.Equal(t, 4, len(settings)) - assert.Equal(t, "2s", v.Get("processors::batch::timeout")) - assert.Equal(t, "3s", v.Get("processors::batch/foo::timeout")) - assert.Equal(t, "foo:9200,foo2:9200", v.Get("exporters::kafka::brokers")) - assert.Equal(t, "localhost:1818", v.Get("receivers::otlp::protocols::grpc::endpoint")) + keys := cp.AllKeys() + assert.Len(t, keys, 4) + assert.Equal(t, "2s", cp.Get("processors::batch::timeout")) + assert.Equal(t, "3s", cp.Get("processors::batch/foo::timeout")) + assert.Equal(t, "foo:9200,foo2:9200", cp.Get("exporters::kafka::brokers")) + assert.Equal(t, "localhost:1818", cp.Get("receivers::otlp::protocols::grpc::endpoint")) } func TestSetFlags_err_set_flag(t *testing.T) { cmd := &cobra.Command{} - v := viper.New() - err := AddSetFlagProperties(v, cmd) - require.Error(t, err) + require.Error(t, AddSetFlagProperties(config.NewParser(), cmd)) } func TestSetFlags_empty(t *testing.T) { cmd := &cobra.Command{} addSetFlag(cmd.Flags()) - v := viper.New() - err := AddSetFlagProperties(v, cmd) + cp := config.NewParser() + err := AddSetFlagProperties(cp, cmd) require.NoError(t, err) - assert.Equal(t, 0, len(v.AllSettings())) + assert.Equal(t, 0, len(cp.AllKeys())) } diff --git a/testbed/testbed/otelcol_runner.go b/testbed/testbed/otelcol_runner.go index 17ce7248563..cdaa1c17472 100644 --- a/testbed/testbed/otelcol_runner.go +++ b/testbed/testbed/otelcol_runner.go @@ -20,12 +20,11 @@ import ( "github.com/shirou/gopsutil/process" "github.com/spf13/cobra" - "github.com/spf13/viper" "go.uber.org/zap" "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configload" + "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/config/configmodels" "go.opentelemetry.io/collector/config/configparser" "go.opentelemetry.io/collector/internal/version" @@ -83,10 +82,10 @@ func (ipp *InProcessCollector) PrepareConfig(configStr string) (configCleanup fu return configCleanup, err } ipp.logger = logger - v := configload.NewViper() + v := config.NewViper() v.SetConfigType("yaml") v.ReadConfig(strings.NewReader(configStr)) - cfg, err := configparser.Load(v, ipp.factories) + cfg, err := configparser.Load(config.ParserFromViper(v), ipp.factories) if err != nil { return configCleanup, err } @@ -106,7 +105,7 @@ func (ipp *InProcessCollector) Start(args StartParams) error { Version: version.Version, GitHash: version.GitHash, }, - ConfigFactory: func(_ *viper.Viper, _ *cobra.Command, _ component.Factories) (*configmodels.Config, error) { + ConfigFactory: func(_ *cobra.Command, _ component.Factories) (*configmodels.Config, error) { return ipp.config, nil }, Factories: ipp.factories,