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.

Sign up for GitHub

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
Merged

[otelcol] Remove all default providers/converters #10436

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ce55ff2
Remove all default providers/converters
TylerHelmuth Jun 18, 2024
b74c7a0
cleanup
TylerHelmuth Jun 20, 2024
761374e
Fix windows command
TylerHelmuth Jun 20, 2024
e17dc92
make crosslink
TylerHelmuth Jun 20, 2024
7d34bc6
remove more uses of providers
TylerHelmuth Jun 20, 2024
cc34643
Fix buildr tests
TylerHelmuth Jun 20, 2024
c814faa
fix lint
TylerHelmuth Jun 20, 2024
6cc7ac2
fix lint
TylerHelmuth Jun 20, 2024
84a8916
Fix window test
TylerHelmuth Jun 21, 2024
e03cfb8
Fix lint
TylerHelmuth Jun 21, 2024
30d443a
Fix windows test
TylerHelmuth Jun 21, 2024
1883c2f
Merge remote-tracking branch 'upstream/main' into otelcol-update-commane
TylerHelmuth Jun 21, 2024
acb7362
Update another test that was still using imported providers
TylerHelmuth Jun 21, 2024
97b529d
Fix lint
TylerHelmuth Jun 21, 2024
b0ff6b4
apply feedback
TylerHelmuth Jun 24, 2024
d32d22d
merge in upstream/main
TylerHelmuth Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/otelcol-update-commane.yaml
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ func TestGenerateAndCompile(t *testing.T) {
testCase: "Default Configuration Compilation",
cfgBuilder: func(t *testing.T) Config {
cfg := newTestConfig()
err := cfg.SetBackwardsCompatibility()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests all pass in a list of providers, and therefore depend on those providers in the go mod list. The builder templates only add them when .Distribution.SupportsConfmapFactories is true. That value was not being set so all these tests should have been failing. They were saved instead by the fact that otelcol depended on the providers. The changes in this PR remove those dependencies when otelcoltest is not being used, so these tests started failing. The fix was to make sure .Distribution.SupportsConfmapFactories is true, which is included in cfg.SetBackwardsCompatibility().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thanks for the explanation.

require.NoError(t, err)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
return cfg
Expand All @@ -267,6 +269,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 @@ -277,6 +281,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 @@ -288,6 +294,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 @@ -298,6 +306,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 @@ -309,6 +319,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 @@ -320,6 +332,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
View file
Open in desktop
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
View file
Open in desktop
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
Loading
Loading