Skip to content

Commit

Permalink
Fix panic on metricbeat test modules (elastic#18797)
Browse files Browse the repository at this point in the history
Since metricbeat light modules support processors (elastic#15923), module
initialization requires a publisher in the beat so modules can attach
their processors. `metricbeat test modules` is not initializing as
normal metricbeat commands, and it is not initializing any output or
publisher pipeline, so metricbeat panics when trying to initialize
modules with the new method.

This change adds a dummy publisher for this case, and fixes also a
condition that was adding a `nil` module option, causing additional
panics. A test that reproduced the issues is also added.

(cherry picked from commit 25b8bf1)
  • Loading branch information
jsoriano committed May 29, 2020
1 parent 1b13b6d commit 37ace56
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix storage metricset to allow config without region/zone. {issue}17623[17623] {pull}17624[17624]
- Fix overflow on Prometheus rates when new buckets are added on the go. {pull}17753[17753]
- Fix tags_filter for cloudwatch metricset in aws. {pull}18524[18524]
- Fix panic on `metricbeat test modules` when modules are configured in `metricbeat.modules`. {issue}18789[18789] {pull}18797[18797]
- Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802]

*Packetbeat*
Expand Down
17 changes: 17 additions & 0 deletions metricbeat/cmd/test/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/elastic/beats/v7/libbeat/beat"
"github.com/elastic/beats/v7/libbeat/cmd/instance"
"github.com/elastic/beats/v7/libbeat/publisher/pipeline"
"github.com/elastic/beats/v7/libbeat/testing"
"github.com/elastic/beats/v7/metricbeat/beater"
)
Expand All @@ -49,6 +50,8 @@ func GenTestModulesCmd(name, beatVersion string, create beat.Creator) *cobra.Com
os.Exit(1)
}

// A publisher is needed for modules that add their own pipelines
b.Beat.Publisher = newPublisher()
mb, err := create(&b.Beat, b.Beat.BeatConfig)
if err != nil {
fmt.Fprintf(os.Stderr, "Error initializing metricbeat: %s\n", err)
Expand Down Expand Up @@ -78,3 +81,17 @@ func GenTestModulesCmd(name, beatVersion string, create beat.Creator) *cobra.Com
},
}
}

type publisher struct {
beat.PipelineConnector
}

// newPublisher returns a functional publisher that does nothing.
func newPublisher() *publisher {
return &publisher{pipeline.NewNilPipeline()}
}

// SetACKHandler is a dummy implementation of the ack handler for the test publisher.
func (*publisher) SetACKHandler(beat.PipelineACKHandler) error {
return nil
}
2 changes: 1 addition & 1 deletion metricbeat/mb/module/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func ConfiguredModules(modulesData []*common.Config, configModulesData *common.C
var modules []*Wrapper

for _, moduleCfg := range modulesData {
module, err := NewWrapper(moduleCfg, mb.Registry, nil)
module, err := NewWrapper(moduleCfg, mb.Registry, moduleOptions...)
if err != nil {
return nil, err
}
Expand Down
15 changes: 15 additions & 0 deletions metricbeat/tests/system/test_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ def test_modules_test(self):
assert self.log_contains("cpu...OK")
assert self.log_contains("memory...OK")

def test_modules_test_with_module_in_main_config(self):
self.render_config_template(reload=False, modules=[{
"name": "system",
"metricsets": ["cpu", "memory"],
"period": "10s",
}])

exit_code = self.run_beat(
logging_args=None,
extra_args=["test", "modules"])

assert exit_code == 0
assert self.log_contains("cpu...OK")
assert self.log_contains("memory...OK")

def test_modules_test_error(self):
"""
Test test modules command with an error result
Expand Down

0 comments on commit 37ace56

Please sign in to comment.