Metricbeat: Remove duplicated filesystems in system module

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
 }