Skip to content

Commit

Permalink
Remove ConfigProviderOptions and NewDefaultConfigProvider
Browse files Browse the repository at this point in the history
  • Loading branch information
noisersup committed Feb 21, 2022
1 parent 0c8c405 commit e55d31a
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 54 deletions.
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@

- Deprecate `pdata.NumberDataPoint.Type()` and `pdata.Exemplar.Type()` in favor of `NumberDataPoint.ValueType()` and
`Exemplar.ValueType()` (#4850)
- Deprecate `service.MustNewConfigProvider` and `service.MustNewDefaultConfigProvider`in favor of `service.NewConfigProvider` and
`service.NewDefaultConfigProvider` (#4762)
- Deprecate `service.MustNewConfigProvider` and `service.MustNewDefaultConfigProvider`in favor of `service.NewConfigProvider` (#4762)

## v0.44.0 Beta

Expand Down
18 changes: 13 additions & 5 deletions service/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import (
"go.uber.org/zap/zapcore"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configmapprovider"
"go.opentelemetry.io/collector/internal/testcomponents"
"go.opentelemetry.io/collector/internal/testutil"
)
Expand All @@ -51,7 +53,7 @@ func TestCollector_StartAsGoRoutine(t *testing.T) {
factories, err := testcomponents.NewDefaultFactories()
require.NoError(t, err)

configProvider, err := NewDefaultConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")}, nil)
configProvider, err := NewConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")})
require.NoError(t, err)

set := CollectorSettings{
Expand Down Expand Up @@ -96,9 +98,15 @@ func TestCollector_Start(t *testing.T) {
}

metricsPort := testutil.GetAvailablePort(t)
configProvider, err := NewDefaultConfigProvider(
[]string{filepath.Join("testdata", "otelcol-config.yaml")},
[]string{"service.telemetry.metrics.address=localhost:" + strconv.FormatUint(uint64(metricsPort), 10)})

cfgMapConverter := []config.MapConverterFunc{
configmapprovider.NewOverwritePropertiesConverter(
[]string{"service.telemetry.metrics.address=localhost:" + strconv.FormatUint(uint64(metricsPort), 10)},
),
}

configProvider, err := NewConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")},
WithCfgMapConverters(cfgMapConverter))

require.NoError(t, err)

Expand Down Expand Up @@ -179,7 +187,7 @@ func TestCollector_ReportError(t *testing.T) {
factories, err := testcomponents.NewDefaultFactories()
require.NoError(t, err)

configProvider, err := NewDefaultConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")}, nil)
configProvider, err := NewConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")})
require.NoError(t, err)

col, err := New(CollectorSettings{
Expand Down
7 changes: 6 additions & 1 deletion service/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package service // import "go.opentelemetry.io/collector/service"
import (
"github.com/spf13/cobra"

"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configmapprovider"
"go.opentelemetry.io/collector/service/featuregate"
)

Expand All @@ -30,7 +32,10 @@ func NewCommand(set CollectorSettings) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
featuregate.Apply(featuregate.GetFlags())
if set.ConfigProvider == nil {
configProvider, err := NewDefaultConfigProvider(getConfigFlag(), getSetFlag())
cfgMapConverter := []config.MapConverterFunc{
configmapprovider.NewOverwritePropertiesConverter(getSetFlag()),
}
configProvider, err := NewConfigProvider(getConfigFlag(), WithCfgMapConverters(cfgMapConverter))
if err != nil {
return err
}
Expand Down
77 changes: 32 additions & 45 deletions service/config_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,32 +75,24 @@ type configProvider struct {
watcher chan error
}

// ConfigProviderOptions has options that change the behavior of ConfigProvider
// returned by NewConfigProvider()
type ConfigProviderOptions struct {
configMapProviders map[string]configmapprovider.Provider
cfgMapConverters []config.MapConverterFunc
configUnmarshaler configunmarshaler.ConfigUnmarshaler
}

// ConfigProviderOption is an option to change the behavior of ConfigProvider
// returned by NewConfigProvider()
type ConfigProviderOption func(opts *ConfigProviderOptions)
type ConfigProviderOption func(opts *configProvider)

func WithConfigMapProviders(c map[string]configmapprovider.Provider) ConfigProviderOption {
return func(opts *ConfigProviderOptions) {
return func(opts *configProvider) {
opts.configMapProviders = c
}
}

func WithCfgMapConverters(c []config.MapConverterFunc) ConfigProviderOption {
return func(opts *ConfigProviderOptions) {
return func(opts *configProvider) {
opts.cfgMapConverters = c
}
}

func WithConfigUnmarshaler(c configunmarshaler.ConfigUnmarshaler) ConfigProviderOption {
return func(opts *ConfigProviderOptions) {
return func(opts *configProvider) {
opts.configUnmarshaler = c
}
}
Expand Down Expand Up @@ -134,13 +126,14 @@ func MustNewConfigProvider(
// The `configMapProviders` is a map of pairs <scheme,Provider>.
//
// Notice: This API is experimental.
func NewConfigProvider(
locations []string,
opts ...ConfigProviderOption) (ConfigProvider, error) {

configOpts := &ConfigProviderOptions{}
for _, o := range opts {
o(configOpts)
func NewConfigProvider(locations []string, opts ...ConfigProviderOption) (ConfigProvider, error) {
// Set default ConfigProvider values
configMapProvider := map[string]configmapprovider.Provider{
"file": configmapprovider.NewFile(),
"env": configmapprovider.NewEnv(),
}
cfgMapConverter := []config.MapConverterFunc{
configmapprovider.NewExpandConverter(),
}

if len(locations) == 0 {
Expand All @@ -149,43 +142,37 @@ func NewConfigProvider(
// Safe copy, ensures the slice cannot be changed from the caller.
locationsCopy := make([]string, len(locations))
copy(locationsCopy, locations)
return &configProvider{

provider := configProvider{
locations: locationsCopy,
configMapProviders: configOpts.configMapProviders,
cfgMapConverters: configOpts.cfgMapConverters,
configUnmarshaler: configOpts.configUnmarshaler,
configMapProviders: configMapProvider,
cfgMapConverters: cfgMapConverter,
configUnmarshaler: configunmarshaler.NewDefault(),
watcher: make(chan error, 1),
}, nil
}
}

// Deprecated: use NewDefaultConfigProvider instead
// MustNewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
// defined by the given configFile and overwrites fields using properties.
func MustNewDefaultConfigProvider(configLocations []string, properties []string) ConfigProvider {
configProvider, err := NewDefaultConfigProvider(configLocations, properties)
if err != nil {
panic(err)
// Override default values with user options
for _, o := range opts {
o(&provider)
}
return configProvider

return &provider, nil
}

// NewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
// Deprecated: use NewConfigProvider instead
// MustNewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
// defined by the given configFile and overwrites fields using properties.
func NewDefaultConfigProvider(configLocations []string, properties []string) (ConfigProvider, error) {
configMapProvider := map[string]configmapprovider.Provider{
"file": configmapprovider.NewFile(),
"env": configmapprovider.NewEnv(),
}
func MustNewDefaultConfigProvider(configLocations []string, properties []string) ConfigProvider {
cfgMapConverter := []config.MapConverterFunc{
configmapprovider.NewOverwritePropertiesConverter(properties),
configmapprovider.NewExpandConverter(),
}
return NewConfigProvider(
configLocations,
WithCfgMapConverters(cfgMapConverter),
WithConfigUnmarshaler(configunmarshaler.NewDefault()),
WithConfigMapProviders(configMapProvider),
)

configProvider, err := NewConfigProvider(configLocations, WithCfgMapConverters(cfgMapConverter))
if err != nil {
panic(err)
}
return configProvider
}

func (cm *configProvider) Get(ctx context.Context, factories component.Factories) (*config.Config, error) {
Expand Down
2 changes: 1 addition & 1 deletion service/config_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ func TestBackwardsCompatibilityForFilePath(t *testing.T) {
},
}
for _, tt := range tests {
provider, err := NewDefaultConfigProvider([]string{tt.location}, []string{})
provider, err := NewConfigProvider([]string{tt.location})
assert.NoError(t, err)
_, err = provider.Get(context.Background(), factories)
assert.Contains(t, err.Error(), tt.errMessage, tt.name)
Expand Down

0 comments on commit e55d31a

Please sign in to comment.