Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions #12212

narph · 2019-05-20T20:40:42Z

Using the PdhExpandWildCardPathW func to retrieve all counter paths instead of PdhGetFormattedCounterArrayW.
Will fix recovered from panic while fetching 'windows/perfmon' for host '' ERROR in Metricbeats Windows Module #10660
Separated methods in different go files by responsibility (will be useful for the iis module as well)
Added extra tests for query, pdh and reader
Removed unused functionality and refactored the retrieval of counter values by format

metricbeat/module/windows/perfmon/perfmon_helper.go

metricbeat/module/windows/perfmon/pdh_query_windows.go

jsoriano

Nice change, but I think we should split this PR if possible in two or three PRs, specially to make it clearer what is changed code and what was moved.
This will also need some changelog entries.

metricbeat/module/windows/perfmon/zpdh_windows.go

metricbeat/module/windows/perfmon/perfmon_helper.go

ruflin

Great to see this refactoring happening.

metricbeat/module/windows/perfmon/_meta/docs.asciidoc

metricbeat/module/windows/perfmon/_meta/fields.yml

metricbeat/module/windows/perfmon/pdh_query_windows.go

ruflin · 2019-05-22T07:25:49Z

metricbeat/module/windows/perfmon/pdh_query_windows.go

+	for path, counter := range q.counters {
+		_, value, err := PdhGetFormattedCounterValue(counter.handle, counter.format|PdhFmtNoCap100)
+		if err != nil {
+			rtn[path] = append(rtn[path], CounterValue{Err: err})


I was reading this line and 116 / 118 a few times. Is rtn[path] always set or could we run into a panic here?

This shouldn't be a problem here, appending to a nil array works.

metricbeat/module/windows/perfmon/pdh_query_windows.go

…unter

- Used the [PdhExpandWildCardPathW](https://docs.microsoft.com/en-us/windows/desktop/api/pdh/nf-pdh-pdhexpandwildcardpathw) func to retrieve all counter paths. Will fix elastic#10660 - Small refactoring in order to separate methods by responsability (will be usefull for the iis module as well) - Added `perfmon.append_instance_counter: true` config option in order to retrieve the full instance name (with counter)

…ferent PR

prakash1243 · 2019-06-02T12:55:31Z

@narph - I have checked out your pull request and built meticbeat package locally to collect Windows Perfmon Counters on a Windows-7 32 bit server, I have added a wild card query to collect Private Bytes across all the processes running, The output seems to be different now:

Configuration:
- instance_label: process_private_bytes measurement_label: process.private.bytes query: '\Process(*)\Private Bytes'

Output:
"metricset": { "name": "perfmon", "module": "windows", "rtt": 32001 }, "event": { "dataset": "windows.perfmon", "duration": 32001800 }, "windows": { "perfmon": { "process_private_bytes": "*", "process": { "private": { "bytes": 0 } } } } }

It doesn't collect the Private Bytes for all the processes, instead it had sent only one document with the process name as '*' . I have also added 'perfmon.append_instance_counter: true' to my windows configuration.
I have been waiting for my issue to get fixed #10660, got to know it would be fixed with this PR, so just did a test and wanted to let you know my findings. Thanks !

…eats into refactor/perfmon-queries

narph · 2019-06-03T10:10:45Z

@prakash1243, I am getting different results here when testing locally with - instance_label: process_private_bytes measurement_label: process.private.bytes query: '\Process(*)\Private Bytes

},
          "metricset" : {
            "name" : "perfmon"
          },
          "service" : {
            "type" : "windows"
          },
          "windows" : {
            "perfmon" : {
              "process" : {
                "private" : {
                  "bytes" : 2564096.0
                }
              },
              "process_private_bytes" : "winpty-agent"
            }
          },
          "ecs" : {
            "version" : "1.0.0"
          },

Nonetheless, I have added an integration test TestWildcardQueryNoInstanceName in pdh_integration_windows_test.go which should pass, mind giving that one a go?
Can you provide me with the entire windows module config if you are still dealing with this issue.

Regarding perfmon.append_instance_counter , in later conversations (#12212 (comment)) I am pointing out that I would leave this feature out and integrate it in a new PR.

prakash1243 · 2019-06-03T10:18:26Z

Hi @narph - Here is module config:

`- module: windows
metricsets: [perfmon]
period: 5s
perfmon.counters:

instance_label: process_private_bytes
measurement_label: process.private.bytes
query: '\Process(*)\Private Bytes'
`
Will I be able to get a snapshot build with the latest code changes you have in your PR for the Windows 32 bit (metricbeat.exe)? I can just test and let you know.
I doubt if am doing something wrong while building the package; since am new to the Go lang. :)
Please let me know. Thanks !

prakash1243 · 2019-06-03T10:54:35Z

Hi @narph -
Am getting this error, when I try to build the metricbeat package :(
C:\Users\Admin\go\src\github.com\elastic\beats\metricbeat>go build github.com/elastic/beats/vendor/github.com/DataDog/zstd In file included from ./zstd.h:43:0, from ..\vendor\github.com\DataDog\zstd\zstd.go:4: C:/Users/Admin/Desktop/gcc-5.1.0-tdm-1-core/lib/gcc/mingw32/5.1.0/include/stdint .h:9:26: fatal error: stdint.h: No such file or directory compilation terminated.

Would you mind sharing the snapshot build, which could be a huge help for me to test and let you know my findings. Thanks !

narph · 2019-06-04T07:52:11Z

@prakash1243 , I believe you can download a snapshot from one of the latest CI builds here https://beats-ci.elastic.co/job/elastic+beats+master+multijob-package-linux/663/gcsObjects/

prakash1243 · 2019-06-04T08:00:13Z

@prakash1243 , I believe you can download a snapshot from one of the latest CI builds here https://beats-ci.elastic.co/job/elastic+beats+master+multijob-package-linux/663/gcsObjects/

@narph - does this snapshot builds include the changes you had on this PR?

ruflin

LGTM. It seems this was mostly moving core around.

Could you update the PR description accordingly?

ruflin · 2019-06-05T08:08:09Z

metricbeat/module/windows/perfmon/perfmon.go

 	for _, value := range config.CounterConfig {
 		form := strings.ToLower(value.Format)
 		switch form {
-		case "":
+		case "", "float":


Was this a bug before?

metricbeat/module/windows/perfmon/pdh_query_windows.go

CHANGELOG.next.asciidoc

metricbeat/module/windows/perfmon/pdh_query_windows.go

narph · 2019-06-12T07:15:29Z

jenkins, test this

narph requested a review from a team as a code owner May 20, 2019 20:40

houndci-bot reviewed May 20, 2019

View reviewed changes

narph self-assigned this May 20, 2019

narph added Team:Integrations Label for the Integrations team :Windows [zube]: In Progress labels May 20, 2019

zube bot added [zube]: Inbox and removed [zube]: In Progress labels May 20, 2019

narph added [zube]: In Progress and removed [zube]: Inbox labels May 20, 2019

narph requested a review from andrewkroh May 20, 2019 20:42

narph added [zube]: In Review and removed [zube]: In Progress labels May 21, 2019

narph requested a review from ruflin May 21, 2019 08:49

jsoriano reviewed May 21, 2019

View reviewed changes

metricbeat/module/windows/perfmon/zpdh_windows.go Outdated Show resolved Hide resolved

metricbeat/module/windows/perfmon/perfmon_helper.go Outdated Show resolved Hide resolved

metricbeat/module/windows/perfmon/perfmon_helper.go Outdated Show resolved Hide resolved

ruflin reviewed May 22, 2019

View reviewed changes

martinscholz83 and others added 11 commits May 22, 2019 11:23

Fixing wrong instance names for wildcard queries

853bef8

Fixing wrong usage wildcard

fa8f1f0

Adding accidentally removed license header

d0aafbd

Extending integration tests to test for correct instance names

dfcee40

Changing wildcard regex to catch wildcards for parent,instance and co…

9c94556

…unter

Changing error handling for not existing counters

e38832e

Adding comments to exported functions

7701ca3

Fixing wrong comments

369bc9e

Moving variable declaration

1412a92

Updating fields.go

a607832

narph added [zube]: In Progress and removed [zube]: In Review labels May 22, 2019

Separate append_instance_counter config option functionality to a dif…

325172f

…ferent PR

narph added [zube]: In Review and removed [zube]: In Progress labels May 27, 2019

narph added 2 commits June 3, 2019 10:21

Add test for wildcards and chck on float

27ae86f

Merge branch 'refactor/perfmon-queries' of https://github.com/narph/b…

2e45968

…eats into refactor/perfmon-queries

narph requested review from ruflin and jsoriano and removed request for ruflin June 4, 2019 09:19

ruflin approved these changes Jun 5, 2019

View reviewed changes

jsoriano approved these changes Jun 5, 2019

View reviewed changes

metricbeat/module/windows/perfmon/pdh_query_windows.go Show resolved Hide resolved

Add extra check on format when retrieving coutner value

7569898

jsoriano approved these changes Jun 6, 2019

View reviewed changes

andrewkroh reviewed Jun 9, 2019

View reviewed changes

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved

metricbeat/module/windows/perfmon/pdh_query_windows.go Outdated Show resolved Hide resolved

metricbeat/module/windows/perfmon/pdh_query_windows.go Outdated Show resolved Hide resolved

narph changed the title ~~Small improvements in perfmon~~ Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions. Jun 11, 2019

Small text changes

d294191

narph requested a review from andrewkroh June 11, 2019 13:45

andrewkroh approved these changes Jun 11, 2019

View reviewed changes

narph merged commit e3d32af into elastic:master Jun 12, 2019

zube bot added [zube]: Done and removed [zube]: In Review labels Jun 12, 2019

narph deleted the refactor/perfmon-queries branch June 12, 2019 10:42

andresrc removed the [zube]: Done label Jun 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions #12212

Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions #12212

narph commented May 20, 2019 •

edited

Loading

jsoriano left a comment

ruflin left a comment

ruflin May 22, 2019

jsoriano May 24, 2019

prakash1243 commented Jun 2, 2019 •

edited

Loading

narph commented Jun 3, 2019

prakash1243 commented Jun 3, 2019 •

edited

Loading

prakash1243 commented Jun 3, 2019 •

edited

Loading

narph commented Jun 4, 2019

prakash1243 commented Jun 4, 2019 •

edited

Loading

ruflin left a comment

ruflin Jun 5, 2019

narph commented Jun 12, 2019

Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions #12212

Refactored Windows perfmon metricset: replaced method to retrieve counter paths with PdhExpandWildCardPathW, separated code by responsibility, removed unused functions #12212

Conversation

narph commented May 20, 2019 • edited Loading

jsoriano left a comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

ruflin May 22, 2019

Choose a reason for hiding this comment

jsoriano May 24, 2019

Choose a reason for hiding this comment

prakash1243 commented Jun 2, 2019 • edited Loading

narph commented Jun 3, 2019

prakash1243 commented Jun 3, 2019 • edited Loading

prakash1243 commented Jun 3, 2019 • edited Loading

narph commented Jun 4, 2019

prakash1243 commented Jun 4, 2019 • edited Loading

ruflin left a comment

Choose a reason for hiding this comment

ruflin Jun 5, 2019

Choose a reason for hiding this comment

narph commented Jun 12, 2019

narph commented May 20, 2019 •

edited

Loading

prakash1243 commented Jun 2, 2019 •

edited

Loading

prakash1243 commented Jun 3, 2019 •

edited

Loading

prakash1243 commented Jun 3, 2019 •

edited

Loading

prakash1243 commented Jun 4, 2019 •

edited

Loading