From b2e86f15df3d127f30eb9a5473e150c5fe2774a7 Mon Sep 17 00:00:00 2001 From: Martin Scholz Date: Wed, 21 Feb 2018 11:28:39 +0100 Subject: [PATCH 1/7] Add option to ignore non existent counters --- .../windows/perfmon/_meta/docs.asciidoc | 6 +- .../perfmon/pdh_integration_windows_test.go | 75 ++++++++++++++----- .../module/windows/perfmon/pdh_windows.go | 16 +++- metricbeat/module/windows/perfmon/perfmon.go | 11 ++- 4 files changed, 78 insertions(+), 30 deletions(-) diff --git a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc index e1a8e980c1a3..dd8d1e01653b 100644 --- a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc +++ b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc @@ -6,8 +6,9 @@ performance counters. You must configure queries for the Windows performance counters that you wish to collect. The example below collects processor time and disk writes. -With `format` you can set the output format for a specific counter. Possible values are -`float` and `long`. If nothing is selected the default value is `float`. +`ignore_non_existent_counters` ignores failures for non-existent counters without +to interrupt the service. With `format` you can set the output format for a specific counter. +Possible values are `float` and `long`. If nothing is selected the default value is `float`. With `instance_name`, you can specify the name of the instance. Use this setting when: - You want to use an instance name that is different from the computed name. For example, `Total` instead of `_Total`. - You specify a counter that has no instance. For example, `\TCPIP Performance Diagnostics\IPv4 NBLs/sec indicated without prevalidation`. @@ -19,6 +20,7 @@ For wildcard queries this setting has no effect. - module: windows metricsets: ["perfmon"] period: 10s + perfmon.ignore_non_existent_counters: true perfmon.counters: - instance_label: "processor.name" instance_name: "Total" diff --git a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go index 89ba63157183..b7c234d87de5 100644 --- a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go +++ b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go @@ -85,11 +85,13 @@ func TestQuery(t *testing.T) { } func TestExistingCounter(t *testing.T) { - config := make([]CounterConfig, 1) - config[0].InstanceLabel = "processor.name" - config[0].MeasurementLabel = "processor.time.total.pct" - config[0].Query = processorTimeCounter - config[0].Format = "float" + config := PerfmonConfig{ + CounterConfig: make([]CounterConfig, 1), + } + config.CounterConfig[0].InstanceLabel = "processor.name" + config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct" + config.CounterConfig[0].Query = processorTimeCounter + config.CounterConfig[0].Format = "float" handle, err := NewPerfmonReader(config) if err != nil { t.Fatal(err) @@ -105,11 +107,13 @@ func TestExistingCounter(t *testing.T) { } func TestNonExistingCounter(t *testing.T) { - config := make([]CounterConfig, 1) - config[0].InstanceLabel = "processor.name" - config[0].MeasurementLabel = "processor.time.total.pct" - config[0].Query = "\\Processor Information(_Total)\\not existing counter" - config[0].Format = "float" + config := PerfmonConfig{ + CounterConfig: make([]CounterConfig, 1), + } + config.CounterConfig[0].InstanceLabel = "processor.name" + config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct" + config.CounterConfig[0].Query = "\\Processor Information(_Total)\\not existing counter" + config.CounterConfig[0].Format = "float" handle, err := NewPerfmonReader(config) if assert.Error(t, err) { assert.EqualValues(t, PDH_CSTATUS_NO_COUNTER, errors.Cause(err)) @@ -121,12 +125,41 @@ func TestNonExistingCounter(t *testing.T) { } } +func TestIgnoreNonExistentCounter(t *testing.T) { + config := PerfmonConfig{ + CounterConfig: make([]CounterConfig, 1), + IgnoreNECounters: true, + } + config.CounterConfig[0].InstanceLabel = "processor.name" + config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct" + config.CounterConfig[0].Query = "\\Processor Information(_Total)\\not existing counter" + config.CounterConfig[0].Format = "float" + handle, err := NewPerfmonReader(config) + + values, err := handle.Read() + + time.Sleep(time.Millisecond * 1000) + + if assert.Error(t, err) { + assert.EqualValues(t, PDH_NO_DATA, errors.Cause(err)) + } + + if handle != nil { + err = handle.query.Close() + assert.NoError(t, err) + } + + t.Log(values) +} + func TestNonExistingObject(t *testing.T) { - config := make([]CounterConfig, 1) - config[0].InstanceLabel = "processor.name" - config[0].MeasurementLabel = "processor.time.total.pct" - config[0].Query = "\\non existing object\\% Processor Performance" - config[0].Format = "float" + config := PerfmonConfig{ + CounterConfig: make([]CounterConfig, 1), + } + config.CounterConfig[0].InstanceLabel = "processor.name" + config.CounterConfig[0].MeasurementLabel = "processor.time.total.pct" + config.CounterConfig[0].Query = "\\non existing object\\% Processor Performance" + config.CounterConfig[0].Format = "float" handle, err := NewPerfmonReader(config) if assert.Error(t, err) { assert.EqualValues(t, PDH_CSTATUS_NO_OBJECT, errors.Cause(err)) @@ -253,11 +286,13 @@ func TestRawValues(t *testing.T) { } func TestWildcardQuery(t *testing.T) { - config := make([]CounterConfig, 1) - config[0].InstanceLabel = "processor.name" - config[0].MeasurementLabel = "processor.time.pct" - config[0].Query = `\Processor Information(*)\% Processor Time` - config[0].Format = "float" + config := PerfmonConfig{ + CounterConfig: make([]CounterConfig, 1), + } + config.CounterConfig[0].InstanceLabel = "processor.name" + config.CounterConfig[0].MeasurementLabel = "processor.time.pct" + config.CounterConfig[0].Query = `\Processor Information(*)\% Processor Time` + config.CounterConfig[0].Format = "float" handle, err := NewPerfmonReader(config) if err != nil { t.Fatal(err) diff --git a/metricbeat/module/windows/perfmon/pdh_windows.go b/metricbeat/module/windows/perfmon/pdh_windows.go index 34016d291283..4daff9a7a7ef 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -10,6 +10,8 @@ import ( "unicode/utf16" "unsafe" + "github.com/elastic/beats/libbeat/logp" + "github.com/joeshaw/multierror" "github.com/pkg/errors" "golang.org/x/sys/windows" @@ -222,7 +224,7 @@ func (q *Query) AddCounter(counterPath string, format Format, instanceName strin h, err := PdhAddCounter(q.handle, counterPath, 0) if err != nil { - return errors.Wrapf(err, `failed to add counter (path="%v")`, counterPath) + return err } wildcard := wildcardRegexp.MatchString(counterPath) @@ -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) { query, err := NewQuery("") if err != nil { return nil, err @@ -326,7 +328,7 @@ func NewPerfmonReader(config []CounterConfig) (*PerfmonReader, error) { measurement: map[string]string{}, } - for _, counter := range config { + for _, counter := range config.CounterConfig { var format Format switch counter.Format { case "float": @@ -335,8 +337,14 @@ func NewPerfmonReader(config []CounterConfig) (*PerfmonReader, error) { format = LongFormat } if err := query.AddCounter(counter.Query, format, counter.InstanceName); err != nil { + if config.IgnoreNECounters { + if err == PDH_CSTATUS_NO_COUNTER { + logp.Info(`ignore non existent counter (path="%v")`, counter.Query) + continue + } + } query.Close() - return nil, err + return nil, errors.Wrapf(err, `failed to add counter (path="%v")`, counter.Query) } r.instanceLabel[counter.Query] = counter.InstanceLabel diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 29bc10048ae7..3d36e00278ab 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -21,6 +21,11 @@ type CounterConfig struct { Format string `config:"format"` } +type PerfmonConfig struct { + IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` + CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` +} + func init() { if err := mb.Registry.AddMetricSet("windows", "perfmon", New); err != nil { panic(err) @@ -36,9 +41,7 @@ type MetricSet struct { func New(base mb.BaseMetricSet) (mb.MetricSet, error) { cfgwarn.Beta("The perfmon metricset is beta") - config := struct { - CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` - }{} + config := PerfmonConfig{} if err := base.Module().UnpackConfig(&config); err != nil { return nil, err @@ -57,7 +60,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { } - reader, err := NewPerfmonReader(config.CounterConfig) + reader, err := NewPerfmonReader(config) if err != nil { return nil, errors.Wrap(err, "initialization failed") } From b56740d35a1ab83a47236d32ef571e676248da72 Mon Sep 17 00:00:00 2001 From: Martin Scholz Date: Mon, 26 Feb 2018 08:22:11 +0100 Subject: [PATCH 2/7] Add changelog. --- CHANGELOG.asciidoc | 1 + metricbeat/module/windows/perfmon/pdh_windows.go | 1 + metricbeat/module/windows/perfmon/perfmon.go | 2 ++ 3 files changed, 4 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index fdf7d3f6ef10..b4e6578b81c1 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -62,6 +62,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - Fix the default configuration for Logstash to include the default port. {pull}6279[6279] - Add filtering option by exact device names in system.diskio. `diskio.include_devices`. {pull}6085[6085] - Fix dealing with new process status codes in Linux kernel 4.14+. {pull}6306[6306] +- Add config option for windows/perfmon metricset to ignore non existent counters. {pull}6432[6432] *Packetbeat* diff --git a/metricbeat/module/windows/perfmon/pdh_windows.go b/metricbeat/module/windows/perfmon/pdh_windows.go index 4daff9a7a7ef..3923ec0fc7c4 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -316,6 +316,7 @@ type PerfmonReader struct { executed bool // Indicates if the query has been executed. } +// Creates a new instance of PerfmonReader. func NewPerfmonReader(config PerfmonConfig) (*PerfmonReader, error) { query, err := NewQuery("") if err != nil { diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 3d36e00278ab..1c114d0becbe 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -13,6 +13,7 @@ import ( "github.com/elastic/beats/metricbeat/mb" ) +// Config for perfmon counters. type CounterConfig struct { InstanceLabel string `config:"instance_label" validate:"required"` InstanceName string `config:"instance_name"` @@ -21,6 +22,7 @@ type CounterConfig struct { Format string `config:"format"` } +// Config for the windows perfmon metricset. type PerfmonConfig struct { IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` From 8e8b3ad3c37cbe366a3d69a9d06500d7a42abff9 Mon Sep 17 00:00:00 2001 From: Martin Scholz Date: Mon, 26 Feb 2018 08:28:38 +0100 Subject: [PATCH 3/7] FIx houndci --- .../windows/perfmon/pdh_integration_windows_test.go | 8 ++++---- metricbeat/module/windows/perfmon/pdh_windows.go | 2 +- metricbeat/module/windows/perfmon/perfmon.go | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go index b7c234d87de5..d354d844c09d 100644 --- a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go +++ b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go @@ -85,7 +85,7 @@ func TestQuery(t *testing.T) { } func TestExistingCounter(t *testing.T) { - config := PerfmonConfig{ + config := Config{ CounterConfig: make([]CounterConfig, 1), } config.CounterConfig[0].InstanceLabel = "processor.name" @@ -126,7 +126,7 @@ func TestNonExistingCounter(t *testing.T) { } func TestIgnoreNonExistentCounter(t *testing.T) { - config := PerfmonConfig{ + config := Config{ CounterConfig: make([]CounterConfig, 1), IgnoreNECounters: true, } @@ -153,7 +153,7 @@ func TestIgnoreNonExistentCounter(t *testing.T) { } func TestNonExistingObject(t *testing.T) { - config := PerfmonConfig{ + config := Config{ CounterConfig: make([]CounterConfig, 1), } config.CounterConfig[0].InstanceLabel = "processor.name" @@ -286,7 +286,7 @@ func TestRawValues(t *testing.T) { } func TestWildcardQuery(t *testing.T) { - config := PerfmonConfig{ + config := Config{ CounterConfig: make([]CounterConfig, 1), } config.CounterConfig[0].InstanceLabel = "processor.name" diff --git a/metricbeat/module/windows/perfmon/pdh_windows.go b/metricbeat/module/windows/perfmon/pdh_windows.go index 3923ec0fc7c4..cc43493883a9 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -316,7 +316,7 @@ type PerfmonReader struct { executed bool // Indicates if the query has been executed. } -// Creates a new instance of PerfmonReader. +// NewPerfmonReader creates a new instance of PerfmonReader. func NewPerfmonReader(config PerfmonConfig) (*PerfmonReader, error) { query, err := NewQuery("") if err != nil { diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 1c114d0becbe..ed44d86645c7 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -23,7 +23,7 @@ type CounterConfig struct { } // Config for the windows perfmon metricset. -type PerfmonConfig struct { +type Config struct { IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` } @@ -43,7 +43,7 @@ type MetricSet struct { func New(base mb.BaseMetricSet) (mb.MetricSet, error) { cfgwarn.Beta("The perfmon metricset is beta") - config := PerfmonConfig{} + config := Config{} if err := base.Module().UnpackConfig(&config); err != nil { return nil, err From 72ac918e595b4cdaba358e63474020d30c1283d7 Mon Sep 17 00:00:00 2001 From: Martin Scholz Date: Mon, 26 Feb 2018 11:05:42 +0100 Subject: [PATCH 4/7] Fix new config param --- metricbeat/module/windows/perfmon/pdh_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metricbeat/module/windows/perfmon/pdh_windows.go b/metricbeat/module/windows/perfmon/pdh_windows.go index cc43493883a9..f8f9f6e08119 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -317,7 +317,7 @@ type PerfmonReader struct { } // NewPerfmonReader creates a new instance of PerfmonReader. -func NewPerfmonReader(config PerfmonConfig) (*PerfmonReader, error) { +func NewPerfmonReader(config Config) (*PerfmonReader, error) { query, err := NewQuery("") if err != nil { return nil, err From 1cb711fa032fad8e1df7459c43149969b82669e2 Mon Sep 17 00:00:00 2001 From: Martin Scholz Date: Tue, 27 Feb 2018 15:44:47 +0100 Subject: [PATCH 5/7] Fix comment --- metricbeat/module/windows/perfmon/perfmon.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index ed44d86645c7..113549fece47 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -13,7 +13,7 @@ import ( "github.com/elastic/beats/metricbeat/mb" ) -// Config for perfmon counters. +// CounterConfig for perfmon counters. type CounterConfig struct { InstanceLabel string `config:"instance_label" validate:"required"` InstanceName string `config:"instance_name"` From 666f838e6b10c5ca94dd97f1b34b6047cf1d2b64 Mon Sep 17 00:00:00 2001 From: Martin Scholz Date: Tue, 27 Feb 2018 15:45:53 +0100 Subject: [PATCH 6/7] Remove unnecessary time.Sleep --- .../module/windows/perfmon/pdh_integration_windows_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go index d354d844c09d..e62fb2a12ce3 100644 --- a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go +++ b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go @@ -138,8 +138,6 @@ func TestIgnoreNonExistentCounter(t *testing.T) { values, err := handle.Read() - time.Sleep(time.Millisecond * 1000) - if assert.Error(t, err) { assert.EqualValues(t, PDH_NO_DATA, errors.Cause(err)) } From a359acfa3b64efa2a15828d5c7db7b1e2906321b Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Fri, 9 Mar 2018 14:13:23 -0500 Subject: [PATCH 7/7] Remove reference to PerfmonConfig --- .../module/windows/perfmon/pdh_integration_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go index e62fb2a12ce3..e4f6d6ccdb7d 100644 --- a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go +++ b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go @@ -107,7 +107,7 @@ func TestExistingCounter(t *testing.T) { } func TestNonExistingCounter(t *testing.T) { - config := PerfmonConfig{ + config := Config{ CounterConfig: make([]CounterConfig, 1), } config.CounterConfig[0].InstanceLabel = "processor.name"