From 9f15d68fb2f8e018d3657e6d20fc676e73dd2a60 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 14 Jul 2021 16:00:04 +0200 Subject: [PATCH 01/12] work on perfmon improvements --- .../helper/windows/pdh/pdh_query_windows.go | 31 +++++++++++- metricbeat/helper/windows/pdh/pdh_windows.go | 13 ++++- metricbeat/helper/windows/pdh/zpdh_windows.go | 11 ++++- metricbeat/module/windows/perfmon/config.go | 11 +++-- metricbeat/module/windows/perfmon/perfmon.go | 6 +-- metricbeat/module/windows/perfmon/reader.go | 47 ++++++++++++++++++- .../{reader_test.go => readeralt_test.go} | 0 7 files changed, 105 insertions(+), 14 deletions(-) rename metricbeat/module/windows/perfmon/{reader_test.go => readeralt_test.go} (100%) diff --git a/metricbeat/helper/windows/pdh/pdh_query_windows.go b/metricbeat/helper/windows/pdh/pdh_query_windows.go index 8cc3a46edc8c..8ce475429524 100644 --- a/metricbeat/helper/windows/pdh/pdh_query_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_query_windows.go @@ -26,6 +26,8 @@ import ( "syscall" "unsafe" + "golang.org/x/sys/windows" + "github.com/pkg/errors" ) @@ -110,7 +112,16 @@ func (q *Query) AddCounter(counterPath string, instance string, format string, w func (q *Query) GetCounterPaths(counterPath string) ([]string, error) { paths, err := q.ExpandWildCardPath(counterPath) if err == nil { - return paths, err + // in several cases ExpandWildCardPath win32 api seems to return initial wildcard without any errors, adding some waiting time between the 2 ExpandWildCardPath api calls seems to be succesfull but that will delay data retrieval + // A call is trigegred again + if len(paths) == 1 && strings.Contains(paths[0], "*") && paths[0] == counterPath { + paths, err = q.ExpandWildCardPath(counterPath) + if err == nil { + return paths, err + } + } else { + return paths, err + } } //check if Windows installed language is not ENG, the ExpandWildCardPath will return either one of the errors below. if err == PDH_CSTATUS_NO_OBJECT || err == PDH_CSTATUS_NO_COUNTER { @@ -158,6 +169,18 @@ func (q *Query) RemoveUnusedCounters(counters []string) error { return nil } +// RemoveAllCounters will remove all counter handles for the paths configured +func (q *Query) RemoveAllCounters() error { + for counterPath, cnt := range q.Counters { + err := PdhRemoveCounter(cnt.handle) + if err != nil { + return err + } + delete(q.Counters, counterPath) + } + return nil +} + func matchCounter(counterPath string, counterList []string) bool { for _, cn := range counterList { if cn == counterPath { @@ -172,6 +195,11 @@ func (q *Query) CollectData() error { return PdhCollectQueryData(q.Handle) } +// CollectData collects the value for all counters in the query. +func (q *Query) CollectDataEx(interval uint32, event windows.Handle) error { + return PdhCollectQueryDataEx(q.Handle, interval, event) +} + // GetFormattedCounterValues returns an array of formatted values for a query. func (q *Query) GetFormattedCounterValues() (map[string][]CounterValue, error) { if q.Counters == nil || len(q.Counters) == 0 { @@ -218,6 +246,7 @@ func (q *Query) ExpandWildCardPath(wildCardPath string) ([]string, error) { if err == PDH_MORE_DATA { expdPaths, err = PdhExpandCounterPath(utfPath) } + expdPaths, err = PdhExpandCounterPath(utfPath) } if err != nil { return nil, err diff --git a/metricbeat/helper/windows/pdh/pdh_windows.go b/metricbeat/helper/windows/pdh/pdh_windows.go index 4d109549175d..1c437480d9ea 100644 --- a/metricbeat/helper/windows/pdh/pdh_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_windows.go @@ -34,6 +34,7 @@ import ( //sys _PdhAddCounter(query PdhQueryHandle, counterPath string, userData uintptr, counter *PdhCounterHandle) (errcode error) [failretval!=0] = pdh.PdhAddCounterW //sys _PdhRemoveCounter(counter PdhCounterHandle) (errcode error) [failretval!=0] = pdh.PdhRemoveCounter //sys _PdhCollectQueryData(query PdhQueryHandle) (errcode error) [failretval!=0] = pdh.PdhCollectQueryData +//sys _PdhCollectQueryDataEx(query PdhQueryHandle, interval uint32, event windows.Handle) (errcode error) [failretval!=0] = pdh.PdhCollectQueryDataEx //sys _PdhGetFormattedCounterValueDouble(counter PdhCounterHandle, format PdhCounterFormat, counterType *uint32, value *PdhCounterValueDouble) (errcode error) [failretval!=0] = pdh.PdhGetFormattedCounterValue //sys _PdhGetFormattedCounterValueLarge(counter PdhCounterHandle, format PdhCounterFormat, counterType *uint32, value *PdhCounterValueLarge) (errcode error) [failretval!=0] = pdh.PdhGetFormattedCounterValue //sys _PdhGetFormattedCounterValueLong(counter PdhCounterHandle, format PdhCounterFormat, counterType *uint32, value *PdhCounterValueLong) (errcode error) [failretval!=0]= pdh.PdhGetFormattedCounterValue @@ -156,6 +157,14 @@ func PdhCollectQueryData(query PdhQueryHandle) error { return nil } +// PdhCollectQueryDataEx collects the current raw data value for all counters in the specified query. +func PdhCollectQueryDataEx(query PdhQueryHandle, interval uint32, event windows.Handle) error { + if err := _PdhCollectQueryDataEx(query, interval, event); err != nil { + return PdhErrno(err.(syscall.Errno)) + } + return nil +} + // PdhGetFormattedCounterValueDouble computes a displayable double value for the specified counter. func PdhGetFormattedCounterValueDouble(counter PdhCounterHandle) (uint32, *PdhCounterValueDouble, error) { var counterType uint32 @@ -200,9 +209,9 @@ func PdhExpandWildCardPath(utfPath *uint16) ([]uint16, error) { if err := _PdhExpandWildCardPath(nil, utfPath, &expandPaths[0], &bufferSize); err != nil { return nil, PdhErrno(err.(syscall.Errno)) } - return expandPaths, nil + return expandPaths, err } - return nil, nil + return nil, PdhErrno(syscall.ERROR_NOT_FOUND) } // PdhExpandCounterPath returns counter paths that match the given counter path, for 32 bit windows. diff --git a/metricbeat/helper/windows/pdh/zpdh_windows.go b/metricbeat/helper/windows/pdh/zpdh_windows.go index 15473a7f5a23..bd644246595b 100644 --- a/metricbeat/helper/windows/pdh/zpdh_windows.go +++ b/metricbeat/helper/windows/pdh/zpdh_windows.go @@ -54,13 +54,13 @@ func errnoErr(e syscall.Errno) error { } var ( - modpdh = windows.NewLazySystemDLL("pdh.dll") - + modpdh = windows.NewLazySystemDLL("pdh.dll") procPdhOpenQueryW = modpdh.NewProc("PdhOpenQueryW") procPdhAddEnglishCounterW = modpdh.NewProc("PdhAddEnglishCounterW") procPdhAddCounterW = modpdh.NewProc("PdhAddCounterW") procPdhRemoveCounter = modpdh.NewProc("PdhRemoveCounter") procPdhCollectQueryData = modpdh.NewProc("PdhCollectQueryData") + procPdhCollectQueryDataEx = modpdh.NewProc("PdhCollectQueryDataEx") procPdhGetFormattedCounterValue = modpdh.NewProc("PdhGetFormattedCounterValue") procPdhCloseQuery = modpdh.NewProc("PdhCloseQuery") procPdhExpandWildCardPathW = modpdh.NewProc("PdhExpandWildCardPathW") @@ -126,6 +126,13 @@ func _PdhCollectQueryData(query PdhQueryHandle) (errcode error) { } return } +func _PdhCollectQueryDataEx(query PdhQueryHandle, interval uint32, event windows.Handle) (errcode error) { + r0, _, _ := syscall.Syscall(procPdhCollectQueryDataEx.Addr(), 1, uintptr(query), uintptr(interval), uintptr(event)) + if r0 != 0 { + errcode = syscall.Errno(r0) + } + return +} func _PdhGetFormattedCounterValueDouble(counter PdhCounterHandle, format PdhCounterFormat, counterType *uint32, value *PdhCounterValueDouble) (errcode error) { r0, _, _ := syscall.Syscall6(procPdhGetFormattedCounterValue.Addr(), 4, uintptr(counter), uintptr(format), uintptr(unsafe.Pointer(counterType)), uintptr(unsafe.Pointer(value)), 0, 0) diff --git a/metricbeat/module/windows/perfmon/config.go b/metricbeat/module/windows/perfmon/config.go index 971d4629b273..58b75ca50cbe 100644 --- a/metricbeat/module/windows/perfmon/config.go +++ b/metricbeat/module/windows/perfmon/config.go @@ -29,11 +29,12 @@ var allowedFormats = []string{"float", "large", "long"} // Config for the windows perfmon metricset. type Config struct { - IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` - GroupMeasurements bool `config:"perfmon.group_measurements_by_instance"` - Counters []Counter `config:"perfmon.counters"` - Queries []Query `config:"perfmon.queries"` - GroupAllCountersTo string `config:"perfmon.group_all_counter"` + IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` + GroupMeasurements bool `config:"perfmon.group_measurements_by_instance"` + RefreshWildcardCounters bool `config:"perfmon.refresh_wildcard_counters"` + Counters []Counter `config:"perfmon.counters"` + Queries []Query `config:"perfmon.queries"` + GroupAllCountersTo string `config:"perfmon.group_all_counter"` } // Counter for the perfmon counters (old implementation deprecated). diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 52865a281074..3d68687ab125 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -36,7 +36,7 @@ func init() { type MetricSet struct { mb.BaseMetricSet - reader *Reader + reader *ReaderAlt log *logp.Logger } @@ -46,7 +46,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { if err := base.Module().UnpackConfig(&config); err != nil { return nil, err } - reader, err := NewReader(config) + reader, err := NewReaderAlt(config) if err != nil { return nil, errors.Wrap(err, "initialization of reader failed") } @@ -68,7 +68,7 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { // Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue. // For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data). // A flag is set if the second call has been executed else refresh will fail (reader.executed) - if m.reader.executed { + if m.reader.executed && m.reader.config.RefreshWildcardCounters { err := m.reader.RefreshCounterPaths() if err != nil { return errors.Wrap(err, "failed retrieving counters") diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go index a1f430dc905a..78f9fa8d552f 100644 --- a/metricbeat/module/windows/perfmon/reader.go +++ b/metricbeat/module/windows/perfmon/reader.go @@ -23,12 +23,17 @@ import ( "fmt" "regexp" "strings" + "time" "unicode" "github.com/elastic/beats/v7/metricbeat/helper/windows/pdh" "github.com/pkg/errors" + "math/rand" + + "golang.org/x/sys/windows" + "github.com/elastic/beats/v7/libbeat/logp" "github.com/elastic/beats/v7/metricbeat/mb" ) @@ -108,7 +113,7 @@ func (re *Reader) Read() ([]mb.Event, error) { } // Get the values. - values, err := re.query.GetFormattedCounterValues() + values, err := re.getValues() if err != nil { return nil, errors.Wrap(err, "failed formatting counter values") } @@ -124,6 +129,46 @@ func (re *Reader) Read() ([]mb.Event, error) { return events, nil } +func (re *Reader) getValues() (map[string][]pdh.CounterValue, error) { + var interval uint32 = 10 + var val map[string][]pdh.CounterValue + rand.Seed(time.Now().UnixNano()) + title := windows.StringToUTF16Ptr(randSeq(10)) + event, err := windows.CreateEvent(nil, 0, 0, title) + if err != nil { + return nil, err + } + err = re.query.CollectDataEx(interval, event) + if err != nil { + return nil, err + } + waitfor, err := windows.WaitForSingleObject(event, windows.INFINITE) + if err != nil { + return nil, err + } + if waitfor == windows.WAIT_OBJECT_0 { + val, err = re.query.GetFormattedCounterValues() + if err != nil { + return nil, err + } + } + if waitfor == windows.WAIT_FAILED { + _ = "fff" + } + + err = windows.CloseHandle(event) + return val, err +} + +func randSeq(n int) string { + var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + b := make([]rune, n) + for i := range b { + b[i] = letters[rand.Intn(len(letters))] + } + return string(b) +} + // Close will close the PDH query for now. func (re *Reader) Close() error { return re.query.Close() diff --git a/metricbeat/module/windows/perfmon/reader_test.go b/metricbeat/module/windows/perfmon/readeralt_test.go similarity index 100% rename from metricbeat/module/windows/perfmon/reader_test.go rename to metricbeat/module/windows/perfmon/readeralt_test.go From 4837a57f59dd5306964f7d98914c855800700d0b Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 14 Jul 2021 16:05:50 +0200 Subject: [PATCH 02/12] changelog --- CHANGELOG.next.asciidoc | 1 + metricbeat/module/windows/perfmon/perfmon.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index fb483e884f19..04be74a626eb 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -244,6 +244,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - In the script processor, the `decode_xml` and `decode_xml_wineventlog` processors are now available as `DecodeXML` and `DecodeXMLWineventlog` respectively. - Fix encoding errors when using the disk queue on nested data with multi-byte characters {pull}26484[26484] - Fix `add_process_metadata` processor complaining about valid pid fields not being valid integers. {pull}26829[26829] {issue}26830[26830] +- Improve `perfmon` metricset performance. {pull}26886[26886] *Auditbeat* diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 3d68687ab125..f668197e2a28 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -36,7 +36,7 @@ func init() { type MetricSet struct { mb.BaseMetricSet - reader *ReaderAlt + reader *Reader log *logp.Logger } @@ -46,7 +46,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { if err := base.Module().UnpackConfig(&config); err != nil { return nil, err } - reader, err := NewReaderAlt(config) + reader, err := NewReader(config) if err != nil { return nil, errors.Wrap(err, "initialization of reader failed") } From 639d7e156b1596c984551dfb6956393fd665a6b8 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 14 Jul 2021 16:07:42 +0200 Subject: [PATCH 03/12] rever changes --- .../module/windows/perfmon/{readeralt_test.go => reader_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename metricbeat/module/windows/perfmon/{readeralt_test.go => reader_test.go} (100%) diff --git a/metricbeat/module/windows/perfmon/readeralt_test.go b/metricbeat/module/windows/perfmon/reader_test.go similarity index 100% rename from metricbeat/module/windows/perfmon/readeralt_test.go rename to metricbeat/module/windows/perfmon/reader_test.go From 7522834a770d50b158874ba005533380ec7a0978 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 14 Jul 2021 16:12:37 +0200 Subject: [PATCH 04/12] complete checks --- metricbeat/module/windows/perfmon/reader.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go index 78f9fa8d552f..064901b9cc17 100644 --- a/metricbeat/module/windows/perfmon/reader.go +++ b/metricbeat/module/windows/perfmon/reader.go @@ -151,11 +151,9 @@ func (re *Reader) getValues() (map[string][]pdh.CounterValue, error) { if err != nil { return nil, err } + } else if waitfor == windows.WAIT_FAILED { + return nil, errors.New("WaitForSingleObject has failed") } - if waitfor == windows.WAIT_FAILED { - _ = "fff" - } - err = windows.CloseHandle(event) return val, err } From 0f0ca1d6a6f050a5ea5adfacb68aacaf31821d35 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 14 Jul 2021 17:00:08 +0200 Subject: [PATCH 05/12] review action --- .../helper/windows/pdh/pdh_query_windows.go | 12 ------------ metricbeat/module/windows/perfmon/config.go | 17 +++++++++-------- metricbeat/module/windows/perfmon/perfmon.go | 1 - metricbeat/module/windows/perfmon/reader.go | 13 ++++++++----- 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/metricbeat/helper/windows/pdh/pdh_query_windows.go b/metricbeat/helper/windows/pdh/pdh_query_windows.go index 8ce475429524..bf508baa696f 100644 --- a/metricbeat/helper/windows/pdh/pdh_query_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_query_windows.go @@ -169,18 +169,6 @@ func (q *Query) RemoveUnusedCounters(counters []string) error { return nil } -// RemoveAllCounters will remove all counter handles for the paths configured -func (q *Query) RemoveAllCounters() error { - for counterPath, cnt := range q.Counters { - err := PdhRemoveCounter(cnt.handle) - if err != nil { - return err - } - delete(q.Counters, counterPath) - } - return nil -} - func matchCounter(counterPath string, counterList []string) bool { for _, cn := range counterList { if cn == counterPath { diff --git a/metricbeat/module/windows/perfmon/config.go b/metricbeat/module/windows/perfmon/config.go index 58b75ca50cbe..5f3d3d2c7c9d 100644 --- a/metricbeat/module/windows/perfmon/config.go +++ b/metricbeat/module/windows/perfmon/config.go @@ -20,21 +20,22 @@ package perfmon import ( - "github.com/pkg/errors" - "github.com/elastic/beats/v7/libbeat/common/cfgwarn" + "github.com/pkg/errors" + "time" ) var allowedFormats = []string{"float", "large", "long"} // Config for the windows perfmon metricset. type Config struct { - IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` - GroupMeasurements bool `config:"perfmon.group_measurements_by_instance"` - RefreshWildcardCounters bool `config:"perfmon.refresh_wildcard_counters"` - Counters []Counter `config:"perfmon.counters"` - Queries []Query `config:"perfmon.queries"` - GroupAllCountersTo string `config:"perfmon.group_all_counter"` + Period time.Duration `config:"period" validate:"required"` + IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` + GroupMeasurements bool `config:"perfmon.group_measurements_by_instance"` + RefreshWildcardCounters bool `config:"perfmon.refresh_wildcard_counters"` + Counters []Counter `config:"perfmon.counters"` + Queries []Query `config:"perfmon.queries"` + GroupAllCountersTo string `config:"perfmon.group_all_counter"` } // Counter for the perfmon counters (old implementation deprecated). diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index f668197e2a28..83cf3bec9771 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -63,7 +63,6 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { if len(m.reader.query.Counters) == 0 { m.log.Error("no counter paths were found") } - // refresh performance counter list // Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue. // For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data). diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go index 064901b9cc17..13839e4c033f 100644 --- a/metricbeat/module/windows/perfmon/reader.go +++ b/metricbeat/module/windows/perfmon/reader.go @@ -130,7 +130,6 @@ func (re *Reader) Read() ([]mb.Event, error) { } func (re *Reader) getValues() (map[string][]pdh.CounterValue, error) { - var interval uint32 = 10 var val map[string][]pdh.CounterValue rand.Seed(time.Now().UnixNano()) title := windows.StringToUTF16Ptr(randSeq(10)) @@ -138,22 +137,26 @@ func (re *Reader) getValues() (map[string][]pdh.CounterValue, error) { if err != nil { return nil, err } - err = re.query.CollectDataEx(interval, event) + err = re.query.CollectDataEx(uint32(re.config.Period.Seconds()), event) if err != nil { return nil, err } - waitfor, err := windows.WaitForSingleObject(event, windows.INFINITE) + waitFor, err := windows.WaitForSingleObject(event, windows.INFINITE) if err != nil { return nil, err } - if waitfor == windows.WAIT_OBJECT_0 { + switch waitFor { + case windows.WAIT_OBJECT_0: val, err = re.query.GetFormattedCounterValues() if err != nil { return nil, err } - } else if waitfor == windows.WAIT_FAILED { + case windows.WAIT_FAILED: return nil, errors.New("WaitForSingleObject has failed") + default: + return nil, errors.New("WaitForSingleObject was abandoned or still waiting for completion") } + err = windows.CloseHandle(event) return val, err } From a7bb99d58e510c17d17ef1ed637ff399b75aa7e6 Mon Sep 17 00:00:00 2001 From: narph Date: Mon, 19 Jul 2021 13:03:43 +0200 Subject: [PATCH 06/12] fmt --- metricbeat/module/windows/perfmon/config.go | 6 ++++-- metricbeat/module/windows/perfmon/reader.go | 5 ++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/metricbeat/module/windows/perfmon/config.go b/metricbeat/module/windows/perfmon/config.go index 5f3d3d2c7c9d..39d86b2cc80e 100644 --- a/metricbeat/module/windows/perfmon/config.go +++ b/metricbeat/module/windows/perfmon/config.go @@ -20,9 +20,11 @@ package perfmon import ( - "github.com/elastic/beats/v7/libbeat/common/cfgwarn" - "github.com/pkg/errors" "time" + + "github.com/pkg/errors" + + "github.com/elastic/beats/v7/libbeat/common/cfgwarn" ) var allowedFormats = []string{"float", "large", "long"} diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go index 13839e4c033f..ad45c90123a1 100644 --- a/metricbeat/module/windows/perfmon/reader.go +++ b/metricbeat/module/windows/perfmon/reader.go @@ -132,11 +132,12 @@ func (re *Reader) Read() ([]mb.Event, error) { func (re *Reader) getValues() (map[string][]pdh.CounterValue, error) { var val map[string][]pdh.CounterValue rand.Seed(time.Now().UnixNano()) - title := windows.StringToUTF16Ptr(randSeq(10)) + title := windows.StringToUTF16Ptr("metricbeat_perfmon" + randSeq(5)) event, err := windows.CreateEvent(nil, 0, 0, title) if err != nil { return nil, err } + defer windows.CloseHandle(event) err = re.query.CollectDataEx(uint32(re.config.Period.Seconds()), event) if err != nil { return nil, err @@ -156,8 +157,6 @@ func (re *Reader) getValues() (map[string][]pdh.CounterValue, error) { default: return nil, errors.New("WaitForSingleObject was abandoned or still waiting for completion") } - - err = windows.CloseHandle(event) return val, err } From 1cfe54335e8ed573d7c9c964fa9ceeb1498d961f Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 28 Jul 2021 13:40:16 +0200 Subject: [PATCH 07/12] clean up --- .../helper/windows/pdh/pdh_query_windows.go | 55 ++++++++++--------- metricbeat/module/windows/perfmon/data.go | 10 ---- metricbeat/module/windows/perfmon/perfmon.go | 3 +- metricbeat/module/windows/perfmon/reader.go | 2 - 4 files changed, 31 insertions(+), 39 deletions(-) diff --git a/metricbeat/helper/windows/pdh/pdh_query_windows.go b/metricbeat/helper/windows/pdh/pdh_query_windows.go index bf508baa696f..2301b9f5d588 100644 --- a/metricbeat/helper/windows/pdh/pdh_query_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_query_windows.go @@ -112,16 +112,7 @@ func (q *Query) AddCounter(counterPath string, instance string, format string, w func (q *Query) GetCounterPaths(counterPath string) ([]string, error) { paths, err := q.ExpandWildCardPath(counterPath) if err == nil { - // in several cases ExpandWildCardPath win32 api seems to return initial wildcard without any errors, adding some waiting time between the 2 ExpandWildCardPath api calls seems to be succesfull but that will delay data retrieval - // A call is trigegred again - if len(paths) == 1 && strings.Contains(paths[0], "*") && paths[0] == counterPath { - paths, err = q.ExpandWildCardPath(counterPath) - if err == nil { - return paths, err - } - } else { - return paths, err - } + return paths, err } //check if Windows installed language is not ENG, the ExpandWildCardPath will return either one of the errors below. if err == PDH_CSTATUS_NO_OBJECT || err == PDH_CSTATUS_NO_COUNTER { @@ -225,24 +216,38 @@ func (q *Query) ExpandWildCardPath(wildCardPath string) ([]string, error) { // PdhExpandWildCardPath will not return the counter paths for windows 32 bit systems but PdhExpandCounterPath will. if runtime.GOARCH == "386" { - expdPaths, err = PdhExpandCounterPath(utfPath) + if expdPaths, err = PdhExpandCounterPath(utfPath); err != nil { + return nil, err + } + if expdPaths == nil { + return nil, errors.New("no counter paths found") + } + return UTF16ToStringArray(expdPaths), nil } else { - expdPaths, err = PdhExpandWildCardPath(utfPath) - // rarely the PdhExpandWildCardPathW will not retrieve the expanded buffer size initially so the next call will encounter the PDH_MORE_DATA error since the specified size on the input is still less than - // the required size. If this is the case we will fallback on the PdhExpandCounterPathW api since it looks to act in a more stable manner. The PdhExpandCounterPathW api does come with some limitations but will - // satisfy most cases and return valid paths. - if err == PDH_MORE_DATA { - expdPaths, err = PdhExpandCounterPath(utfPath) + if expdPaths, err = PdhExpandWildCardPath(utfPath); err != nil { + // rarely the PdhExpandWildCardPathW will not retrieve the expanded buffer size initially so the next call will encounter the PDH_MORE_DATA error since the specified size on the input is still less than + // the required size. If this is the case we will fallback on the PdhExpandCounterPathW api since it looks to act in a more stable manner. The PdhExpandCounterPathW api does come with some limitations but will + // satisfy most cases and return valid paths. + if err == PDH_MORE_DATA { + expdPaths, err = PdhExpandWildCardPath(utfPath) + } else { + return nil, err + } + } + paths := UTF16ToStringArray(expdPaths) + // in several cases ExpandWildCardPath win32 api seems to return initial wildcard without any errors, adding some waiting time between the 2 ExpandWildCardPath api calls seems to be succesfull but that will delay data retrieval + // A call is triggered again + if len(paths) == 1 && strings.Contains(paths[0], "*") && paths[0] == wildCardPath { + expdPaths, err = PdhExpandWildCardPath(utfPath) + if err == nil { + return paths, err + } + } else { + return paths, err } - expdPaths, err = PdhExpandCounterPath(utfPath) - } - if err != nil { - return nil, err - } - if expdPaths == nil { - return nil, errors.New("no counter paths found") } - return UTF16ToStringArray(expdPaths), nil + + return nil, PdhErrno(syscall.ERROR_NOT_FOUND) } // Close closes the query and all of its counters. diff --git a/metricbeat/module/windows/perfmon/data.go b/metricbeat/module/windows/perfmon/data.go index 3dad6dc729ef..3a2adb413eaa 100644 --- a/metricbeat/module/windows/perfmon/data.go +++ b/metricbeat/module/windows/perfmon/data.go @@ -47,11 +47,6 @@ func (re *Reader) groupToEvents(counters map[string][]pdh.CounterValue) []mb.Eve // Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue. // For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data). if val.Err.Error != nil { - if !re.executed { - re.log.Debugw("Ignoring the first measurement because the data isn't ready", - "error", val.Err.Error, logp.Namespace("perfmon"), "query", counterPath) - continue - } // The counter has a negative value or the counter was successfully found, but the data returned is not valid. // This error can occur if the counter value is less than the previous value. (Because counter values always increment, the counter value rolls over to zero when it reaches its maximum value.) // This is not an error that stops the application from running successfully and a positive counter value should be retrieved in the later calls. @@ -118,11 +113,6 @@ func (re *Reader) groupToSingleEvent(counters map[string][]pdh.CounterValue) mb. // Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue. // For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data). if val.Err.Error != nil { - if !re.executed { - re.log.Debugw("Ignoring the first measurement because the data isn't ready", - "error", val.Err, logp.Namespace("perfmon"), "query", counterPath) - continue - } if val.Err.Error == pdh.PDH_CALC_NEGATIVE_VALUE || val.Err.Error == pdh.PDH_INVALID_DATA { re.log.Debugw("Counter value retrieval returned", "error", val.Err.Error, "cstatus", pdh.PdhErrno(val.Err.CStatus), logp.Namespace("perfmon"), "query", counterPath) diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 83cf3bec9771..6dd03e92def3 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -66,8 +66,7 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { // refresh performance counter list // Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case we must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue. // For more information, see Collecting Performance Data (https://docs.microsoft.com/en-us/windows/desktop/PerfCtrs/collecting-performance-data). - // A flag is set if the second call has been executed else refresh will fail (reader.executed) - if m.reader.executed && m.reader.config.RefreshWildcardCounters { + if m.reader.config.RefreshWildcardCounters { err := m.reader.RefreshCounterPaths() if err != nil { return errors.Wrap(err, "failed retrieving counters") diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go index ad45c90123a1..3e62e1978490 100644 --- a/metricbeat/module/windows/perfmon/reader.go +++ b/metricbeat/module/windows/perfmon/reader.go @@ -49,7 +49,6 @@ const ( // Reader will contain the config options type Reader struct { query pdh.Query // PDH Query - executed bool // Indicates if the query has been executed. log *logp.Logger // config Config // Metricset configuration counters []PerfCounter @@ -125,7 +124,6 @@ func (re *Reader) Read() ([]mb.Event, error) { } else { events = re.groupToEvents(values) } - re.executed = true return events, nil } From 7583f85850b0798c858e2b94f4ffea39c9a91623 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 28 Jul 2021 14:00:12 +0200 Subject: [PATCH 08/12] docs --- CHANGELOG.next.asciidoc | 1 - metricbeat/module/windows/perfmon/_meta/docs.asciidoc | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 1bc1da7e70a5..9d19010f50d8 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -245,7 +245,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Omit full index template from errors that occur while loading the template. {pull}25743[25743] - In the script processor, the `decode_xml` and `decode_xml_wineventlog` processors are now available as `DecodeXML` and `DecodeXMLWineventlog` respectively. - Fix encoding errors when using the disk queue on nested data with multi-byte characters {pull}26484[26484] -- Fix `add_process_metadata` processor complaining about valid pid fields not being valid integers. {pull}26829[26829] {issue}26830[26830] - Improve `perfmon` metricset performance. {pull}26886[26886] - Preserve annotations in a kubernetes namespace metadata {pull}27045[27045] diff --git a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc index f04c9247c049..93c39acf1d35 100644 --- a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc +++ b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc @@ -63,6 +63,8 @@ event. In the above example, this will cause the physical_disk.write.per_sec and physical_disk.write.time.pct measurements to be sent as a single event. The default behaviour is for all measurements to be sent as separate events. +*`refresh_wildcard_counters`*:: A boolean option to refresh the counter list at each fetch. By default, the counter list will be retrieved at the starting time, to refresh the list at each fetch, users will have to enable this setting. + *`counters`*:: Counters specifies a list of queries to perform. Each individual counter requires three config options - `instance_label`, `measurement_label`, and `query`. From 7cf69f507a18c61a98986b061bc1bd426a6a9fc5 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 28 Jul 2021 15:48:16 +0200 Subject: [PATCH 09/12] fix tests --- metricbeat/module/windows/perfmon/data_test.go | 2 -- metricbeat/module/windows/perfmon/reader_test.go | 1 - 2 files changed, 3 deletions(-) diff --git a/metricbeat/module/windows/perfmon/data_test.go b/metricbeat/module/windows/perfmon/data_test.go index 2554a87a9ce8..f11fe12c08ac 100644 --- a/metricbeat/module/windows/perfmon/data_test.go +++ b/metricbeat/module/windows/perfmon/data_test.go @@ -34,7 +34,6 @@ func TestGroupToEvents(t *testing.T) { GroupMeasurements: true, }, query: pdh.Query{}, - executed: true, log: nil, counters: []PerfCounter{ { @@ -150,7 +149,6 @@ func TestGroupToEvents(t *testing.T) { func TestGroupToSingleEvent(t *testing.T) { reader := Reader{ query: pdh.Query{}, - executed: true, log: nil, config: Config{ GroupAllCountersTo: "processor_count", diff --git a/metricbeat/module/windows/perfmon/reader_test.go b/metricbeat/module/windows/perfmon/reader_test.go index 12b2f0cfb034..1bbce6d2c0e6 100644 --- a/metricbeat/module/windows/perfmon/reader_test.go +++ b/metricbeat/module/windows/perfmon/reader_test.go @@ -30,7 +30,6 @@ import ( func TestGetCounter(t *testing.T) { reader := Reader{ query: pdh.Query{}, - executed: true, log: nil, counters: []PerfCounter{ { From 75811559d6489702b7bec87ca9f2712cc84b2074 Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 28 Jul 2021 17:09:59 +0200 Subject: [PATCH 10/12] update --- metricbeat/module/windows/perfmon/data_test.go | 8 ++++---- metricbeat/module/windows/perfmon/reader_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/metricbeat/module/windows/perfmon/data_test.go b/metricbeat/module/windows/perfmon/data_test.go index f11fe12c08ac..ea5f72fe143f 100644 --- a/metricbeat/module/windows/perfmon/data_test.go +++ b/metricbeat/module/windows/perfmon/data_test.go @@ -33,8 +33,8 @@ func TestGroupToEvents(t *testing.T) { config: Config{ GroupMeasurements: true, }, - query: pdh.Query{}, - log: nil, + query: pdh.Query{}, + log: nil, counters: []PerfCounter{ { QueryField: "datagrams_sent_per_sec", @@ -148,8 +148,8 @@ func TestGroupToEvents(t *testing.T) { func TestGroupToSingleEvent(t *testing.T) { reader := Reader{ - query: pdh.Query{}, - log: nil, + query: pdh.Query{}, + log: nil, config: Config{ GroupAllCountersTo: "processor_count", }, diff --git a/metricbeat/module/windows/perfmon/reader_test.go b/metricbeat/module/windows/perfmon/reader_test.go index 1bbce6d2c0e6..818f07441e34 100644 --- a/metricbeat/module/windows/perfmon/reader_test.go +++ b/metricbeat/module/windows/perfmon/reader_test.go @@ -29,8 +29,8 @@ import ( func TestGetCounter(t *testing.T) { reader := Reader{ - query: pdh.Query{}, - log: nil, + query: pdh.Query{}, + log: nil, counters: []PerfCounter{ { QueryField: "datagrams_sent_per_sec", From daa9f85e963b498f38258108129992963963d776 Mon Sep 17 00:00:00 2001 From: narph Date: Thu, 29 Jul 2021 10:18:59 +0200 Subject: [PATCH 11/12] fix tests --- metricbeat/helper/windows/pdh/pdh_query_windows.go | 9 +-------- metricbeat/helper/windows/pdh/pdh_windows.go | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/metricbeat/helper/windows/pdh/pdh_query_windows.go b/metricbeat/helper/windows/pdh/pdh_query_windows.go index 2301b9f5d588..83b0eba548a4 100644 --- a/metricbeat/helper/windows/pdh/pdh_query_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_query_windows.go @@ -225,14 +225,7 @@ func (q *Query) ExpandWildCardPath(wildCardPath string) ([]string, error) { return UTF16ToStringArray(expdPaths), nil } else { if expdPaths, err = PdhExpandWildCardPath(utfPath); err != nil { - // rarely the PdhExpandWildCardPathW will not retrieve the expanded buffer size initially so the next call will encounter the PDH_MORE_DATA error since the specified size on the input is still less than - // the required size. If this is the case we will fallback on the PdhExpandCounterPathW api since it looks to act in a more stable manner. The PdhExpandCounterPathW api does come with some limitations but will - // satisfy most cases and return valid paths. - if err == PDH_MORE_DATA { - expdPaths, err = PdhExpandWildCardPath(utfPath) - } else { - return nil, err - } + return nil, err } paths := UTF16ToStringArray(expdPaths) // in several cases ExpandWildCardPath win32 api seems to return initial wildcard without any errors, adding some waiting time between the 2 ExpandWildCardPath api calls seems to be succesfull but that will delay data retrieval diff --git a/metricbeat/helper/windows/pdh/pdh_windows.go b/metricbeat/helper/windows/pdh/pdh_windows.go index 1c437480d9ea..52b7c9d2d3ed 100644 --- a/metricbeat/helper/windows/pdh/pdh_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_windows.go @@ -206,7 +206,7 @@ func PdhExpandWildCardPath(utfPath *uint16) ([]uint16, error) { return nil, PdhErrno(err.(syscall.Errno)) } expandPaths := make([]uint16, bufferSize) - if err := _PdhExpandWildCardPath(nil, utfPath, &expandPaths[0], &bufferSize); err != nil { + if err = _PdhExpandWildCardPath(nil, utfPath, &expandPaths[0], &bufferSize); err != nil { return nil, PdhErrno(err.(syscall.Errno)) } return expandPaths, err From a9ef8a8df82a62c5ff359723f149ca578ae52141 Mon Sep 17 00:00:00 2001 From: narph Date: Thu, 5 Aug 2021 10:30:06 +0200 Subject: [PATCH 12/12] fix changelog --- CHANGELOG.next.asciidoc | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index e33cae7a17d8..5364aee8e000 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -191,17 +191,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix index template loading when the new index format is selected. {issue}22482[22482] {pull}22682[22682] - Periodic metrics in logs will now report `libbeat.output.events.active` and `beat.memstats.rss` as gauges (rather than counters). {pull}22877[22877] -- Use PROGRAMDATA environment variable instead of C:\ProgramData for windows install service {pull}22874[22874] -- Fix reporting of cgroup metrics when running under Docker {pull}22879[22879] -- Fix typo in config docs {pull}23185[23185] -- Add FAQ entry for madvdontneed variable {pull}23429[23429] -- Fix panic due to unhandled DeletedFinalStateUnknown in k8s OnDelete {pull}23419[23419] -- Fix error loop with runaway CPU use when the Kafka output encounters some connection errors {pull}23484[23484] -- Fix issue discovering docker containers and metadata after reconnections {pull}24318[24318] -- Fix ILM alias creation when write alias exists and initial index does not exist {pull}26143[26143] -- Omit full index template from errors that occur while loading the template. {pull}25743[25743] -- In the script processor, the `decode_xml` and `decode_xml_wineventlog` processors are now available as `DecodeXML` and `DecodeXMLWineventlog` respectively. -- Fix encoding errors when using the disk queue on nested data with multi-byte characters {pull}26484[26484] - Improve `perfmon` metricset performance. {pull}26886[26886] - Preserve annotations in a kubernetes namespace metadata {pull}27045[27045] - Allow conditional processing in `decode_xml` and `decode_xml_wineventlog`. {pull}27159[27159]