Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[otelcol] Remove all default providers/converters #10436

Merged
merged 16 commits into from
Jun 28, 2024
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
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
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 {
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
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
Loading