Cherry-pick elastic#22261 to 7.10: Fix instance name in perfmon metri…

…cset (elastic#22411) * Fix instance name in perfmon metricset (elastic#22261) * mofidy doc * fix * changelog * add test (cherry picked from commit f2a55fd) * changelog
kvch · Nov 4, 2020 · 16bf6d6 · 16bf6d6
1 parent fee1238
commit 16bf6d6
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 62 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -427,6 +427,7 @@ field. You can revert this change by configuring tags for the module and omittin
 - 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]
+- Fix instance name in perfmon metricset. {issue}22218[22218] {pull}22261[22261]
 - Add interval information to `monitor` metricset in azure. {pull}22152[22152]
 - Remove io.time from windows {pull}22237[22237]
 

diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go
@@ -73,78 +73,23 @@ func NewReader(config Config) (*Reader, error) {
 		config: config,
 	}
 	r.mapCounters(config)
-	for i, counter := range r.counters {
-		r.counters[i].ChildQueries = []string{}
-		childQueries, err := query.GetCounterPaths(counter.QueryName)
-		if err != nil {
-			if config.IgnoreNECounters {
-				switch err {
-				case pdh.PDH_CSTATUS_NO_COUNTER, pdh.PDH_CSTATUS_NO_COUNTERNAME,
-					pdh.PDH_CSTATUS_NO_INSTANCE, pdh.PDH_CSTATUS_NO_OBJECT:
-					r.log.Infow("Ignoring non existent counter", "error", err,
-						logp.Namespace("perfmon"), "query", counter.QueryName)
-					continue
-				}
-			} else {
-				query.Close()
-				return nil, errors.Wrapf(err, `failed to expand counter (query="%v")`, counter.QueryName)
-			}
-		}
-		// check if the pdhexpandcounterpath/pdhexpandwildcardpath functions have expanded the counter successfully.
-		if len(childQueries) == 0 || (len(childQueries) == 1 && strings.Contains(childQueries[0], "*")) {
-			// covering cases when PdhExpandWildCardPathW returns no counter paths or is unable to expand and the ignore_non_existent_counters flag is set
-			if config.IgnoreNECounters {
-				r.log.Infow("Ignoring non existent counter", "initial query", counter.QueryName,
-					logp.Namespace("perfmon"), "expanded query", childQueries)
-				continue
-			}
-			return nil, errors.Errorf(`failed to expand counter (query="%v"), no error returned`, counter.QueryName)
-		}
-		for _, v := range childQueries {
-			if err := query.AddCounter(v, counter.InstanceName, counter.Format, len(childQueries) > 1); err != nil {
-				return nil, errors.Wrapf(err, `failed to add counter (query="%v")`, counter.QueryName)
-			}
-			r.counters[i].ChildQueries = append(r.counters[i].ChildQueries, v)
-		}
+	_, err := r.getCounterPaths()
+	if err != nil {
+		return nil, err
 	}
 	return r, nil
 }
 
 // RefreshCounterPaths will recheck for any new instances and add them to the counter list
 func (re *Reader) RefreshCounterPaths() error {
-	var newCounters []string
-	for i, counter := range re.counters {
-		re.counters[i].ChildQueries = []string{}
-		childQueries, err := re.query.GetCounterPaths(counter.QueryName)
-		if err != nil {
-			if re.config.IgnoreNECounters {
-				switch err {
-				case pdh.PDH_CSTATUS_NO_COUNTER, pdh.PDH_CSTATUS_NO_COUNTERNAME,
-					pdh.PDH_CSTATUS_NO_INSTANCE, pdh.PDH_CSTATUS_NO_OBJECT:
-					re.log.Infow("Ignoring non existent counter", "error", err,
-						logp.Namespace("perfmon"), "query", counter.QueryName)
-					continue
-				}
-			} else {
-				return errors.Wrapf(err, `failed to expand counter (query="%v")`, counter.QueryName)
-			}
-		}
-		newCounters = append(newCounters, childQueries...)
-		// there are cases when the ExpandWildCardPath will retrieve a successful status but not an expanded query so we need to check for the size of the list
-		if err == nil && len(childQueries) >= 1 && !strings.Contains(childQueries[0], "*") {
-			for _, v := range childQueries {
-				if err := re.query.AddCounter(v, counter.InstanceName, counter.Format, len(childQueries) > 1); err != nil {
-					return errors.Wrapf(err, "failed to add counter (query='%v')", counter.QueryName)
-				}
-				re.counters[i].ChildQueries = append(re.counters[i].ChildQueries, v)
-			}
-		}
+	newCounters, err := re.getCounterPaths()
+	if err != nil {
+		return errors.Wrap(err, "failed retrieving counter paths")
 	}
-	err := re.query.RemoveUnusedCounters(newCounters)
+	err = re.query.RemoveUnusedCounters(newCounters)
 	if err != nil {
 		return errors.Wrap(err, "failed removing unused counter values")
 	}
-
 	return nil
 }
 
@@ -184,6 +129,39 @@ func (re *Reader) Close() error {
 	return re.query.Close()
 }
 
+// getCounterPaths func will process the counter paths based on the configuration options entered
+func (re *Reader) getCounterPaths() ([]string, error) {
+	var newCounters []string
+	for i, counter := range re.counters {
+		re.counters[i].ChildQueries = []string{}
+		childQueries, err := re.query.GetCounterPaths(counter.QueryName)
+		if err != nil {
+			if re.config.IgnoreNECounters {
+				switch err {
+				case pdh.PDH_CSTATUS_NO_COUNTER, pdh.PDH_CSTATUS_NO_COUNTERNAME,
+					pdh.PDH_CSTATUS_NO_INSTANCE, pdh.PDH_CSTATUS_NO_OBJECT:
+					re.log.Infow("Ignoring non existent counter", "error", err,
+						logp.Namespace("perfmon"), "query", counter.QueryName)
+					continue
+				}
+			} else {
+				return newCounters, errors.Wrapf(err, `failed to expand counter (query="%v")`, counter.QueryName)
+			}
+		}
+		newCounters = append(newCounters, childQueries...)
+		// there are cases when the ExpandWildCardPath will retrieve a successful status but not an expanded query so we need to check for the size of the list
+		if err == nil && len(childQueries) >= 1 && !strings.Contains(childQueries[0], "*") {
+			for _, v := range childQueries {
+				if err := re.query.AddCounter(v, counter.InstanceName, counter.Format, isWildcard(childQueries, counter.InstanceName)); err != nil {
+					return newCounters, errors.Wrapf(err, "failed to add counter (query='%v')", counter.QueryName)
+				}
+				re.counters[i].ChildQueries = append(re.counters[i].ChildQueries, v)
+			}
+		}
+	}
+	return newCounters, nil
+}
+
 func (re *Reader) getCounter(query string) (bool, PerfCounter) {
 	for _, counter := range re.counters {
 		for _, childQuery := range counter.ChildQueries {
@@ -310,3 +288,14 @@ func replaceUpperCase(src string) string {
 		return newStr
 	})
 }
+
+// isWildcard function checks if users has configured a wildcard inside the instance configuration option and if the wildcard has been resulted in a valid number of queries
+func isWildcard(queries []string, instance string) bool {
+	if len(queries) > 1 {
+		return true
+	}
+	if len(queries) == 1 && strings.Contains(instance, "*") {
+		return true
+	}
+	return false
+}
diff --git a/metricbeat/module/windows/perfmon/reader_test.go b/metricbeat/module/windows/perfmon/reader_test.go
@@ -158,3 +158,16 @@ func TestMapCounterPathLabel(t *testing.T) {
 	assert.Equal(t, result, "metrics.logicaldisk_avg_disk_sec_per_transfer")
 
 }
+
+func TestIsWildcard(t *testing.T) {
+	queries := []string{"\\Process(chrome)\\% User Time", "\\Process(chrome#1)\\% User Time", "\\Process(svchost)\\% User Time"}
+	instance := "*"
+	result := isWildcard(queries, instance)
+	assert.True(t, result)
+	queries = []string{"\\Process(chrome)\\% User Time"}
+	result = isWildcard(queries, instance)
+	assert.True(t, result)
+	instance = "chrome"
+	result = isWildcard(queries, instance)
+	assert.False(t, result)
+}