Enhance windows/perfmon metricset

[ruflin]

This is a meta issue to track enhancements to the windows/perfmon metricset created in #3758.

• Cleanup fields.yml to only contain the minimal fields Refactoring config handling for perfmon metricset #3896
• Refactoring config handling to use one list instead of groups Refactoring config handling for perfmon metricset #3854
• Improve error handling Refactor Windows perf data helper code #3871
• Improve first polling cycle to not depend on sleep: https://github.com/elastic/beats/pull/3758/files#diff-90bec44359d2b80d45705c5b909c3643R74 Refactor Windows perf data helper code #3871
• Add two // go:generate <cmd> lines to doc.go for generating defs_windows_<goarch>.go from defs_windows.go. Add missing go generate lines for perfmon metricset #3856 Update Metricbeat perfmon go:generate commands #3862
• Update data.json with a real example generated from a _test.go file. Perfmon metricset add real data example #4287
• Add many config examples to docs so people know how to collect common counters.
• Create example dashboards
• Use wildcard queries to collect data from multiple instances from a counter
• Add config option to ignore failures for non-existent counters [discuss]

[martinscholz83]

Hi @ruflin, how do you want to go on with this issue. Should we create a PR for every point or just one big PR?

[ruflin]

@maddin2016 I'm a fan of lots of small PR's as it makes it easier to review and get the changes in quickly but one large would also work.

[martinscholz83]

I would like to start with Improve first polling cycle. Would that be an alternative?

if firstFetch {
		// Most counters require two sample values in order to compute a displayable value. So wait and then collect the second value
		select {
		case <-time.After(time.Second * 2):
			err = _PdhCollectQueryData(q.query)
		}
	}

[ruflin]

@maddin2016 The above will still block for 2 seconds. I was more thinking of finding an option to only wait until the all the data is available or even skip it in the first run. Not sure what the best behaviour is. That is probably one of the trickiest once in the list above.

I would suggest to first focus on the cleanup part, for example change to the config options.

[martinscholz83]

Ok. Here is an description of the problem

Some counters, such as rate counters, require two counter values in order to compute a displayable value. In this case you must call PdhCollectQueryData twice before calling PdhGetFormattedCounterValue. For more information, see Collecting Performance Data.

So if we don't wait the first value is always 0. So an option is to skip the first run. Maybe @andrewkroh has an idea. I start with the other ones.

[martinscholz83]

I have started two PR's for config handling and the missing go generate lines. I wait for them to be merged before i go on with the samples for the docs and dashboards.

[andrewkroh]

I opened an intermediate PR to refactor the code and error handling. I made a change to remove the sleep, but the first value will always be 0. I'm thinking for move the core PDH code into its own package.

[martinscholz83]

As #3871 now is merged, i would go on with #3854.

[andrewkroh]

Congrats @maddin2016, this is being released today with 6.0.0-alpha1. 🎉

Since more people will be trying it now, it would be nice to get some more example in the docs and put some real data into the data.json.

[martinscholz83]

Great to hear @andrewkroh 🎉 Many thanks for your help to get this finished!!
Is it ok to put some stuff from my locale machine into data.json or has it to be some specific data?

[ruflin]

I think it is fine as long as it is clear it is only an example. Is there a way to auto generate the data.json?

[martinscholz83]

@ruflin, i currently have rebase my master. If i try to debug, i get the following error

module\system\process\helper.go:398: undefined: sort.Slice
module\system\process\helper.go:405: undefined: sort.Slice

Can you confirm this?

[martinscholz83]

I use go 1.7.3

[martinscholz83]

🙈 Sorry. Just read that the current go version for developing is 1.8.1. Updated and error is gone.

[martinscholz83]

This would be an example output. Is this ok

{
  "@timestamp": "2017-05-10T06:42:11.339Z",
  "beat": {
    "hostname": "beathost",
    "name": "beathost"
  },
  "metricset": {
    "module": "windows",
    "name": "perfmon"
  },
  "windows": {
    "perfmon": {
      "processor": {
        "time": {
          "total": {
            "pct": 22.486702
          }
        }
      }
    }
  }
}

[ruflin]

@maddin2016 is this auto generated or manually?

[martinscholz83]

This is generated manually. Just changed hostname and name.

[ruflin]

Any chance it could be generated automatically? Reason I'm asking is that in case we change something in the data model, it is very easy for us to recreate all the files again.

[martinscholz83]

I have debug the project and just copy the output. How do you normally generate these data.json files?

[ruflin]

Each metricset normally has a test called TestDatathat uses some mbtest helper to create the events and write them to disk. See for example: https://github.com/elastic/beats/blob/master/metricbeat/module/system/memory/memory_test.go#L11

To then run this test you have to run go test github.com/elastic/beats/metricbeat/module/your-module/... -v In case it has an integration test dependency, you have to add -tags=integration.

[martinscholz83]

Ok, then i have just to extend my current test file. Thanks @ruflin

[martinscholz83]

@ruflin, if i run this test i get this

=== RUN   TestData
--- SKIP: TestData (0.38s)
        data_generator.go:25: Skip data generation tests

[martinscholz83]

Got it. Have to add -data=true

[martinscholz83]

I have opened #4287

[martinscholz83]

@ruflin, @andrewkroh. Can you please track a new feature to the list above. It's about using wildcard querys to collect data from multiple instances from a counter.
For example \PhysicalDisk(*)\% Disk Time. If you have wildcards in your query you have to call a different function which is actually not implemented in perfmon. From MSDN

If the counter path contains a wildcard character for the instance name, call PdhGetFormattedCounterArray to retrieve an array of formatted counter values for each instance collected.

Lets say you have 3 disks. Then the following counters are available.

_Total, *, 0, 1, 2

Currently we just can call _Total or select one of the three disks. But not *.

This is also needed for #3798. I will add this ASAP and open a new PR.

Thanks.

PS: Maybe the actual docs should point out to this missing feature.

[krasekhi]

Is this enhancement still being considered for a future release? -
Add config option to ignore failures for non-existent counters

Currently we have metricbeat deployed to over 10,000 windows endpoints and when one of the defined WMI counters no longer exists the service stops. Being able to ignore the failures for non-existent counters would be massive improvement so we don't lose all data from the endpoint.

Thanks!

[ruflin]

@krasekhi Lets open a separate issue for this one to better track it instead of just the list above. Could you open one?

[martinscholz83]

Hi @ruflin, @andrewkroh for my inclusion (😄) back to beats, i think this is a good start. I would just catch if an counter not exists and write a warning into log.

[martinscholz83]

And of course make this optional as a config option.

[ruflin]

@martinscholz83 You have a PR in mind? :-D

[ruflin]

I'm closing this meta issue as we have covered allmost all the points above. For all the new things to come we should open separate issues / PR's.

@martinscholz83 Thanks a lot for getting us to this point.