Add mapping for munin and options to override service type and name

CHANGELOG.next.asciidoc

@@ -73,6 +73,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 
 *Metricbeat*
 
+- Refactor munin module to collect an event per plugin and to have more strict field mappings {pull}10322[10322]
 - Refactor Prometheus metric mappings {pull}9948[9948]
 - Removed Prometheus stats metricset in favor of just using Prometheus collector {pull}9948[9948]
 - Adjust Redis.info metricset fields to ECS. {pull}10319[10319]

metricbeat/docs/fields.asciidoc

@@ -17339,11 +17339,25 @@ Munin node metrics exporter
 
 
 
-[float]
-== munin fields
+*`munin.metrics.*`*::
++
+--
+type: object
+
+Metrics exposed by a plugin of a munin node agent.
+
+
+--
 
-munin contains metrics exposed by a munin node agent
+*`munin.plugin.name`*::
++
+--
+type: keyword
 
+Name of the plugin collecting these metrics.
+
+
+--
 
 
 [[exported-fields-mysql]]

metricbeat/docs/modules/munin.asciidoc

@@ -33,7 +33,15 @@ metricbeat.modules:
   enabled: true
   period: 10s
   hosts: ["localhost:4949"]
-  node.namespace: node
+
+  # List of plugins to collect metrics from, by default it collects from
+  # all the available ones.
+  #munin.plugins: []
+
+  # If set to true, it sanitizes fields names in concordance with munin
+  # implementation (all characters that are not alphanumeric, or underscore
+  # are replaced by underscores).
+  #munin.sanitize: false
 ----
 
 [float]

metricbeat/metricbeat.reference.yml

@@ -507,7 +507,15 @@ metricbeat.modules:
   enabled: true
   period: 10s
   hosts: ["localhost:4949"]
-  node.namespace: node
+
+  # List of plugins to collect metrics from, by default it collects from
+  # all the available ones.
+  #munin.plugins: []
+
+  # If set to true, it sanitizes fields names in concordance with munin
+  # implementation (all characters that are not alphanumeric, or underscore
+  # are replaced by underscores).
+  #munin.sanitize: false
 
 #-------------------------------- MySQL Module -------------------------------
 - module: mysql

metricbeat/module/munin/_meta/config.reference.yml

@@ -3,4 +3,12 @@
   enabled: true
   period: 10s
   hosts: ["localhost:4949"]
-  node.namespace: node
+
+  # List of plugins to collect metrics from, by default it collects from
+  # all the available ones.
+  #munin.plugins: []
+
+  # If set to true, it sanitizes fields names in concordance with munin
+  # implementation (all characters that are not alphanumeric, or underscore
+  # are replaced by underscores).
+  #munin.sanitize: false

metricbeat/module/munin/_meta/config.yml

@@ -3,4 +3,3 @@
   #  - node
   period: 10s
   hosts: ["localhost:4949"]
-  node.namespace: node

metricbeat/module/munin/_meta/fields.yml

@@ -4,8 +4,16 @@
     Munin node metrics exporter
   release: beta
   fields:
+    - name: munin.metrics.*
+      type: object
+      object_type: double
+      object_type_mapping_type: '*'
+      description: >
+        Metrics exposed by a plugin of a munin node agent.
+    - name: munin.plugin.name
+      type: keyword
+      description: >
+        Name of the plugin collecting these metrics.
     - name: munin
       type: group
-      description: >
-          munin contains metrics exposed by a munin node agent
       fields:

metricbeat/module/munin/munin.go

@@ -19,23 +19,29 @@ package munin
 
 import (
 	"bufio"
-	"fmt"
 	"io"
 	"net"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
 
-	"github.com/joeshaw/multierror"
 	"github.com/pkg/errors"
 
 	"github.com/elastic/beats/libbeat/common"
+	"github.com/elastic/beats/libbeat/logp"
 )
 
 const (
 	unknownValue = "U"
 )
 
+var (
+	// Field names must match with this expression
+	// http://guide.munin-monitoring.org/en/latest/reference/plugin.html#notes-on-fieldnames
+	nameRegexp = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9_]*$")
+)
+
 // Node connection
 type Node struct {
 	conn net.Conn
@@ -66,7 +72,7 @@ func (n *Node) Close() error {
 	return n.conn.Close()
 }
 
-// List of items exposed by the node
+// List of plugins exposed by the node
 func (n *Node) List() ([]string, error) {
 	_, err := io.WriteString(n.writer, "list\n")
 	if err != nil {
@@ -79,54 +85,64 @@ func (n *Node) List() ([]string, error) {
 }
 
 // Fetch metrics from munin node
-func (n *Node) Fetch(items ...string) (common.MapStr, error) {
-	var errs multierror.Errors
+func (n *Node) Fetch(plugin string, sanitize bool) (common.MapStr, error) {
+	_, err := io.WriteString(n.writer, "fetch "+plugin+"\n")
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to fetch metrics for plugin '%s'", plugin)
+	}
+
 	event := common.MapStr{}
+	scanner := bufio.NewScanner(n.reader)
+	scanner.Split(bufio.ScanWords)
+	for scanner.Scan() {
+		name := strings.TrimSpace(scanner.Text())
 
-	for _, item := range items {
-		_, err := io.WriteString(n.writer, "fetch "+item+"\n")
-		if err != nil {
-			errs = append(errs, err)
-			continue
+		// Munin delimits metrics with a dot
+		if name == "." {
+			break
 		}
 
-		scanner := bufio.NewScanner(n.reader)
-		scanner.Split(bufio.ScanWords)
-		for scanner.Scan() {
-			name := strings.TrimSpace(scanner.Text())
-
-			// Munin delimits metrics with a dot
-			if name == "." {
-				break
-			}
-
-			name = strings.TrimSuffix(name, ".value")
-
-			if !scanner.Scan() {
-				if scanner.Err() == nil {
-					errs = append(errs, errors.New("unexpected EOF when expecting value"))
-				}
-				break
+		name = strings.TrimSuffix(name, ".value")
+		if !scanner.Scan() {
+			if scanner.Err() == nil {
+				return nil, errors.New("unexpected EOF when expecting value")
 			}
-			value := scanner.Text()
+		}
+		value := scanner.Text()
 
-			key := fmt.Sprintf("%s.%s", item, name)
+		if strings.Contains(name, ".") {
+			logp.Debug("munin", "ignoring field name with dot '%s'", name)
+			continue
+		}
 
-			if value == unknownValue {
-				errs = append(errs, errors.Errorf("unknown value for %s", key))
-				continue
-			}
-			if f, err := strconv.ParseFloat(value, 64); err == nil {
-				event.Put(key, f)
-				continue
-			}
-			event.Put(key, value)
+		if value == unknownValue {
+			logp.Debug("munin", "unknown value for '%s'", name)
+			continue
 		}
 
-		if scanner.Err() != nil {
-			errs = append(errs, scanner.Err())
+		if sanitize && !nameRegexp.MatchString(name) {
+			logp.Debug("munin", "sanitizing name with invalid characters '%s'", name)
+			name = sanitizeName(name)
+		}
+		if f, err := strconv.ParseFloat(value, 64); err == nil {
+			event[name] = f
+			continue
 		}
 	}
 
-	return event, errs.Err()
+	if err := scanner.Err(); err != nil {
+		return nil, err
+	}
+
+	return event, nil
+}
+
+var (
+	invalidCharactersRegexp = regexp.MustCompile("(^[^a-zA-Z_]|[^a-zA-Z_0-9])")
+)
+
+// Mimic munin master implementation
+// https://github.com/munin-monitoring/munin/blob/20abb861/lib/Munin/Master/Node.pm#L385
+func sanitizeName(name string) string {
+	return invalidCharactersRegexp.ReplaceAllString(name, "_")
 }