From a8600a23b9dc678f58287b0c5343960a0d7839c8 Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Fri, 28 Oct 2016 15:12:47 -0400 Subject: [PATCH] Metricbeat system module updates for Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added system core metricset for Windows. Per core metrics were implemented in elastic/gosigar. - Added logging of process/system details on Windows to aid in debugging (user, arch, cores, sid, privs). - Fixes #2860 (PPID is zero on Windows). - Fixes #1704 (Server 2003 - PID’s not recognized). The command line arguments for the process will not be reported on XP and 2003. - Fixes #1897 (OpenProcess access denied on Windows). Added code to enable the SeDebugPrivilege when it is available. - Fixes #2885 (diskio metricset fails on XP and 2003). - Enabled fsstats by default in Metricbeat config. (cherry picked from commit eb88c2cfe91178a886ccc5233e886510819f9b86) --- CHANGELOG.asciidoc | 9 ++ metricbeat/docs/modules/system.asciidoc | 2 +- metricbeat/etc/beat.full.yml | 2 +- metricbeat/etc/beat.short.yml | 58 ------------ metricbeat/etc/beat.yml | 2 +- metricbeat/make.bat | 34 +++++++ metricbeat/metricbeat.full.yml | 2 +- metricbeat/metricbeat.yml | 2 +- .../module/system/_meta/config.full.yml | 2 +- metricbeat/module/system/_meta/config.yml | 2 +- .../module/system/core/_meta/docs.asciidoc | 1 + metricbeat/module/system/core/core.go | 7 +- metricbeat/module/system/core/core_test.go | 2 +- metricbeat/module/system/filesystem/helper.go | 4 +- .../module/system/filesystem/helper_test.go | 21 +++-- metricbeat/module/system/process/helper.go | 6 +- metricbeat/module/system/system.go | 2 +- metricbeat/module/system/system_linux.go | 4 + metricbeat/module/system/system_other.go | 4 +- metricbeat/module/system/system_windows.go | 89 +++++++++++++++++++ metricbeat/tests/system/test_system.py | 4 +- 21 files changed, 167 insertions(+), 92 deletions(-) delete mode 100644 metricbeat/etc/beat.short.yml create mode 100644 metricbeat/make.bat create mode 100644 metricbeat/module/system/system_windows.go diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 27aa6e960cf..8965190c90f 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,10 @@ https://github.com/elastic/beats/compare/v5.0.0...5.0[Check the HEAD diff] *Metricbeat* - Fix `system.process.start_time` on Windows. {pull}2848[2848] +- Fix `system.process.ppid` on Windows. {issue}2860[2860] +- Fix system process metricset for Windows XP and 2003. `cmdline` will be unavailable. {issue}1704[1704] +- Fix access denied issues in system process metricset by enabling SeDebugPrivilege on Windows. {issue}1897[1897] +- Fix system diskio metricset for Windows XP and 2003. {issue}2885[2885] *Packetbeat* @@ -49,6 +53,11 @@ https://github.com/elastic/beats/compare/v5.0.0...5.0[Check the HEAD diff] *Metricbeat* - Add username and password config options to the PostgreSQL module. {pull}2889[2890] +- Add experimental filebeat metricset in the beats module. {pull}2297[2297] +- Add experimental libbeat metricset in the beats module. {pull}2339[2339] +- Add experimental docker module. Provided by Ingensi and @douaejeouit based on dockbeat. +- Add username and password config options to the MongoDB module. {pull}2889[2889] +- Add system core metricset for Windows. {pull}2883}[2883] *Packetbeat* diff --git a/metricbeat/docs/modules/system.asciidoc b/metricbeat/docs/modules/system.asciidoc index 881934d18c2..1754c9da10c 100644 --- a/metricbeat/docs/modules/system.asciidoc +++ b/metricbeat/docs/modules/system.asciidoc @@ -83,7 +83,7 @@ metricbeat.modules: - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/etc/beat.full.yml b/metricbeat/etc/beat.full.yml index 30b923e9b9f..1241e26eb08 100644 --- a/metricbeat/etc/beat.full.yml +++ b/metricbeat/etc/beat.full.yml @@ -29,7 +29,7 @@ metricbeat.modules: - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/etc/beat.short.yml b/metricbeat/etc/beat.short.yml deleted file mode 100644 index c76d6c5368f..00000000000 --- a/metricbeat/etc/beat.short.yml +++ /dev/null @@ -1,58 +0,0 @@ -########################## Metricbeat Configuration ########################### - -# This file is a full configuration example documenting all non-deprecated -# options in comments. For a shorter configuration example, that contains only -# the most common options, please see metricbeat.short.yml in the same directory. -# -# You can find the full configuration reference here: -# https://www.elastic.co/guide/en/beats/metricbeat/index.html - -#========================== Modules configuration ============================ -metricbeat.modules: - -#---------------------------- Apache Status Module --------------------------- -- module: apache - metricsets: ["status"] - enabled: true - period: 1s - - # Apache hosts - hosts: ["http://127.0.0.1/"] - -#---------------------------- MySQL Status Module ---------------------------- -- module: mysql - metricsets: ["status"] - enabled: true - period: 2s - - # Host DSN should be defined as "tcp(127.0.0.1:3306)/" - # The username and password can either be set in the DSN or for all hosts in username and password config option - hosts: ["root@tcp(127.0.0.1:3306)/"] - -#---------------------------- Nginx Status Module ---------------------------- -- module: nginx - metricsets: ["stubstatus"] - enabled: true - period: 1s - - # Nginx hosts - hosts: ["http://127.0.0.1/"] - - -#---------------------------- Redis Status Module ---------------------------- -- module: redis - metricsets: ["info"] - enabled: true - period: 1s - - # Redis hosts - hosts: ["127.0.0.1:6379"] - -#---------------------------- System Status Module --------------------------- -- module: system - metricsets: ["cpu", "cores", "filesystem", "fsstats", "memory", "process"] - enabled: true - period: 2s - processes: ['.*'] - - diff --git a/metricbeat/etc/beat.yml b/metricbeat/etc/beat.yml index 984356ef6ac..592c10be776 100644 --- a/metricbeat/etc/beat.yml +++ b/metricbeat/etc/beat.yml @@ -29,7 +29,7 @@ metricbeat.modules: - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/make.bat b/metricbeat/make.bat new file mode 100644 index 00000000000..7edecf7aaca --- /dev/null +++ b/metricbeat/make.bat @@ -0,0 +1,34 @@ +@echo off + +REM +REM Batch script to build and test on Windows. You can use this in conjunction +REM with the Vagrant machine. +REM + +go install github.com/elastic/beats/vendor/github.com/pierrre/gotestcover +if %errorlevel% neq 0 exit /b %errorlevel% + +echo Building +go build +if %errorlevel% neq 0 exit /b %errorlevel% + +echo Testing +mkdir build\coverage +gotestcover -race -coverprofile=build/coverage/integration.cov github.com/elastic/beats/metricbeat/... +if %errorlevel% neq 0 exit /b %errorlevel% + +echo System Testing +go test -c -covermode=atomic -coverpkg ./... +if %errorlevel% neq 0 exit /b %errorlevel% +nosetests -v -w tests\system --process-timeout=30 +if %errorlevel% neq 0 exit /b %errorlevel% + +echo Aggregating Coverage Reports +python ..\dev-tools\aggregate_coverage.py -o build\coverage\system.cov .\build\system-tests\run +if %errorlevel% neq 0 exit /b %errorlevel% +python ..\dev-tools\aggregate_coverage.py -o build\coverage\full.cov .\build\coverage +if %errorlevel% neq 0 exit /b %errorlevel% +go tool cover -html=build\coverage\full.cov -o build\coverage\full.html +if %errorlevel% neq 0 exit /b %errorlevel% + +echo Success diff --git a/metricbeat/metricbeat.full.yml b/metricbeat/metricbeat.full.yml index 97ae95bef2a..701861f8b8a 100644 --- a/metricbeat/metricbeat.full.yml +++ b/metricbeat/metricbeat.full.yml @@ -29,7 +29,7 @@ metricbeat.modules: - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/metricbeat.yml b/metricbeat/metricbeat.yml index 5678d6364ee..54328186bf6 100644 --- a/metricbeat/metricbeat.yml +++ b/metricbeat/metricbeat.yml @@ -29,7 +29,7 @@ metricbeat.modules: - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/module/system/_meta/config.full.yml b/metricbeat/module/system/_meta/config.full.yml index 94499b6f516..dd4050d5333 100644 --- a/metricbeat/module/system/_meta/config.full.yml +++ b/metricbeat/module/system/_meta/config.full.yml @@ -16,7 +16,7 @@ - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/module/system/_meta/config.yml b/metricbeat/module/system/_meta/config.yml index 9ff213b1754..a4548f82b91 100644 --- a/metricbeat/module/system/_meta/config.yml +++ b/metricbeat/module/system/_meta/config.yml @@ -16,7 +16,7 @@ - filesystem # File system summary stats - #- fsstat + - fsstat # Memory stats - memory diff --git a/metricbeat/module/system/core/_meta/docs.asciidoc b/metricbeat/module/system/core/_meta/docs.asciidoc index 90b5106546c..5b4f2797030 100644 --- a/metricbeat/module/system/core/_meta/docs.asciidoc +++ b/metricbeat/module/system/core/_meta/docs.asciidoc @@ -8,3 +8,4 @@ This metricset is available on: - FreeBSD - Linux - OpenBSD +- Windows diff --git a/metricbeat/module/system/core/core.go b/metricbeat/module/system/core/core.go index f94da810363..a87232d8232 100644 --- a/metricbeat/module/system/core/core.go +++ b/metricbeat/module/system/core/core.go @@ -1,4 +1,4 @@ -// +build darwin freebsd linux openbsd +// +build darwin freebsd linux openbsd windows package core @@ -24,7 +24,6 @@ type MetricSet struct { // New is a mb.MetricSetFactory that returns a cores.MetricSet. func New(base mb.BaseMetricSet) (mb.MetricSet, error) { - config := struct { CpuTicks bool `config:"cpu_ticks"` // export CPU usage in ticks }{ @@ -46,7 +45,6 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { // Fetch fetches CPU core metrics from the OS. func (m *MetricSet) Fetch() ([]common.MapStr, error) { - cpuCoreStat, err := cpu.GetCpuTimesList() if err != nil { return nil, errors.Wrap(err, "cpu core times") @@ -54,8 +52,7 @@ func (m *MetricSet) Fetch() ([]common.MapStr, error) { m.cpu.AddCpuPercentageList(cpuCoreStat) - cores := []common.MapStr{} - + cores := make([]common.MapStr, 0, len(cpuCoreStat)) for core, stat := range cpuCoreStat { coreStat := common.MapStr{ diff --git a/metricbeat/module/system/core/core_test.go b/metricbeat/module/system/core/core_test.go index 819a8ff0a36..107a496a5b5 100644 --- a/metricbeat/module/system/core/core_test.go +++ b/metricbeat/module/system/core/core_test.go @@ -1,4 +1,4 @@ -// +build darwin freebsd linux openbsd +// +build darwin freebsd linux openbsd windows package core diff --git a/metricbeat/module/system/filesystem/helper.go b/metricbeat/module/system/filesystem/helper.go index 3d48340c418..c1e93909954 100644 --- a/metricbeat/module/system/filesystem/helper.go +++ b/metricbeat/module/system/filesystem/helper.go @@ -31,10 +31,8 @@ func GetFileSystemList() ([]sigar.FileSystem, error) { } func GetFileSystemStat(fs sigar.FileSystem) (*FileSystemStat, error) { - stat := sigar.FileSystemUsage{} - err := stat.Get(fs.DirName) - if err != nil { + if err := stat.Get(fs.DirName); err != nil { return nil, err } diff --git a/metricbeat/module/system/filesystem/helper_test.go b/metricbeat/module/system/filesystem/helper_test.go index 8479a234ea5..cffee33232f 100644 --- a/metricbeat/module/system/filesystem/helper_test.go +++ b/metricbeat/module/system/filesystem/helper_test.go @@ -12,24 +12,27 @@ import ( ) func TestFileSystemList(t *testing.T) { - if runtime.GOOS == "darwin" && os.Getenv("TRAVIS") == "true" { t.Skip("FileSystem test fails on Travis/OSX with i/o error") } fss, err := GetFileSystemList() - - assert.Nil(t, err) + if err != nil { + t.Fatal("GetFileSystemList", err) + } assert.True(t, (len(fss) > 0)) for _, fs := range fss { + if fs.TypeName == "cdrom" { + continue + } stat, err := GetFileSystemStat(fs) - assert.NoError(t, err) - - assert.True(t, (stat.Total >= 0)) - assert.True(t, (stat.Free >= 0)) - assert.True(t, (stat.Avail >= 0)) - assert.True(t, (stat.Used >= 0)) + if assert.NoError(t, err, "%v", err) { + assert.True(t, (stat.Total >= 0)) + assert.True(t, (stat.Free >= 0)) + assert.True(t, (stat.Avail >= 0)) + assert.True(t, (stat.Used >= 0)) + } } } diff --git a/metricbeat/module/system/process/helper.go b/metricbeat/module/system/process/helper.go index 541825de37b..c31b2c9e3f1 100644 --- a/metricbeat/module/system/process/helper.go +++ b/metricbeat/module/system/process/helper.go @@ -77,7 +77,7 @@ func (proc *Process) getDetails(cmdline string) error { if cmdline == "" { args := sigar.ProcArgs{} - if err := args.Get(proc.Pid); err != nil { + if err := args.Get(proc.Pid); err != nil && !sigar.IsNotImplemented(err) { return fmt.Errorf("error getting process arguments for pid=%d: %v", proc.Pid, err) } proc.CmdLine = strings.Join(args.List, " ") @@ -330,14 +330,12 @@ func (procStats *ProcStats) GetProcStats() ([]common.MapStr, error) { } func (procStats *ProcStats) GetProcStatsEvents() ([]common.MapStr, error) { - - events := []common.MapStr{} - processes, err := procStats.GetProcStats() if err != nil { return nil, err } + events := make([]common.MapStr, len(processes)) for _, proc := range processes { event := common.MapStr{ "@timestamp": common.Time(time.Now()), diff --git a/metricbeat/module/system/system.go b/metricbeat/module/system/system.go index 61b09afbad5..1e8c9401120 100644 --- a/metricbeat/module/system/system.go +++ b/metricbeat/module/system/system.go @@ -29,7 +29,7 @@ type Module struct { func NewModule(base mb.BaseModule) (mb.Module, error) { // This only needs to be configured once for all system modules. once.Do(func() { - configureHostFS() + initModule() }) return &Module{BaseModule: base, HostFS: *HostFS}, nil diff --git a/metricbeat/module/system/system_linux.go b/metricbeat/module/system/system_linux.go index a137a21a583..b5ec941cd73 100644 --- a/metricbeat/module/system/system_linux.go +++ b/metricbeat/module/system/system_linux.go @@ -7,6 +7,10 @@ import ( "github.com/elastic/gosigar" ) +func initModule() { + configureHostFS() +} + func configureHostFS() { dir := *HostFS if dir == "" { diff --git a/metricbeat/module/system/system_other.go b/metricbeat/module/system/system_other.go index 71c24ff5706..8e00133736e 100644 --- a/metricbeat/module/system/system_other.go +++ b/metricbeat/module/system/system_other.go @@ -1,7 +1,7 @@ -// +build !linux +// +build !linux,!windows package system -func configureHostFS() { +func initModule() { // Stub method for non-linux. } diff --git a/metricbeat/module/system/system_windows.go b/metricbeat/module/system/system_windows.go new file mode 100644 index 00000000000..c2bb8604463 --- /dev/null +++ b/metricbeat/module/system/system_windows.go @@ -0,0 +1,89 @@ +package system + +import ( + "syscall" + + "github.com/elastic/beats/libbeat/logp" + "github.com/elastic/gosigar/sys/windows" + "github.com/pkg/errors" +) + +// errMissingSeDebugPrivilege indicates that the SeDebugPrivilege is not +// present in the process's token. This is distinct from disabled. The token +// would be missing if the user does not have "Debug programs" rights. By +// default, only administrators and LocalSystem accounts have the privileges to +// debug programs. +var errMissingSeDebugPrivilege = errors.New("Metricbeat is running without " + + "SeDebugPrivilege, a Windows privilege that allows it to collect metrics " + + "from other processes. The user running Metricbeat may not have the " + + "appropriate privileges or the security policy disallows it.") + +func initModule() { + if err := checkAndEnableSeDebugPrivilege(); err != nil { + logp.Warn("%v", err) + } +} + +// checkAndEnableSeDebugPrivilege checks if the process's token has the +// SeDebugPrivilege and enables it if it is disabled. +func checkAndEnableSeDebugPrivilege() error { + info, err := windows.GetDebugInfo() + if err != nil { + return errors.Wrap(err, "GetDebugInfo failed") + } + logp.Info("Metricbeat process and system info: %v", info) + + seDebug, found := info.ProcessPrivs[windows.SeDebugPrivilege] + if !found { + return errMissingSeDebugPrivilege + } + + if seDebug.Enabled { + logp.Info("SeDebugPrivilege is enabled. %v", seDebug) + return nil + } + + if err = enableSeDebugPrivilege(); err != nil { + logp.Warn("Failure while attempting to enable SeDebugPrivilege. %v", err) + } + + info, err = windows.GetDebugInfo() + if err != nil { + return errors.Wrap(err, "GetDebugInfo failed") + } + + seDebug, found = info.ProcessPrivs[windows.SeDebugPrivilege] + if !found { + return errMissingSeDebugPrivilege + } + + if !seDebug.Enabled { + return errors.Errorf("Metricbeat failed to enable the "+ + "SeDebugPrivilege, a Windows privilege that allows it to collect "+ + "metrics from other processes. %v", seDebug) + } + + logp.Info("SeDebugPrivilege is now enabled. %v", seDebug) + return nil +} + +// enableSeDebugPrivilege enables the SeDebugPrivilege if it is present in +// the process's token. +func enableSeDebugPrivilege() error { + self, err := syscall.GetCurrentProcess() + if err != nil { + return err + } + + var token syscall.Token + err = syscall.OpenProcessToken(self, syscall.TOKEN_QUERY|syscall.TOKEN_ADJUST_PRIVILEGES, &token) + if err != nil { + return err + } + + if err = windows.EnableTokenPrivileges(token, windows.SeDebugPrivilege); err != nil { + return errors.Wrap(err, "EnableTokenPrivileges failed") + } + + return nil +} diff --git a/metricbeat/tests/system/test_system.py b/metricbeat/tests/system/test_system.py index d248cf256cf..e9910f3f82b 100644 --- a/metricbeat/tests/system/test_system.py +++ b/metricbeat/tests/system/test_system.py @@ -97,7 +97,7 @@ def test_cpu_ticks_option(self): cpuStats = evt["system"]["cpu"] self.assertItemsEqual(self.de_dot(SYSTEM_CPU_FIELDS_ALL), cpuStats.keys()) - @unittest.skipUnless(re.match("(?i)linux|darwin|freebsd|openbsd", sys.platform), "os") + @unittest.skipUnless(re.match("(?i)win|linux|darwin|freebsd|openbsd", sys.platform), "os") def test_core(self): """ Test core system output. @@ -123,7 +123,7 @@ def test_core(self): core = evt["system"]["core"] self.assertItemsEqual(self.de_dot(SYSTEM_CORE_FIELDS), core.keys()) - @unittest.skipUnless(re.match("(?i)linux|darwin|freebsd|openbsd", sys.platform), "os") + @unittest.skipUnless(re.match("(?i)win|linux|darwin|freebsd|openbsd", sys.platform), "os") def test_core_with_cpu_ticks(self): """ Test core system output.