From eb06809ddebf0df62f05100a6d85cb123ef54643 Mon Sep 17 00:00:00 2001 From: Mariana Dima Date: Wed, 28 Oct 2020 13:35:57 +0000 Subject: [PATCH] Perfmon - Fix regular expressions to comply to multiple parentheses in instance name and object (#22146) * mofidy doc * work on instance * changelog * fix tests (cherry picked from commit 36775b2f909942bfd54cc8aa2aa809186f04c3a4) --- CHANGELOG.next.asciidoc | 40 +++++++++++ .../helper/windows/pdh/pdh_query_windows.go | 44 +++++++++++-- .../windows/pdh/pdh_query_windows_test.go | 66 ++++++++++++++++++- metricbeat/module/windows/perfmon/data.go | 4 +- .../module/windows/perfmon/data_test.go | 6 +- 5 files changed, 150 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 2ef01cb2642..aa0a1cc4299 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -94,6 +94,46 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Add privileged option for Auditbeat in Openshift {pull}17637[17637] - Fix storage metricset to allow config without region/zone. {issue}17623[17623] {pull}17624[17624] - Fix overflow on Prometheus rates when new buckets are added on the go. {pull}17753[17753] +- Remove specific win32 api errors from events in perfmon. {issue}18292[18292] {pull}18361[18361] +- Fix application_pool metricset after pdh changes. {pull}18477[18477] +- Fix tags_filter for cloudwatch metricset in aws. {pull}18524[18524] +- Fix panic on `metricbeat test modules` when modules are configured in `metricbeat.modules`. {issue}18789[18789] {pull}18797[18797] +- Fix getting gcp compute instance metadata with partial zone/region in config. {pull}18757[18757] +- Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802] +- Fix compute and pubsub dashboard for googlecloud module. {issue}18962[18962] {pull}18980[18980] +- Fix crash on vsphere module when Host information is not available. {issue}18996[18996] {pull}19078[19078] +- Fix incorrect usage of hints builder when exposed port is a substring of the hint {pull}19052[19052] +- Remove dedot for tag values in aws module. {issue}19112[19112] {pull}19221[19221] +- Stop counterCache only when already started {pull}19103[19103] +- Fix empty field name errors in the application pool metricset. {pull}19537[19537] +- Set tags correctly if the dimension value is ARN {issue}19111[19111] {pull}19433[19433] +- Fix bug incorrect parsing of float numbers as integers in Couchbase module {issue}18949[18949] {pull}19055[19055] +- Fix mapping of service start type in the service metricset, windows module. {pull}19551[19551] +- Fix config example in the perfmon configuration files. {pull}19539[19539] +- Add missing info about the rest of the azure metricsets in the documentation. {pull}19601[19601] +- Fix k8s scheduler compatibility issue. {pull}19699[19699] +- Fix SQL module mapping NULL values as string {pull}18955[18955] {issue}18898[18898 +- Add support for azure light metricset app_stats. {pull}20639[20639] +- Fix ec2 disk and network metrics to use Sum statistic method. {pull}20680[20680] +- Fill cloud.account.name with accountID if account alias doesn't exist. {pull}20736[20736] +- The Kibana collector applies backoff when errored at getting usage stats {pull}20772[20772] +- Update fields.yml in the azure module, missing metrics field. {pull}20918[20918] +- The `elasticsearch/index` metricset only requests wildcard expansion for hidden indices if the monitored Elasticsearch cluster supports it. {pull}20938[20938] +- Disable Kafka metricsets based on Jolokia by default. They require a different configuration. {pull}20989[20989] +- Fix panic index out of range error when getting AWS account name. {pull}21101[21101] {issue}21095[21095] +- Handle missing counters in the application_pool metricset. {pull}21071[21071] +- Fix timestamp handling in remote_write. {pull}21166[21166] +- Fix remote_write flaky test. {pull}21173[21173] +- Visualization title fixes in aws, azure and googlecloud compute dashboards. {pull}21098[21098] +- Add a switch to the driver definition on SQL module to use pretty names {pull}17378[17378] +- Fix retrieving resources by ID for the azure module. {pull}21711[21711] {issue}21707[21707] +- Use timestamp from CloudWatch API when creating events. {pull}21498[21498] +- Report the correct windows events for system/filesystem {pull}21758[21758] +- Fix regular expression in windows/permfon. {pull}22146[22146] {issue}21125[21125] +- Fix azure storage event format. {pull}21845[21845] +- Fix panic in kubernetes autodiscover related to keystores {issue}21843[21843] {pull}21880[21880] +- [Kubernetes] Remove redundant dockersock volume mount {pull}22009[22009] +- Revert change to report `process.memory.rss` as `process.memory.wss` on Windows. {pull}22055[22055] - Add a switch to the driver definition on SQL module to use pretty names {pull}17378[17378] *Packetbeat* diff --git a/metricbeat/helper/windows/pdh/pdh_query_windows.go b/metricbeat/helper/windows/pdh/pdh_query_windows.go index 3c51df5073a..8cc3a46edc8 100644 --- a/metricbeat/helper/windows/pdh/pdh_query_windows.go +++ b/metricbeat/helper/windows/pdh/pdh_query_windows.go @@ -30,7 +30,7 @@ import ( ) var ( - instanceNameRegexp = regexp.MustCompile(`.*?\((.*?)\).*`) + instanceNameRegexp = regexp.MustCompile(`(\(.+\))\\`) objectNameRegexp = regexp.MustCompile(`(?:^\\\\[^\\]+\\|^\\)([^\\]+)`) ) @@ -86,7 +86,7 @@ func (q *Query) AddCounter(counterPath string, instance string, format string, w var instanceName string // Extract the instance name from the counterPath. if instance == "" || wildcard { - instanceName, err = MatchInstanceName(counterPath) + instanceName, err = matchInstanceName(counterPath) if err != nil { return err } @@ -233,18 +233,50 @@ func (q *Query) Close() error { return PdhCloseQuery(q.Handle) } -// MatchInstanceName will check first for instance and then for any objects names. -func MatchInstanceName(counterPath string) (string, error) { +// matchInstanceName will check first for instance and then for any objects names. +func matchInstanceName(counterPath string) (string, error) { matches := instanceNameRegexp.FindStringSubmatch(counterPath) - if len(matches) != 2 { - matches = objectNameRegexp.FindStringSubmatch(counterPath) + if len(matches) == 2 { + return returnLastInstance(matches[1]), nil } + matches = objectNameRegexp.FindStringSubmatch(counterPath) if len(matches) == 2 { return matches[1], nil } return "", errors.New("query doesn't contain an instance name. In this case you have to define 'instance_name'") } +// returnLastInstance will return the content from the last parentheses, this covers cases as `\WF (System.Workflow) 4.0.0.0(*)\Workflows Created`. +func returnLastInstance(match string) string { + var openedParanth int + var innerMatch string + var matches []string + runeMatch := []rune(match) + for i := 0; i < len(runeMatch); i++ { + char := string(runeMatch[i]) + + // check if string ends between parentheses + if char == ")" { + openedParanth -= 1 + } + if openedParanth > 0 { + innerMatch += char + } + if openedParanth == 0 && innerMatch != "" { + matches = append(matches, innerMatch) + innerMatch = "" + } + // check if string starts between parentheses + if char == "(" { + openedParanth += 1 + } + } + if len(matches) > 0 { + return matches[len(matches)-1] + } + return match +} + // getCounterValue will retrieve the counter value based on the format applied in the config options func getCounterValue(counter *Counter) CounterValue { counterValue := CounterValue{Instance: counter.instanceName, Err: CounterValueError{CStatus: 0}} diff --git a/metricbeat/helper/windows/pdh/pdh_query_windows_test.go b/metricbeat/helper/windows/pdh/pdh_query_windows_test.go index 276aba7313d..69eeeca852c 100644 --- a/metricbeat/helper/windows/pdh/pdh_query_windows_test.go +++ b/metricbeat/helper/windows/pdh/pdh_query_windows_test.go @@ -89,6 +89,48 @@ func TestSuccessfulQuery(t *testing.T) { assert.NotNil(t, list) } +func TestMatchInstanceName(t *testing.T) { + query := "\\SQLServer:Databases(*)\\Log File(s) Used Size (KB)" + match, err := matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "*") + + query = " \\\\desktop-rfooe09\\per processor network interface card activity(3, microsoft wi-fi directvirtual (gyfyg) adapter #2)\\dpcs queued/sec" + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "3, microsoft wi-fi directvirtual (gyfyg) adapter #2") + + query = " \\\\desktop-rfooe09\\ (test this scenario) per processor network interface card activity(3, microsoft wi-fi directvirtual (gyfyg) adapter #2)\\dpcs queued/sec" + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "3, microsoft wi-fi directvirtual (gyfyg) adapter #2") + + query = "\\RAS\\Bytes Received By Disconnected Clients" + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "RAS") + + query = `\\Process (chrome.exe#4)\\Bytes Received By Disconnected Clients` + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "chrome.exe#4") + + query = "\\BranchCache\\Local Cache: Cache complete file segments" + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "BranchCache") + + query = `\Synchronization(*)\Exec. Resource no-Waits AcqShrdStarveExcl/sec` + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "*") + + query = `\.NET CLR Exceptions(test hellp (dsdsd) #rfsfs #3)\# of Finallys / sec` + match, err = matchInstanceName(query) + assert.NoError(t, err) + assert.Equal(t, match, "test hellp (dsdsd) #rfsfs #3") +} + // TestInstanceNameRegexp tests regular expression for instance. func TestInstanceNameRegexp(t *testing.T) { queryPaths := []string{`\SQLServer:Databases(*)\Log File(s) Used Size (KB)`, `\Search Indexer(*)\L0 Indexes (Wordlists)`, @@ -96,7 +138,7 @@ func TestInstanceNameRegexp(t *testing.T) { for _, path := range queryPaths { matches := instanceNameRegexp.FindStringSubmatch(path) if assert.Len(t, matches, 2, "regular expression did not return any matches") { - assert.Equal(t, matches[1], "*") + assert.Equal(t, matches[1], "(*)") } } } @@ -114,6 +156,28 @@ func TestObjectNameRegexp(t *testing.T) { } } +func TestReturnLastInstance(t *testing.T) { + query := "(*)" + match := returnLastInstance(query) + assert.Equal(t, match, "*") + + query = "(3, microsoft wi-fi directvirtual (gyfyg) adapter #2)" + match = returnLastInstance(query) + assert.Equal(t, match, "3, microsoft wi-fi directvirtual (gyfyg) adapter #2") + + query = "(test this scenario) per processor network interface card activity(3, microsoft wi-fi directvirtual (gyfyg) adapter #2)" + match = returnLastInstance(query) + assert.Equal(t, match, "3, microsoft wi-fi directvirtual (gyfyg) adapter #2") + + query = `(chrome.exe#4)` + match = returnLastInstance(query) + assert.Equal(t, match, "chrome.exe#4") + + query = `(test hellp (dsdsd) #rfsfs #3)` + match = returnLastInstance(query) + assert.Equal(t, match, "test hellp (dsdsd) #rfsfs #3") +} + func TestUTF16ToStringArray(t *testing.T) { var array = []string{ "\\\\DESKTOP-RFOOE09\\Physikalischer Datenträger(0 C:)\\Schreibvorgänge/s", diff --git a/metricbeat/module/windows/perfmon/data.go b/metricbeat/module/windows/perfmon/data.go index 833c5757220..6e4e9c6ef0a 100644 --- a/metricbeat/module/windows/perfmon/data.go +++ b/metricbeat/module/windows/perfmon/data.go @@ -33,7 +33,7 @@ import ( "github.com/elastic/beats/v7/metricbeat/mb" ) -var processRegexp = regexp.MustCompile(`(.+?)#[1-9]+`) +var processRegexp = regexp.MustCompile(`(.+?[^\s])(?:#\d+|$)`) func (re *Reader) groupToEvents(counters map[string][]pdh.CounterValue) []mb.Event { eventMap := make(map[string]*mb.Event) @@ -73,7 +73,7 @@ func (re *Reader) groupToEvents(counters map[string][]pdh.CounterValue) []mb.Eve Error: errors.Wrapf(val.Err.Error, "failed on query=%v", counterPath), } if val.Instance != "" { - //will ignore instance counter + //will ignore instance index if ok, match := matchesParentProcess(val.Instance); ok { eventMap[eventKey].MetricSetFields.Put(counter.InstanceField, match) } else { diff --git a/metricbeat/module/windows/perfmon/data_test.go b/metricbeat/module/windows/perfmon/data_test.go index df23d1667ff..7203963d2cc 100644 --- a/metricbeat/module/windows/perfmon/data_test.go +++ b/metricbeat/module/windows/perfmon/data_test.go @@ -159,9 +159,13 @@ func TestGroupToSingleEvent(t *testing.T) { func TestMatchesParentProcess(t *testing.T) { ok, val := matchesParentProcess("svchost") - assert.False(t, ok) + assert.True(t, ok) assert.Equal(t, val, "svchost") ok, val = matchesParentProcess("svchost#54") assert.True(t, ok) assert.Equal(t, val, "svchost") + + ok, val = matchesParentProcess("svchost (test) another #54") + assert.True(t, ok) + assert.Equal(t, val, "svchost (test) another #54") }