From 799c1a88af842c4f7cfd2ecf2f6fd3d23db5bff3 Mon Sep 17 00:00:00 2001 From: Manoj Shetty <97392538+ManojS-shetty@users.noreply.github.com> Date: Tue, 26 Apr 2022 17:50:10 +0530 Subject: [PATCH] Improve handling of disabled commands in Zookeeper Metricbeat module (#31013) * Improve handing of disabled commands in Zookeeper Metricbeat module * Added fmt library to connection in zookeeper * Removed the "error" argument in conns.parseCons func * Update CHANGELOG.next.asciidoc * Added errors.unwrap() to remove the package error * added a handler over second return value * adding string type in errorf statement removing all type * Added zookeeper asciidoc file * improved Error message in data.go Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- CHANGELOG.next.asciidoc | 1 + metricbeat/docs/modules/zookeeper.asciidoc | 2 +- metricbeat/module/zookeeper/_meta/docs.asciidoc | 2 +- .../module/zookeeper/connection/connection.go | 12 ++++++------ .../module/zookeeper/connection/connection_test.go | 5 +---- metricbeat/module/zookeeper/connection/data.go | 13 +++++-------- 6 files changed, 15 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 7d208e2f155a..d9cef64a49a4 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -74,6 +74,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...main[Check the HEAD dif - Add back missing metrics to system/linux. {pull}30774[30774] - GCP metrics query instances with aggregatedList API to improve efficiency. {pull}30154[#30153] - Fix Jolokia module to print URI for one of the debug logs. {pull}30943[#30943] +- Improve handling of disabled commands in Zookeeper Metricbeat module. {pull}31013[#31013] - Handle docker reporting different capitalization for disk usage metrics. {pull}30978[#30978] *Packetbeat* diff --git a/metricbeat/docs/modules/zookeeper.asciidoc b/metricbeat/docs/modules/zookeeper.asciidoc index a74b767df289..7798a40a8013 100644 --- a/metricbeat/docs/modules/zookeeper.asciidoc +++ b/metricbeat/docs/modules/zookeeper.asciidoc @@ -22,7 +22,7 @@ metricsets are `mntr` and `server`. The ZooKeeper metricsets were tested with ZooKeeper 3.4.8, 3.6.0 and 3.7.0. They are expected to work with all versions >= 3.4.0. Versions prior to 3.4 do not support the `mntr` command. -Note that from ZooKeeper 3.6.0, `mntr` command must be explicitly enabled at ZooKeeper side using the `4lw.commands.whitelist` configuration parameter. +Note that from ZooKeeper 3.6.0, `mntr`, `stat`, `ruok`, `conf`, `isro`, `cons` command must be explicitly enabled at ZooKeeper side using the `4lw.commands.whitelist` configuration parameter. [float] === Dashboard diff --git a/metricbeat/module/zookeeper/_meta/docs.asciidoc b/metricbeat/module/zookeeper/_meta/docs.asciidoc index b2d85523396f..2193d21ab009 100644 --- a/metricbeat/module/zookeeper/_meta/docs.asciidoc +++ b/metricbeat/module/zookeeper/_meta/docs.asciidoc @@ -11,7 +11,7 @@ metricsets are `mntr` and `server`. The ZooKeeper metricsets were tested with ZooKeeper 3.4.8, 3.6.0 and 3.7.0. They are expected to work with all versions >= 3.4.0. Versions prior to 3.4 do not support the `mntr` command. -Note that from ZooKeeper 3.6.0, `mntr` command must be explicitly enabled at ZooKeeper side using the `4lw.commands.whitelist` configuration parameter. +Note that from ZooKeeper 3.6.0, `mntr`, `stat`, `ruok`, `conf`, `isro`, `cons` command must be explicitly enabled at ZooKeeper side using the `4lw.commands.whitelist` configuration parameter. [float] === Dashboard diff --git a/metricbeat/module/zookeeper/connection/connection.go b/metricbeat/module/zookeeper/connection/connection.go index 0ea86368209a..3075f7ff8044 100644 --- a/metricbeat/module/zookeeper/connection/connection.go +++ b/metricbeat/module/zookeeper/connection/connection.go @@ -18,7 +18,7 @@ package connection import ( - "github.com/pkg/errors" + "fmt" "github.com/elastic/beats/v7/metricbeat/mb" "github.com/elastic/beats/v7/metricbeat/mb/parse" @@ -61,21 +61,21 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { func (m *MetricSet) Fetch(reporter mb.ReporterV2) error { outputReader, err := zookeeper.RunCommand("cons", m.Host(), m.Module().Config().Timeout) if err != nil { - return errors.Wrap(err, "'cons' command failed") + return fmt.Errorf("'cons' command failed: %w", err) } - events, err := m.parseCons(outputReader) + events := m.parseCons(outputReader) if err != nil { - return errors.Wrap(err, "error parsing response from zookeeper") + return fmt.Errorf("error parsing response from zookeeper: %w", err) } serverID, err := zookeeper.ServerID(m.Host(), m.Module().Config().Timeout) if err != nil { - return errors.Wrap(err, "error obtaining server id") + return fmt.Errorf("error obtaining server id %w", err) } for _, event := range events { - event.RootFields.Put("service.node.name", serverID) + _, _ = event.RootFields.Put("service.node.name", serverID) reporter.Event(event) } diff --git a/metricbeat/module/zookeeper/connection/connection_test.go b/metricbeat/module/zookeeper/connection/connection_test.go index 6477a7776320..7576183addcc 100644 --- a/metricbeat/module/zookeeper/connection/connection_test.go +++ b/metricbeat/module/zookeeper/connection/connection_test.go @@ -34,10 +34,7 @@ var srvrTestInput = `/172.17.0.1:55218[0](queued=0,recved=1,sent=0) func TestParser(t *testing.T) { conns := MetricSet{} - mapStr, err := conns.parseCons(bytes.NewReader([]byte(srvrTestInput))) - if err != nil { - t.Fatal(err) - } + mapStr := conns.parseCons(bytes.NewReader([]byte(srvrTestInput))) assert.True(t, len(mapStr) == 3) firstLine := mapStr[0] secondLine := mapStr[1] diff --git a/metricbeat/module/zookeeper/connection/data.go b/metricbeat/module/zookeeper/connection/data.go index e205ba8594f9..5e1842dcc7bf 100644 --- a/metricbeat/module/zookeeper/connection/data.go +++ b/metricbeat/module/zookeeper/connection/data.go @@ -19,12 +19,11 @@ package connection import ( "bufio" + "fmt" "io" "regexp" "strconv" - "github.com/pkg/errors" - "github.com/elastic/beats/v7/metricbeat/mb" "github.com/elastic/beats/v7/libbeat/common" @@ -32,7 +31,7 @@ import ( var capturer = regexp.MustCompile(`/(?P.*):(?P\d+)\[(?P\d*)]\(queued=(?P\d*),recved=(?P\d*),sent=(?P\d*)\)`) -func (m *MetricSet) parseCons(i io.Reader) ([]mb.Event, error) { +func (m *MetricSet) parseCons(i io.Reader) []mb.Event { scanner := bufio.NewScanner(i) result := make([]mb.Event, 0) @@ -45,7 +44,7 @@ func (m *MetricSet) parseCons(i io.Reader) ([]mb.Event, error) { oneParsingIsCorrect := false keyMap, err := lineToMap(line) if err != nil { - m.Logger().Debugf(err.Error()) + m.Logger().Errorf("Error while parsing zookeeper 'cons' command %s", err.Error()) continue } @@ -70,14 +69,14 @@ func (m *MetricSet) parseCons(i io.Reader) ([]mb.Event, error) { } } - return result, nil + return result } func lineToMap(line string) (map[string]string, error) { capturedPatterns := capturer.FindStringSubmatch(line) if len(capturedPatterns) < 1 { //Nothing captured - return nil, errors.Errorf("no data captured in '%s'", line) + return nil, fmt.Errorf("no data captured,'%s'", line) } keyMap := make(map[string]string) @@ -105,6 +104,4 @@ func (m *MetricSet) checkRegexAndSetInt(output common.MapStr, capturedData strin } else { m.Logger().Errorf("parse error: empty data for key '%s'", key) } - - return }