Refactor config: pipelines now under service (#376)

* Refactor config: pipelines now under service

This change moves 'pipelines' sections from the top level to under the 'service' section.

* Fix typo in old comment

Author: Paulo Janotti

README.md

@@ -139,7 +139,8 @@ exporters:
   ...
 processors:
   ...
-pipelines:
+service:
+  pipelines:
   ...
 ```
 
@@ -218,12 +219,12 @@ a receiver/exporter is reference by all the pipelines.
 The following is an example pipeline configuration. For more information, refer
 to [pipeline documentation](docs/pipelines.md)
 ```yaml
-pipelines:
-  traces:
-    receivers: [examplereceiver]
-    processors: [exampleprocessor]
-    exporters: [exampleexporter]
-
+service:
+  pipelines:
+    traces:
+      receivers: [examplereceiver]
+      processors: [exampleprocessor]
+      exporters: [exampleexporter]
 ```
 
 ### <a name="config-diagnostics"></a>Diagnostics

config/config.go

@@ -137,7 +137,6 @@ func Load(
 		Receivers  map[string]interface{} `mapstructure:"receivers"`
 		Processors map[string]interface{} `mapstructure:"processors"`
 		Exporters  map[string]interface{} `mapstructure:"exporters"`
-		Pipelines  map[string]interface{} `mapstructure:"pipelines"`
 	}
 
 	if err := v.UnmarshalExact(&topLevelSections); err != nil {
@@ -147,21 +146,15 @@ func Load(
 		}
 	}
 
-	// Start with extensions and service.
+	// Start with the service extensions.
 
 	extensions, err := loadExtensions(v, factories.Extensions)
 	if err != nil {
 		return nil, err
 	}
 	config.Extensions = extensions
 
-	service, err := loadService(v)
-	if err != nil {
-		return nil, err
-	}
-	config.Service = service
-
-	// Load data components (receivers, exporters, processores, and pipelines).
+	// Load data components (receivers, exporters, and processors).
 
 	receivers, err := loadReceivers(v, factories.Receivers)
 	if err != nil {
@@ -181,11 +174,12 @@ func Load(
 	}
 	config.Processors = processors
 
-	pipelines, err := loadPipelines(v)
+	// Load the service and its data pipelines.
+	service, err := loadService(v)
 	if err != nil {
 		return nil, err
 	}
-	config.Pipelines = pipelines
+	config.Service = service
 
 	// Config is loaded. Now validate it.
 
@@ -296,13 +290,27 @@ func loadExtensions(v *viper.Viper, factories map[string]extension.Factory) (con
 
 func loadService(v *viper.Viper) (configmodels.Service, error) {
 	var service configmodels.Service
-	if err := v.UnmarshalKey(serviceKeyName, &service, errorOnUnused); err != nil {
+	serviceSub := getConfigSection(v, serviceKeyName)
+
+	// Process the pipelines first so in case of error on them it can be properly
+	// reported.
+	pipelines, err := loadPipelines(serviceSub)
+	if err != nil {
+		return service, err
+	}
+
+	// Do an exact match to find any unused section on config.
+	if err := serviceSub.UnmarshalExact(&service); err != nil {
 		return service, &configError{
 			code: errUnmarshalErrorOnService,
 			msg:  fmt.Sprintf("error reading settings for %q: %v", serviceKeyName, err),
 		}
 	}
 
+	// Unmarshal cannot properly build Pipelines field, set it to the value
+	// previously loaded.
+	service.Pipelines = pipelines
+
 	return service, nil
 }
 
@@ -640,12 +648,12 @@ func validateServiceExtensions(
 
 func validatePipelines(cfg *configmodels.Config, logger *zap.Logger) error {
 	// Must have at least one pipeline.
-	if len(cfg.Pipelines) < 1 {
+	if len(cfg.Service.Pipelines) < 1 {
 		return &configError{code: errMissingPipelines, msg: "must have at least one pipeline"}
 	}
 
 	// Validate pipelines.
-	for _, pipeline := range cfg.Pipelines {
+	for _, pipeline := range cfg.Service.Pipelines {
 		if err := validatePipeline(cfg, pipeline, logger); err != nil {
 			return err
 		}
@@ -849,7 +857,7 @@ func validateProcessors(cfg *configmodels.Config) {
 // getConfigSection returns a sub-config from the viper config that has the corresponding given key.
 // It also expands all the string values.
 func getConfigSection(v *viper.Viper, key string) *viper.Viper {
-	// Unmarsh only the subconfig for this processor.
+	// Unmarshal only the subconfig for this processor.
 	sv := v.Sub(key)
 	if sv == nil {
 		// When the config for this key is empty Sub returns nil. In order to avoid nil checks

config/config_test.go

@@ -111,7 +111,7 @@ func TestDecodeConfig(t *testing.T) {
 		"Did not load processor config correctly")
 
 	// Verify Pipelines
-	assert.Equal(t, 1, len(config.Pipelines), "Incorrect pipelines count")
+	assert.Equal(t, 1, len(config.Service.Pipelines), "Incorrect pipelines count")
 
 	assert.Equal(t,
 		&configmodels.Pipeline{
@@ -121,7 +121,7 @@ func TestDecodeConfig(t *testing.T) {
 			Processors: []string{"exampleprocessor"},
 			Exporters:  []string{"exampleexporter"},
 		},
-		config.Pipelines["traces"],
+		config.Service.Pipelines["traces"],
 		"Did not load pipeline config correctly")
 }
 
@@ -275,7 +275,7 @@ func TestSimpleConfig(t *testing.T) {
 			"TEST[%s] Did not load processor config correctly", test.name)
 
 		// Verify Pipelines
-		assert.Equalf(t, 1, len(config.Pipelines), "TEST[%s]", test.name)
+		assert.Equalf(t, 1, len(config.Service.Pipelines), "TEST[%s]", test.name)
 
 		assert.Equalf(t,
 			&configmodels.Pipeline{
@@ -285,7 +285,7 @@ func TestSimpleConfig(t *testing.T) {
 				Processors: []string{"exampleprocessor"},
 				Exporters:  []string{"exampleexporter"},
 			},
-			config.Pipelines["traces"],
+			config.Service.Pipelines["traces"],
 			"TEST[%s] Did not load pipeline config correctly", test.name)
 	}
 }

config/configmodels/configmodels.go

@@ -36,7 +36,6 @@ type Config struct {
 	Receivers  Receivers
 	Exporters  Exporters
 	Processors Processors
-	Pipelines  Pipelines
 	Extensions Extensions
 	Service    Service
 }
@@ -144,6 +143,9 @@ type Extensions map[string]Extension
 type Service struct {
 	// Extensions is the ordered list of extensions configured for the service.
 	Extensions []string `mapstructure:"extensions"`
+
+	// Pipelines is the set of data pipelines configured for the service.
+	Pipelines Pipelines `mapstructure:"pipelines"`
 }
 
 // Below are common setting structs for Receivers, Exporters and Processors.

config/testdata/duplicate-exporter.yaml

@@ -5,8 +5,9 @@ exporters:
   exampleexporter/ exp :
 processors:
   exampleprocessor:
-pipelines:
-  traces:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
+service:
+  pipelines:
+    traces:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]

config/testdata/duplicate-pipeline.yaml

@@ -4,12 +4,13 @@ exporters:
   exampleexporter:
 processors:
   exampleprocessor:
-pipelines:
-  traces/default:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
-  traces/ default:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
+service:
+  pipelines:
+    traces/default:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]
+    traces/ default:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]

config/testdata/duplicate-processor.yaml

@@ -5,8 +5,9 @@ exporters:
 processors:
   exampleprocessor/       abc:
   exampleprocessor/abc:
-pipelines:
-  traces:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
+service:
+  pipelines:
+    traces:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]

config/testdata/duplicate-receiver.yaml

@@ -5,8 +5,9 @@ exporters:
   exampleexporter:
 processors:
   exampleprocessor:
-pipelines:
-  traces:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
+service:
+  pipelines:
+    traces:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]

config/testdata/invalid-bool-value.yaml

@@ -5,8 +5,9 @@ exporters:
 processors:
   exampleprocessor:
     disabled: [2,3]
-pipelines:
-  traces:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
+service:
+  pipelines:
+    traces:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]

config/testdata/invalid-disabled-bool-value.yaml

@@ -5,8 +5,9 @@ exporters:
     disabled: "string for bool"
 processors:
   exampleprocessor:
-pipelines:
-  traces:
-    receivers: [examplereceiver]
-    exporters: [exampleexporter]
-    processors: [exampleprocessor]
+service:
+  pipelines:
+    traces:
+      receivers: [examplereceiver]
+      exporters: [exampleexporter]
+      processors: [exampleprocessor]