Skip to content

Commit

Permalink
Remove all default providers/converters
Browse files Browse the repository at this point in the history
  • Loading branch information
TylerHelmuth committed Jun 18, 2024
1 parent a138356 commit ce55ff2
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 93 deletions.
25 changes: 25 additions & 0 deletions .chloggen/otelcol-update-commane.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: otelcol

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: `otelcol.NewCommand` now requires at least one provider be set.

# One or more tracking issues or pull requests related to the change
issues: [10436]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
122 changes: 107 additions & 15 deletions otelcol/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package otelcol
import (
"context"
"errors"
"os"
"path/filepath"
"sync"
"syscall"
Expand All @@ -15,6 +16,8 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"gopkg.in/yaml.v3"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/confmap"
Expand All @@ -34,7 +37,7 @@ func TestCollectorStartAsGoRoutine(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
}
col, err := NewCollector(set)
require.NoError(t, err)
Expand All @@ -55,7 +58,7 @@ func TestCollectorCancelContext(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
}
col, err := NewCollector(set)
require.NoError(t, err)
Expand Down Expand Up @@ -87,10 +90,10 @@ func TestCollectorStateAfterConfigChange(t *testing.T) {
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
// this will be overwritten, but we need something to get past validation
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
})
require.NoError(t, err)
provider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}))
provider, err := NewConfigProvider(newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}))
require.NoError(t, err)
col.configProvider = &mockCfgProvider{ConfigProvider: provider, watcher: watcher}

Expand All @@ -116,7 +119,7 @@ func TestCollectorReportError(t *testing.T) {
col, err := NewCollector(CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
})
require.NoError(t, err)

Expand Down Expand Up @@ -162,7 +165,7 @@ func TestComponentStatusWatcher(t *testing.T) {
col, err := NewCollector(CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: func() (Factories, error) { return factories, nil },
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-statuswatcher.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-statuswatcher.yaml")}),
})
require.NoError(t, err)

Expand Down Expand Up @@ -225,7 +228,7 @@ func TestCollectorSendSignal(t *testing.T) {
col, err := NewCollector(CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
})
require.NoError(t, err)

Expand Down Expand Up @@ -253,7 +256,7 @@ func TestCollectorFailedShutdown(t *testing.T) {
col, err := NewCollector(CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
})
require.NoError(t, err)

Expand All @@ -278,7 +281,7 @@ func TestCollectorStartInvalidConfig(t *testing.T) {
col, err := NewCollector(CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
})
require.NoError(t, err)
assert.Error(t, col.Run(context.Background()))
Expand All @@ -294,7 +297,7 @@ func TestNewCollectorInvalidConfigProviderSettings(t *testing.T) {
}

func TestNewCollectorUseConfig(t *testing.T) {
set := newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")})
set := newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")})

col, err := NewCollector(CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Expand Down Expand Up @@ -335,7 +338,7 @@ func TestCollectorStartWithTraceContextPropagation(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", tt.file)}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", tt.file)}),
}

col, err := NewCollector(set)
Expand Down Expand Up @@ -367,7 +370,7 @@ func TestCollectorRun(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", tt.file)}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", tt.file)}),
}
col, err := NewCollector(set)
require.NoError(t, err)
Expand All @@ -385,7 +388,7 @@ func TestCollectorShutdownBeforeRun(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
}
col, err := NewCollector(set)
require.NoError(t, err)
Expand All @@ -405,7 +408,7 @@ func TestCollectorClosedStateOnStartUpError(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
}
col, err := NewCollector(set)
require.NoError(t, err)
Expand All @@ -426,7 +429,7 @@ func TestCollectorDryRun(t *testing.T) {
settings: CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
},
expectedErr: `service::pipelines::traces: references processor "invalid" which is not configured`,
},
Expand Down Expand Up @@ -492,3 +495,92 @@ func (*failureProvider) Scheme() string {
func (*failureProvider) Shutdown(context.Context) error {
return nil
}

type fakeProvider struct {
scheme string
ret func(ctx context.Context, uri string, watcher confmap.WatcherFunc) (*confmap.Retrieved, error)
logger *zap.Logger
}

func (f *fakeProvider) Retrieve(ctx context.Context, uri string, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) {
return f.ret(ctx, uri, watcher)
}

func (f *fakeProvider) Scheme() string {
return f.scheme
}

func (f *fakeProvider) Shutdown(context.Context) error {
return nil
}

func newFakeProvider(scheme string, ret func(ctx context.Context, uri string, watcher confmap.WatcherFunc) (*confmap.Retrieved, error)) confmap.ProviderFactory {
return confmap.NewProviderFactory(func(ps confmap.ProviderSettings) confmap.Provider {
return &fakeProvider{
scheme: scheme,
ret: ret,
logger: ps.Logger,
}
})
}

func newEnvProvider() confmap.ProviderFactory {
return newFakeProvider("env", func(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
// When using `env` as the default scheme for tests, the uri will not include `env:`.
// Instead of duplicating the switch cases, the scheme is added instead.
if uri[0:4] != "env:" {
uri = "env:" + uri
}
switch uri {
case "env:COMPLEX_VALUE":
return confmap.NewRetrieved([]any{"localhost:3042"})
case "env:HOST":
return confmap.NewRetrieved("localhost")
case "env:OS":
return confmap.NewRetrieved("ubuntu")
case "env:PR":
return confmap.NewRetrieved("amd")
case "env:PORT":
return confmap.NewRetrieved(3044)
case "env:INT":
return confmap.NewRetrieved(1)
case "env:INT32":
return confmap.NewRetrieved(32)
case "env:INT64":
return confmap.NewRetrieved(64)
case "env:FLOAT32":
return confmap.NewRetrieved(float32(3.25))
case "env:FLOAT64":
return confmap.NewRetrieved(float64(6.4))
case "env:BOOL":
return confmap.NewRetrieved(true)
}
return nil, errors.New("impossible")
})
}

func newDefaultConfigProviderSettings(t testing.TB, uris []string) ConfigProviderSettings {
fileProvider := newFakeProvider("file", func(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
return confmap.NewRetrieved(newConfFromFile(t, uri[5:]))
})
return ConfigProviderSettings{
ResolverSettings: confmap.ResolverSettings{
URIs: uris,
ProviderFactories: []confmap.ProviderFactory{
fileProvider,
newEnvProvider(),
},
},
}
}

// newConfFromFile creates a new Conf by reading the given file.
func newConfFromFile(t testing.TB, fileName string) map[string]any {
content, err := os.ReadFile(filepath.Clean(fileName))
require.NoErrorf(t, err, "unable to read the file %v", fileName)

var data map[string]any
require.NoError(t, yaml.Unmarshal(content, &data), "unable to parse yaml")

return confmap.NewFromStringMap(data).ToStringMap()
}
41 changes: 16 additions & 25 deletions otelcol/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,15 @@ import (
// Any URIs specified in CollectorSettings.ConfigProviderSettings.ResolverSettings.URIs
// are considered defaults and will be overwritten by config flags passed as
// command-line arguments to the executable.
//
// Deprecated: [v0.103.0] use NewCommandMustSetProvider instead
// At least one Provider must be set.
func NewCommand(set CollectorSettings) *cobra.Command {
return commandHelper(set, false)
}

// NewCommandMustSetProvider constructs a new cobra.Command using the given CollectorSettings.
// Any URIs specified in CollectorSettings.ConfigProviderSettings.ResolverSettings.URIs
// are considered defaults and will be overwritten by config flags passed as
// command-line arguments to the executable.
// At least one Provider must be supplied via CollectorSettings.ConfigProviderSettings.ResolverSettings.ProviderFactories.
func NewCommandMustSetProvider(set CollectorSettings) *cobra.Command {
return commandHelper(set, true)
}

func commandHelper(set CollectorSettings, enforceProviders bool) *cobra.Command {
flagSet := flags(featuregate.GlobalRegistry())
rootCmd := &cobra.Command{
Use: set.BuildInfo.Command,
Version: set.BuildInfo.Version,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, _ []string) error {
err := updateSettingsUsingFlags(&set, flagSet, enforceProviders)
err := updateSettingsUsingFlags(&set, flagSet)
if err != nil {
return err
}
Expand All @@ -52,13 +38,24 @@ func commandHelper(set CollectorSettings, enforceProviders bool) *cobra.Command
},
}
rootCmd.AddCommand(newComponentsCommand(set))
rootCmd.AddCommand(newValidateSubCommand(set, flagSet, enforceProviders))
rootCmd.AddCommand(newValidateSubCommand(set, flagSet))
rootCmd.Flags().AddGoFlagSet(flagSet)
return rootCmd
}

// NewCommandMustSetProvider constructs a new cobra.Command using the given CollectorSettings.
// Any URIs specified in CollectorSettings.ConfigProviderSettings.ResolverSettings.URIs
// are considered defaults and will be overwritten by config flags passed as
// command-line arguments to the executable.
// At least one Provider must be supplied via CollectorSettings.ConfigProviderSettings.ResolverSettings.ProviderFactories.
//
// Deprecated: [v0.104.0] use NewCommand instead
func NewCommandMustSetProvider(set CollectorSettings) *cobra.Command {
return NewCommand(set)
}

// Puts command line flags from flags into the CollectorSettings, to be used during config resolution.
func updateSettingsUsingFlags(set *CollectorSettings, flags *flag.FlagSet, enforceProviders bool) error {
func updateSettingsUsingFlags(set *CollectorSettings, flags *flag.FlagSet) error {
resolverSet := &set.ConfigProviderSettings.ResolverSettings
configFlags := getConfigFlag(flags)

Expand All @@ -73,14 +70,8 @@ func updateSettingsUsingFlags(set *CollectorSettings, flags *flag.FlagSet, enfor
set.ConfigProviderSettings.ResolverSettings.DefaultScheme = "env"
}

// Provide a default set of providers and converters if none have been specified.
// TODO: Remove this after CollectorSettings.ConfigProvider is removed and instead
// do it in the builder.
if len(resolverSet.ProviderFactories) == 0 && len(resolverSet.ConverterFactories) == 0 {
if enforceProviders {
return errors.New("at least one Provider must be supplied")
}
set.ConfigProviderSettings = newDefaultConfigProviderSettings(resolverSet.URIs)
return errors.New("at least one Provider must be supplied")
}
return nil
}
4 changes: 2 additions & 2 deletions otelcol/command_components_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ func TestNewBuildSubCommand(t *testing.T) {
set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: nopFactories,
ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}),
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}),
}
cmd := NewCommandMustSetProvider(set)
cmd := NewCommand(set)
cmd.SetArgs([]string{"components"})

ExpectedOutput, err := os.ReadFile(filepath.Join("testdata", "components-output.yaml"))
Expand Down
Loading

0 comments on commit ce55ff2

Please sign in to comment.