From 2c61ce91878a0a7f297964dde05bbc544a738f1d Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 27 May 2020 20:54:03 +0200 Subject: [PATCH] Fix panic on `metricbeat test modules` Since metricbeat light modules support processors (#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 issue is also added. --- CHANGELOG.next.asciidoc | 1 + metricbeat/cmd/test/modules.go | 16 ++++++++++++++++ metricbeat/mb/module/configuration.go | 2 +- metricbeat/tests/system/test_cmd.py | 15 +++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 446ece49a55..610ebf91410 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -204,6 +204,7 @@ field. You can revert this change by configuring tags for the module and omittin - Remove specific win32 api errors from events in perfmon. {issue}18292[18292] {pull}18361[18361] - Fix application_pool metricset after pdh changes. {pull}18477[18477] - 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}[] *Packetbeat* diff --git a/metricbeat/cmd/test/modules.go b/metricbeat/cmd/test/modules.go index 93950973a5d..efc169653cb 100644 --- a/metricbeat/cmd/test/modules.go +++ b/metricbeat/cmd/test/modules.go @@ -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" ) @@ -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) @@ -78,3 +81,16 @@ 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()} +} + +func (*publisher) SetACKHandler(beat.PipelineACKHandler) error { + return nil +} diff --git a/metricbeat/mb/module/configuration.go b/metricbeat/mb/module/configuration.go index 8b0701c0ae9..d5049690b48 100644 --- a/metricbeat/mb/module/configuration.go +++ b/metricbeat/mb/module/configuration.go @@ -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 } diff --git a/metricbeat/tests/system/test_cmd.py b/metricbeat/tests/system/test_cmd.py index 740beedb97a..ad9a507d08c 100644 --- a/metricbeat/tests/system/test_cmd.py +++ b/metricbeat/tests/system/test_cmd.py @@ -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