Skip to content

Commit

Permalink
[otelcol] Remove all default providers/converters (#10436)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Removed deprecated `NewCommand`. Deprecates `NewCommandMustSetProvider`.
Adds `NewCommand` that requires at least one provider be set.

Removes usage of imported providers/converters from otelcol (but they
still live in otelcoltest until
#10417 is
merged.

<!-- Issue number if applicable -->
#### Link to tracking issue
closes
#10290.

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit tests
  • Loading branch information
TylerHelmuth committed Jun 28, 2024
1 parent c60fb1d commit 5309d60
Show file tree
Hide file tree
Showing 15 changed files with 249 additions and 140 deletions.
25 changes: 25 additions & 0 deletions .chloggen/otelcol-update-commane-2.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: deprecation

# 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: The `otelcol.NewCommandMustSetProvider` is deprecated. Use `otelcol.NewCommand` instead.

# 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]
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: The `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]
14 changes: 14 additions & 0 deletions cmd/builder/internal/builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "Default Configuration Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
return cfg
Expand All @@ -268,6 +270,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "LDFlags Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.LDFlags = `-X "test.gitVersion=0743dc6c6411272b98494a9b32a63378e84c34da" -X "test.gitTag=local-testing" -X "test.goVersion=go version go1.20.7 darwin/amd64"`
Expand All @@ -278,6 +282,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "Debug Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Logger = zap.NewNop()
Expand All @@ -289,6 +295,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "No providers",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Providers = &[]Module{}
Expand All @@ -299,6 +307,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "Pre-confmap factories",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Distribution.OtelColVersion = "0.98.0"
Expand All @@ -310,6 +320,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "With confmap factories",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.Distribution.OtelColVersion = "0.99.0"
Expand All @@ -321,6 +333,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "ConfResolverDefaultURIScheme set",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
require.NoError(t, err)
cfg.ConfResolver = ConfResolver{
DefaultURIScheme: "env",
}
Expand Down
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()
}
2 changes: 1 addition & 1 deletion otelcol/collector_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *windowsService) start(elog *eventlog.Log, colErrorChannel chan error) e
}

var err error
err = updateSettingsUsingFlags(&s.settings, s.flags, false)
err = updateSettingsUsingFlags(&s.settings, s.flags)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions otelcol/collector_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import (
func TestNewSvcHandler(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()
os.Args = []string{"otelcol", "--config", filepath.Join("testdata", "otelcol-nop.yaml")}
filePath := filepath.Join("testdata", "otelcol-nop.yaml")
os.Args = []string{"otelcol", "--config", filePath}

s := NewSvcHandler(CollectorSettings{BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories})
s := NewSvcHandler(CollectorSettings{BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories, ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filePath})})

colDone := make(chan struct{})
requests := make(chan svc.ChangeRequest)
Expand Down
Loading

0 comments on commit 5309d60

Please sign in to comment.