Refactor use of system.hostfs to fix cgroup metrics

CHANGELOG.next.asciidoc

@@ -514,6 +514,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Unskip s3_request integration test. {pull}23887[23887]
 - Add system.hostfs configuration option for system module. {pull}23831[23831]
 - Fix GCP not able to request Cloudfunctions metrics if a region filter was set {pull}24218[24218]
+- Allow cgroup self-monitoring to see alternate `hostfs` paths {pull}24334[24334]
 
 *Packetbeat*
 

libbeat/cmd/instance/beat.go

@@ -1083,13 +1083,16 @@ func initPaths(cfg *common.Config) error {
 	// the paths field. After we will unpack the complete configuration and keystore reference
 	// will be correctly replaced.
 	partialConfig := struct {
-		Path paths.Path `config:"path"`
+		Path   paths.Path `config:"path"`
+		Hostfs string     `config:"system.hostfs"`
 	}{}
 
 	if err := cfg.Unpack(&partialConfig); err != nil {
 		return fmt.Errorf("error extracting default paths: %+v", err)
 	}
 
+	partialConfig.Path.Hostfs = partialConfig.Hostfs
+
 	if err := paths.InitPaths(&partialConfig.Path); err != nil {
 		return fmt.Errorf("error setting default paths: %+v", err)
 	}

libbeat/cmd/instance/metrics/metrics.go

@@ -29,6 +29,7 @@ import (
 	"github.com/elastic/beats/v7/libbeat/metric/system/cpu"
 	"github.com/elastic/beats/v7/libbeat/metric/system/process"
 	"github.com/elastic/beats/v7/libbeat/monitoring"
+	"github.com/elastic/beats/v7/libbeat/paths"
 	"github.com/elastic/gosigar/cgroup"
 )
 
@@ -284,7 +285,9 @@ func reportBeatCgroups(_ monitoring.Mode, V monitoring.Visitor) {
 		return
 	}
 
+
 	cgroups, err := cgroup.NewReaderOptions(cgroup.ReaderOptions{
+		RootfsMountpoint: paths.Paths.Hostfs,
 		IgnoreRootCgroups:        true,
 		CgroupsHierarchyOverride: os.Getenv(libbeatMonitoringCgroupsHierarchyOverride),
 	})

libbeat/paths/paths.go

@@ -28,6 +28,8 @@
 //
 // path.config - Configuration files and Elasticsearch template default location
 //
+// system.hostfs - supplies an alternate filesystem root for containerized environments
+//
 // These settings can be set via the configuration file or via command line flags.
 // The CLI flags overwrite the configuration file options.
 //
@@ -38,27 +40,45 @@
 package paths
 
 import (
+	"flag"
 	"fmt"
 	"os"
 	"path/filepath"
+
+	"github.com/elastic/beats/v7/libbeat/common/cfgwarn"
+)
+
+var (
+	// TODO: remove this flag in 8.0 since it should be replaced by system.hostfs configuration option (config.HostFS)
+	// HostFS is an alternate mountpoint for the filesytem root, for when metricbeat is running inside a container.
+	hostFS = flag.String("system.hostfs", "", "mountpoint of the host's filesystem for use in monitoring a host from within a container")
 )
 
+// Path tracks user-configurable path locations and directories
 type Path struct {
 	Home   string
 	Config string
 	Data   string
 	Logs   string
+	Hostfs string
 }
 
 // FileType is an enumeration type representing the file types.
 // Currently existing file types are: Home, Config, Data
 type FileType string
 
 const (
-	Home   FileType = "home"
+	// Home is the "root" directory for the running beats instance
+	Home FileType = "home"
+	// Config is the path to the beat config
 	Config FileType = "config"
-	Data   FileType = "data"
-	Logs   FileType = "logs"
+	// Data is the path to the beat data directory
+	Data FileType = "data"
+	// Logs is the path to the beats logs directory
+	Logs FileType = "logs"
+	// Hostfs is an alternate path to the filesystem root,
+	// used for system metrics that interact with procfs and sysfs.
+	Hostfs FileType = "hostfs"
 )
 
 // Paths is the Path singleton on which the top level functions from this
@@ -115,15 +135,24 @@ func (paths *Path) initPaths(cfg *Path) error {
 		paths.Logs = filepath.Join(paths.Home, "logs")
 	}
 
+	if *hostFS != "" {
+		cfgwarn.Deprecate("8.0.0", "This flag will be removed in the future and replaced by a config value.")
+		paths.Hostfs = *hostFS
+	}
+
+	if paths.Hostfs == "" {
+		paths.Hostfs = "/"
+	}
+
 	return nil
 }
 
 // Resolve resolves a path to a location in one of the default
 // folders. For example, Resolve(Home, "test") returns an absolute
 // path for "test" in the home path.
 func (paths *Path) Resolve(fileType FileType, path string) string {
-	// absolute paths are not changed
-	if filepath.IsAbs(path) {
+	// absolute paths are not changed for non-hostfs file types, since hostfs is a little odd
+	if filepath.IsAbs(path) && fileType != Hostfs {
 		return path
 	}
 
@@ -136,6 +165,8 @@ func (paths *Path) Resolve(fileType FileType, path string) string {
 		return filepath.Join(paths.Data, path)
 	case Logs:
 		return filepath.Join(paths.Logs, path)
+	case Hostfs:
+		return filepath.Join(paths.Hostfs, path)
 	default:
 		panic(fmt.Sprintf("Unknown file type: %s", fileType))
 	}

metricbeat/module/system/diskio/diskio_test.go

@@ -26,16 +26,23 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/elastic/beats/v7/libbeat/paths"
 	mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing"
-	"github.com/elastic/beats/v7/metricbeat/module/system"
 )
 
+func setHostfs(pathString string) {
+	path := paths.Path{
+		Hostfs: pathString,
+	}
+	paths.InitPaths(&path)
+}
+
 func TestDataNameFilter(t *testing.T) {
-	oldFS := system.HostFS
-	newFS := "_meta/testdata"
-	system.HostFS = &newFS
+	oldFS := paths.Paths.Hostfs
+	setHostfs("_meta/testdata")
+
 	defer func() {
-		system.HostFS = oldFS
+		setHostfs(oldFS)
 	}()
 
 	conf := map[string]interface{}{
@@ -51,11 +58,11 @@ func TestDataNameFilter(t *testing.T) {
 }
 
 func TestDataEmptyFilter(t *testing.T) {
-	oldFS := system.HostFS
-	newFS := "_meta/testdata"
-	system.HostFS = &newFS
+	oldFS := paths.Paths.Hostfs
+	setHostfs("_meta/testdata")
+
 	defer func() {
-		system.HostFS = oldFS
+		setHostfs(oldFS)
 	}()
 
 	conf := map[string]interface{}{
@@ -67,6 +74,7 @@ func TestDataEmptyFilter(t *testing.T) {
 	data, errs := mbtest.ReportingFetchV2Error(f)
 	assert.Empty(t, errs)
 	assert.Equal(t, 10, len(data))
+
 }
 
 func TestFetch(t *testing.T) {

metricbeat/module/system/entropy/entropy.go

@@ -29,8 +29,8 @@ import (
 
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/elastic/beats/v7/libbeat/common/cfgwarn"
+	"github.com/elastic/beats/v7/libbeat/paths"
 	"github.com/elastic/beats/v7/metricbeat/mb"
-	"github.com/elastic/beats/v7/metricbeat/module/system"
 )
 
 // init registers the MetricSet with the central registry as soon as the program
@@ -55,12 +55,7 @@ type MetricSet struct {
 func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 	cfgwarn.Beta("The system entropy metricset is beta.")
 
-	systemModule, ok := base.Module().(*system.Module)
-	if !ok {
-		return nil, errors.New("unexpected module type")
-	}
-
-	totalPath := path.Join(systemModule.HostFS, "/proc/sys/kernel/random")
+	totalPath := paths.Resolve(paths.Hostfs, "/proc/sys/kernel/random")
 
 	return &MetricSet{
 		BaseMetricSet: base,

metricbeat/module/system/entropy/entropy_test.go

@@ -24,13 +24,19 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/elastic/beats/v7/libbeat/paths"
 	mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing"
-	"github.com/elastic/beats/v7/metricbeat/module/system"
 )
 
 func TestData(t *testing.T) {
-	testdata := "./_meta/testdata"
-	system.HostFS = &testdata
+	testPath := paths.Path{
+		Hostfs: "./_meta/testdata",
+	}
+
+	if err := paths.InitPaths(&testPath); err != nil {
+		t.Errorf("error setting default paths: %+v", err)
+		t.FailNow()
+	}
 	f := mbtest.NewReportingMetricSetV2Error(t, getConfig())
 	err := mbtest.WriteEventsReporterV2Error(f, t, ".")
 	if err != nil {
@@ -39,8 +45,14 @@ func TestData(t *testing.T) {
 }
 
 func TestFetch(t *testing.T) {
-	testdata := "./_meta/testdata"
-	system.HostFS = &testdata
+	testPath := paths.Path{
+		Hostfs: "./_meta/testdata",
+	}
+
+	if err := paths.InitPaths(&testPath); err != nil {
+		t.Errorf("error setting default paths: %+v", err)
+		t.FailNow()
+	}
 	f := mbtest.NewReportingMetricSetV2Error(t, getConfig())
 	events, errs := mbtest.ReportingFetchV2Error(f)
 

metricbeat/module/system/filesystem/helper.go

@@ -22,15 +22,14 @@ package filesystem
 import (
 	"bufio"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 	"time"
 
 	"runtime"
 
 	"github.com/elastic/beats/v7/libbeat/common"
-	"github.com/elastic/beats/v7/metricbeat/module/system"
+	"github.com/elastic/beats/v7/libbeat/paths"
 	sigar "github.com/elastic/gosigar"
 )
 
@@ -194,7 +193,7 @@ func BuildTypeFilter(ignoreType ...string) Predicate {
 func DefaultIgnoredTypes() (types []string) {
 	// If /proc/filesystems exist, default ignored types are all marked
 	// as nodev
-	fsListFile := path.Join(*system.HostFS, "/proc/filesystems")
+	fsListFile := paths.Resolve(paths.Hostfs, "/proc/filesystems")
 	if f, err := os.Open(fsListFile); err == nil {
 		scanner := bufio.NewScanner(f)
 		for scanner.Scan() {

metricbeat/module/system/process/process.go

@@ -28,6 +28,7 @@ import (
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/elastic/beats/v7/libbeat/logp"
 	"github.com/elastic/beats/v7/libbeat/metric/system/process"
+	"github.com/elastic/beats/v7/libbeat/paths"
 	"github.com/elastic/beats/v7/metricbeat/mb"
 	"github.com/elastic/beats/v7/metricbeat/mb/parse"
 	"github.com/elastic/beats/v7/metricbeat/module/system"
@@ -83,8 +84,8 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 
 	if runtime.GOOS == "linux" {
 		if config.Cgroups == nil || *config.Cgroups {
-			debugf("process cgroup data collection is enabled, using hostfs='%v'", systemModule.HostFS)
-			m.cgroup, err = cgroup.NewReader(systemModule.HostFS, true)
+			debugf("process cgroup data collection is enabled, using hostfs='%v'", paths.Paths.Hostfs)
+			m.cgroup, err = cgroup.NewReader(paths.Paths.Hostfs, true)
 			if err != nil {
 				if err == cgroup.ErrCgroupsMissing {
 					logp.Warn("cgroup data collection will be disabled: %v", err)

metricbeat/module/system/raid/raid.go

@@ -24,9 +24,9 @@ import (
 	"github.com/prometheus/procfs"
 
 	"github.com/elastic/beats/v7/libbeat/common"
+	"github.com/elastic/beats/v7/libbeat/paths"
 	"github.com/elastic/beats/v7/metricbeat/mb"
 	"github.com/elastic/beats/v7/metricbeat/mb/parse"
-	"github.com/elastic/beats/v7/metricbeat/module/system"
 	"github.com/elastic/beats/v7/metricbeat/module/system/raid/blockinfo"
 )
 
@@ -45,10 +45,6 @@ type MetricSet struct {
 
 // New creates a new instance of the raid metricset.
 func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
-	systemModule, ok := base.Module().(*system.Module)
-	if !ok {
-		return nil, errors.New("unexpected module type")
-	}
 
 	// Additional configuration options
 	config := struct {
@@ -60,7 +56,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 	}
 
 	if config.MountPoint == "" {
-		config.MountPoint = systemModule.HostFS
+		config.MountPoint = paths.Paths.Hostfs
 	}
 
 	mountPoint := filepath.Join(config.MountPoint, procfs.DefaultMountPoint)

metricbeat/module/system/socket/socket.go

@@ -24,18 +24,17 @@ import (
 	"net"
 	"os"
 	"os/user"
-	"path/filepath"
 	"strconv"
 	"syscall"
 
 	"github.com/pkg/errors"
 
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/elastic/beats/v7/libbeat/logp"
+	"github.com/elastic/beats/v7/libbeat/paths"
 	sock "github.com/elastic/beats/v7/metricbeat/helper/socket"
 	"github.com/elastic/beats/v7/metricbeat/mb"
 	"github.com/elastic/beats/v7/metricbeat/mb/parse"
-	"github.com/elastic/beats/v7/metricbeat/module/system"
 	"github.com/elastic/gosigar/sys/linux"
 )
 
@@ -50,6 +49,10 @@ func init() {
 	)
 }
 
+// MetricSet holds any configuration or state information. It must implement
+// the mb.MetricSet interface. And this is best achieved by embedding
+// mb.BaseMetricSet because it implements all of the required mb.MetricSet
+// interface methods except for Fetch.
 type MetricSet struct {
 	mb.BaseMetricSet
 	netlink       *sock.NetlinkSession
@@ -62,18 +65,15 @@ type MetricSet struct {
 	users         UserCache
 }
 
+// New creates a new instance of the MetricSet. New is responsible for unpacking
+// any MetricSet specific configuration options if there are any.
 func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 	c := defaultConfig
 	if err := base.Module().UnpackConfig(&c); err != nil {
 		return nil, err
 	}
 
-	systemModule, ok := base.Module().(*system.Module)
-	if !ok {
-		return nil, errors.New("unexpected module type")
-	}
-
-	ptable, err := sock.NewProcTable(filepath.Join(systemModule.HostFS, "/proc"))
+	ptable, err := sock.NewProcTable(paths.Resolve(paths.Hostfs, "/proc"))
 	if err != nil {
 		return nil, err
 	}

metricbeat/module/system/socket_summary/socket_summary.go

@@ -155,7 +155,7 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error {
 	}
 
 	stats := calculateConnStats(conns)
-	newStats, err := applyEnhancements(stats, m)
+	newStats, err := applyEnhancements(stats)
 	if err != nil {
 		m.Logger().Debugf("error applying enhancements: %s", err)
 		newStats = stats