From fa1969b7636176b419a4d53617a59525a43c6de9 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Tue, 23 Oct 2018 15:53:45 +1000 Subject: [PATCH 1/7] Add group_measurements_by_instance_label flag to perfmon configuration. This flag will send all perfmon measurements with a matching instance label as part of the same event (i.e. all metrics for C:\, Processer X, etc.). Enabling this flag considerably reduces the number of events sent by metricbeat. --- .../module/windows/_meta/config.reference.yml | 1 + .../module/windows/perfmon/pdh_windows.go | 57 ++++++++++++------- metricbeat/module/windows/perfmon/perfmon.go | 5 +- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/metricbeat/module/windows/_meta/config.reference.yml b/metricbeat/module/windows/_meta/config.reference.yml index d891fe62ec5..0c0dea1e418 100644 --- a/metricbeat/module/windows/_meta/config.reference.yml +++ b/metricbeat/module/windows/_meta/config.reference.yml @@ -3,6 +3,7 @@ enabled: true period: 10s perfmon.ignore_non_existent_counters: true + perfmon.group_measurements_by_instance_label: true perfmon.counters: # - instance_label: processor.name # instance_name: total diff --git a/metricbeat/module/windows/perfmon/pdh_windows.go b/metricbeat/module/windows/perfmon/pdh_windows.go index 291e909a5ec..c4b7e7df4d9 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -326,11 +326,12 @@ func (q *Query) Close() error { } type PerfmonReader struct { - query *Query // PDH Query - instanceLabel map[string]string // Mapping of counter path to key used for the label (e.g. processor.name) - measurement map[string]string // Mapping of counter path to key used for the value (e.g. processor.cpu_time). - executed bool // Indicates if the query has been executed. - log *logp.Logger + query *Query // PDH Query + instanceLabel map[string]string // Mapping of counter path to key used for the label (e.g. processor.name) + measurement map[string]string // Mapping of counter path to key used for the value (e.g. processor.cpu_time). + executed bool // Indicates if the query has been executed. + log *logp.Logger // + groupMeasurements bool // Indicates if measurements with the same instance label should be sent in the same event } // NewPerfmonReader creates a new instance of PerfmonReader. @@ -341,10 +342,11 @@ func NewPerfmonReader(config Config) (*PerfmonReader, error) { } r := &PerfmonReader{ - query: query, - instanceLabel: map[string]string{}, - measurement: map[string]string{}, - log: logp.NewLogger("perfmon"), + query: query, + instanceLabel: map[string]string{}, + measurement: map[string]string{}, + log: logp.NewLogger("perfmon"), + groupMeasurements: config.GroupMeasurements, } for _, counter := range config.CounterConfig { @@ -388,36 +390,53 @@ func (r *PerfmonReader) Read() ([]mb.Event, error) { return nil, errors.Wrap(err, "failed formatting counter values") } - // Write the values into the map. - events := make([]mb.Event, 0, len(values)) + eventMap := make(map[string]*mb.Event) for counterPath, values := range values { - for _, val := range values { + for ind, val := range values { if val.Err != nil && !r.executed { r.log.Debugw("Ignoring the first measurement because the data isn't ready", "error", val.Err, logp.Namespace("perfmon"), "query", counterPath) continue } - event := mb.Event{ - MetricSetFields: common.MapStr{}, - Error: errors.Wrapf(val.Err, "failed on query=%v", counterPath), + var eventKey string + if r.groupMeasurements { + // Send measurements with the same instance label as part of the same event + eventKey = val.Instance + } else { + // Send every measurement as an individual event + eventKey = counterPath + str(ind) } - if val.Instance != "" { - event.MetricSetFields.Put(r.instanceLabel[counterPath], val.Instance) + // Create a new event if the key doesn't exist in the map + if _, ok := eventMap[eventKey]; !ok { + eventMap[eventKey] = &mb.Event{ + MetricSetFields: common.MapStr{}, + Error: errors.Wrapf(val.Err, "failed on query=%v", counterPath), + } + + if val.Instance != "" { + eventMap[eventKey].MetricSetFields.Put(r.instanceLabel[counterPath], val.Instance) + } } + event := eventMap[eventKey] + if val.Measurement != nil { event.MetricSetFields.Put(r.measurement[counterPath], val.Measurement) } else { event.MetricSetFields.Put(r.measurement[counterPath], 0) } - - events = append(events, event) } } + // Write the values into the map. + events := make([]mb.Event, 0, len(eventMap)) + for _, val := range eventMap { + events = append(events, *val) + } + r.executed = true return events, nil } diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 090a3063238..581683abe23 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -40,8 +40,9 @@ type CounterConfig struct { // Config for the windows perfmon metricset. type Config struct { - IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` - CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` + IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` + GroupMeasurements bool `config:"perfmon.group_measurements_by_instance_label"` + CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` } func init() { From 0e954289c9e824b1b7595c24614282c33184ae29 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Tue, 23 Oct 2018 16:48:58 +1000 Subject: [PATCH 2/7] fix: Add group_measurements_by_instance flag to reference and docs files. --- metricbeat/metricbeat.reference.yml | 5 +++-- metricbeat/module/windows/_meta/config.reference.yml | 2 +- metricbeat/module/windows/perfmon/_meta/docs.asciidoc | 7 +++++++ metricbeat/module/windows/perfmon/perfmon.go | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/metricbeat/metricbeat.reference.yml b/metricbeat/metricbeat.reference.yml index 4db8f49b71f..eb21f4d86f1 100644 --- a/metricbeat/metricbeat.reference.yml +++ b/metricbeat/metricbeat.reference.yml @@ -661,6 +661,7 @@ metricbeat.modules: enabled: true period: 10s perfmon.ignore_non_existent_counters: true + perfmon.group_measurements_by_instance: true perfmon.counters: # - instance_label: processor.name # instance_name: total @@ -798,7 +799,7 @@ metricbeat.modules: # # event -> filter1 -> event1 -> filter2 ->event2 ... # -# The supported processors are drop_fields, drop_event, include_fields, +# The supported processors are drop_fields, drop_event, include_fields, # decode_json_fields, and add_cloud_metadata. # # For example, you can use the following processors to keep the fields that @@ -888,7 +889,7 @@ metricbeat.modules: # match_pids: ["system.process.ppid"] # target: system.process.parent # -# The following example decodes fields containing JSON strings +# The following example decodes fields containing JSON strings # and replaces the strings with valid JSON objects. # #processors: diff --git a/metricbeat/module/windows/_meta/config.reference.yml b/metricbeat/module/windows/_meta/config.reference.yml index 0c0dea1e418..0d06042ceb5 100644 --- a/metricbeat/module/windows/_meta/config.reference.yml +++ b/metricbeat/module/windows/_meta/config.reference.yml @@ -3,7 +3,7 @@ enabled: true period: 10s perfmon.ignore_non_existent_counters: true - perfmon.group_measurements_by_instance_label: true + perfmon.group_measurements_by_instance: true perfmon.counters: # - instance_label: processor.name # instance_name: total diff --git a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc index 28c05d3709d..61d03b5d569 100644 --- a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc +++ b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc @@ -14,6 +14,7 @@ to collect. The example below collects processor time and disk writes every metricsets: [perfmon] period: 10s perfmon.ignore_non_existent_counters: true + perfmon.group_measurements_by_instance: true perfmon.counters: - instance_label: processor.name instance_name: total @@ -34,6 +35,12 @@ metricset to ignore errors caused by counters that do not exist when set to true. Instead of an error, a message will be logged at the info level stating that the counter does not exist. +*`group_measurements_by_instance`*:: A boolean option that causes metricbeat +to send all measurements with a matching perfmon instance as part of a single +event. In the above example, this will cause the physical_disk.write.per_sec +and physical_disk.write.time.pct measurements to be sent as a single event. +The default behaviour is for all measurements to be sent as separate events. + *`counters`*:: Counters specifies a list of queries to perform. Each individual counter requires three config options - `instance_label`, `measurement_label`, and `query`. diff --git a/metricbeat/module/windows/perfmon/perfmon.go b/metricbeat/module/windows/perfmon/perfmon.go index 581683abe23..91cbcfcdc52 100644 --- a/metricbeat/module/windows/perfmon/perfmon.go +++ b/metricbeat/module/windows/perfmon/perfmon.go @@ -41,7 +41,7 @@ type CounterConfig struct { // Config for the windows perfmon metricset. type Config struct { IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` - GroupMeasurements bool `config:"perfmon.group_measurements_by_instance_label"` + GroupMeasurements bool `config:"perfmon.group_measurements_by_instance"` CounterConfig []CounterConfig `config:"perfmon.counters" validate:"required"` } From 9ee48b55079f61236d5fec89048f3a3def20e006 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Thu, 8 Nov 2018 13:39:11 +1000 Subject: [PATCH 3/7] Default group_measurements_by_instance and ignore_non_existent_counters fields to false. Update changelog and ensure windows.asciidoc is up to date. --- CHANGELOG-developer.asciidoc | 1 + metricbeat/docs/modules/windows.asciidoc | 3 ++- metricbeat/metricbeat.reference.yml | 8 ++++---- metricbeat/module/windows/_meta/config.reference.yml | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG-developer.asciidoc b/CHANGELOG-developer.asciidoc index 1581c93df95..58642673352 100644 --- a/CHANGELOG-developer.asciidoc +++ b/CHANGELOG-developer.asciidoc @@ -54,3 +54,4 @@ The list below covers the major changes between 6.3.0 and master only. - Set current year in generator templates. {pull}8396[8396] - You can now override default settings of libbeat by using instance.Settings. {pull}8449[8449] - Add `-space-id` option to `export_dashboards.go` script to support Kibana Spaces {pull}7942[7942] +- Add `group_measurements_by_instance` option to windows perfmon metricset. {pull}8688[8688] diff --git a/metricbeat/docs/modules/windows.asciidoc b/metricbeat/docs/modules/windows.asciidoc index 863abf40385..035dcf723ac 100644 --- a/metricbeat/docs/modules/windows.asciidoc +++ b/metricbeat/docs/modules/windows.asciidoc @@ -24,7 +24,8 @@ metricbeat.modules: metricsets: ["perfmon"] enabled: true period: 10s - perfmon.ignore_non_existent_counters: true + perfmon.ignore_non_existent_counters: false + perfmon.group_measurements_by_instance: false perfmon.counters: # - instance_label: processor.name # instance_name: total diff --git a/metricbeat/metricbeat.reference.yml b/metricbeat/metricbeat.reference.yml index eb21f4d86f1..0ac79abcc45 100644 --- a/metricbeat/metricbeat.reference.yml +++ b/metricbeat/metricbeat.reference.yml @@ -660,8 +660,8 @@ metricbeat.modules: metricsets: ["perfmon"] enabled: true period: 10s - perfmon.ignore_non_existent_counters: true - perfmon.group_measurements_by_instance: true + perfmon.ignore_non_existent_counters: false + perfmon.group_measurements_by_instance: false perfmon.counters: # - instance_label: processor.name # instance_name: total @@ -799,7 +799,7 @@ metricbeat.modules: # # event -> filter1 -> event1 -> filter2 ->event2 ... # -# The supported processors are drop_fields, drop_event, include_fields, +# The supported processors are drop_fields, drop_event, include_fields, # decode_json_fields, and add_cloud_metadata. # # For example, you can use the following processors to keep the fields that @@ -889,7 +889,7 @@ metricbeat.modules: # match_pids: ["system.process.ppid"] # target: system.process.parent # -# The following example decodes fields containing JSON strings +# The following example decodes fields containing JSON strings # and replaces the strings with valid JSON objects. # #processors: diff --git a/metricbeat/module/windows/_meta/config.reference.yml b/metricbeat/module/windows/_meta/config.reference.yml index 0d06042ceb5..dc9b27a9c3d 100644 --- a/metricbeat/module/windows/_meta/config.reference.yml +++ b/metricbeat/module/windows/_meta/config.reference.yml @@ -2,8 +2,8 @@ metricsets: ["perfmon"] enabled: true period: 10s - perfmon.ignore_non_existent_counters: true - perfmon.group_measurements_by_instance: true + perfmon.ignore_non_existent_counters: false + perfmon.group_measurements_by_instance: false perfmon.counters: # - instance_label: processor.name # instance_name: total From f7c07de4dbf237644fa7530b1f944fba931ea2f1 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Thu, 8 Nov 2018 15:07:31 +1000 Subject: [PATCH 4/7] Use correct string conversion. --- 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 c4b7e7df4d9..8414e79a477 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -406,7 +406,7 @@ func (r *PerfmonReader) Read() ([]mb.Event, error) { eventKey = val.Instance } else { // Send every measurement as an individual event - eventKey = counterPath + str(ind) + eventKey = counterPath + strconv.Itoa(ind) } // Create a new event if the key doesn't exist in the map From 663431d9d17021896123f26109c65a17e956d864 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Fri, 11 Jan 2019 17:18:39 +1000 Subject: [PATCH 5/7] If a counter contains an error, it should always be sent as an individual event. This solves the problem of only keeping the error contained in the first event. Also added simple unit test. --- .../perfmon/pdh_integration_windows_test.go | 59 +++++++++++++++++++ .../module/windows/perfmon/pdh_windows.go | 3 +- 2 files changed, 61 insertions(+), 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 1babaa0833d..998c270f0a2 100644 --- a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go +++ b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go @@ -336,3 +336,62 @@ func TestWildcardQuery(t *testing.T) { t.Log(values) } + +func TestGroupByInstance(t *testing.T) { + config := Config{ + CounterConfig: make([]CounterConfig, 3), + GroupMeasurements: true + } + config.CounterConfig[0].InstanceLabel = "processor.name" + config.CounterConfig[0].MeasurementLabel = "processor.time.pct" + config.CounterConfig[0].Query = `\Processor Information(_Total)\% Processor Time` + config.CounterConfig[0].Format = "float" + + config.CounterConfig[1].InstanceLabel = "disk.bytes.name" + config.CounterConfig[1].MeasurementLabel = "disk.bytes.read.total" + config.CounterConfig[1].Query = `\FileSystem Disk Activity(_Total)\FileSystem Bytes Read` + config.CounterConfig[1].Format = "float" + + config.CounterConfig[2].InstanceLabel = "processor.name" + config.CounterConfig[2].MeasurementLabel = "processor.time.idle.average.ns" + config.CounterConfig[2].Query = `\Processor Information(_Total)\Average Idle Time` + config.CounterConfig[2].Format = "float" + + handle, err := NewPerfmonReader(config) + if err != nil { + t.Fatal(err) + } + defer handle.query.Close() + + values, _ := handle.Read() + + time.Sleep(time.Millisecond * 1000) + + values, err = handle.Read() + if err != nil { + t.Fatal(err) + } + + assert.EqualValues(t, 1, len(values)) // Assert all metrics have been grouped into a single event + + // Test all keys exist in the event + pctKey, err := values[0].MetricSetFields.HasKey("processor.time.pct") + if err != nil { + t.Fatal(err) + } + assert.True(t, pctKey) + + pctKey, err := values[0].MetricSetFields.HasKey("disk.bytes.read.total") + if err != nil { + t.Fatal(err) + } + assert.True(t, pctKey) + + pctKey, err := values[0].MetricSetFields.HasKey("processor.time.idle.average.ns") + if err != nil { + t.Fatal(err) + } + assert.True(t, pctKey) + + t.Log(values) +} diff --git a/metricbeat/module/windows/perfmon/pdh_windows.go b/metricbeat/module/windows/perfmon/pdh_windows.go index 8414e79a477..eb755450731 100644 --- a/metricbeat/module/windows/perfmon/pdh_windows.go +++ b/metricbeat/module/windows/perfmon/pdh_windows.go @@ -401,11 +401,12 @@ func (r *PerfmonReader) Read() ([]mb.Event, error) { } var eventKey string - if r.groupMeasurements { + if r.groupMeasurements && val.Err == nil { // Send measurements with the same instance label as part of the same event eventKey = val.Instance } else { // Send every measurement as an individual event + // If a counter contains an error, it will always be sent as an individual event eventKey = counterPath + strconv.Itoa(ind) } From 6bb4187e13ede92848c973681e10d0b20580b83f Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Fri, 11 Jan 2019 17:21:03 +1000 Subject: [PATCH 6/7] fix missing comma --- .../module/windows/perfmon/pdh_integration_windows_test.go | 4 ++-- 1 file changed, 2 insertions(+), 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 998c270f0a2..8cfc24000de 100644 --- a/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go +++ b/metricbeat/module/windows/perfmon/pdh_integration_windows_test.go @@ -339,8 +339,8 @@ func TestWildcardQuery(t *testing.T) { func TestGroupByInstance(t *testing.T) { config := Config{ - CounterConfig: make([]CounterConfig, 3), - GroupMeasurements: true + CounterConfig: make([]CounterConfig, 3), + GroupMeasurements: true, } config.CounterConfig[0].InstanceLabel = "processor.name" config.CounterConfig[0].MeasurementLabel = "processor.time.pct" From 6e1be877dc99e2296ef6970e551f57c351a22a18 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Sat, 12 Jan 2019 20:56:23 +1000 Subject: [PATCH 7/7] Update metricbeat reference in x-pack folder --- x-pack/metricbeat/metricbeat.reference.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/metricbeat/metricbeat.reference.yml b/x-pack/metricbeat/metricbeat.reference.yml index db6d6640e2d..f3f4eee05ad 100644 --- a/x-pack/metricbeat/metricbeat.reference.yml +++ b/x-pack/metricbeat/metricbeat.reference.yml @@ -674,7 +674,8 @@ metricbeat.modules: metricsets: ["perfmon"] enabled: true period: 10s - perfmon.ignore_non_existent_counters: true + perfmon.ignore_non_existent_counters: false + perfmon.group_measurements_by_instance: false perfmon.counters: # - instance_label: processor.name # instance_name: total