Skip to content

Commit

Permalink
Fix instance name in perfmon metricset (elastic#22261)
Browse files Browse the repository at this point in the history
* mofidy doc

* fix

* changelog

* add test

(cherry picked from commit f2a55fd)
  • Loading branch information
narph committed Nov 4, 2020
1 parent fee1238 commit 541519e
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 62 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,9 @@ 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]
- Add a switch to the driver definition on SQL module to use pretty names {pull}17378[17378]
- Fix instance name in perfmon metricset. {issue}22218[22218] {pull}22261[22261]
- Remove io.time from windows {pull}22237[22237]
- Add interval information to `monitor` metricset in azure. {pull}22152[22152]
- Remove io.time from windows {pull}22237[22237]

Expand Down
113 changes: 51 additions & 62 deletions metricbeat/module/windows/perfmon/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
13 changes: 13 additions & 0 deletions metricbeat/module/windows/perfmon/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 541519e

Please sign in to comment.