From ea250c85ea26c7067a590246eff54301f76d791d Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 6 Oct 2021 15:35:05 +0200 Subject: [PATCH 1/2] remove deprecated config --- CHANGELOG.next.asciidoc | 1 + .../windows/perfmon/_meta/docs.asciidoc | 57 ------ metricbeat/module/windows/perfmon/config.go | 33 +--- .../module/windows/perfmon/perfmon_test.go | 186 +----------------- metricbeat/module/windows/perfmon/reader.go | 12 -- .../module/windows/perfmon/reader_test.go | 21 +- 6 files changed, 7 insertions(+), 303 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 85dbbe04827..d4d16034f17 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -115,6 +115,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Remove deprecated fields in Docker module. {issue}11835[11835] {pull}27933[27933] - Remove deprecated fields in Kafka module. {pull}27938[27938] - Remove deprecated config option default_region from aws module. {pull}28120[28120] +- Remove deprecated config option perfmon.counters from windows/perfmon metricset. {pull}28120[28120] *Packetbeat* diff --git a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc index 93c39acf1d3..9550e771ebc 100644 --- a/metricbeat/module/windows/perfmon/_meta/docs.asciidoc +++ b/metricbeat/module/windows/perfmon/_meta/docs.asciidoc @@ -34,22 +34,6 @@ to collect. The example below collects processor time and disk writes every field: "write_time" format: "float" - - // deprecated, will be removed in 8.0 - - perfmon.counters: - - instance_label: processor.name - instance_name: total - measurement_label: processor.time.total.pct - query: '\Processor Information(_Total)\% Processor Time' - - - instance_label: physical_disk.name - measurement_label: physical_disk.write.per_sec - query: '\PhysicalDisk(*)\Disk Writes/sec' - - - instance_label: physical_disk.name - measurement_label: physical_disk.write.time.pct - query: '\PhysicalDisk(*)\% Disk Write Time' ---- *`ignore_non_existent_counters`*:: A boolean option that causes the @@ -65,9 +49,6 @@ The default behaviour is for all measurements to be sent as separate events. *`refresh_wildcard_counters`*:: A boolean option to refresh the counter list at each fetch. By default, the counter list will be retrieved at the starting time, to refresh the list at each fetch, users will have to enable this setting. -*`counters`*:: Counters specifies a list of queries to perform. Each individual -counter requires three config options - `instance_label`, `measurement_label`, -and `query`. [float] ==== Query Configuration @@ -92,41 +73,3 @@ the value for this configuration option will be `% Processor Time`. *`format`*:: Format of the measurement value. The value can be either `float`, `large` or `long`. The default is `float`. - - - -[float] -==== Deprecated Counter Configuration - -Each item in the `counters` list specifies a perfmon query to perform. In the -events generated by the metricset these configuration options map to the field -values as shown below. - ----- -"%[instance_label]": "%[instance_name] or ", -"%[measurement_label]": , ----- - -*`instance_label`*:: The label used to identify the counter instance. - -*`instance_name`*:: The instance name to use in the event when the counter's -path (`query`) does not include an instance or when you want to override the -instance name. For example with `\Processor Information(_Total)` the -instance name would be `_Total` and by setting `instance_name: total` you can -override the value. -+ -The setting has no effect with wildcard queries (e.g. -`\PhysicalDisk(*)\Disk Writes/sec`). - -*`measurement_label`*:: The label used for the value returned by the query. -This field is required. - -*`query`*:: The perfmon query. This is the counter path specified in -Performance Data Helper (PDH) syntax. This field is required. For example -`\Processor Information(_Total)\% Processor Time`. An asterisk can be used in -place of an instance name to perform a wildcard query that generates an event -for each counter instance (e.g. `\PhysicalDisk(*)\Disk Writes/sec`). - -*`format`*:: Format of the measurement value. The value can be either `float`, `large` or -`long`. The default is `float`. - diff --git a/metricbeat/module/windows/perfmon/config.go b/metricbeat/module/windows/perfmon/config.go index 39d86b2cc80..7d8b857613c 100644 --- a/metricbeat/module/windows/perfmon/config.go +++ b/metricbeat/module/windows/perfmon/config.go @@ -23,8 +23,6 @@ import ( "time" "github.com/pkg/errors" - - "github.com/elastic/beats/v7/libbeat/common/cfgwarn" ) var allowedFormats = []string{"float", "large", "long"} @@ -35,20 +33,10 @@ type Config struct { IgnoreNECounters bool `config:"perfmon.ignore_non_existent_counters"` GroupMeasurements bool `config:"perfmon.group_measurements_by_instance"` RefreshWildcardCounters bool `config:"perfmon.refresh_wildcard_counters"` - Counters []Counter `config:"perfmon.counters"` Queries []Query `config:"perfmon.queries"` GroupAllCountersTo string `config:"perfmon.group_all_counter"` } -// Counter for the perfmon counters (old implementation deprecated). -type Counter struct { - InstanceLabel string `config:"instance_label"` - InstanceName string `config:"instance_name"` - MeasurementLabel string `config:"measurement_label" validate:"required"` - Query string `config:"query" validate:"required"` - Format string `config:"format"` -} - // QueryConfig for perfmon queries. This will be used as the new configuration format type Query struct { Name string `config:"object" validate:"required"` @@ -73,19 +61,6 @@ func (counter *QueryCounter) InitDefaults() { counter.Format = "float" } -func (counter *Counter) InitDefaults() { - counter.Format = "float" -} - -func (counter *Counter) Validate() error { - if !isValidFormat(counter.Format) { - return errors.Errorf("initialization failed: format '%s' "+ - "for counter '%s' is invalid (must be float, large or long)", - counter.Format, counter.InstanceLabel) - } - return nil -} - func (counter *QueryCounter) Validate() error { if !isValidFormat(counter.Format) { return errors.Errorf("initialization failed: format '%s' "+ @@ -96,12 +71,8 @@ func (counter *QueryCounter) Validate() error { } func (conf *Config) Validate() error { - if len(conf.Counters) == 0 && len(conf.Queries) == 0 { - return errors.New("no perfmon counters or queries have been configured") - } - if len(conf.Counters) > 0 { - cfgwarn.Deprecate("8.0", "perfmon.counters configuration option is deprecated and will be removed in the future major version, "+ - "we advise using the perfmon.queries configuration option instead.") + if len(conf.Queries) == 0 { + return errors.New("No perfmon queries have been configured. Please follow documentation on allowed configuration settings (perfmon.counters configuration option has been deprecated and is removed in 8.0, perfmon.queries configuration option can be used instead). ") } return nil } diff --git a/metricbeat/module/windows/perfmon/perfmon_test.go b/metricbeat/module/windows/perfmon/perfmon_test.go index cb7961c4b00..573571629e9 100644 --- a/metricbeat/module/windows/perfmon/perfmon_test.go +++ b/metricbeat/module/windows/perfmon/perfmon_test.go @@ -30,7 +30,6 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" - "github.com/elastic/beats/v7/metricbeat/mb" mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing" ) @@ -40,6 +39,7 @@ func TestData(t *testing.T) { config := map[string]interface{}{ "module": "windows", "metricsets": []string{"perfmon"}, + "period": "10s", "perfmon.queries": []map[string]interface{}{ { "object": "Processor Information", @@ -67,49 +67,11 @@ func TestData(t *testing.T) { } -func TestDataDeprecated(t *testing.T) { - config := map[string]interface{}{ - "module": "windows", - "metricsets": []string{"perfmon"}, - "perfmon.counters": []map[string]string{ - { - "instance_label": "processor.name", - "measurement_label": "processor.time.total.pct", - "query": `\Processor Information(_Total)\% Processor Time`, - }, - { - "instance_label": "process.name", - "measurement_label": "process.ID", - "query": `\Process(_Total)\ID Process`, - }, - { - "instance_label": "processor.name", - "measurement_label": "processor.time.user.ns", - "query": `\Processor Information(_Total)\% User Time`, - }, - }, - } - - ms := mbtest.NewReportingMetricSetV2Error(t, config) - mbtest.ReportingFetchV2Error(ms) - time.Sleep(60 * time.Millisecond) - - events, errs := mbtest.ReportingFetchV2Error(ms) - if len(errs) > 0 { - t.Fatal(errs) - } - if len(events) == 0 { - t.Fatal("no events received") - } - - beatEvent := mbtest.StandardizeEvent(ms, events[0], mb.AddMetricSetInfo) - mbtest.WriteEventToDataJSON(t, beatEvent, "") -} - func TestCounterWithNoInstanceName(t *testing.T) { config := map[string]interface{}{ "module": "windows", "metricsets": []string{"perfmon"}, + "period": "10s", "perfmon.queries": []map[string]interface{}{ { "object": "UDPv4", @@ -203,28 +165,6 @@ func TestExistingCounter(t *testing.T) { t.Log(values) } -func TestExistingCounterDeprecated(t *testing.T) { - config := Config{ - Counters: make([]Counter, 1), - } - config.Counters[0].InstanceLabel = "processor.name" - config.Counters[0].MeasurementLabel = "processor.time.total.pct" - config.Counters[0].Query = processorTimeCounter - config.Counters[0].Format = "float" - handle, err := NewReader(config) - if err != nil { - t.Fatal(err) - } - defer handle.query.Close() - - values, err := handle.Read() - if err != nil { - t.Fatal(err) - } - - t.Log(values) -} - func TestNonExistingCounter(t *testing.T) { config := Config{ Queries: make([]Query, 1), @@ -247,25 +187,6 @@ func TestNonExistingCounter(t *testing.T) { } } -func TestNonExistingCounterDeprecated(t *testing.T) { - config := Config{ - Counters: make([]Counter, 1), - } - config.Counters[0].InstanceLabel = "processor.name" - config.Counters[0].MeasurementLabel = "processor.time.total.pct" - config.Counters[0].Query = "\\Processor Information(_Total)\\not existing counter" - config.Counters[0].Format = "float" - handle, err := NewReader(config) - if assert.Error(t, err) { - assert.EqualValues(t, pdh.PDH_CSTATUS_NO_COUNTER, errors.Cause(err)) - } - - if handle != nil { - err = handle.query.Close() - assert.NoError(t, err) - } -} - func TestIgnoreNonExistentCounter(t *testing.T) { config := Config{ Queries: make([]Query, 1), @@ -294,31 +215,6 @@ func TestIgnoreNonExistentCounter(t *testing.T) { t.Log(values) } -func TestIgnoreNonExistentCounterDeprecated(t *testing.T) { - config := Config{ - Counters: make([]Counter, 1), - IgnoreNECounters: true, - } - config.Counters[0].InstanceLabel = "processor.name" - config.Counters[0].MeasurementLabel = "processor.time.total.pct" - config.Counters[0].Query = "\\Processor Information(_Total)\\not existing counter" - config.Counters[0].Format = "float" - handle, err := NewReader(config) - - values, err := handle.Read() - - if assert.Error(t, err) { - assert.EqualValues(t, pdh.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 := Config{ Queries: make([]Query, 1), @@ -341,25 +237,6 @@ func TestNonExistingObject(t *testing.T) { } } -func TestNonExistingObjectDeprecated(t *testing.T) { - config := Config{ - Counters: make([]Counter, 1), - } - config.Counters[0].InstanceLabel = "processor.name" - config.Counters[0].MeasurementLabel = "processor.time.total.pct" - config.Counters[0].Query = "\\non existing object\\% Processor Performance" - config.Counters[0].Format = "float" - handle, err := NewReader(config) - if assert.Error(t, err) { - assert.EqualValues(t, pdh.PDH_CSTATUS_NO_OBJECT, errors.Cause(err)) - } - - if handle != nil { - err = handle.query.Close() - assert.NoError(t, err) - } -} - func TestLongOutputFormat(t *testing.T) { var query pdh.Query err := query.Open() @@ -575,62 +452,3 @@ func TestGroupByInstance(t *testing.T) { t.Log(values) } - -func TestGroupByInstanceDeprecated(t *testing.T) { - config := Config{ - Counters: make([]Counter, 3), - GroupMeasurements: true, - } - config.Counters[0].InstanceLabel = "processor.name" - config.Counters[0].MeasurementLabel = "processor.time.pct" - config.Counters[0].Query = `\Processor Information(_Total)\% Processor Time` - config.Counters[0].Format = "float" - - config.Counters[1].InstanceLabel = "processor.name" - config.Counters[1].MeasurementLabel = "processor.time.user.pct" - config.Counters[1].Query = `\Processor Information(_Total)\% User Time` - config.Counters[1].Format = "float" - - config.Counters[2].InstanceLabel = "processor.name" - config.Counters[2].MeasurementLabel = "processor.time.privileged.ns" - config.Counters[2].Query = `\Processor Information(_Total)\% Privileged Time` - config.Counters[2].Format = "float" - - handle, err := NewReader(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("processor.time.user.pct") - if err != nil { - t.Fatal(err) - } - assert.True(t, pctKey) - - pctKey, err = values[0].MetricSetFields.HasKey("processor.time.privileged.ns") - if err != nil { - t.Fatal(err) - } - assert.True(t, pctKey) - - t.Log(values) -} diff --git a/metricbeat/module/windows/perfmon/reader.go b/metricbeat/module/windows/perfmon/reader.go index 3e62e197849..a97140e2565 100644 --- a/metricbeat/module/windows/perfmon/reader.go +++ b/metricbeat/module/windows/perfmon/reader.go @@ -218,18 +218,6 @@ func (re *Reader) getCounter(query string) (bool, PerfCounter) { func (re *Reader) mapCounters(config Config) { re.counters = []PerfCounter{} - if len(config.Counters) > 0 { - for _, counter := range config.Counters { - re.counters = append(re.counters, PerfCounter{ - InstanceField: counter.InstanceLabel, - InstanceName: counter.InstanceName, - QueryField: counter.MeasurementLabel, - QueryName: counter.Query, - Format: counter.Format, - ChildQueries: nil, - }) - } - } if len(config.Queries) > 0 { for _, query := range config.Queries { for _, counter := range query.Counters { diff --git a/metricbeat/module/windows/perfmon/reader_test.go b/metricbeat/module/windows/perfmon/reader_test.go index 818f07441e3..0234ca243dd 100644 --- a/metricbeat/module/windows/perfmon/reader_test.go +++ b/metricbeat/module/windows/perfmon/reader_test.go @@ -53,15 +53,6 @@ func TestMapCounters(t *testing.T) { config := Config{ IgnoreNECounters: false, GroupMeasurements: false, - Counters: []Counter{ - { - InstanceLabel: "physical_disk.name", - InstanceName: "total", - MeasurementLabel: "physical_disk.write.time.pct", - Query: `\PhysicalDisk(*)\% Disk Write Time`, - Format: "float", - }, - }, Queries: []Query{ { Name: "Process", @@ -91,17 +82,9 @@ func TestMapCounters(t *testing.T) { } reader := Reader{} reader.mapCounters(config) - assert.Equal(t, len(reader.counters), 3) + assert.Equal(t, len(reader.counters), 2) for _, readerCounter := range reader.counters { - if readerCounter.InstanceField == "physical_disk.name" { - assert.Equal(t, readerCounter.InstanceName, "total") - assert.Equal(t, readerCounter.ObjectName, "") - assert.Equal(t, readerCounter.ObjectField, "") - assert.Equal(t, readerCounter.QueryField, "physical_disk.write.time.pct") - assert.Equal(t, readerCounter.QueryName, `\PhysicalDisk(*)\% Disk Write Time`) - assert.Equal(t, len(readerCounter.ChildQueries), 0) - assert.Equal(t, readerCounter.Format, "float") - } else if readerCounter.InstanceName == "svchost*" { + if readerCounter.InstanceName == "svchost*" { assert.Equal(t, readerCounter.ObjectName, "Process") assert.Equal(t, readerCounter.ObjectField, "object") assert.Equal(t, readerCounter.QueryField, "metrics.%_processor_time") From 53623ae74ff88abe6ef2ba1bde00c48074a0811c Mon Sep 17 00:00:00 2001 From: narph Date: Wed, 6 Oct 2021 15:37:19 +0200 Subject: [PATCH 2/2] changelog --- CHANGELOG.next.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index d4d16034f17..75add67868e 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -115,7 +115,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Remove deprecated fields in Docker module. {issue}11835[11835] {pull}27933[27933] - Remove deprecated fields in Kafka module. {pull}27938[27938] - Remove deprecated config option default_region from aws module. {pull}28120[28120] -- Remove deprecated config option perfmon.counters from windows/perfmon metricset. {pull}28120[28120] +- Remove deprecated config option perfmon.counters from windows/perfmon metricset. {pull}28282[28282] *Packetbeat*