diff --git a/.chloggen/otelcol-telemetryfactory.yaml b/.chloggen/otelcol-telemetryfactory.yaml new file mode 100644 index 00000000000..2d97c5d5cd0 --- /dev/null +++ b/.chloggen/otelcol-telemetryfactory.yaml @@ -0,0 +1,28 @@ +# 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: pkg/otelcol + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Require a telemetry factory to be injected through otelcol.Factories + +# One or more tracking issues or pull requests related to the change +issues: [4970] + +# (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: | + otelcol.Factories now has a required Telemetry field, + which contains the telemetry factory to be used by the service. + Set it to otelconftelemetry.NewFactory() for the existing behavior. + +# 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] diff --git a/cmd/builder/internal/builder/templates/components.go.tmpl b/cmd/builder/internal/builder/templates/components.go.tmpl index b8bdc46d8f9..d9722a9cb5d 100644 --- a/cmd/builder/internal/builder/templates/components.go.tmpl +++ b/cmd/builder/internal/builder/templates/components.go.tmpl @@ -10,6 +10,7 @@ import ( "go.opentelemetry.io/collector/otelcol" "go.opentelemetry.io/collector/processor" "go.opentelemetry.io/collector/receiver" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" {{- range .Connectors}} {{.Name}} "{{.Import}}" {{- end}} @@ -29,7 +30,9 @@ import ( func components() (otelcol.Factories, error) { var err error - factories := otelcol.Factories{} + factories := otelcol.Factories{ + Telemetry: otelconftelemetry.NewFactory(), + } factories.Extensions, err = otelcol.MakeFactoryMap[extension.Factory]( {{- range .Extensions}} diff --git a/cmd/otelcorecol/components.go b/cmd/otelcorecol/components.go index 284e5c8c0d2..b0150526bfd 100644 --- a/cmd/otelcorecol/components.go +++ b/cmd/otelcorecol/components.go @@ -21,11 +21,14 @@ import ( "go.opentelemetry.io/collector/receiver" nopreceiver "go.opentelemetry.io/collector/receiver/nopreceiver" otlpreceiver "go.opentelemetry.io/collector/receiver/otlpreceiver" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) func components() (otelcol.Factories, error) { var err error - factories := otelcol.Factories{} + factories := otelcol.Factories{ + Telemetry: otelconftelemetry.NewFactory(), + } factories.Extensions, err = otelcol.MakeFactoryMap[extension.Factory]( memorylimiterextension.NewFactory(), diff --git a/cmd/otelcorecol/go.mod b/cmd/otelcorecol/go.mod index c1ff0c57621..c5b6e270db2 100644 --- a/cmd/otelcorecol/go.mod +++ b/cmd/otelcorecol/go.mod @@ -31,6 +31,7 @@ require ( go.opentelemetry.io/collector/receiver v1.43.0 go.opentelemetry.io/collector/receiver/nopreceiver v0.137.0 go.opentelemetry.io/collector/receiver/otlpreceiver v0.137.0 + go.opentelemetry.io/collector/service v0.137.0 golang.org/x/sys v0.36.0 ) @@ -135,7 +136,6 @@ require ( go.opentelemetry.io/collector/receiver/receiverhelper v0.137.0 // indirect go.opentelemetry.io/collector/receiver/receivertest v0.137.0 // indirect go.opentelemetry.io/collector/receiver/xreceiver v0.137.0 // indirect - go.opentelemetry.io/collector/service v0.137.0 // indirect go.opentelemetry.io/collector/service/hostcapabilities v0.137.0 // indirect go.opentelemetry.io/contrib/bridges/otelzap v0.13.0 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0 // indirect diff --git a/internal/e2e/metric_stability_test.go b/internal/e2e/metric_stability_test.go index caa58ac01bd..d598ae01182 100644 --- a/internal/e2e/metric_stability_test.go +++ b/internal/e2e/metric_stability_test.go @@ -36,6 +36,7 @@ import ( "go.opentelemetry.io/collector/processor/batchprocessor" "go.opentelemetry.io/collector/receiver" "go.opentelemetry.io/collector/receiver/otlpreceiver" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) func assertMetrics(t *testing.T, metricsAddr string, expectedMetrics map[string]bool) bool { @@ -204,6 +205,7 @@ func testMetricStability(t *testing.T, configFile string, expectedMetrics map[st Receivers: map[component.Type]receiver.Factory{otlpreceiver.NewFactory().Type(): otlpreceiver.NewFactory()}, Processors: map[component.Type]processor.Factory{batchprocessor.NewFactory().Type(): batchprocessor.NewFactory()}, Exporters: map[component.Type]exporter.Factory{debugexporter.NewFactory().Type(): debugexporter.NewFactory()}, + Telemetry: otelconftelemetry.NewFactory(), }, nil }, ConfigProviderSettings: otelcol.ConfigProviderSettings{ diff --git a/otelcol/collector.go b/otelcol/collector.go index c6783472041..eb202ecab7b 100644 --- a/otelcol/collector.go +++ b/otelcol/collector.go @@ -54,7 +54,8 @@ func (s State) String() string { // CollectorSettings holds configuration for creating a new Collector. type CollectorSettings struct { - // Factories service factories. + // Factories returns component factories for the collector. + // // TODO(13263) This is a dangerous "bare" function value, should define an interface // following style guidelines. Factories func() (Factories, error) @@ -178,6 +179,10 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error { if err != nil { return fmt.Errorf("failed to initialize factories: %w", err) } + if factories.Telemetry == nil { + factories.Telemetry = otelconftelemetry.NewFactory() + } + cfg, err := col.configProvider.Get(ctx, factories) if err != nil { return fmt.Errorf("failed to get config: %w", err) @@ -219,10 +224,7 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error { }, AsyncErrorChannel: col.asyncErrorChannel, LoggingOptions: col.set.LoggingOptions, - - // TODO: inject the telemetry factory through factories. - // See https://github.com/open-telemetry/opentelemetry-collector/issues/4970 - TelemetryFactory: otelconftelemetry.NewFactory(), + TelemetryFactory: factories.Telemetry, }, cfg.Service) if err != nil { return err @@ -272,6 +274,10 @@ func (col *Collector) DryRun(ctx context.Context) error { if err != nil { return fmt.Errorf("failed to initialize factories: %w", err) } + if factories.Telemetry == nil { + factories.Telemetry = otelconftelemetry.NewFactory() + } + cfg, err := col.configProvider.Get(ctx, factories) if err != nil { return fmt.Errorf("failed to get config: %w", err) @@ -291,6 +297,7 @@ func (col *Collector) DryRun(ctx context.Context) error { ExportersFactories: factories.Exporters, ConnectorsConfigs: cfg.Connectors, ConnectorsFactories: factories.Connectors, + TelemetryFactory: factories.Telemetry, }, service.Config{ Pipelines: cfg.Service.Pipelines, }) diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index 30379887391..b8316fc8c2d 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -7,8 +7,6 @@ package otelcol import ( "context" "errors" - "net/http" - "net/http/httptest" "os" "path/filepath" "sync" @@ -26,6 +24,8 @@ import ( "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/processor/processortest" + "go.opentelemetry.io/collector/service/telemetry" + "go.opentelemetry.io/collector/service/telemetry/telemetrytest" ) func TestStateString(t *testing.T) { @@ -79,47 +79,33 @@ func TestCollectorCancelContext(t *testing.T) { } func TestCollectorStateAfterConfigChange(t *testing.T) { - metricsPushRequests := make(chan chan struct{}) - srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + var watcher confmap.WatcherFunc + fileProvider := newFakeProvider("file", func(_ context.Context, uri string, w confmap.WatcherFunc) (*confmap.Retrieved, error) { + watcher = w + conf := newConfFromFile(t, uri[5:]) + return confmap.NewRetrieved(conf) + }) + + shutdownRequests := make(chan chan struct{}) + shutdown := func(ctx context.Context) error { unblock := make(chan struct{}) select { - case <-r.Context().Done(): - case metricsPushRequests <- unblock: + case <-ctx.Done(): + case shutdownRequests <- unblock: select { case <-unblock: - case <-r.Context().Done(): + case <-ctx.Done(): } } - })) - defer srv.Close() + return nil + } + factories, err := nopFactories() + require.NoError(t, err) + factories.Telemetry = telemetry.NewFactory( + func() component.Config { return fakeTelemetryConfig{} }, + telemetrytest.WithLogger(zap.NewNop(), shutdown), + ) - var watcher confmap.WatcherFunc - fileProvider := newFakeProvider("file", func(_ context.Context, uri string, w confmap.WatcherFunc) (*confmap.Retrieved, error) { - watcher = w - conf := newConfFromFile(t, uri[5:]) - conf["service"].(map[string]any)["telemetry"] = map[string]any{ - "metrics": map[string]any{ - "level": "basic", - "readers": []any{ - map[string]any{ - // Use a periodic metric reader with a long period, - // so we should only see an export on shutdown. - "periodic": map[string]any{ - "interval": 30000, // 30s - "exporter": map[string]any{ - "otlp": map[string]any{ - "endpoint": srv.URL, - "insecure": true, - "protocol": "http/protobuf", - }, - }, - }, - }, - }, - }, - } - return confmap.NewRetrieved(conf) - }) set := ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{filepath.Join("testdata", "otelcol-nop.yaml")}, @@ -130,7 +116,7 @@ func TestCollectorStateAfterConfigChange(t *testing.T) { } col, err := NewCollector(CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), - Factories: nopFactories, + Factories: func() (Factories, error) { return factories, nil }, ConfigProviderSettings: set, }) require.NoError(t, err) @@ -145,7 +131,7 @@ func TestCollectorStateAfterConfigChange(t *testing.T) { // push to the OTLP endpoint. We block the request to check // the state of the collector during the config change event. watcher(&confmap.ChangeEvent{}) - unblock := <-metricsPushRequests + unblock := <-shutdownRequests assert.Equal(t, StateClosing, col.GetState()) close(unblock) assert.Eventually(t, func() bool { @@ -156,13 +142,13 @@ func TestCollectorStateAfterConfigChange(t *testing.T) { // config change to make sure the internal service shutdown // does not influence collector shutdown. watcher(&confmap.ChangeEvent{}) - unblock = <-metricsPushRequests + unblock = <-shutdownRequests assert.Equal(t, StateClosing, col.GetState()) col.Shutdown() close(unblock) // After the config reload, the final shutdown should occur. - close(<-metricsPushRequests) + close(<-shutdownRequests) wg.Wait() assert.Equal(t, StateClosed, col.GetState()) } @@ -402,55 +388,34 @@ func TestNewCollectorValidatesResolverSettings(t *testing.T) { require.Error(t, err) } -func TestCollectorStartWithTraceContextPropagation(t *testing.T) { - tests := []struct { - file string - errExpected bool - }{ - {file: "otelcol-invalidprop.yaml", errExpected: true}, - {file: "otelcol-nop.yaml", errExpected: false}, - {file: "otelcol-validprop.yaml", errExpected: false}, - } - - for _, tt := range tests { - t.Run(tt.file, func(t *testing.T) { - set := CollectorSettings{ - BuildInfo: component.NewDefaultBuildInfo(), - Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", tt.file)}), - } - - col, err := NewCollector(set) - require.NoError(t, err) - - if tt.errExpected { - require.Error(t, col.Run(context.Background())) - assert.Equal(t, StateClosed, col.GetState()) - } else { - wg := startCollector(context.Background(), t, col) - col.Shutdown() - wg.Wait() - assert.Equal(t, StateClosed, col.GetState()) - } - }) - } -} - func TestCollectorRun(t *testing.T) { - tests := []struct { - file string + tests := map[string]struct { + factories func() (Factories, error) + configFile string }{ - {file: "otelcol-noreaders.yaml"}, - {file: "otelcol-emptyreaders.yaml"}, - {file: "otelcol-multipleheaders.yaml"}, + "nop": { + factories: nopFactories, + configFile: "otelcol-nop.yaml", + }, + "defaul_telemetry_factory": { + factories: func() (Factories, error) { + factories, err := nopFactories() + if err != nil { + return Factories{}, err + } + factories.Telemetry = nil + return factories, nil + }, + configFile: "otelcol-otelconftelemetry.yaml", + }, } - for _, tt := range tests { - t.Run(tt.file, func(t *testing.T) { + for name, test := range tests { + t.Run(name, func(t *testing.T) { set := CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), - Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", tt.file)}), + Factories: test.factories, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", test.configFile)}), } col, err := NewCollector(set) require.NoError(t, err) @@ -464,7 +429,7 @@ func TestCollectorRun(t *testing.T) { } } -func TestCollectorShutdownBeforeRun(t *testing.T) { +func TestCollectorRun_AfterShutdown(t *testing.T) { set := CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories, @@ -483,21 +448,50 @@ func TestCollectorShutdownBeforeRun(t *testing.T) { assert.Equal(t, StateClosed, col.GetState()) } -func TestCollectorClosedStateOnStartUpError(t *testing.T) { - // Load a bad config causing startup to fail - set := CollectorSettings{ - BuildInfo: component.NewDefaultBuildInfo(), - Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}), +func TestCollectorRun_Errors(t *testing.T) { + tests := map[string]struct { + settings CollectorSettings + expectedErr string + }{ + "factories_error": { + settings: CollectorSettings{ + Factories: func() (Factories, error) { + return Factories{}, errors.New("no factories for you") + }, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}), + }, + expectedErr: "failed to initialize factories: no factories for you", + }, + "invalid_processor": { + settings: CollectorSettings{ + Factories: nopFactories, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}), + }, + expectedErr: `invalid configuration: service::pipelines::traces: references processor "invalid" which is not configured`, + }, + "invalid_telemetry_config": { + settings: CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: nopFactories, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid-telemetry.yaml")}), + }, + expectedErr: "failed to get config: cannot unmarshal the configuration: decoding failed due to the following error(s):\n\n'service.telemetry' has invalid keys: unknown", + }, } - col, err := NewCollector(set) - require.NoError(t, err) - // Expect run to error - require.Error(t, col.Run(context.Background())) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + col, err := NewCollector(test.settings) + require.NoError(t, err) - // Expect state to be closed - assert.Equal(t, StateClosed, col.GetState()) + // Expect run to error + err = col.Run(context.Background()) + require.EqualError(t, err, test.expectedErr) + + // Expect state to be closed + assert.Equal(t, StateClosed, col.GetState()) + }) + } } func TestCollectorDryRun(t *testing.T) { @@ -505,6 +499,15 @@ func TestCollectorDryRun(t *testing.T) { settings CollectorSettings expectedErr string }{ + "factories_error": { + settings: CollectorSettings{ + Factories: func() (Factories, error) { + return Factories{}, errors.New("no factories for you") + }, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}), + }, + expectedErr: "failed to initialize factories: no factories for you", + }, "invalid_processor": { settings: CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), @@ -537,6 +540,28 @@ func TestCollectorDryRun(t *testing.T) { }, expectedErr: `failed to build pipelines: cycle detected: connector "nop/forward" (traces to traces) -> connector "nop/forward" (traces to traces)`, }, + "invalid_telemetry_config": { + settings: CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: nopFactories, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid-telemetry.yaml")}), + }, + expectedErr: "failed to get config: cannot unmarshal the configuration: decoding failed due to the following error(s):\n\n'service.telemetry' has invalid keys: unknown", + }, + "default_telemetry_factory": { + settings: CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: func() (Factories, error) { + factories, err := nopFactories() + if err != nil { + return Factories{}, err + } + factories.Telemetry = nil + return factories, nil + }, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-otelconftelemetry.yaml")}), + }, + }, } for name, test := range tests { diff --git a/otelcol/collector_windows.go b/otelcol/collector_windows.go index f8fee8d99d6..ec8aaa3d75b 100644 --- a/otelcol/collector_windows.go +++ b/otelcol/collector_windows.go @@ -215,6 +215,9 @@ func (w windowsEventLogCore) Sync() error { func withWindowsCore(elog *eventlog.Log, serviceConfig **service.Config) func(zapcore.Core) zapcore.Core { return func(core zapcore.Core) zapcore.Core { if serviceConfig != nil && *serviceConfig != nil { + // TODO remove the dependency on otelconftelemetry + // + // https://github.com/open-telemetry/opentelemetry-collector/issues/14002 for _, output := range (*serviceConfig).Telemetry.(*otelconftelemetry.Config).Logs.OutputPaths { if output != "stdout" && output != "stderr" { // A log file was specified in the configuration, so we should not use the Windows Event Log diff --git a/otelcol/command_print_test.go b/otelcol/command_print_test.go index 8d5941b0d81..6560742e7b1 100644 --- a/otelcol/command_print_test.go +++ b/otelcol/command_print_test.go @@ -24,6 +24,7 @@ import ( "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/receiver" "go.opentelemetry.io/collector/receiver/xreceiver" + "go.opentelemetry.io/collector/service/telemetry" ) type printExporterConfig struct { @@ -202,6 +203,9 @@ func TestPrintCommand(t *testing.T) { Exporters: map[component.Type]exporter.Factory{ testE: testExporter, }, + Telemetry: telemetry.NewFactory(func() component.Config { + return fakeTelemetryConfig{} + }), }, nil }, ConfigProviderSettings: ConfigProviderSettings{ diff --git a/otelcol/config_test.go b/otelcol/config_test.go index e9c80d96ae3..7b30c1fc26c 100644 --- a/otelcol/config_test.go +++ b/otelcol/config_test.go @@ -9,17 +9,13 @@ import ( "testing" "github.com/stretchr/testify/require" - config "go.opentelemetry.io/contrib/otelconf/v0.3.0" - "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/pipeline" "go.opentelemetry.io/collector/service" "go.opentelemetry.io/collector/service/pipelines" - "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) var ( @@ -297,33 +293,7 @@ func generateConfig() *Config { component.MustNewID("nop"): &errConfig{}, }, Service: service.Config{ - Telemetry: otelconftelemetry.Config{ - Logs: otelconftelemetry.LogsConfig{ - Level: zapcore.DebugLevel, - Development: true, - Encoding: "console", - DisableCaller: true, - DisableStacktrace: true, - OutputPaths: []string{"stderr", "./output-logs"}, - ErrorOutputPaths: []string{"stderr", "./error-output-logs"}, - InitialFields: map[string]any{"fieldKey": "filed-value"}, - }, - Metrics: otelconftelemetry.MetricsConfig{ - Level: configtelemetry.LevelNormal, - MeterProvider: config.MeterProvider{ - Readers: []config.MetricReader{ - { - Pull: &config.PullMetricReader{Exporter: config.PullMetricExporter{ - Prometheus: &config.Prometheus{ - Host: newPtr("localhost"), - Port: newPtr(8080), - }, - }}, - }, - }, - }, - }, - }, + Telemetry: fakeTelemetryConfig{}, Extensions: []component.ID{component.MustNewID("nop")}, Pipelines: pipelines.Config{ pipeline.NewID(pipeline.SignalTraces): { @@ -336,10 +306,6 @@ func generateConfig() *Config { } } -func newPtr[T int | string](str T) *T { - return &str -} - type fakeTelemetryConfig struct { Invalid bool `mapstructure:"invalid"` } diff --git a/otelcol/configprovider_test.go b/otelcol/configprovider_test.go index 63304ba822b..927ee204582 100644 --- a/otelcol/configprovider_test.go +++ b/otelcol/configprovider_test.go @@ -13,95 +13,101 @@ import ( "github.com/stretchr/testify/require" yaml "go.yaml.in/yaml/v3" + "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/connector/connectortest" + "go.opentelemetry.io/collector/exporter/exportertest" + "go.opentelemetry.io/collector/extension/extensiontest" + "go.opentelemetry.io/collector/pipeline" + "go.opentelemetry.io/collector/processor/processortest" + "go.opentelemetry.io/collector/receiver/receivertest" + "go.opentelemetry.io/collector/service" + "go.opentelemetry.io/collector/service/pipelines" ) -func newConfig(yamlBytes []byte, factories Factories) (*Config, error) { - stringMap := map[string]any{} - err := yaml.Unmarshal(yamlBytes, stringMap) - if err != nil { - return nil, err - } - - conf := confmap.NewFromStringMap(stringMap) - - cfg, err := unmarshal(conf, factories) - if err != nil { - return nil, err - } - - return &Config{ - Receivers: cfg.Receivers.Configs(), - Processors: cfg.Processors.Configs(), - Exporters: cfg.Exporters.Configs(), - Connectors: cfg.Connectors.Configs(), - Extensions: cfg.Extensions.Configs(), - Service: cfg.Service, - }, nil -} - -func TestConfigProviderYaml(t *testing.T) { - yamlBytes, err := os.ReadFile(filepath.Join("testdata", "otelcol-nop.yaml")) - require.NoError(t, err) - - uriLocation := "yaml:" + string(yamlBytes) - - yamlProvider := newFakeProvider("yaml", func(_ context.Context, _ string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { - var rawConf any - if yamlErr := yaml.Unmarshal(yamlBytes, &rawConf); yamlErr != nil { - return nil, yamlErr - } - return confmap.NewRetrieved(rawConf) - }) - - set := ConfigProviderSettings{ - ResolverSettings: confmap.ResolverSettings{ - URIs: []string{uriLocation}, - ProviderFactories: []confmap.ProviderFactory{yamlProvider}, +func TestConfigProvider(t *testing.T) { + nopComponentID := component.MustNewID("nop") + nopConComponentID := component.MustNewIDWithName("nop", "con") + + tests := map[string]struct { + filename string + factories func() (Factories, error) + expectedConfig *Config + }{ + "nop": { + filename: "otelcol-nop.yaml", + factories: nopFactories, + expectedConfig: &Config{ + Connectors: map[component.ID]component.Config{ + nopConComponentID: connectortest.NewNopFactory().CreateDefaultConfig(), + }, + Exporters: map[component.ID]component.Config{ + nopComponentID: exportertest.NewNopFactory().CreateDefaultConfig(), + }, + Extensions: map[component.ID]component.Config{ + nopComponentID: extensiontest.NewNopFactory().CreateDefaultConfig(), + }, + Processors: map[component.ID]component.Config{ + nopComponentID: processortest.NewNopFactory().CreateDefaultConfig(), + }, + Receivers: map[component.ID]component.Config{ + nopComponentID: receivertest.NewNopFactory().CreateDefaultConfig(), + }, + Service: service.Config{ + Telemetry: fakeTelemetryConfig{}, + Extensions: []component.ID{nopComponentID}, + Pipelines: map[pipeline.ID]*pipelines.PipelineConfig{ + pipeline.NewID(pipeline.SignalTraces): { + Receivers: []component.ID{nopComponentID}, + Processors: []component.ID{nopComponentID}, + Exporters: []component.ID{nopComponentID, nopConComponentID}, + }, + pipeline.NewID(pipeline.SignalMetrics): { + Receivers: []component.ID{nopComponentID}, + Processors: []component.ID{nopComponentID}, + Exporters: []component.ID{nopComponentID}, + }, + pipeline.NewID(pipeline.SignalLogs): { + Receivers: []component.ID{nopComponentID, nopConComponentID}, + Processors: []component.ID{nopComponentID}, + Exporters: []component.ID{nopComponentID}, + }, + }, + }, + }, }, } - cp, err := NewConfigProvider(set) - require.NoError(t, err) - - factories, err := nopFactories() - require.NoError(t, err) - - cfg, err := cp.Get(context.Background(), factories) - require.NoError(t, err) - - configNop, err := newConfig(yamlBytes, factories) - require.NoError(t, err) - - assert.Equal(t, configNop, cfg) -} - -func TestConfigProviderFile(t *testing.T) { - uriLocation := "file:" + filepath.Join("testdata", "otelcol-nop.yaml") - fileProvider := newFakeProvider("file", func(_ context.Context, _ string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { - return confmap.NewRetrieved(newConfFromFile(t, uriLocation[5:])) - }) - set := ConfigProviderSettings{ - ResolverSettings: confmap.ResolverSettings{ - URIs: []string{uriLocation}, - ProviderFactories: []confmap.ProviderFactory{fileProvider}, - }, + for name, test := range tests { + t.Run(name, func(t *testing.T) { + yamlBytes, err := os.ReadFile(filepath.Join("testdata", test.filename)) + require.NoError(t, err) + + yamlProvider := newFakeProvider("yaml", func(context.Context, string, confmap.WatcherFunc) (*confmap.Retrieved, error) { + var rawConf any + if yamlErr := yaml.Unmarshal(yamlBytes, &rawConf); yamlErr != nil { + return nil, yamlErr + } + return confmap.NewRetrieved(rawConf) + }) + + set := ConfigProviderSettings{ + ResolverSettings: confmap.ResolverSettings{ + URIs: []string{"yaml:" + string(yamlBytes)}, + ProviderFactories: []confmap.ProviderFactory{yamlProvider}, + }, + } + + cp, err := NewConfigProvider(set) + require.NoError(t, err) + + factories, err := test.factories() + require.NoError(t, err) + + cfg, err := cp.Get(context.Background(), factories) + require.NoError(t, err) + + assert.Equal(t, test.expectedConfig, cfg) + }) } - - cp, err := NewConfigProvider(set) - require.NoError(t, err) - - factories, err := nopFactories() - require.NoError(t, err) - - cfg, err := cp.Get(context.Background(), factories) - require.NoError(t, err) - - yamlBytes, err := os.ReadFile(filepath.Join("testdata", "otelcol-nop.yaml")) - require.NoError(t, err) - - configNop, err := newConfig(yamlBytes, factories) - require.NoError(t, err) - - assert.Equal(t, configNop, cfg) } diff --git a/otelcol/factories.go b/otelcol/factories.go index c080dbde584..3f8daa39089 100644 --- a/otelcol/factories.go +++ b/otelcol/factories.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/processor" "go.opentelemetry.io/collector/receiver" + "go.opentelemetry.io/collector/service/telemetry" ) // Factories struct holds in a single type all component factories that @@ -32,6 +33,13 @@ type Factories struct { // Connectors maps connector type names in the config to the respective factory. Connectors map[component.Type]connector.Factory + // Telemetry is the factory to create the telemetry providers for the service. + // + // If Telemetry is nil, otelconftelemetry will be used by default. + // TODO remove the backwards compatibility and require a non-nil factory + // https://github.com/open-telemetry/opentelemetry-collector/issues/14003 + Telemetry telemetry.Factory + // ReceiverModules maps receiver types to their respective go modules. ReceiverModules map[component.Type]string diff --git a/otelcol/factories_test.go b/otelcol/factories_test.go index 808a4e8e9d1..52a5f0299e1 100644 --- a/otelcol/factories_test.go +++ b/otelcol/factories_test.go @@ -20,6 +20,7 @@ import ( "go.opentelemetry.io/collector/processor/processortest" "go.opentelemetry.io/collector/receiver" "go.opentelemetry.io/collector/receiver/receivertest" + "go.opentelemetry.io/collector/service/telemetry" ) func nopFactories() (Factories, error) { @@ -66,6 +67,10 @@ func nopFactories() (Factories, error) { factories.ProcessorModules[proc.Type()] = "go.opentelemetry.io/collector/processor/processortest v1.2.3" } + factories.Telemetry = telemetry.NewFactory(func() component.Config { + return fakeTelemetryConfig{} + }) + return factories, err } diff --git a/otelcol/go.mod b/otelcol/go.mod index 7c05bb61c5c..84b2bc97a37 100644 --- a/otelcol/go.mod +++ b/otelcol/go.mod @@ -8,7 +8,6 @@ require ( go.opentelemetry.io/collector/component v1.43.0 go.opentelemetry.io/collector/component/componentstatus v0.137.0 go.opentelemetry.io/collector/config/configopaque v1.43.0 - go.opentelemetry.io/collector/config/configtelemetry v0.137.0 go.opentelemetry.io/collector/confmap v1.43.0 go.opentelemetry.io/collector/confmap/provider/fileprovider v1.43.0 go.opentelemetry.io/collector/confmap/xconfmap v0.137.0 @@ -28,7 +27,7 @@ require ( go.opentelemetry.io/collector/receiver/receivertest v0.137.0 go.opentelemetry.io/collector/receiver/xreceiver v0.137.0 go.opentelemetry.io/collector/service v0.137.0 - go.opentelemetry.io/contrib/otelconf v0.18.0 + go.opentelemetry.io/collector/service/telemetry/telemetrytest v0.137.0 go.uber.org/goleak v1.3.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 @@ -79,6 +78,7 @@ require ( github.com/yusufpapurcu/wmi v1.2.4 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/collector/component/componenttest v0.137.0 // indirect + go.opentelemetry.io/collector/config/configtelemetry v0.137.0 // indirect go.opentelemetry.io/collector/connector/xconnector v0.137.0 // indirect go.opentelemetry.io/collector/consumer/consumererror v0.137.0 // indirect go.opentelemetry.io/collector/consumer/consumertest v0.137.0 // indirect @@ -94,6 +94,7 @@ require ( go.opentelemetry.io/collector/processor/xprocessor v0.137.0 // indirect go.opentelemetry.io/collector/service/hostcapabilities v0.137.0 // indirect go.opentelemetry.io/contrib/bridges/otelzap v0.13.0 // indirect + go.opentelemetry.io/contrib/otelconf v0.18.0 // indirect go.opentelemetry.io/contrib/propagators/b3 v1.38.0 // indirect go.opentelemetry.io/otel v1.38.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.14.0 // indirect diff --git a/otelcol/otelcoltest/config.go b/otelcol/otelcoltest/config.go index 55087dfda29..a36cdcc583d 100644 --- a/otelcol/otelcoltest/config.go +++ b/otelcol/otelcoltest/config.go @@ -13,10 +13,18 @@ import ( "go.opentelemetry.io/collector/confmap/provider/yamlprovider" "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/otelcol" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) // LoadConfig loads a config.Config from file, and does NOT validate the configuration. +// +// If factories.Telemetry is nil, otelconftelemetry will be used by default. +// TODO remove the backwards compatibility and require a non-nil factory +// https://github.com/open-telemetry/opentelemetry-collector/issues/14003 func LoadConfig(fileName string, factories otelcol.Factories) (*otelcol.Config, error) { + if factories.Telemetry == nil { + factories.Telemetry = otelconftelemetry.NewFactory() + } provider, err := otelcol.NewConfigProvider(otelcol.ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{fileName}, diff --git a/otelcol/otelcoltest/config_test.go b/otelcol/otelcoltest/config_test.go index 5a95e52c606..dfebac41d7e 100644 --- a/otelcol/otelcoltest/config_test.go +++ b/otelcol/otelcoltest/config_test.go @@ -13,6 +13,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/pipeline" "go.opentelemetry.io/collector/service/pipelines" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) func TestLoadConfig(t *testing.T) { @@ -58,6 +59,20 @@ func TestLoadConfig(t *testing.T) { }, cfg.Service.Pipelines[pipeline.NewID(pipeline.SignalTraces)], "Did not load pipeline config correctly") + + // Verify telemetry + assert.Equal(t, struct{}{}, cfg.Service.Telemetry) +} + +func TestLoadConfig_DefaultTelemetry(t *testing.T) { + factories, err := NopFactories() + require.NoError(t, err) + factories.Telemetry = nil + + cfg, err := LoadConfig(filepath.Join("testdata", "config.yaml"), factories) + require.NoError(t, err) + + assert.Equal(t, otelconftelemetry.NewFactory().CreateDefaultConfig(), cfg.Service.Telemetry) } func TestLoadConfigAndValidate(t *testing.T) { diff --git a/otelcol/otelcoltest/nop_factories.go b/otelcol/otelcoltest/nop_factories.go index 326430f8ecb..c1a559f228d 100644 --- a/otelcol/otelcoltest/nop_factories.go +++ b/otelcol/otelcoltest/nop_factories.go @@ -4,12 +4,14 @@ package otelcoltest // import "go.opentelemetry.io/collector/otelcol/otelcoltest" import ( + "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/exporter/exportertest" "go.opentelemetry.io/collector/extension/extensiontest" "go.opentelemetry.io/collector/otelcol" "go.opentelemetry.io/collector/processor/processortest" "go.opentelemetry.io/collector/receiver/receivertest" + "go.opentelemetry.io/collector/service/telemetry" ) // NopFactories returns a otelcol.Factories with all nop factories. @@ -22,6 +24,9 @@ func NopFactories() (otelcol.Factories, error) { factories.Exporters, _ = otelcol.MakeFactoryMap(exportertest.NewNopFactory()) factories.Processors, _ = otelcol.MakeFactoryMap(processortest.NewNopFactory()) factories.Connectors, _ = otelcol.MakeFactoryMap(connectortest.NewNopFactory()) + factories.Telemetry = telemetry.NewFactory( + func() component.Config { return struct{}{} }, + ) return factories, nil } diff --git a/otelcol/testdata/otelcol-invalid-receiver-type.yaml b/otelcol/testdata/otelcol-invalid-receiver-type.yaml index 9662a40384b..b1443b4bf7f 100644 --- a/otelcol/testdata/otelcol-invalid-receiver-type.yaml +++ b/otelcol/testdata/otelcol-invalid-receiver-type.yaml @@ -8,14 +8,6 @@ exporters: nop: service: - telemetry: - metrics: - readers: - - pull: - exporter: - prometheus: - host: "localhost" - port: 9999 pipelines: traces: receivers: [nop_logs] diff --git a/otelcol/testdata/otelcol-noreaders.yaml b/otelcol/testdata/otelcol-invalid-telemetry.yaml similarity index 80% rename from otelcol/testdata/otelcol-noreaders.yaml rename to otelcol/testdata/otelcol-invalid-telemetry.yaml index a35cf1fbbff..de79c1f01db 100644 --- a/otelcol/testdata/otelcol-noreaders.yaml +++ b/otelcol/testdata/otelcol-invalid-telemetry.yaml @@ -6,8 +6,7 @@ exporters: service: telemetry: - metrics: - level: none + unknown: key pipelines: metrics: receivers: [nop] diff --git a/otelcol/testdata/otelcol-invalid.yaml b/otelcol/testdata/otelcol-invalid.yaml index bdef77034f4..d6cab57658b 100644 --- a/otelcol/testdata/otelcol-invalid.yaml +++ b/otelcol/testdata/otelcol-invalid.yaml @@ -8,14 +8,6 @@ exporters: nop: service: - telemetry: - metrics: - readers: - - pull: - exporter: - prometheus: - host: "localhost" - port: 9999 pipelines: traces: receivers: [nop] diff --git a/otelcol/testdata/otelcol-invalidprop.yaml b/otelcol/testdata/otelcol-invalidprop.yaml deleted file mode 100644 index a47e5902cde..00000000000 --- a/otelcol/testdata/otelcol-invalidprop.yaml +++ /dev/null @@ -1,27 +0,0 @@ -receivers: - nop: - -processors: - nop: - -exporters: - nop: - -service: - telemetry: - traces: - propagators: - - "unknown" - - "tracecontext" - metrics: - readers: - - pull: - exporter: - prometheus: - host: "localhost" - port: 9999 - pipelines: - traces: - receivers: [nop] - processors: [nop] - exporters: [nop] diff --git a/otelcol/testdata/otelcol-multipleheaders.yaml b/otelcol/testdata/otelcol-multipleheaders.yaml deleted file mode 100644 index 6247f38318d..00000000000 --- a/otelcol/testdata/otelcol-multipleheaders.yaml +++ /dev/null @@ -1,24 +0,0 @@ -receivers: - nop: - -exporters: - nop: - -service: - telemetry: - metrics: - level: none - traces: - processors: - - batch: - exporter: - otlp: - endpoint: localhost:4318 - headers: - first: val1 - second: val2 - protocol: http/protobuf - pipelines: - metrics: - receivers: [nop] - exporters: [nop] diff --git a/otelcol/testdata/otelcol-nop.yaml b/otelcol/testdata/otelcol-nop.yaml index 15e1e884b6f..bf5806e0d20 100644 --- a/otelcol/testdata/otelcol-nop.yaml +++ b/otelcol/testdata/otelcol-nop.yaml @@ -14,14 +14,6 @@ connectors: nop/con: service: - telemetry: - metrics: - readers: - - pull: - exporter: - prometheus: - host: "localhost" - port: 9999 extensions: [nop] pipelines: traces: diff --git a/otelcol/testdata/otelcol-emptyreaders.yaml b/otelcol/testdata/otelcol-otelconftelemetry.yaml similarity index 74% rename from otelcol/testdata/otelcol-emptyreaders.yaml rename to otelcol/testdata/otelcol-otelconftelemetry.yaml index f8f78dc7ba6..4a55d99723a 100644 --- a/otelcol/testdata/otelcol-emptyreaders.yaml +++ b/otelcol/testdata/otelcol-otelconftelemetry.yaml @@ -1,14 +1,13 @@ receivers: nop: - exporters: nop: service: telemetry: + # Set configuration understood by otelconftelemetry metrics: level: none - readers: [] pipelines: metrics: receivers: [nop] diff --git a/otelcol/testdata/otelcol-statuswatcher.yaml b/otelcol/testdata/otelcol-statuswatcher.yaml index 8f79ec0552d..08a2ff4cf98 100644 --- a/otelcol/testdata/otelcol-statuswatcher.yaml +++ b/otelcol/testdata/otelcol-statuswatcher.yaml @@ -12,14 +12,6 @@ extensions: statuswatcher: service: - telemetry: - metrics: - readers: - - pull: - exporter: - prometheus: - host: "localhost" - port: 9999 extensions: [statuswatcher] pipelines: traces: diff --git a/otelcol/testdata/otelcol-validprop.yaml b/otelcol/testdata/otelcol-validprop.yaml deleted file mode 100644 index 10c573686aa..00000000000 --- a/otelcol/testdata/otelcol-validprop.yaml +++ /dev/null @@ -1,27 +0,0 @@ -receivers: - nop: - -processors: - nop: - -exporters: - nop: - -service: - telemetry: - traces: - propagators: - - "b3" - - "tracecontext" - metrics: - readers: - - pull: - exporter: - prometheus: - host: "localhost" - port: 9999 - pipelines: - traces: - receivers: [nop] - processors: [nop] - exporters: [nop] diff --git a/otelcol/unmarshaler.go b/otelcol/unmarshaler.go index 0874e48f417..3e0dde0eb05 100644 --- a/otelcol/unmarshaler.go +++ b/otelcol/unmarshaler.go @@ -12,7 +12,6 @@ import ( "go.opentelemetry.io/collector/processor" "go.opentelemetry.io/collector/receiver" "go.opentelemetry.io/collector/service" - "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) type configSettings struct { @@ -27,11 +26,6 @@ type configSettings struct { // unmarshal the configSettings from a confmap.Conf. // After the config is unmarshalled, `Validate()` must be called to validate. func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) { - // TODO: inject the telemetry factory through factories, once available. - // See https://github.com/open-telemetry/opentelemetry-collector/issues/4970 - telFactory := otelconftelemetry.NewFactory() - defaultTelConfig := telFactory.CreateDefaultConfig().(*otelconftelemetry.Config) - // Unmarshal top level sections and validate. cfg := &configSettings{ Receivers: configunmarshaler.NewConfigs(factories.Receivers), @@ -41,7 +35,7 @@ func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) { Extensions: configunmarshaler.NewConfigs(factories.Extensions), // TODO: Add a component.ServiceFactory to allow this to be defined by the Service. Service: service.Config{ - Telemetry: defaultTelConfig, + Telemetry: factories.Telemetry.CreateDefaultConfig(), }, } err := v.Unmarshal(&cfg) diff --git a/otelcol/unmarshaler_test.go b/otelcol/unmarshaler_test.go index 66f370ebebf..fafde031f47 100644 --- a/otelcol/unmarshaler_test.go +++ b/otelcol/unmarshaler_test.go @@ -5,16 +5,13 @@ package otelcol import ( "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/zap" "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/service" "go.opentelemetry.io/collector/service/pipelines" - "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) func TestUnmarshalEmpty(t *testing.T) { @@ -40,23 +37,7 @@ func TestUnmarshalEmptyAllSections(t *testing.T) { cfg, err := unmarshal(conf, factories) require.NoError(t, err) - zapProdCfg := zap.NewProductionConfig() - assert.Equal(t, otelconftelemetry.LogsConfig{ - Level: zapProdCfg.Level.Level(), - Development: zapProdCfg.Development, - Encoding: "console", - Sampling: &otelconftelemetry.LogsSamplingConfig{ - Enabled: true, - Tick: 10 * time.Second, - Initial: 10, - Thereafter: 100, - }, - DisableCaller: zapProdCfg.DisableCaller, - DisableStacktrace: zapProdCfg.DisableStacktrace, - OutputPaths: zapProdCfg.OutputPaths, - ErrorOutputPaths: zapProdCfg.ErrorOutputPaths, - InitialFields: zapProdCfg.InitialFields, - }, cfg.Service.Telemetry.(*otelconftelemetry.Config).Logs) + assert.Equal(t, fakeTelemetryConfig{}, cfg.Service.Telemetry) } func TestUnmarshalUnknownTopLevel(t *testing.T) { diff --git a/service/go.mod b/service/go.mod index 0c6d7008fd7..2e794613054 100644 --- a/service/go.mod +++ b/service/go.mod @@ -47,7 +47,7 @@ require ( go.opentelemetry.io/collector/receiver/receivertest v0.137.0 go.opentelemetry.io/collector/receiver/xreceiver v0.137.0 go.opentelemetry.io/collector/service/hostcapabilities v0.137.0 - go.opentelemetry.io/collector/service/telemetry/telemetrytest v0.0.0-20251010094443-567586048b9f + go.opentelemetry.io/collector/service/telemetry/telemetrytest v0.137.0 go.opentelemetry.io/contrib/otelconf v0.18.0 go.opentelemetry.io/contrib/propagators/b3 v1.38.0 go.opentelemetry.io/otel v1.38.0