From 2ef36a148058682117d2184f0f862b85919e85aa Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 9 Oct 2025 12:46:34 +0800 Subject: [PATCH 01/13] otelcol: add telemetry.Factory to Factories --- .chloggen/otelcol-telemetryfactory.yaml | 25 +++ otelcol/collector.go | 16 +- otelcol/collector_test.go | 199 +++++++++--------- otelcol/factories.go | 6 + otelcol/factories_test.go | 5 + otelcol/go.mod | 1 + otelcol/otelcoltest/nop_factories.go | 5 + .../otelcol-invalid-receiver-type.yaml | 8 - otelcol/testdata/otelcol-invalid.yaml | 8 - otelcol/testdata/otelcol-invalidprop.yaml | 27 --- otelcol/testdata/otelcol-multipleheaders.yaml | 24 --- otelcol/testdata/otelcol-nop.yaml | 8 - otelcol/testdata/otelcol-noreaders.yaml | 14 -- ...rs.yaml => otelcol-otelconftelemetry.yaml} | 2 +- otelcol/testdata/otelcol-statuswatcher.yaml | 8 - otelcol/testdata/otelcol-validprop.yaml | 27 --- otelcol/unmarshaler.go | 8 +- otelcol/unmarshaler_test.go | 21 +- 18 files changed, 159 insertions(+), 253 deletions(-) create mode 100644 .chloggen/otelcol-telemetryfactory.yaml delete mode 100644 otelcol/testdata/otelcol-invalidprop.yaml delete mode 100644 otelcol/testdata/otelcol-multipleheaders.yaml delete mode 100644 otelcol/testdata/otelcol-noreaders.yaml rename otelcol/testdata/{otelcol-emptyreaders.yaml => otelcol-otelconftelemetry.yaml} (74%) delete mode 100644 otelcol/testdata/otelcol-validprop.yaml diff --git a/.chloggen/otelcol-telemetryfactory.yaml b/.chloggen/otelcol-telemetryfactory.yaml new file mode 100644 index 00000000000..c577d949101 --- /dev/null +++ b/.chloggen/otelcol-telemetryfactory.yaml @@ -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: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: all + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Enable injecting a telemetry factory 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: + +# 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/otelcol/collector.go b/otelcol/collector.go index 419a855d742..4c36699fe07 100644 --- a/otelcol/collector.go +++ b/otelcol/collector.go @@ -54,7 +54,7 @@ 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. Factories func() (Factories, error) // BuildInfo provides collector start information. @@ -176,6 +176,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) @@ -217,10 +221,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 @@ -270,6 +271,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) @@ -289,6 +294,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..ca425422dcf 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,41 @@ 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 + }{ + "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-otelconftelemetry.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: metrics", + }, } - 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) { @@ -537,6 +522,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-otelconftelemetry.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: metrics", + }, + "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/factories.go b/otelcol/factories.go index c080dbde584..0885a8218cb 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,11 @@ 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. + 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 ae73717df15..97e6b599b92 100644 --- a/otelcol/go.mod +++ b/otelcol/go.mod @@ -25,6 +25,7 @@ require ( go.opentelemetry.io/collector/receiver v1.43.0 go.opentelemetry.io/collector/receiver/receivertest v0.137.0 go.opentelemetry.io/collector/service v0.137.0 + go.opentelemetry.io/collector/service/telemetry/telemetrytest v0.137.0 go.opentelemetry.io/contrib/otelconf v0.18.0 go.uber.org/goleak v1.3.0 go.uber.org/multierr v1.11.0 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-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-noreaders.yaml b/otelcol/testdata/otelcol-noreaders.yaml deleted file mode 100644 index a35cf1fbbff..00000000000 --- a/otelcol/testdata/otelcol-noreaders.yaml +++ /dev/null @@ -1,14 +0,0 @@ -receivers: - nop: - -exporters: - nop: - -service: - telemetry: - metrics: - level: none - pipelines: - metrics: - receivers: [nop] - exporters: [nop] 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..cd9e0260b94 100644 --- a/otelcol/testdata/otelcol-emptyreaders.yaml +++ b/otelcol/testdata/otelcol-otelconftelemetry.yaml @@ -6,9 +6,9 @@ exporters: 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) { From 0f6f32f3a42681e5fb42e50242ede0f026e164a0 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 9 Oct 2025 13:34:09 +0800 Subject: [PATCH 02/13] Add otelcol component --- .chloggen/config.yaml | 1 + .chloggen/otelcol-telemetryfactory.yaml | 2 +- .github/CODEOWNERS | 1 + .github/ISSUE_TEMPLATE/bug_report.yaml | 1 + .github/ISSUE_TEMPLATE/feature_request.yaml | 1 + .github/ISSUE_TEMPLATE/other.yaml | 1 + otelcol/metadata.yaml | 7 +++++++ 7 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 otelcol/metadata.yaml diff --git a/.chloggen/config.yaml b/.chloggen/config.yaml index d34a8d7caa4..3c11f7c81d7 100644 --- a/.chloggen/config.yaml +++ b/.chloggen/config.yaml @@ -25,6 +25,7 @@ components: - pdata/pprofile - pkg/confmap - pkg/exporterhelper + - pkg/otelcol - pkg/pdata - pkg/processorhelper - pkg/queuebatch diff --git a/.chloggen/otelcol-telemetryfactory.yaml b/.chloggen/otelcol-telemetryfactory.yaml index c577d949101..6ef832034ca 100644 --- a/.chloggen/otelcol-telemetryfactory.yaml +++ b/.chloggen/otelcol-telemetryfactory.yaml @@ -4,7 +4,7 @@ change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) -component: all +component: pkg/otelcol # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). note: Enable injecting a telemetry factory through otelcol.Factories diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 171fc539906..c0e647a4e8a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -47,6 +47,7 @@ extension/memorylimiterextension/ @open-telemetry/collector-approvers extension/xextension/ @open-telemetry/collector-approvers extension/xextension/storage/ @open-telemetry/collector-approvers @swiatekm extension/zpagesextension/ @open-telemetry/collector-approvers +otelcol/ @open-telemetry/collector-approvers pdata/ @open-telemetry/collector-approvers @bogdandrutu @dmitryax pdata/pprofile/ @open-telemetry/collector-approvers @mx-psi @dmathieu processor/batchprocessor/ @open-telemetry/collector-approvers diff --git a/.github/ISSUE_TEMPLATE/bug_report.yaml b/.github/ISSUE_TEMPLATE/bug_report.yaml index 377af0c14e3..86efd36f12d 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yaml +++ b/.github/ISSUE_TEMPLATE/bug_report.yaml @@ -47,6 +47,7 @@ body: - extension/x - extension/x/storage - extension/zpages + - otelcol - pdata - pdata/pprofile - processor/batch diff --git a/.github/ISSUE_TEMPLATE/feature_request.yaml b/.github/ISSUE_TEMPLATE/feature_request.yaml index 360ef56c511..67edb417591 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yaml +++ b/.github/ISSUE_TEMPLATE/feature_request.yaml @@ -41,6 +41,7 @@ body: - extension/x - extension/x/storage - extension/zpages + - otelcol - pdata - pdata/pprofile - processor/batch diff --git a/.github/ISSUE_TEMPLATE/other.yaml b/.github/ISSUE_TEMPLATE/other.yaml index 06354bad964..0bf98cdb356 100644 --- a/.github/ISSUE_TEMPLATE/other.yaml +++ b/.github/ISSUE_TEMPLATE/other.yaml @@ -40,6 +40,7 @@ body: - extension/x - extension/x/storage - extension/zpages + - otelcol - pdata - pdata/pprofile - processor/batch diff --git a/otelcol/metadata.yaml b/otelcol/metadata.yaml new file mode 100644 index 00000000000..b27f5133bec --- /dev/null +++ b/otelcol/metadata.yaml @@ -0,0 +1,7 @@ +type: otelcol +github_project: open-telemetry/opentelemetry-collector + +status: + disable_codecov_badge: true + class: pkg + distributions: [core, contrib] From 2cf0667483cc90dd2340bedb62b5a3ac359704a5 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 9 Oct 2025 14:00:37 +0800 Subject: [PATCH 03/13] Revert "Add otelcol component" This reverts commit 0f6f32f3a42681e5fb42e50242ede0f026e164a0. --- .chloggen/config.yaml | 1 - .chloggen/otelcol-telemetryfactory.yaml | 2 +- .github/CODEOWNERS | 1 - .github/ISSUE_TEMPLATE/bug_report.yaml | 1 - .github/ISSUE_TEMPLATE/feature_request.yaml | 1 - .github/ISSUE_TEMPLATE/other.yaml | 1 - otelcol/metadata.yaml | 7 ------- 7 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 otelcol/metadata.yaml diff --git a/.chloggen/config.yaml b/.chloggen/config.yaml index 3c11f7c81d7..d34a8d7caa4 100644 --- a/.chloggen/config.yaml +++ b/.chloggen/config.yaml @@ -25,7 +25,6 @@ components: - pdata/pprofile - pkg/confmap - pkg/exporterhelper - - pkg/otelcol - pkg/pdata - pkg/processorhelper - pkg/queuebatch diff --git a/.chloggen/otelcol-telemetryfactory.yaml b/.chloggen/otelcol-telemetryfactory.yaml index 6ef832034ca..c577d949101 100644 --- a/.chloggen/otelcol-telemetryfactory.yaml +++ b/.chloggen/otelcol-telemetryfactory.yaml @@ -4,7 +4,7 @@ change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) -component: pkg/otelcol +component: all # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). note: Enable injecting a telemetry factory through otelcol.Factories diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c0e647a4e8a..171fc539906 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -47,7 +47,6 @@ extension/memorylimiterextension/ @open-telemetry/collector-approvers extension/xextension/ @open-telemetry/collector-approvers extension/xextension/storage/ @open-telemetry/collector-approvers @swiatekm extension/zpagesextension/ @open-telemetry/collector-approvers -otelcol/ @open-telemetry/collector-approvers pdata/ @open-telemetry/collector-approvers @bogdandrutu @dmitryax pdata/pprofile/ @open-telemetry/collector-approvers @mx-psi @dmathieu processor/batchprocessor/ @open-telemetry/collector-approvers diff --git a/.github/ISSUE_TEMPLATE/bug_report.yaml b/.github/ISSUE_TEMPLATE/bug_report.yaml index 86efd36f12d..377af0c14e3 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yaml +++ b/.github/ISSUE_TEMPLATE/bug_report.yaml @@ -47,7 +47,6 @@ body: - extension/x - extension/x/storage - extension/zpages - - otelcol - pdata - pdata/pprofile - processor/batch diff --git a/.github/ISSUE_TEMPLATE/feature_request.yaml b/.github/ISSUE_TEMPLATE/feature_request.yaml index 67edb417591..360ef56c511 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yaml +++ b/.github/ISSUE_TEMPLATE/feature_request.yaml @@ -41,7 +41,6 @@ body: - extension/x - extension/x/storage - extension/zpages - - otelcol - pdata - pdata/pprofile - processor/batch diff --git a/.github/ISSUE_TEMPLATE/other.yaml b/.github/ISSUE_TEMPLATE/other.yaml index 0bf98cdb356..06354bad964 100644 --- a/.github/ISSUE_TEMPLATE/other.yaml +++ b/.github/ISSUE_TEMPLATE/other.yaml @@ -40,7 +40,6 @@ body: - extension/x - extension/x/storage - extension/zpages - - otelcol - pdata - pdata/pprofile - processor/batch diff --git a/otelcol/metadata.yaml b/otelcol/metadata.yaml deleted file mode 100644 index b27f5133bec..00000000000 --- a/otelcol/metadata.yaml +++ /dev/null @@ -1,7 +0,0 @@ -type: otelcol -github_project: open-telemetry/opentelemetry-collector - -status: - disable_codecov_badge: true - class: pkg - distributions: [core, contrib] From 8df43ff0d4ea61793f0f10932b856d79e7444ffb Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 9 Oct 2025 15:36:51 +0800 Subject: [PATCH 04/13] Set default factory in one more place --- otelcol/configprovider.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/otelcol/configprovider.go b/otelcol/configprovider.go index f813ce2d5a0..258dcee8141 100644 --- a/otelcol/configprovider.go +++ b/otelcol/configprovider.go @@ -8,6 +8,7 @@ import ( "fmt" "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) // ConfigProvider provides the service configuration. @@ -58,6 +59,10 @@ func (cm *ConfigProvider) Get(ctx context.Context, factories Factories) (*Config return nil, fmt.Errorf("cannot resolve the configuration: %w", err) } + if factories.Telemetry == nil { + factories.Telemetry = otelconftelemetry.NewFactory() + } + var cfg *configSettings if cfg, err = unmarshal(conf, factories); err != nil { return nil, fmt.Errorf("cannot unmarshal the configuration: %w", err) From cf73f32f196fd4c008c6068b984f4955f9e1f5ff Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 9 Oct 2025 17:20:04 +0800 Subject: [PATCH 05/13] Improve covere of and and tidy tests Improve the coverage of TestConfigProvider* so it also covers the default telemetry provider code path. Tidy the TestConfigProvider* tests: - Just use a single confmap provider. They're both fakes, so testing one is enough. - Rather than comparing unmarshaled config to unmarshaled config, which is a bit circuitous, construct the expected config struct --- otelcol/configprovider_test.go | 208 ++++++++++++++++++++------------- 1 file changed, 124 insertions(+), 84 deletions(-) diff --git a/otelcol/configprovider_test.go b/otelcol/configprovider_test.go index 63304ba822b..05e22954016 100644 --- a/otelcol/configprovider_test.go +++ b/otelcol/configprovider_test.go @@ -13,95 +13,135 @@ import ( "github.com/stretchr/testify/require" yaml "go.yaml.in/yaml/v3" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configtelemetry" "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" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) -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}, + "default_telemetry": { + filename: "otelcol-otelconftelemetry.yaml", + factories: func() (Factories, error) { + factories, err := nopFactories() + if err != nil { + return Factories{}, err + } + factories.Telemetry = nil + return factories, nil + }, + expectedConfig: &Config{ + Exporters: map[component.ID]component.Config{ + nopComponentID: exportertest.NewNopFactory().CreateDefaultConfig(), + }, + Receivers: map[component.ID]component.Config{ + nopComponentID: receivertest.NewNopFactory().CreateDefaultConfig(), + }, + Service: service.Config{ + Telemetry: func() component.Config { + cfg := otelconftelemetry.NewFactory().CreateDefaultConfig().(*otelconftelemetry.Config) + cfg.Metrics.Level = configtelemetry.LevelNone + return cfg + }(), + Pipelines: map[pipeline.ID]*pipelines.PipelineConfig{ + pipeline.NewID(pipeline.SignalMetrics): { + Receivers: []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) - - 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) + 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) + }) + } } From f83391fdba58f45f5359ac14acd76918ce79485e Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Fri, 10 Oct 2025 16:34:51 +0800 Subject: [PATCH 06/13] Update component --- .chloggen/otelcol-telemetryfactory.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/otelcol-telemetryfactory.yaml b/.chloggen/otelcol-telemetryfactory.yaml index c577d949101..6ef832034ca 100644 --- a/.chloggen/otelcol-telemetryfactory.yaml +++ b/.chloggen/otelcol-telemetryfactory.yaml @@ -4,7 +4,7 @@ change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) -component: all +component: pkg/otelcol # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). note: Enable injecting a telemetry factory through otelcol.Factories From b4310442dfcfd70892ce78e7819c30dbb7964647 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 11:22:12 +0800 Subject: [PATCH 07/13] Make it a breaking change --- .chloggen/otelcol-telemetryfactory.yaml | 9 ++- .../builder/templates/components.go.tmpl | 5 +- cmd/otelcorecol/components.go | 5 +- cmd/otelcorecol/go.mod | 2 +- otelcol/collector.go | 7 -- otelcol/collector_test.go | 73 ++++++++----------- otelcol/collector_windows.go | 3 + otelcol/command_print_test.go | 4 + otelcol/config_test.go | 32 +------- otelcol/configprovider.go | 5 -- otelcol/configprovider_test.go | 34 --------- otelcol/factories.go | 2 - ...ry.yaml => otelcol-invalid-telemetry.yaml} | 4 +- otelcol/unmarshaler.go | 8 ++ 14 files changed, 64 insertions(+), 129 deletions(-) rename otelcol/testdata/{otelcol-otelconftelemetry.yaml => otelcol-invalid-telemetry.yaml} (60%) diff --git a/.chloggen/otelcol-telemetryfactory.yaml b/.chloggen/otelcol-telemetryfactory.yaml index 6ef832034ca..2d97c5d5cd0 100644 --- a/.chloggen/otelcol-telemetryfactory.yaml +++ b/.chloggen/otelcol-telemetryfactory.yaml @@ -1,13 +1,13 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: enhancement +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: Enable injecting a telemetry factory through otelcol.Factories +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] @@ -15,7 +15,10 @@ 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: +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]' 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 f5b285d9413..ec7e6266afa 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/otelcol/collector.go b/otelcol/collector.go index eb202ecab7b..fecd77d0f99 100644 --- a/otelcol/collector.go +++ b/otelcol/collector.go @@ -25,7 +25,6 @@ import ( "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/otelcol/internal/grpclog" "go.opentelemetry.io/collector/service" - "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) // State defines Collector's state. @@ -179,9 +178,6 @@ 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 { @@ -274,9 +270,6 @@ 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 { diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index ca425422dcf..4b108bbd3e6 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -389,44 +389,19 @@ func TestNewCollectorValidatesResolverSettings(t *testing.T) { } func TestCollectorRun(t *testing.T) { - tests := map[string]struct { - factories func() (Factories, error) - configFile string - }{ - "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", - }, + set := CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: nopFactories, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}), } + col, err := NewCollector(set) + require.NoError(t, err) - for name, test := range tests { - t.Run(name, func(t *testing.T) { - set := CollectorSettings{ - BuildInfo: component.NewDefaultBuildInfo(), - Factories: test.factories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", test.configFile)}), - } - col, err := NewCollector(set) - require.NoError(t, err) - - wg := startCollector(context.Background(), t, col) + wg := startCollector(context.Background(), t, col) - col.Shutdown() - wg.Wait() - assert.Equal(t, StateClosed, col.GetState()) - }) - } + col.Shutdown() + wg.Wait() + assert.Equal(t, StateClosed, col.GetState()) } func TestCollectorRun_AfterShutdown(t *testing.T) { @@ -464,9 +439,24 @@ func TestCollectorRun_Errors(t *testing.T) { settings: CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-otelconftelemetry.yaml")}), + 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", + }, + "missing_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-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: metrics", + expectedErr: "failed to get config: cannot unmarshal the configuration: Factories.Telemetry must not be nil", }, } @@ -526,11 +516,11 @@ func TestCollectorDryRun(t *testing.T) { settings: CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-otelconftelemetry.yaml")}), + 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: metrics", + 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": { + "missing_telemetry_factory": { settings: CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: func() (Factories, error) { @@ -541,8 +531,9 @@ func TestCollectorDryRun(t *testing.T) { factories.Telemetry = nil return factories, nil }, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-otelconftelemetry.yaml")}), + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid-telemetry.yaml")}), }, + expectedErr: "failed to get config: cannot unmarshal the configuration: Factories.Telemetry must not be nil", }, } 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..6c367f3c3a0 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): { diff --git a/otelcol/configprovider.go b/otelcol/configprovider.go index 258dcee8141..f813ce2d5a0 100644 --- a/otelcol/configprovider.go +++ b/otelcol/configprovider.go @@ -8,7 +8,6 @@ import ( "fmt" "go.opentelemetry.io/collector/confmap" - "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) // ConfigProvider provides the service configuration. @@ -59,10 +58,6 @@ func (cm *ConfigProvider) Get(ctx context.Context, factories Factories) (*Config return nil, fmt.Errorf("cannot resolve the configuration: %w", err) } - if factories.Telemetry == nil { - factories.Telemetry = otelconftelemetry.NewFactory() - } - var cfg *configSettings if cfg, err = unmarshal(conf, factories); err != nil { return nil, fmt.Errorf("cannot unmarshal the configuration: %w", err) diff --git a/otelcol/configprovider_test.go b/otelcol/configprovider_test.go index 05e22954016..927ee204582 100644 --- a/otelcol/configprovider_test.go +++ b/otelcol/configprovider_test.go @@ -14,7 +14,6 @@ import ( yaml "go.yaml.in/yaml/v3" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/exporter/exportertest" @@ -24,7 +23,6 @@ import ( "go.opentelemetry.io/collector/receiver/receivertest" "go.opentelemetry.io/collector/service" "go.opentelemetry.io/collector/service/pipelines" - "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) func TestConfigProvider(t *testing.T) { @@ -78,38 +76,6 @@ func TestConfigProvider(t *testing.T) { }, }, }, - "default_telemetry": { - filename: "otelcol-otelconftelemetry.yaml", - factories: func() (Factories, error) { - factories, err := nopFactories() - if err != nil { - return Factories{}, err - } - factories.Telemetry = nil - return factories, nil - }, - expectedConfig: &Config{ - Exporters: map[component.ID]component.Config{ - nopComponentID: exportertest.NewNopFactory().CreateDefaultConfig(), - }, - Receivers: map[component.ID]component.Config{ - nopComponentID: receivertest.NewNopFactory().CreateDefaultConfig(), - }, - Service: service.Config{ - Telemetry: func() component.Config { - cfg := otelconftelemetry.NewFactory().CreateDefaultConfig().(*otelconftelemetry.Config) - cfg.Metrics.Level = configtelemetry.LevelNone - return cfg - }(), - Pipelines: map[pipeline.ID]*pipelines.PipelineConfig{ - pipeline.NewID(pipeline.SignalMetrics): { - Receivers: []component.ID{nopComponentID}, - Exporters: []component.ID{nopComponentID}, - }, - }, - }, - }, - }, } for name, test := range tests { diff --git a/otelcol/factories.go b/otelcol/factories.go index 0885a8218cb..76377059189 100644 --- a/otelcol/factories.go +++ b/otelcol/factories.go @@ -34,8 +34,6 @@ type Factories struct { 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. Telemetry telemetry.Factory // ReceiverModules maps receiver types to their respective go modules. diff --git a/otelcol/testdata/otelcol-otelconftelemetry.yaml b/otelcol/testdata/otelcol-invalid-telemetry.yaml similarity index 60% rename from otelcol/testdata/otelcol-otelconftelemetry.yaml rename to otelcol/testdata/otelcol-invalid-telemetry.yaml index cd9e0260b94..de79c1f01db 100644 --- a/otelcol/testdata/otelcol-otelconftelemetry.yaml +++ b/otelcol/testdata/otelcol-invalid-telemetry.yaml @@ -6,9 +6,7 @@ exporters: service: telemetry: - # Set configuration understood by otelconftelemetry - metrics: - level: none + unknown: key pipelines: metrics: receivers: [nop] diff --git a/otelcol/unmarshaler.go b/otelcol/unmarshaler.go index 3e0dde0eb05..3e733e62c54 100644 --- a/otelcol/unmarshaler.go +++ b/otelcol/unmarshaler.go @@ -4,6 +4,8 @@ package otelcol // import "go.opentelemetry.io/collector/otelcol" import ( + "errors" + "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/connector" "go.opentelemetry.io/collector/exporter" @@ -14,6 +16,8 @@ import ( "go.opentelemetry.io/collector/service" ) +var errNilTelemetryFactory = errors.New("Factories.Telemetry must not be nil") + type configSettings struct { Receivers *configunmarshaler.Configs[receiver.Factory] `mapstructure:"receivers"` Processors *configunmarshaler.Configs[processor.Factory] `mapstructure:"processors"` @@ -26,6 +30,10 @@ 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) { + if factories.Telemetry == nil { + return nil, errNilTelemetryFactory + } + // Unmarshal top level sections and validate. cfg := &configSettings{ Receivers: configunmarshaler.NewConfigs(factories.Receivers), From 46c87e667713a9c5996b7c7cadf1fb87a34ea9f6 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 13:04:47 +0800 Subject: [PATCH 08/13] Remove unused code --- otelcol/config_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/otelcol/config_test.go b/otelcol/config_test.go index 6c367f3c3a0..7b30c1fc26c 100644 --- a/otelcol/config_test.go +++ b/otelcol/config_test.go @@ -306,10 +306,6 @@ func generateConfig() *Config { } } -func newPtr[T int | string](str T) *T { - return &str -} - type fakeTelemetryConfig struct { Invalid bool `mapstructure:"invalid"` } From dadb0b1eee9ff20dcc16a2d0f17c9548bce756b9 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 13:10:21 +0800 Subject: [PATCH 09/13] make gotidy --- otelcol/go.mod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/otelcol/go.mod b/otelcol/go.mod index 5071c4045ef..00ee5fcd664 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.42.0 go.opentelemetry.io/collector/confmap/xconfmap v0.137.0 @@ -29,7 +28,6 @@ require ( go.opentelemetry.io/collector/receiver/xreceiver v0.137.0 go.opentelemetry.io/collector/service v0.137.0 go.opentelemetry.io/collector/service/telemetry/telemetrytest v0.137.0 - go.opentelemetry.io/contrib/otelconf v0.18.0 go.uber.org/goleak v1.3.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 @@ -80,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 @@ -95,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 From 298017b9a83c09e27e8fd8c0730d9affa574ae5e Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 13:18:55 +0800 Subject: [PATCH 10/13] internal/e2e: set Telemetry factory --- internal/e2e/metric_stability_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/e2e/metric_stability_test.go b/internal/e2e/metric_stability_test.go index 8cbf46e9a89..7880cd9a69b 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{ From 0778c8da2d5f80c5d5d2b35c49a907f2a22537fe Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 13:24:04 +0800 Subject: [PATCH 11/13] make gotidy --- service/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From a8670dd55ac98ccd17e833e3cd263423975e9005 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 14:20:11 +0800 Subject: [PATCH 12/13] otelcoltest: set nop telemetry if unspecified --- otelcol/otelcoltest/config.go | 5 +++++ otelcol/otelcoltest/nop_factories.go | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/otelcol/otelcoltest/config.go b/otelcol/otelcoltest/config.go index 55087dfda29..f37b6437366 100644 --- a/otelcol/otelcoltest/config.go +++ b/otelcol/otelcoltest/config.go @@ -16,7 +16,12 @@ import ( ) // LoadConfig loads a config.Config from file, and does NOT validate the configuration. +// +// If factories.Telemetry is nil, it uses a no-op telemetry factory. func LoadConfig(fileName string, factories otelcol.Factories) (*otelcol.Config, error) { + if factories.Telemetry == nil { + factories.Telemetry = nopFactories().Telemetry + } provider, err := otelcol.NewConfigProvider(otelcol.ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: []string{fileName}, diff --git a/otelcol/otelcoltest/nop_factories.go b/otelcol/otelcoltest/nop_factories.go index c1a559f228d..4366030ae5a 100644 --- a/otelcol/otelcoltest/nop_factories.go +++ b/otelcol/otelcoltest/nop_factories.go @@ -16,6 +16,10 @@ import ( // NopFactories returns a otelcol.Factories with all nop factories. func NopFactories() (otelcol.Factories, error) { + return nopFactories(), nil +} + +func nopFactories() otelcol.Factories { var factories otelcol.Factories // MakeFactoryMap can never return an error with a single Factory @@ -28,5 +32,5 @@ func NopFactories() (otelcol.Factories, error) { func() component.Config { return struct{}{} }, ) - return factories, nil + return factories } From 9bdcb360020c10718b715ad57167b8a62ab7893a Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 13 Oct 2025 16:30:26 +0800 Subject: [PATCH 13/13] otelcol/...: reinstate backwards compatibility Keep the cmd/builder changes, but retain backwards compatibility in otelcol and otelcoltest to ease the transition in contrib. --- otelcol/collector.go | 7 ++ otelcol/collector_test.go | 83 ++++++++++++------- otelcol/factories.go | 4 + otelcol/otelcoltest/config.go | 7 +- otelcol/otelcoltest/config_test.go | 15 ++++ otelcol/otelcoltest/nop_factories.go | 6 +- .../testdata/otelcol-otelconftelemetry.yaml | 14 ++++ otelcol/unmarshaler.go | 8 -- 8 files changed, 101 insertions(+), 43 deletions(-) create mode 100644 otelcol/testdata/otelcol-otelconftelemetry.yaml diff --git a/otelcol/collector.go b/otelcol/collector.go index fecd77d0f99..eb202ecab7b 100644 --- a/otelcol/collector.go +++ b/otelcol/collector.go @@ -25,6 +25,7 @@ import ( "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/otelcol/internal/grpclog" "go.opentelemetry.io/collector/service" + "go.opentelemetry.io/collector/service/telemetry/otelconftelemetry" ) // State defines Collector's state. @@ -178,6 +179,9 @@ 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 { @@ -270,6 +274,9 @@ 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 { diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index 4b108bbd3e6..b8316fc8c2d 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -389,19 +389,44 @@ func TestNewCollectorValidatesResolverSettings(t *testing.T) { } func TestCollectorRun(t *testing.T) { - set := CollectorSettings{ - BuildInfo: component.NewDefaultBuildInfo(), - Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-nop.yaml")}), + tests := map[string]struct { + factories func() (Factories, error) + configFile string + }{ + "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", + }, } - col, err := NewCollector(set) - require.NoError(t, err) - wg := startCollector(context.Background(), t, col) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + set := CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: test.factories, + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", test.configFile)}), + } + col, err := NewCollector(set) + require.NoError(t, err) - col.Shutdown() - wg.Wait() - assert.Equal(t, StateClosed, col.GetState()) + wg := startCollector(context.Background(), t, col) + + col.Shutdown() + wg.Wait() + assert.Equal(t, StateClosed, col.GetState()) + }) + } } func TestCollectorRun_AfterShutdown(t *testing.T) { @@ -428,6 +453,15 @@ func TestCollectorRun_Errors(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{ Factories: nopFactories, @@ -443,21 +477,6 @@ func TestCollectorRun_Errors(t *testing.T) { }, 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", }, - "missing_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-invalid-telemetry.yaml")}), - }, - expectedErr: "failed to get config: cannot unmarshal the configuration: Factories.Telemetry must not be nil", - }, } for name, test := range tests { @@ -480,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(), @@ -520,7 +548,7 @@ func TestCollectorDryRun(t *testing.T) { }, 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", }, - "missing_telemetry_factory": { + "default_telemetry_factory": { settings: CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: func() (Factories, error) { @@ -531,9 +559,8 @@ func TestCollectorDryRun(t *testing.T) { factories.Telemetry = nil return factories, nil }, - ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid-telemetry.yaml")}), + ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-otelconftelemetry.yaml")}), }, - expectedErr: "failed to get config: cannot unmarshal the configuration: Factories.Telemetry must not be nil", }, } diff --git a/otelcol/factories.go b/otelcol/factories.go index 76377059189..3f8daa39089 100644 --- a/otelcol/factories.go +++ b/otelcol/factories.go @@ -34,6 +34,10 @@ type Factories struct { 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. diff --git a/otelcol/otelcoltest/config.go b/otelcol/otelcoltest/config.go index f37b6437366..a36cdcc583d 100644 --- a/otelcol/otelcoltest/config.go +++ b/otelcol/otelcoltest/config.go @@ -13,14 +13,17 @@ 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, it uses a no-op telemetry factory. +// 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 = nopFactories().Telemetry + factories.Telemetry = otelconftelemetry.NewFactory() } provider, err := otelcol.NewConfigProvider(otelcol.ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ 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 4366030ae5a..c1a559f228d 100644 --- a/otelcol/otelcoltest/nop_factories.go +++ b/otelcol/otelcoltest/nop_factories.go @@ -16,10 +16,6 @@ import ( // NopFactories returns a otelcol.Factories with all nop factories. func NopFactories() (otelcol.Factories, error) { - return nopFactories(), nil -} - -func nopFactories() otelcol.Factories { var factories otelcol.Factories // MakeFactoryMap can never return an error with a single Factory @@ -32,5 +28,5 @@ func nopFactories() otelcol.Factories { func() component.Config { return struct{}{} }, ) - return factories + return factories, nil } diff --git a/otelcol/testdata/otelcol-otelconftelemetry.yaml b/otelcol/testdata/otelcol-otelconftelemetry.yaml new file mode 100644 index 00000000000..4a55d99723a --- /dev/null +++ b/otelcol/testdata/otelcol-otelconftelemetry.yaml @@ -0,0 +1,14 @@ +receivers: + nop: +exporters: + nop: + +service: + telemetry: + # Set configuration understood by otelconftelemetry + metrics: + level: none + pipelines: + metrics: + receivers: [nop] + exporters: [nop] diff --git a/otelcol/unmarshaler.go b/otelcol/unmarshaler.go index 3e733e62c54..3e0dde0eb05 100644 --- a/otelcol/unmarshaler.go +++ b/otelcol/unmarshaler.go @@ -4,8 +4,6 @@ package otelcol // import "go.opentelemetry.io/collector/otelcol" import ( - "errors" - "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/connector" "go.opentelemetry.io/collector/exporter" @@ -16,8 +14,6 @@ import ( "go.opentelemetry.io/collector/service" ) -var errNilTelemetryFactory = errors.New("Factories.Telemetry must not be nil") - type configSettings struct { Receivers *configunmarshaler.Configs[receiver.Factory] `mapstructure:"receivers"` Processors *configunmarshaler.Configs[processor.Factory] `mapstructure:"processors"` @@ -30,10 +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) { - if factories.Telemetry == nil { - return nil, errNilTelemetryFactory - } - // Unmarshal top level sections and validate. cfg := &configSettings{ Receivers: configunmarshaler.NewConfigs(factories.Receivers),