Perfmon ignore non existent counters #6432

martinscholz83 · 2018-02-21T11:50:52Z

Like discussed in #3828 this PR add a config option to ignore non existent counters.

elasticmachine · 2018-02-21T11:50:54Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

houndci-bot · 2018-02-21T11:51:00Z

metricbeat/module/windows/perfmon/perfmon.go

@@ -21,6 +21,11 @@ type CounterConfig struct {
 	Format           string `config:"format"`
 }

+type PerfmonConfig struct {


exported type PerfmonConfig should have comment or be unexported
type name will be used as perfmon.PerfmonConfig by other packages, and that stutters; consider calling this Config

houndci-bot · 2018-02-21T11:51:00Z

metricbeat/module/windows/perfmon/pdh_windows.go

@@ -314,7 +316,7 @@ type PerfmonReader struct {
 	executed      bool // Indicates if the query has been executed.
 }

-func NewPerfmonReader(config []CounterConfig) (*PerfmonReader, error) {
+func NewPerfmonReader(config PerfmonConfig) (*PerfmonReader, error) {


exported function NewPerfmonReader should have comment or be unexported

houndci-bot · 2018-02-21T11:51:00Z

metricbeat/module/windows/perfmon/perfmon.go

@@ -21,6 +21,11 @@ type CounterConfig struct {
 	Format           string `config:"format"`
 }

+type PerfmonConfig struct {


exported type PerfmonConfig should have comment or be unexported
type name will be used as perfmon.PerfmonConfig by other packages, and that stutters; consider calling this Config

houndci-bot · 2018-02-21T11:51:00Z

metricbeat/module/windows/perfmon/pdh_windows.go

@@ -314,7 +316,7 @@ type PerfmonReader struct {
 	executed      bool // Indicates if the query has been executed.
 }

-func NewPerfmonReader(config []CounterConfig) (*PerfmonReader, error) {
+func NewPerfmonReader(config PerfmonConfig) (*PerfmonReader, error) {


exported function NewPerfmonReader should have comment or be unexported

houndci-bot · 2018-02-21T11:51:01Z

metricbeat/module/windows/perfmon/perfmon.go

@@ -21,6 +21,11 @@ type CounterConfig struct {
 	Format           string `config:"format"`
 }

+type PerfmonConfig struct {


exported type PerfmonConfig should have comment or be unexported
type name will be used as perfmon.PerfmonConfig by other packages, and that stutters; consider calling this Config

houndci-bot · 2018-02-21T11:51:01Z

metricbeat/module/windows/perfmon/pdh_windows.go

@@ -314,7 +316,7 @@ type PerfmonReader struct {
 	executed      bool // Indicates if the query has been executed.
 }

-func NewPerfmonReader(config []CounterConfig) (*PerfmonReader, error) {
+func NewPerfmonReader(config PerfmonConfig) (*PerfmonReader, error) {


exported function NewPerfmonReader should have comment or be unexported

kvch · 2018-02-21T12:09:43Z

jenkins test it

ruflin · 2018-02-23T07:27:02Z

@martinscholz83 Thanks for the addition. Could you add a CHANGELOG and make HoundCI happy? The config name is pretty long but I don't have a better suggestion.

kvch · 2018-02-23T11:00:22Z

@martinscholz83 Ignore the AppVeyor failure. We don't use it anymore.

houndci-bot · 2018-02-26T07:24:23Z

metricbeat/module/windows/perfmon/perfmon.go

@@ -21,6 +22,12 @@ type CounterConfig struct {
 	Format           string `config:"format"`
 }

+// Config for the windows perfmon metricset.
+type PerfmonConfig struct {


type name will be used as perfmon.PerfmonConfig by other packages, and that stutters; consider calling this Config

houndci-bot · 2018-02-26T07:24:23Z

metricbeat/module/windows/perfmon/perfmon.go

@@ -21,6 +22,12 @@ type CounterConfig struct {
 	Format           string `config:"format"`
 }

+// Config for the windows perfmon metricset.


comment on exported type PerfmonConfig should be of the form "PerfmonConfig ..." (with optional leading article)

houndci-bot · 2018-02-26T07:24:23Z

metricbeat/module/windows/perfmon/perfmon.go

@@ -13,6 +13,7 @@ import (
 	"github.com/elastic/beats/metricbeat/mb"
 )

+// Config for perfmon counters.


comment on exported type CounterConfig should be of the form "CounterConfig ..." (with optional leading article)

I think houndci has a point here :-)

houndci-bot · 2018-02-26T07:24:23Z

metricbeat/module/windows/perfmon/pdh_windows.go

@@ -314,7 +316,8 @@ type PerfmonReader struct {
 	executed      bool // Indicates if the query has been executed.
 }

-func NewPerfmonReader(config []CounterConfig) (*PerfmonReader, error) {
+// Creates a new instance of PerfmonReader.


comment on exported function NewPerfmonReader should be of the form "NewPerfmonReader ..."

ruflin · 2018-02-27T00:24:36Z

metricbeat/module/windows/perfmon/pdh_integration_windows_test.go

+
+	values, err := handle.Read()
+
+	time.Sleep(time.Millisecond * 1000)


Any other way instead of using Sleep here? It makes the test suite much slower and has always potential to break.

ruflin · 2018-03-04T04:46:26Z

jenkins, test it

krasekhi · 2018-03-05T21:16:12Z

Any chance this enhancement will make it in the 6.2.x or 6.3 release?

andrewkroh · 2018-03-08T04:14:02Z

It looks like we have a build error with this:

module\windows\perfmon\pdh_integration_windows_test.go:110:12: undefined: PerfmonConfig

https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-windows/3383/beat=metricbeat,label=windows/

andrewkroh · 2018-03-08T04:18:13Z

@krasekhi Since this is new feature it will likely make it into 6.3 rather than a 6.2 patch release.

Once it merges into master would you be up for testing the feature using one of our snapshot releases? Those artifacts are updated after every commit to master. After you test it please drop a message here (good or bad, any feedback helps). Thanks

martinscholz83 · 2018-03-10T10:03:32Z

@andrewkroh, thanks for the commit! Didn't had the time last week.

Add option to ignore non existent counters

b2e86f1

houndci-bot reviewed Feb 21, 2018

View reviewed changes

kvch added review Metricbeat Metricbeat labels Feb 21, 2018

ruflin requested a review from andrewkroh February 23, 2018 07:27

Add changelog.

b56740d

houndci-bot reviewed Feb 26, 2018

View reviewed changes

ruflin added the module label Feb 26, 2018

martinscholz83 added 2 commits February 26, 2018 08:28

FIx houndci

8e8b3ad

Fix new config param

72ac918

ruflin reviewed Feb 27, 2018

View reviewed changes

martinscholz83 added 2 commits February 27, 2018 15:44

Fix comment

1cb711f

Remove unnecessary time.Sleep

666f838

krasekhi mentioned this pull request Mar 5, 2018

Metricbeat - Add config to Ignore failures for non-existent counters #6362

Closed

Remove reference to PerfmonConfig

a359acf

andrewkroh merged commit 75228cc into elastic:master Mar 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perfmon ignore non existent counters #6432

Perfmon ignore non existent counters #6432

martinscholz83 commented Feb 21, 2018

elasticmachine commented Feb 21, 2018

houndci-bot Feb 21, 2018

houndci-bot Feb 21, 2018

houndci-bot Feb 21, 2018

houndci-bot Feb 21, 2018

houndci-bot Feb 21, 2018

houndci-bot Feb 21, 2018

kvch commented Feb 21, 2018

ruflin commented Feb 23, 2018

kvch commented Feb 23, 2018

houndci-bot Feb 26, 2018

houndci-bot Feb 26, 2018

houndci-bot Feb 26, 2018

ruflin Feb 27, 2018

houndci-bot Feb 26, 2018

ruflin Feb 27, 2018

ruflin commented Mar 4, 2018

krasekhi commented Mar 5, 2018

andrewkroh commented Mar 8, 2018

andrewkroh commented Mar 8, 2018 •

edited

Loading

martinscholz83 commented Mar 10, 2018


		values, err := handle.Read()

		time.Sleep(time.Millisecond * 1000)

Perfmon ignore non existent counters #6432

Perfmon ignore non existent counters #6432

Conversation

martinscholz83 commented Feb 21, 2018

elasticmachine commented Feb 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvch commented Feb 21, 2018

ruflin commented Feb 23, 2018

kvch commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Mar 4, 2018

krasekhi commented Mar 5, 2018

andrewkroh commented Mar 8, 2018

andrewkroh commented Mar 8, 2018 • edited Loading

martinscholz83 commented Mar 10, 2018

andrewkroh commented Mar 8, 2018 •

edited

Loading