Metricbeat: Remove duplicated filesystems in system module (#6819)

* Metricbeat: Remove duplicated filesystems in system module

Mountpoints whose device name is the same absolute path are considered
to be the same and are counted only once. The directory name used is
the shorter between them. This avoids duplication of filesystems as can
happen with bind mounts in Linux (#3384).

* Metricbeat: Ignore nodev filesystems by default

In system module, when no filesystem type is set to be ignored, it
tries to make a sane guess. In systems with /proc/filesystems file it
ignores all devices marked as `nodev`.

CHANGELOG.asciidoc

@@ -76,6 +76,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
 - Fix namespace disambiguation in Kubernetes state_* metricsets. {issue}6281[6281]
 - Fix Windows perfmon metricset so that it sends metrics when an error occurs. {pull}6542[6542]
 - Fix Kubernetes calculated fields store. {pull}6564{6564}
+- Exclude bind mounts in fsstat and filesystem metricsets. {pull}6819[6819]
 
 *Packetbeat*
 
@@ -188,6 +189,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
 - Set `stubstatus` as default metricset for nginx module. {pull}6770[6770]
 - Added support for haproxy 1.7 and 1.8. {pull}6793[6793]
 - Add accumulated I/O stats to diskio in the line of `docker stats`. {pull}6701[6701]
+- Ignore virtual filesystem types by default in system module. {pull}6819[6819]
 
 *Packetbeat*
 

metricbeat/docs/fields.asciidoc

@@ -12914,7 +12914,7 @@ The percentage of used disk space.
 [float]
 == fsstat fields
 
-`system.fsstat` contains filesystem metrics aggregated from all mounted filesystems, similar with what `df -a` prints out.
+`system.fsstat` contains filesystem metrics aggregated from all mounted filesystems.
 
 
 

metricbeat/docs/modules/system.asciidoc

@@ -54,6 +54,9 @@ metricbeat.modules:
   # A list of filesystem types to ignore. The filesystem metricset will not
   # collect data from filesystems matching any of the specified types, and
   # fsstats will not include data from these filesystems in its summary stats.
+  # If not set, types associated to virtual filesystems are automatically
+  # added when this information is available in the system (e.g. the list of
+  # `nodev` types in `/proc/filesystem`).
   #filesystem.ignore_types: []
 
   # These options allow you to filter out all processes that are not

metricbeat/metricbeat.reference.yml

@@ -74,6 +74,9 @@ metricbeat.modules:
   # A list of filesystem types to ignore. The filesystem metricset will not
   # collect data from filesystems matching any of the specified types, and
   # fsstats will not include data from these filesystems in its summary stats.
+  # If not set, types associated to virtual filesystems are automatically
+  # added when this information is available in the system (e.g. the list of
+  # `nodev` types in `/proc/filesystem`).
   #filesystem.ignore_types: []
 
   # These options allow you to filter out all processes that are not

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

@@ -24,6 +24,9 @@
   # A list of filesystem types to ignore. The filesystem metricset will not
   # collect data from filesystems matching any of the specified types, and
   # fsstats will not include data from these filesystems in its summary stats.
+  # If not set, types associated to virtual filesystems are automatically
+  # added when this information is available in the system (e.g. the list of
+  # `nodev` types in `/proc/filesystem`).
   #filesystem.ignore_types: []
 
   # These options allow you to filter out all processes that are not

metricbeat/module/system/filesystem/_meta/docs.asciidoc

@@ -14,7 +14,9 @@ This metricset is available on:
 
 *`filesystem.ignore_types`* - A list of filesystem types to ignore. Metrics will
 not be collected from filesystems matching these types. This setting also
-affects the `fsstats` metricset.
+affects the `fsstats` metricset. If this option is not set, metricbeat ignores
+all types for virtual devices in systems where this information is available (e.g.
+all types marked as `nodev` in `/proc/filesystems` in Linux systems).
 
 [float]
 === Filtering

metricbeat/module/system/filesystem/filesystem.go

@@ -3,6 +3,8 @@
 package filesystem
 
 import (
+	"strings"
+
 	"github.com/elastic/beats/libbeat/common"
 	"github.com/elastic/beats/libbeat/logp"
 	"github.com/elastic/beats/metricbeat/mb"
@@ -32,6 +34,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 		return nil, err
 	}
 
+	if config.IgnoreTypes == nil {
+		config.IgnoreTypes = DefaultIgnoredTypes()
+	}
+	if len(config.IgnoreTypes) > 0 {
+		logp.Info("Ignoring filesystem types: %s", strings.Join(config.IgnoreTypes, ", "))
+	}
+
 	return &MetricSet{
 		BaseMetricSet: base,
 		config:        config,

metricbeat/module/system/filesystem/helper.go

@@ -3,12 +3,17 @@
 package filesystem
 
 import (
+	"bufio"
+	"os"
+	"path"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"runtime"
 
 	"github.com/elastic/beats/libbeat/common"
+	"github.com/elastic/beats/metricbeat/module/system"
 	sigar "github.com/elastic/gosigar"
 )
 
@@ -31,19 +36,57 @@ func GetFileSystemList() ([]sigar.FileSystem, error) {
 		return nil, err
 	}
 
-	// Ignore relative mount points, which are present for example
-	// in /proc/mounts on Linux with network namespaces.
-	filtered := fss.List[:0]
-	for _, fs := range fss.List {
-		if filepath.IsAbs(fs.DirName) {
+	if runtime.GOOS == "windows" {
+		// No filtering on Windows
+		return fss.List, nil
+	}
+
+	return filterFileSystemList(fss.List), nil
+}
+
+// filterFileSystemList filters mountpoints to avoid virtual filesystems
+// and duplications
+func filterFileSystemList(fsList []sigar.FileSystem) []sigar.FileSystem {
+	var filtered []sigar.FileSystem
+	devices := make(map[string]sigar.FileSystem)
+	for _, fs := range fsList {
+		// Ignore relative mount points, which are present for example
+		// in /proc/mounts on Linux with network namespaces.
+		if !filepath.IsAbs(fs.DirName) {
+			debugf("Filtering filesystem with relative mountpoint %+v", fs)
+			continue
+		}
+
+		// Don't do further checks in special devices
+		if !filepath.IsAbs(fs.DevName) {
 			filtered = append(filtered, fs)
 			continue
 		}
-		debugf("Filtering filesystem with relative mountpoint %+v", fs)
+
+		// If the device name is a directory, this is a bind mount or nullfs,
+		// don't count it as it'd be counting again its parent filesystem.
+		devFileInfo, _ := os.Stat(fs.DevName)
+		if devFileInfo != nil && devFileInfo.IsDir() {
+			continue
+		}
+
+		// If a block device is mounted multiple times (e.g. with some bind mounts),
+		// store it only once, and use the shorter mount point path.
+		if seen, found := devices[fs.DevName]; found {
+			if len(fs.DirName) < len(seen.DirName) {
+				devices[fs.DevName] = fs
+			}
+			continue
+		}
+
+		devices[fs.DevName] = fs
+	}
+
+	for _, fs := range devices {
+		filtered = append(filtered, fs)
 	}
-	fss.List = filtered
 
-	return fss.List, nil
+	return filtered
 }
 
 func GetFileSystemStat(fs sigar.FileSystem) (*FileSystemStat, error) {
@@ -126,3 +169,21 @@ func BuildTypeFilter(ignoreType ...string) Predicate {
 		return true
 	}
 }
+
+// DefaultIgnoredTypes tries to guess a sane list of filesystem types that
+// could be ignored in the running system
+func DefaultIgnoredTypes() (types []string) {
+	// If /proc/filesystems exist, default ignored types are all marked
+	// as nodev
+	fsListFile := path.Join(*system.HostFS, "/proc/filesystems")
+	if f, err := os.Open(fsListFile); err == nil {
+		scanner := bufio.NewScanner(f)
+		for scanner.Scan() {
+			line := strings.Fields(scanner.Text())
+			if len(line) == 2 && line[0] == "nodev" {
+				types = append(types, line[1])
+			}
+		}
+	}
+	return
+}

metricbeat/module/system/filesystem/helper_test.go

@@ -4,6 +4,7 @@
 package filesystem
 
 import (
+	"io/ioutil"
 	"os"
 	"runtime"
 	"testing"
@@ -47,6 +48,98 @@ func TestFileSystemList(t *testing.T) {
 	}
 }
 
+func TestFileSystemListFiltering(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("These cases don't need to work on Windows")
+	}
+
+	fakeDevDir, err := ioutil.TempDir(os.TempDir(), "dir")
+	assert.Empty(t, err)
+	defer os.RemoveAll(fakeDevDir)
+
+	cases := []struct {
+		description   string
+		fss, expected []sigar.FileSystem
+	}{
+		{
+			fss: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+				{DirName: "/", DevName: "/dev/sda1"},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+			},
+		},
+		{
+			description: "Don't repeat devices, shortest of dir names should be used",
+			fss: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+				{DirName: "/bind", DevName: "/dev/sda1"},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+			},
+		},
+		{
+			description: "Don't repeat devices, shortest of dir names should be used",
+			fss: []sigar.FileSystem{
+				{DirName: "/bind", DevName: "/dev/sda1"},
+				{DirName: "/", DevName: "/dev/sda1"},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+			},
+		},
+		{
+			description: "Keep tmpfs",
+			fss: []sigar.FileSystem{
+				{DirName: "/run", DevName: "tmpfs"},
+				{DirName: "/tmp", DevName: "tmpfs"},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/run", DevName: "tmpfs"},
+				{DirName: "/tmp", DevName: "tmpfs"},
+			},
+		},
+		{
+			description: "Don't repeat devices, shortest of dir names should be used, keep tmpfs",
+			fss: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+				{DirName: "/bind", DevName: "/dev/sda1"},
+				{DirName: "/run", DevName: "tmpfs"},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+				{DirName: "/run", DevName: "tmpfs"},
+			},
+		},
+		{
+			description: "Don't keep the fs if the device is a directory (it'd be a bind mount)",
+			fss: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+				{DirName: "/bind", DevName: fakeDevDir},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/", DevName: "/dev/sda1"},
+			},
+		},
+		{
+			description: "Don't filter out NFS",
+			fss: []sigar.FileSystem{
+				{DirName: "/srv/data", DevName: "192.168.42.42:/exports/nfs1"},
+			},
+			expected: []sigar.FileSystem{
+				{DirName: "/srv/data", DevName: "192.168.42.42:/exports/nfs1"},
+			},
+		},
+	}
+
+	for _, c := range cases {
+		filtered := filterFileSystemList(c.fss)
+		assert.ElementsMatch(t, c.expected, filtered, c.description)
+	}
+}
+
 func TestFilter(t *testing.T) {
 	in := []sigar.FileSystem{
 		{SysTypeName: "nfs"},

metricbeat/module/system/fsstat/_meta/docs.asciidoc

@@ -13,4 +13,6 @@ This metricset is available on:
 
 *`filesystem.ignore_types`* - A list of filesystem types to ignore. Metrics will
 not be collected from filesystems matching these types. This setting also
-affects the `filesystem` metricset.
+affects the `filesystem` metricset. If this option is not set, metricbeat ignores
+all types for virtual devices in systems where this information is available (e.g.
+all types marked as `nodev` in `/proc/filesystems` in Linux systems).

metricbeat/module/system/fsstat/_meta/fields.yml

@@ -2,7 +2,7 @@
   type: group
   description: >
     `system.fsstat` contains filesystem metrics aggregated from all mounted
-    filesystems, similar with what `df -a` prints out.
+    filesystems.
   release: ga
   fields:
     - name: count

metricbeat/module/system/fsstat/fsstat.go

@@ -3,6 +3,8 @@
 package fsstat
 
 import (
+	"strings"
+
 	"github.com/elastic/beats/libbeat/common"
 	"github.com/elastic/beats/libbeat/logp"
 	"github.com/elastic/beats/metricbeat/mb"
@@ -33,6 +35,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 		return nil, err
 	}
 
+	if config.IgnoreTypes == nil {
+		config.IgnoreTypes = filesystem.DefaultIgnoredTypes()
+	}
+	if len(config.IgnoreTypes) > 0 {
+		logp.Info("Ignoring filesystem types: %s", strings.Join(config.IgnoreTypes, ", "))
+	}
+
 	return &MetricSet{
 		BaseMetricSet: base,
 		config:        config,
@@ -53,7 +62,6 @@ func (m *MetricSet) Fetch() (common.MapStr, error) {
 
 	// These values are optional and could also be calculated by Kibana
 	var totalFiles, totalSize, totalSizeFree, totalSizeUsed uint64
-	dict := map[string]bool{}
 
 	for _, fs := range fss {
 		stat, err := filesystem.GetFileSystemStat(fs)
@@ -63,18 +71,10 @@ func (m *MetricSet) Fetch() (common.MapStr, error) {
 		}
 		logp.Debug("fsstat", "filesystem: %s total=%d, used=%d, free=%d", stat.Mount, stat.Total, stat.Used, stat.Free)
 
-		if _, ok := dict[stat.Mount]; ok {
-			// ignore filesystem with the same mounting point
-			continue
-		}
-
 		totalFiles += stat.Files
 		totalSize += stat.Total
 		totalSizeFree += stat.Free
 		totalSizeUsed += stat.Used
-
-		dict[stat.Mount] = true
-
 	}
 
 	return common.MapStr{
@@ -83,7 +83,7 @@ func (m *MetricSet) Fetch() (common.MapStr, error) {
 			"used":  totalSizeUsed,
 			"total": totalSize,
 		},
-		"count":       len(dict),
+		"count":       len(fss),
 		"total_files": totalFiles,
 	}, nil
 }