From 07603a09f7c0a37b60ec2327d3c0d4a0322b3de6 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Thu, 27 Oct 2022 12:58:53 -0700 Subject: [PATCH] Move default unmarshaler to service to avoid using deprecate configs. (#6395) Remove deprecated config.Service. Signed-off-by: Bogdan Signed-off-by: Bogdan --- .chloggen/mvunmarshaler.yaml | 11 + config/moved_config.go | 17 -- service/config.go | 13 +- service/config_provider.go | 5 +- .../configunmarshaler/defaultunmarshaler.go | 102 --------- .../defaultunmarshaler_test.go | 157 ------------- service/internal/configunmarshaler/error.go | 30 +++ .../testdata/duplicate-pipeline.yaml | 16 -- .../testdata/invalid-logs-level.yaml | 19 -- .../testdata/invalid-metrics-level.yaml | 19 -- .../invalid-pipeline-name-after-slash.yaml | 13 -- .../testdata/invalid-pipeline-section.yaml | 20 -- .../testdata/invalid-pipeline-sub-config.yaml | 9 - .../testdata/invalid-pipeline-type.yaml | 13 -- .../testdata/invalid-sequence-value.yaml | 14 -- .../invalid-service-extensions-section.yaml | 18 -- .../testdata/invalid-service-section.yaml | 21 -- .../testdata/invalid-top-level-section.yaml | 21 -- .../testdata/valid-config.yaml | 34 --- service/internal/pipelines/pipelines_test.go | 25 +- service/unmarshaler.go | 66 ++++++ service/unmarshaler_test.go | 214 ++++++++++++++++++ 22 files changed, 356 insertions(+), 501 deletions(-) create mode 100755 .chloggen/mvunmarshaler.yaml delete mode 100644 service/internal/configunmarshaler/defaultunmarshaler.go delete mode 100644 service/internal/configunmarshaler/defaultunmarshaler_test.go create mode 100644 service/internal/configunmarshaler/error.go delete mode 100644 service/internal/configunmarshaler/testdata/duplicate-pipeline.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-logs-level.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-metrics-level.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-pipeline-name-after-slash.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-pipeline-section.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-pipeline-sub-config.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-pipeline-type.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-sequence-value.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-service-extensions-section.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-service-section.yaml delete mode 100644 service/internal/configunmarshaler/testdata/invalid-top-level-section.yaml delete mode 100644 service/internal/configunmarshaler/testdata/valid-config.yaml create mode 100644 service/unmarshaler.go create mode 100644 service/unmarshaler_test.go diff --git a/.chloggen/mvunmarshaler.yaml b/.chloggen/mvunmarshaler.yaml new file mode 100755 index 000000000000..2803fd9b366e --- /dev/null +++ b/.chloggen/mvunmarshaler.yaml @@ -0,0 +1,11 @@ +# 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: config + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Remove already deprecates `config.Service`. + +# One or more tracking issues or pull requests related to the change +issues: [6395] diff --git a/config/moved_config.go b/config/moved_config.go index 35780df67182..2af429427468 100644 --- a/config/moved_config.go +++ b/config/moved_config.go @@ -14,23 +14,6 @@ package config // import "go.opentelemetry.io/collector/config" -import ( - "go.opentelemetry.io/collector/service/telemetry" -) - -// Service defines the configurable components of the service. -// Deprecated: [v0.52.0] Use service.ConfigService -type Service struct { - // Telemetry is the configuration for collector's own telemetry. - Telemetry telemetry.Config `mapstructure:"telemetry"` - - // Extensions are the ordered list of extensions configured for the service. - Extensions []ComponentID `mapstructure:"extensions"` - - // Pipelines are the set of data pipelines configured for the service. - Pipelines map[ComponentID]*Pipeline `mapstructure:"pipelines"` -} - // Pipeline defines a single pipeline. // Deprecated: [v0.52.0] Use service.ConfigServicePipeline type Pipeline struct { diff --git a/service/config.go b/service/config.go index bff1b2456a2b..a96c415864be 100644 --- a/service/config.go +++ b/service/config.go @@ -19,6 +19,7 @@ import ( "fmt" "go.opentelemetry.io/collector/config" + "go.opentelemetry.io/collector/service/telemetry" ) var ( @@ -151,6 +152,16 @@ func (cfg *Config) validateService() error { return nil } -type ConfigService = config.Service +// ConfigService defines the configurable components of the service. +type ConfigService struct { + // Telemetry is the configuration for collector's own telemetry. + Telemetry telemetry.Config `mapstructure:"telemetry"` + + // Extensions are the ordered list of extensions configured for the service. + Extensions []config.ComponentID `mapstructure:"extensions"` + + // Pipelines are the set of data pipelines configured for the service. + Pipelines map[config.ComponentID]*ConfigServicePipeline `mapstructure:"pipelines"` +} type ConfigServicePipeline = config.Pipeline diff --git a/service/config_provider.go b/service/config_provider.go index 3119f14233c2..3feba9741fe1 100644 --- a/service/config_provider.go +++ b/service/config_provider.go @@ -25,7 +25,6 @@ import ( "go.opentelemetry.io/collector/confmap/provider/fileprovider" "go.opentelemetry.io/collector/confmap/provider/httpprovider" "go.opentelemetry.io/collector/confmap/provider/yamlprovider" - "go.opentelemetry.io/collector/service/internal/configunmarshaler" ) // ConfigProvider provides the service configuration. @@ -105,8 +104,8 @@ func (cm *configProvider) Get(ctx context.Context, factories component.Factories return nil, fmt.Errorf("cannot resolve the configuration: %w", err) } - cfg, err := configunmarshaler.Unmarshal(conf, factories) - if err != nil { + var cfg *configSettings + if cfg, err = unmarshal(conf, factories); err != nil { return nil, fmt.Errorf("cannot unmarshal the configuration: %w", err) } diff --git a/service/internal/configunmarshaler/defaultunmarshaler.go b/service/internal/configunmarshaler/defaultunmarshaler.go deleted file mode 100644 index da4d67859a88..000000000000 --- a/service/internal/configunmarshaler/defaultunmarshaler.go +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configunmarshaler // import "go.opentelemetry.io/collector/service/internal/configunmarshaler" - -import ( - "fmt" - "reflect" - - "go.uber.org/zap/zapcore" - - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config" - "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/collector/confmap" - "go.opentelemetry.io/collector/service/telemetry" -) - -// These are errors that can be returned by Unmarshal(). Note that error codes are not part -// of Unmarshal()'s public API, they are for internal unit testing only. -type configErrorCode int - -const ( - // Skip 0, start errors codes from 1. - _ configErrorCode = iota - - errUnmarshalTopLevelStructure -) - -type configError struct { - // the original error. - error - - // internal error code. - code configErrorCode -} - -type Config struct { - Receivers *Receivers `mapstructure:"receivers"` - Processors *Processors `mapstructure:"processors"` - Exporters *Exporters `mapstructure:"exporters"` - Extensions *Extensions `mapstructure:"extensions"` - Service config.Service `mapstructure:"service"` -} - -// Unmarshal the Config from a confmap.Conf. -// After the config is unmarshalled, `Validate()` must be called to validate. -func Unmarshal(v *confmap.Conf, factories component.Factories) (*Config, error) { - // Unmarshal top level sections and validate. - cfg := &Config{ - Receivers: NewReceivers(factories.Receivers), - Processors: NewProcessors(factories.Processors), - Exporters: NewExporters(factories.Exporters), - Extensions: NewExtensions(factories.Extensions), - // TODO: Add a component.ServiceFactory to allow this to be defined by the Service. - Service: config.Service{ - Telemetry: telemetry.Config{ - Logs: telemetry.LogsConfig{ - Level: zapcore.InfoLevel, - Development: false, - Encoding: "console", - OutputPaths: []string{"stderr"}, - ErrorOutputPaths: []string{"stderr"}, - DisableCaller: false, - DisableStacktrace: false, - InitialFields: map[string]interface{}(nil), - }, - Metrics: telemetry.MetricsConfig{ - Level: configtelemetry.LevelBasic, - Address: ":8888", - }, - }, - }, - } - if err := v.Unmarshal(&cfg, confmap.WithErrorUnused()); err != nil { - return nil, configError{ - error: fmt.Errorf("error reading top level configuration sections: %w", err), - code: errUnmarshalTopLevelStructure, - } - } - - return cfg, nil -} - -func errorUnknownType(component string, id config.ComponentID, factories []reflect.Value) error { - return fmt.Errorf("unknown %s type: %q for id: %q (valid values: %v)", component, id.Type(), id, factories) -} - -func errorUnmarshalError(component string, id config.ComponentID, err error) error { - return fmt.Errorf("error reading %s configuration for %q: %w", component, id, err) -} diff --git a/service/internal/configunmarshaler/defaultunmarshaler_test.go b/service/internal/configunmarshaler/defaultunmarshaler_test.go deleted file mode 100644 index e1153ab09b4d..000000000000 --- a/service/internal/configunmarshaler/defaultunmarshaler_test.go +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configunmarshaler - -import ( - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" - - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/component/componenttest" - "go.opentelemetry.io/collector/config" - "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/collector/confmap" - "go.opentelemetry.io/collector/confmap/confmaptest" - "go.opentelemetry.io/collector/service/telemetry" -) - -func TestLoadEmpty(t *testing.T) { - factories, err := componenttest.NopFactories() - assert.NoError(t, err) - - _, err = Unmarshal(confmap.New(), factories) - assert.NoError(t, err) -} - -func TestLoadEmptyAllSections(t *testing.T) { - factories, err := componenttest.NopFactories() - assert.NoError(t, err) - - conf := confmap.NewFromStringMap(map[string]interface{}{ - "receivers": nil, - "processors": nil, - "exporters": nil, - "extensions": nil, - "service": nil, - }) - cfg, err := Unmarshal(conf, factories) - assert.NoError(t, err) - - zapProdCfg := zap.NewProductionConfig() - assert.Equal(t, telemetry.LogsConfig{ - Level: zapProdCfg.Level.Level(), - Development: zapProdCfg.Development, - Encoding: "console", - DisableCaller: zapProdCfg.DisableCaller, - DisableStacktrace: zapProdCfg.DisableStacktrace, - OutputPaths: zapProdCfg.OutputPaths, - ErrorOutputPaths: zapProdCfg.ErrorOutputPaths, - InitialFields: zapProdCfg.InitialFields, - }, cfg.Service.Telemetry.Logs) -} - -func TestUnmarshal(t *testing.T) { - factories, err := componenttest.NopFactories() - require.NoError(t, err) - - // Unmarshal the config - cfg, err := loadConfigFile(t, filepath.Join("testdata", "valid-config.yaml"), factories) - require.NoError(t, err, "Unable to load config") - - assert.Equal(t, map[config.ComponentID]config.Receiver{config.NewComponentID("nop"): factories.Receivers["nop"].CreateDefaultConfig()}, cfg.Receivers.GetReceivers()) - assert.Equal(t, map[config.ComponentID]config.Processor{config.NewComponentID("nop"): factories.Processors["nop"].CreateDefaultConfig()}, cfg.Processors.GetProcessors()) - assert.Equal(t, map[config.ComponentID]config.Exporter{config.NewComponentID("nop"): factories.Exporters["nop"].CreateDefaultConfig()}, cfg.Exporters.GetExporters()) - assert.Equal(t, map[config.ComponentID]config.Extension{config.NewComponentID("nop"): factories.Extensions["nop"].CreateDefaultConfig()}, cfg.Extensions.GetExtensions()) - - assert.Equal(t, - config.Service{ - Extensions: []config.ComponentID{config.NewComponentID("nop")}, - Pipelines: map[config.ComponentID]*config.Pipeline{ - config.NewComponentID("traces"): { - Receivers: []config.ComponentID{config.NewComponentID("nop")}, - Processors: []config.ComponentID{config.NewComponentID("nop")}, - Exporters: []config.ComponentID{config.NewComponentID("nop")}, - }, - }, - Telemetry: telemetry.Config{ - Logs: telemetry.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]interface{}{"field_key": "filed_value"}, - }, - Metrics: telemetry.MetricsConfig{ - Level: configtelemetry.LevelNormal, - Address: ":8081", - }, - }, - }, cfg.Service) -} - -func TestUnmarshalError(t *testing.T) { - var testCases = []struct { - name string // test case name (also file name containing config yaml) - expected configErrorCode // expected error (if nil any error is acceptable) - expectedMessage string // string that the error must contain - }{ - {name: "duplicate-pipeline", expected: errUnmarshalTopLevelStructure, expectedMessage: "duplicate name"}, - {name: "invalid-logs-level", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-metrics-level", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-pipeline-name-after-slash", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-pipeline-section", expected: errUnmarshalTopLevelStructure, expectedMessage: "pipelines"}, - {name: "invalid-pipeline-sub-config", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-pipeline-type", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-sequence-value", expected: errUnmarshalTopLevelStructure, expectedMessage: "pipelines"}, - {name: "invalid-service-extensions-section", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-service-section", expected: errUnmarshalTopLevelStructure}, - {name: "invalid-top-level-section", expected: errUnmarshalTopLevelStructure, expectedMessage: "top level"}, - } - - factories, err := componenttest.NopFactories() - assert.NoError(t, err) - - for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { - _, err = loadConfigFile(t, filepath.Join("testdata", test.name+".yaml"), factories) - require.Error(t, err) - if test.expected != 0 { - var cfgErr configError - require.ErrorAs(t, err, &cfgErr) - assert.Equal(t, test.expected, cfgErr.code, err) - if test.expectedMessage != "" { - assert.Contains(t, cfgErr.Error(), test.expectedMessage) - } - assert.NotEmpty(t, cfgErr.Error(), "returned config error %v with empty error message", cfgErr.code) - } - }) - } -} - -func loadConfigFile(t *testing.T, fileName string, factories component.Factories) (*Config, error) { - cm, err := confmaptest.LoadConf(fileName) - require.NoError(t, err) - - // Unmarshal the config from the confmap.Conf using the given factories. - return Unmarshal(cm, factories) -} diff --git a/service/internal/configunmarshaler/error.go b/service/internal/configunmarshaler/error.go new file mode 100644 index 000000000000..102c21851d58 --- /dev/null +++ b/service/internal/configunmarshaler/error.go @@ -0,0 +1,30 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configunmarshaler // import "go.opentelemetry.io/collector/service/internal/configunmarshaler" + +import ( + "fmt" + "reflect" + + "go.opentelemetry.io/collector/config" +) + +func errorUnknownType(component string, id config.ComponentID, factories []reflect.Value) error { + return fmt.Errorf("unknown %s type: %q for id: %q (valid values: %v)", component, id.Type(), id, factories) +} + +func errorUnmarshalError(component string, id config.ComponentID, err error) error { + return fmt.Errorf("error reading %s configuration for %q: %w", component, id, err) +} diff --git a/service/internal/configunmarshaler/testdata/duplicate-pipeline.yaml b/service/internal/configunmarshaler/testdata/duplicate-pipeline.yaml deleted file mode 100644 index de1cda0a3979..000000000000 --- a/service/internal/configunmarshaler/testdata/duplicate-pipeline.yaml +++ /dev/null @@ -1,16 +0,0 @@ -receivers: - nop: -exporters: - nop: -processors: - nop: -service: - pipelines: - traces /pipe: - receivers: [nop] - exporters: [nop] - processors: [nop] - traces/ pipe : - receivers: [nop] - exporters: [nop] - processors: [nop] diff --git a/service/internal/configunmarshaler/testdata/invalid-logs-level.yaml b/service/internal/configunmarshaler/testdata/invalid-logs-level.yaml deleted file mode 100644 index a30cd0e71d6d..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-logs-level.yaml +++ /dev/null @@ -1,19 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: -extensions: - nop: -service: - telemetry: - logs: - level: "UNKNOWN" - extensions: [nop] - pipelines: - traces: - receivers: [nop] - processors: [nop] - exporters: [nop] - diff --git a/service/internal/configunmarshaler/testdata/invalid-metrics-level.yaml b/service/internal/configunmarshaler/testdata/invalid-metrics-level.yaml deleted file mode 100644 index d57a14d0ff7b..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-metrics-level.yaml +++ /dev/null @@ -1,19 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: -extensions: - nop: -service: - telemetry: - metrics: - level: "unknown" - extensions: [nop] - pipelines: - traces: - receivers: [nop] - processors: [nop] - exporters: [nop] - diff --git a/service/internal/configunmarshaler/testdata/invalid-pipeline-name-after-slash.yaml b/service/internal/configunmarshaler/testdata/invalid-pipeline-name-after-slash.yaml deleted file mode 100644 index 6df9ffd1e506..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-pipeline-name-after-slash.yaml +++ /dev/null @@ -1,13 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: - -service: - pipelines: - metrics/: - receivers: [nop] - processors: [nop] - exporters: [nop] diff --git a/service/internal/configunmarshaler/testdata/invalid-pipeline-section.yaml b/service/internal/configunmarshaler/testdata/invalid-pipeline-section.yaml deleted file mode 100644 index c507b87bc148..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-pipeline-section.yaml +++ /dev/null @@ -1,20 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: -extensions: - nop: -service: - extensions: - - nop - pipelines: - traces: - receivers: - - nop - processors: - - nop - exporters: - - nop - unknown_section: 1 diff --git a/service/internal/configunmarshaler/testdata/invalid-pipeline-sub-config.yaml b/service/internal/configunmarshaler/testdata/invalid-pipeline-sub-config.yaml deleted file mode 100644 index dbe067632caa..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-pipeline-sub-config.yaml +++ /dev/null @@ -1,9 +0,0 @@ -receivers: - nop: -exporters: - nop: -processors: - nop: -service: - pipelines: - traces diff --git a/service/internal/configunmarshaler/testdata/invalid-pipeline-type.yaml b/service/internal/configunmarshaler/testdata/invalid-pipeline-type.yaml deleted file mode 100644 index 937202168c46..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-pipeline-type.yaml +++ /dev/null @@ -1,13 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: - -service: - pipelines: - /metrics: - receivers: [nop] - processors: [nop] - exporters: [nop] diff --git a/service/internal/configunmarshaler/testdata/invalid-sequence-value.yaml b/service/internal/configunmarshaler/testdata/invalid-sequence-value.yaml deleted file mode 100644 index b537378dbf97..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-sequence-value.yaml +++ /dev/null @@ -1,14 +0,0 @@ -receivers: - nop: -exporters: - nop: -processors: - nop: -service: - pipelines: - traces: - receivers: - nop: - some: config - exporters: [nop] - processors: [nop] diff --git a/service/internal/configunmarshaler/testdata/invalid-service-extensions-section.yaml b/service/internal/configunmarshaler/testdata/invalid-service-extensions-section.yaml deleted file mode 100644 index 1386878e5a89..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-service-extensions-section.yaml +++ /dev/null @@ -1,18 +0,0 @@ -extensions: - nop: -receivers: - nop: -processors: - nop: -exporters: - nop: - -service: - extensions: - nop: - error: true - pipelines: - traces: - receivers: [nop] - processors: [nop] - exporters: [nop] diff --git a/service/internal/configunmarshaler/testdata/invalid-service-section.yaml b/service/internal/configunmarshaler/testdata/invalid-service-section.yaml deleted file mode 100644 index 7dcf713e035e..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-service-section.yaml +++ /dev/null @@ -1,21 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: -extensions: - nop: -service: - extenstions: - - examapleextension - unknown_section: - a_num: 2 - pipelines: - traces: - receivers: - - nop - processors: - - nop - exporters: - - nop diff --git a/service/internal/configunmarshaler/testdata/invalid-top-level-section.yaml b/service/internal/configunmarshaler/testdata/invalid-top-level-section.yaml deleted file mode 100644 index a40aa2efd6e2..000000000000 --- a/service/internal/configunmarshaler/testdata/invalid-top-level-section.yaml +++ /dev/null @@ -1,21 +0,0 @@ -receivers: - nop: -processors: - nop: -exporters: - nop: -extensions: - nop: -service: - extenstions: - - examapleextension - pipelines: - traces: - receivers: - - nop - processors: - - nop - exporters: - - nop -unknown_section: - a_num: 2 diff --git a/service/internal/configunmarshaler/testdata/valid-config.yaml b/service/internal/configunmarshaler/testdata/valid-config.yaml deleted file mode 100644 index 471aa3dcf848..000000000000 --- a/service/internal/configunmarshaler/testdata/valid-config.yaml +++ /dev/null @@ -1,34 +0,0 @@ -receivers: - nop: - -processors: - nop: - -exporters: - nop: - -extensions: - nop: - -service: - telemetry: - logs: - level: "DEBUG" - development: true - encoding: "console" - disable_caller: true - disable_stacktrace: true - output_paths: ["stderr", "./output-logs"] - error_output_paths: ["stderr", "./error-output-logs"] - initial_fields: - field_key: "filed_value" - metrics: - level: "normal" - address: ":8081" - extensions: [nop] - pipelines: - traces: - receivers: [nop] - processors: [nop] - exporters: [nop] - diff --git a/service/internal/pipelines/pipelines_test.go b/service/internal/pipelines/pipelines_test.go index 8c9d2d0d4ea3..858d05c04dd0 100644 --- a/service/internal/pipelines/pipelines_test.go +++ b/service/internal/pipelines/pipelines_test.go @@ -26,6 +26,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config" + "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/confmaptest" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/consumer/consumertest" @@ -434,7 +435,7 @@ func newErrExporterFactory() component.ExporterFactory { ) } -func toSettings(factories component.Factories, cfg *configunmarshaler.Config) Settings { +func toSettings(factories component.Factories, cfg *configSettings) Settings { return Settings{ Telemetry: componenttest.NewNopTelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo(), @@ -464,11 +465,27 @@ func (e errComponent) Shutdown(context.Context) error { return errors.New("my error") } -func loadConfig(t *testing.T, fileName string, factories component.Factories) *configunmarshaler.Config { +// TODO: Remove this by not reading the input from the files, or by providing something similar outside service package. +type configSettings struct { + Receivers *configunmarshaler.Receivers `mapstructure:"receivers"` + Processors *configunmarshaler.Processors `mapstructure:"processors"` + Exporters *configunmarshaler.Exporters `mapstructure:"exporters"` + Service *serviceSettings `mapstructure:"service"` +} + +type serviceSettings struct { + Pipelines map[config.ComponentID]*config.Pipeline `mapstructure:"pipelines"` +} + +func loadConfig(t *testing.T, fileName string, factories component.Factories) *configSettings { // Read yaml config from file conf, err := confmaptest.LoadConf(fileName) require.NoError(t, err) - cfg, err := configunmarshaler.Unmarshal(conf, factories) - require.NoError(t, err) + cfg := &configSettings{ + Receivers: configunmarshaler.NewReceivers(factories.Receivers), + Processors: configunmarshaler.NewProcessors(factories.Processors), + Exporters: configunmarshaler.NewExporters(factories.Exporters), + } + require.NoError(t, conf.Unmarshal(cfg, confmap.WithErrorUnused())) return cfg } diff --git a/service/unmarshaler.go b/service/unmarshaler.go new file mode 100644 index 000000000000..52f85b41cd14 --- /dev/null +++ b/service/unmarshaler.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service // import "go.opentelemetry.io/collector/service" + +import ( + "go.uber.org/zap/zapcore" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/service/internal/configunmarshaler" + "go.opentelemetry.io/collector/service/telemetry" +) + +type configSettings struct { + Receivers *configunmarshaler.Receivers `mapstructure:"receivers"` + Processors *configunmarshaler.Processors `mapstructure:"processors"` + Exporters *configunmarshaler.Exporters `mapstructure:"exporters"` + Extensions *configunmarshaler.Extensions `mapstructure:"extensions"` + Service ConfigService `mapstructure:"service"` +} + +// unmarshal the configSettings from a confmap.Conf. +// After the config is unmarshalled, `Validate()` must be called to validate. +func unmarshal(v *confmap.Conf, factories component.Factories) (*configSettings, error) { + // Unmarshal top level sections and validate. + cfg := &configSettings{ + Receivers: configunmarshaler.NewReceivers(factories.Receivers), + Processors: configunmarshaler.NewProcessors(factories.Processors), + Exporters: configunmarshaler.NewExporters(factories.Exporters), + Extensions: configunmarshaler.NewExtensions(factories.Extensions), + // TODO: Add a component.ServiceFactory to allow this to be defined by the Service. + Service: ConfigService{ + Telemetry: telemetry.Config{ + Logs: telemetry.LogsConfig{ + Level: zapcore.InfoLevel, + Development: false, + Encoding: "console", + OutputPaths: []string{"stderr"}, + ErrorOutputPaths: []string{"stderr"}, + DisableCaller: false, + DisableStacktrace: false, + InitialFields: map[string]interface{}(nil), + }, + Metrics: telemetry.MetricsConfig{ + Level: configtelemetry.LevelBasic, + Address: ":8888", + }, + }, + }, + } + + return cfg, v.Unmarshal(&cfg, confmap.WithErrorUnused()) +} diff --git a/service/unmarshaler_test.go b/service/unmarshaler_test.go new file mode 100644 index 000000000000..1f385047bdcd --- /dev/null +++ b/service/unmarshaler_test.go @@ -0,0 +1,214 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "go.opentelemetry.io/collector/component/componenttest" + "go.opentelemetry.io/collector/config" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/service/telemetry" +) + +func TestUnmarshalEmpty(t *testing.T) { + factories, err := componenttest.NopFactories() + assert.NoError(t, err) + + _, err = unmarshal(confmap.New(), factories) + assert.NoError(t, err) +} + +func TestUnmarshalEmptyAllSections(t *testing.T) { + factories, err := componenttest.NopFactories() + assert.NoError(t, err) + + conf := confmap.NewFromStringMap(map[string]interface{}{ + "receivers": nil, + "processors": nil, + "exporters": nil, + "extensions": nil, + "service": nil, + }) + cfg, err := unmarshal(conf, factories) + assert.NoError(t, err) + + zapProdCfg := zap.NewProductionConfig() + assert.Equal(t, telemetry.LogsConfig{ + Level: zapProdCfg.Level.Level(), + Development: zapProdCfg.Development, + Encoding: "console", + DisableCaller: zapProdCfg.DisableCaller, + DisableStacktrace: zapProdCfg.DisableStacktrace, + OutputPaths: zapProdCfg.OutputPaths, + ErrorOutputPaths: zapProdCfg.ErrorOutputPaths, + InitialFields: zapProdCfg.InitialFields, + }, cfg.Service.Telemetry.Logs) +} + +func TestUnmarshalUnknownTopLevel(t *testing.T) { + factories, err := componenttest.NopFactories() + assert.NoError(t, err) + + conf := confmap.NewFromStringMap(map[string]interface{}{ + "unknown_section": nil, + }) + _, err = unmarshal(conf, factories) + require.Error(t, err) + assert.Contains(t, err.Error(), "'' has invalid keys: unknown_section") +} + +func TestConfigServicePipelineUnmarshalError(t *testing.T) { + var testCases = []struct { + // test case name (also file name containing config yaml) + name string + conf *confmap.Conf + // string that the error must contain + expectError string + }{ + { + name: "duplicate-pipeline", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "traces/ pipe": nil, + "traces /pipe": nil, + }), + expectError: "duplicate name", + }, + { + name: "invalid-pipeline-name-after-slash", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "metrics/": nil, + }), + expectError: "in \"metrics/\" id: the part after / should not be empty", + }, + { + name: "invalid-pipeline-section", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "traces": map[string]interface{}{ + "unknown_section": nil, + }, + }), + expectError: "'[traces]' has invalid keys: unknown_section", + }, + { + name: "invalid-pipeline-sub-config", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "traces": "string", + }), + expectError: "'[traces]' expected a map, got 'string'", + }, + { + name: "invalid-pipeline-type", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "/metrics": nil, + }), + expectError: "in \"/metrics\" id: the part before / should not be empty", + }, + { + name: "invalid-sequence-value", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "traces": map[string]interface{}{ + "receivers": map[string]interface{}{ + "nop": map[string]interface{}{ + "some": "config", + }, + }, + }, + }), + expectError: "'[traces].receivers[0]' has invalid keys: nop", + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + pipelines := make(map[config.ComponentID]ConfigServicePipeline) + err := tt.conf.Unmarshal(&pipelines, confmap.WithErrorUnused()) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectError) + }) + } +} + +func TestServiceUnmarshalError(t *testing.T) { + var testCases = []struct { + // test case name (also file name containing config yaml) + name string + conf *confmap.Conf + // string that the error must contain + expectError string + }{ + { + name: "invalid-logs-level", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "telemetry": map[string]interface{}{ + "logs": map[string]interface{}{ + "level": "UNKNOWN", + }, + }, + }), + expectError: "error decoding 'telemetry.logs.level': unrecognized level: \"UNKNOWN\"", + }, + { + name: "invalid-metrics-level", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "telemetry": map[string]interface{}{ + "metrics": map[string]interface{}{ + "level": "unknown", + }, + }, + }), + expectError: "error decoding 'telemetry.metrics.level': unknown metrics level \"unknown\"", + }, + { + name: "invalid-service-extensions-section", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "extensions": []interface{}{ + map[string]interface{}{ + "nop": map[string]interface{}{ + "some": "config", + }, + }, + }, + }), + expectError: "'extensions[0]' has invalid keys: nop", + }, + { + name: "invalid-service-section", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "unknown_section": "string", + }), + expectError: "'' has invalid keys: unknown_section", + }, + { + name: "invalid-pipelines-config", + conf: confmap.NewFromStringMap(map[string]interface{}{ + "pipelines": "string", + }), + expectError: "'pipelines' expected a map, got 'string'", + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + err := tt.conf.Unmarshal(&ConfigService{}, confmap.WithErrorUnused()) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectError) + }) + } +}