[Metricbeat] Fix: mixed modules fail loading standard and light metri…

…csets (elastic#15011)

* Prepare test case for mixed module

* Ignore registered modules while loading light metricsets

* Test case: fail on unregistered module

* Fix: mage fmt

* Fix: replace String with ...Info

* Rename method to ModulesInfo()

(cherry picked from commit 1769f67)

metricbeat/mb/lightmodules.go

@@ -68,8 +68,8 @@ func (s *LightModulesSource) HasModule(moduleName string) bool {
 }
 
 // DefaultMetricSets list the default metricsets for a given module
-func (s *LightModulesSource) DefaultMetricSets(moduleName string) ([]string, error) {
-	module, err := s.loadModule(moduleName)
+func (s *LightModulesSource) DefaultMetricSets(r *Register, moduleName string) ([]string, error) {
+	module, err := s.loadModule(r, moduleName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to get default metricsets for module '%s'", moduleName)
 	}
@@ -83,8 +83,8 @@ func (s *LightModulesSource) DefaultMetricSets(moduleName string) ([]string, err
 }
 
 // MetricSets list the available metricsets for a given module
-func (s *LightModulesSource) MetricSets(moduleName string) ([]string, error) {
-	module, err := s.loadModule(moduleName)
+func (s *LightModulesSource) MetricSets(r *Register, moduleName string) ([]string, error) {
+	module, err := s.loadModule(r, moduleName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to get metricsets for module '%s'", moduleName)
 	}
@@ -118,7 +118,7 @@ func (s *LightModulesSource) HasMetricSet(moduleName, metricSetName string) bool
 
 // MetricSetRegistration obtains a registration for a light metric set
 func (s *LightModulesSource) MetricSetRegistration(register *Register, moduleName, metricSetName string) (MetricSetRegistration, error) {
-	lightModule, err := s.loadModule(moduleName)
+	lightModule, err := s.loadModule(register, moduleName)
 	if err != nil {
 		return MetricSetRegistration{}, errors.Wrapf(err, "failed to load module '%s'", moduleName)
 	}
@@ -131,12 +131,12 @@ func (s *LightModulesSource) MetricSetRegistration(register *Register, moduleNam
 	return ms.Registration(register)
 }
 
-// String returns a string representation of this source, with a list of known metricsets
-func (s *LightModulesSource) String() string {
+// ModulesInfo returns a string representation of this source, with a list of known metricsets
+func (s *LightModulesSource) ModulesInfo(r *Register) string {
 	var metricSets []string
 	modules, _ := s.Modules()
 	for _, module := range modules {
-		moduleMetricSets, _ := s.MetricSets(module)
+		moduleMetricSets, _ := s.MetricSets(r, module)
 		for _, name := range moduleMetricSets {
 			metricSets = append(metricSets, fmt.Sprintf("%s/%s", module, name))
 		}
@@ -156,7 +156,7 @@ type LightModule struct {
 	MetricSets map[string]LightMetricSet
 }
 
-func (s *LightModulesSource) loadModule(moduleName string) (*LightModule, error) {
+func (s *LightModulesSource) loadModule(register *Register, moduleName string) (*LightModule, error) {
 	modulePath, found := s.findModulePath(moduleName)
 	if !found {
 		return nil, fmt.Errorf("module '%s' not found", moduleName)
@@ -167,7 +167,7 @@ func (s *LightModulesSource) loadModule(moduleName string) (*LightModule, error)
 		return nil, errors.Wrapf(err, "failed to load light module '%s' definition", moduleName)
 	}
 
-	metricSets, err := s.loadMetricSets(filepath.Dir(modulePath), moduleConfig.Name, moduleConfig.MetricSets)
+	metricSets, err := s.loadMetricSets(register, filepath.Dir(modulePath), moduleConfig.Name, moduleConfig.MetricSets)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to load metric sets for light module '%s'", moduleName)
 	}
@@ -198,9 +198,15 @@ func (s *LightModulesSource) loadModuleConfig(modulePath string) (*lightModuleCo
 	return &moduleConfig, nil
 }
 
-func (s *LightModulesSource) loadMetricSets(moduleDirPath, moduleName string, metricSetNames []string) (map[string]LightMetricSet, error) {
+func (s *LightModulesSource) loadMetricSets(register *Register, moduleDirPath, moduleName string, metricSetNames []string) (map[string]LightMetricSet, error) {
 	metricSets := make(map[string]LightMetricSet)
 	for _, metricSet := range metricSetNames {
+		if moduleMetricSets, exists := register.metricSets[moduleName]; exists {
+			if _, exists := moduleMetricSets[metricSet]; exists {
+				continue
+			}
+		}
+
 		manifestPath := filepath.Join(moduleDirPath, metricSet, manifestYML)
 
 		metricSetConfig, err := s.loadMetricSetConfig(manifestPath)

metricbeat/mb/lightmodules_test.go

@@ -168,9 +168,10 @@ func TestLoadModule(t *testing.T) {
 	}
 
 	for _, c := range cases {
+		register := NewRegister()
 		r := NewLightModulesSource("testdata/lightmodules")
 		t.Run(c.name, func(t *testing.T) {
-			_, err := r.loadModule(c.name)
+			_, err := r.loadModule(register, c.name)
 			if c.err {
 				assert.Error(t, err)
 			}
@@ -227,10 +228,20 @@ func TestNewModuleFromConfig(t *testing.T) {
 			config: common.MapStr{"module": "service", "enabled": false},
 			err:    true,
 		},
+		"mixed module with standard and light metricsets": {
+			config:         common.MapStr{"module": "mixed", "metricsets": []string{"standard", "light"}},
+			expectedOption: "default",
+		},
+		"mixed module with unregistered and light metricsets": {
+			config: common.MapStr{"module": "mixedbroken", "metricsets": []string{"unregistered", "light"}},
+			err:    true,
+		},
 	}
 
 	r := NewRegister()
 	r.MustAddMetricSet("foo", "bar", newMetricSetWithOption)
+	r.MustAddMetricSet("foo", "light", newMetricSetWithOption)
+	r.MustAddMetricSet("mixed", "standard", newMetricSetWithOption)
 	r.SetSecondarySource(NewLightModulesSource("testdata/lightmodules"))
 
 	for title, c := range cases {

metricbeat/mb/registry.go

@@ -116,11 +116,11 @@ type Register struct {
 type ModulesSource interface {
 	Modules() ([]string, error)
 	HasModule(module string) bool
-	MetricSets(module string) ([]string, error)
-	DefaultMetricSets(module string) ([]string, error)
+	MetricSets(r *Register, module string) ([]string, error)
+	DefaultMetricSets(r *Register, module string) ([]string, error)
 	HasMetricSet(module, name string) bool
 	MetricSetRegistration(r *Register, module, name string) (MetricSetRegistration, error)
-	String() string
+	ModulesInfo(r *Register) string
 }
 
 // NewRegister creates and returns a new Register.
@@ -280,7 +280,7 @@ func (r *Register) DefaultMetricSets(module string) ([]string, error) {
 	// List also default metrics from secondary sources
 	if source := r.secondarySource; source != nil && source.HasModule(module) {
 		exists = true
-		sourceDefaults, err := source.DefaultMetricSets(module)
+		sourceDefaults, err := source.DefaultMetricSets(r, module)
 		if err != nil {
 			r.log.Errorf("Failed to get default metric sets for module '%s' from secondary source: %s", module, err)
 		} else if len(sourceDefaults) > 0 {
@@ -352,7 +352,7 @@ func (r *Register) MetricSets(module string) []string {
 
 	// List also metric sets from secondary sources
 	if source := r.secondarySource; source != nil && source.HasModule(module) {
-		sourceMetricSets, err := source.MetricSets(module)
+		sourceMetricSets, err := source.MetricSets(r, module)
 		if err != nil {
 			r.log.Errorf("Failed to get metricsets from secondary source: %s", err)
 		}
@@ -390,7 +390,7 @@ func (r *Register) String() string {
 
 	var secondarySource string
 	if source := r.secondarySource; source != nil {
-		secondarySource = ", " + source.String()
+		secondarySource = fmt.Sprintf(", LightModules:[%s]", source.ModulesInfo(r))
 	}
 
 	sort.Strings(modules)

metricbeat/mb/testdata/lightmodules/mixed/light/manifest.yml

@@ -0,0 +1,4 @@
+default: true
+input:
+  module: foo
+  metricset: light

metricbeat/mb/testdata/lightmodules/mixed/module.yml

@@ -0,0 +1,4 @@
+name: mixed
+metricsets:
+- light
+- standard

metricbeat/mb/testdata/lightmodules/mixedbroken/light/manifest.yml

@@ -0,0 +1,4 @@
+default: true
+input:
+  module: foo
+  metricset: light

metricbeat/mb/testdata/lightmodules/mixedbroken/module.yml

@@ -0,0 +1,4 @@
+name: mixedbroken
+metricsets:
+- light
+- unregistered