From a7e1f30b0aa30393cf6567bddba47df1b33911a1 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 2 Mar 2020 12:01:50 -0700 Subject: [PATCH 1/4] Rename tags to tags_filter for cloudwatch metricset --- .../module/aws/cloudwatch/cloudwatch.go | 31 ++++++++++++++++--- .../module/aws/cloudwatch/cloudwatch_test.go | 10 +++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go index 43b021aa3656..194f8055e9ee 100644 --- a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go +++ b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go @@ -18,6 +18,7 @@ import ( "github.com/pkg/errors" "github.com/elastic/beats/libbeat/common" + "github.com/elastic/beats/libbeat/common/cfgwarn" "github.com/elastic/beats/metricbeat/mb" awscommon "github.com/elastic/beats/x-pack/libbeat/common/aws" "github.com/elastic/beats/x-pack/metricbeat/module/aws" @@ -66,7 +67,16 @@ type Config struct { Dimensions []Dimension `config:"dimensions"` ResourceTypeFilter string `config:"tags.resource_type_filter"` Statistic []string `config:"statistic"` - Tags []aws.Tag `config:"tags"` + TagsFilter []aws.Tag `config:"tags_filter"` + Tags []aws.Tag `config:"tags"` // Deprecated. +} + +// Validate checks for deprecated config options +func (c Config) Validate() error { + if c.Tags != nil { + cfgwarn.Deprecate("8.0.0", "tags is deprecated. Use tags_filter instead") + } + return nil } type metricsWithStatistics struct { @@ -305,10 +315,17 @@ func (m *MetricSet) readCloudwatchConfig() (listMetricWithDetail, map[string][]n if config.ResourceTypeFilter != "" { if _, ok := resourceTypesWithTags[config.ResourceTypeFilter]; ok { - resourceTypesWithTags[config.ResourceTypeFilter] = config.Tags - + if config.TagsFilter != nil { + resourceTypesWithTags[config.ResourceTypeFilter] = config.TagsFilter + } else { + resourceTypesWithTags[config.ResourceTypeFilter] = config.Tags + } } else { - resourceTypesWithTags[config.ResourceTypeFilter] = append(resourceTypesWithTags[config.ResourceTypeFilter], config.Tags...) + if config.TagsFilter != nil { + resourceTypesWithTags[config.ResourceTypeFilter] = append(resourceTypesWithTags[config.ResourceTypeFilter], config.TagsFilter...) + } else { + resourceTypesWithTags[config.ResourceTypeFilter] = append(resourceTypesWithTags[config.ResourceTypeFilter], config.Tags...) + } } } continue @@ -316,11 +333,15 @@ func (m *MetricSet) readCloudwatchConfig() (listMetricWithDetail, map[string][]n configPerNamespace := namespaceDetail{ names: config.MetricName, - tags: config.Tags, statistics: config.Statistic, resourceTypeFilter: config.ResourceTypeFilter, dimensions: cloudwatchDimensions, } + if config.TagsFilter != nil { + configPerNamespace.tags = config.TagsFilter + } else { + configPerNamespace.tags = config.Tags + } if _, ok := namespaceDetailTotal[config.Namespace]; ok { namespaceDetailTotal[config.Namespace] = append(namespaceDetailTotal[config.Namespace], configPerNamespace) diff --git a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go index 9ce957adc7b1..7dd1cdf2b4f8 100644 --- a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go +++ b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go @@ -500,7 +500,7 @@ func TestReadCloudwatchConfig(t *testing.T) { Namespace: "AWS/EC2", MetricName: []string{"CPUUtilization"}, ResourceTypeFilter: resourceTypeEC2, - Tags: []aws.Tag{ + TagsFilter: []aws.Tag{ { Key: "name", Value: "test-ec2", @@ -512,7 +512,7 @@ func TestReadCloudwatchConfig(t *testing.T) { MetricName: []string{"BackendConnectionErrors", "HTTPCode_Backend_2XX", "HTTPCode_Backend_3XX"}, Statistic: []string{"Sum"}, ResourceTypeFilter: "elasticloadbalancing", - Tags: []aws.Tag{ + TagsFilter: []aws.Tag{ { Key: "name", Value: "test-elb1", @@ -524,7 +524,7 @@ func TestReadCloudwatchConfig(t *testing.T) { MetricName: []string{"HealthyHostCount", "SurgeQueueLength", "UnHealthyHostCount"}, Statistic: []string{"Maximum"}, ResourceTypeFilter: "elasticloadbalancing", - Tags: []aws.Tag{ + TagsFilter: []aws.Tag{ { Key: "name", Value: "test-elb2", @@ -545,7 +545,7 @@ func TestReadCloudwatchConfig(t *testing.T) { MetricName: []string{"BackendConnectionErrors", "HTTPCode_Backend_2XX", "HTTPCode_Backend_3XX"}, Statistic: []string{"Sum"}, ResourceTypeFilter: "elasticloadbalancing", - Tags: []aws.Tag{ + TagsFilter: []aws.Tag{ { Key: "name", Value: "test-elb1", @@ -557,7 +557,7 @@ func TestReadCloudwatchConfig(t *testing.T) { MetricName: []string{"HealthyHostCount", "SurgeQueueLength", "UnHealthyHostCount"}, Statistic: []string{"Maximum"}, ResourceTypeFilter: "elasticloadbalancing", - Tags: []aws.Tag{ + TagsFilter: []aws.Tag{ { Key: "name", Value: "test-elb2", From b0203fd22a149bcf6f90cf2cb3e18c39a4e20d39 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Tue, 3 Mar 2020 14:53:37 -0700 Subject: [PATCH 2/4] check config.Tags or config.TagsFilter once at the beginning of the for loop --- .../module/aws/cloudwatch/cloudwatch.go | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go index 345f4fabb0db..6448172043ae 100644 --- a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go +++ b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go @@ -18,6 +18,7 @@ import ( "github.com/pkg/errors" "github.com/elastic/beats/v7/libbeat/common" + "github.com/elastic/beats/v7/libbeat/common/cfgwarn" "github.com/elastic/beats/v7/metricbeat/mb" awscommon "github.com/elastic/beats/v7/x-pack/libbeat/common/aws" "github.com/elastic/beats/v7/x-pack/metricbeat/module/aws" @@ -283,6 +284,11 @@ func (m *MetricSet) readCloudwatchConfig() (listMetricWithDetail, map[string][]n resourceTypesWithTags := map[string][]aws.Tag{} for _, config := range m.CloudwatchConfigs { + tagsFilter := config.Tags + if config.TagsFilter != nil { + tagsFilter = config.TagsFilter + } + // If there is no statistic method specified, then use the default. if config.Statistic == nil { config.Statistic = defaultStatistics @@ -314,17 +320,9 @@ func (m *MetricSet) readCloudwatchConfig() (listMetricWithDetail, map[string][]n if config.ResourceTypeFilter != "" { if _, ok := resourceTypesWithTags[config.ResourceTypeFilter]; ok { - if config.TagsFilter != nil { - resourceTypesWithTags[config.ResourceTypeFilter] = config.TagsFilter - } else { - resourceTypesWithTags[config.ResourceTypeFilter] = config.Tags - } + resourceTypesWithTags[config.ResourceTypeFilter] = tagsFilter } else { - if config.TagsFilter != nil { - resourceTypesWithTags[config.ResourceTypeFilter] = append(resourceTypesWithTags[config.ResourceTypeFilter], config.TagsFilter...) - } else { - resourceTypesWithTags[config.ResourceTypeFilter] = append(resourceTypesWithTags[config.ResourceTypeFilter], config.Tags...) - } + resourceTypesWithTags[config.ResourceTypeFilter] = append(resourceTypesWithTags[config.ResourceTypeFilter], tagsFilter...) } } continue @@ -332,15 +330,11 @@ func (m *MetricSet) readCloudwatchConfig() (listMetricWithDetail, map[string][]n configPerNamespace := namespaceDetail{ names: config.MetricName, + tags: tagsFilter, statistics: config.Statistic, resourceTypeFilter: config.ResourceTypeFilter, dimensions: cloudwatchDimensions, } - if config.TagsFilter != nil { - configPerNamespace.tags = config.TagsFilter - } else { - configPerNamespace.tags = config.Tags - } if _, ok := namespaceDetailTotal[config.Namespace]; ok { namespaceDetailTotal[config.Namespace] = append(namespaceDetailTotal[config.Namespace], configPerNamespace) From 33d49768e6e74b992e3c527ed19c6c5d691c9cd2 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 18 May 2020 12:41:31 -0600 Subject: [PATCH 3/4] update unit test --- .../module/aws/cloudwatch/cloudwatch.go | 1 - .../module/aws/cloudwatch/cloudwatch_test.go | 118 ++++++++++++------ 2 files changed, 82 insertions(+), 37 deletions(-) diff --git a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go index 6b5e4700977f..2ec9fe5461e1 100644 --- a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go +++ b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go @@ -70,7 +70,6 @@ type Config struct { Dimensions []Dimension `config:"dimensions"` ResourceTypeFilter string `config:"tags.resource_type_filter"` Statistic []string `config:"statistic"` - TagsFilter []aws.Tag `config:"tags_filter"` Tags []aws.Tag `config:"tags"` // Deprecated. } diff --git a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go index a43fdbeb7356..e6b5cdebe5a0 100644 --- a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go +++ b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch_test.go @@ -213,6 +213,53 @@ func TestReadCloudwatchConfig(t *testing.T) { resourceTypeFilters: resourceTypeFiltersEC2RDS, } + resourceTypeFiltersEC2RDSWithTag := map[string][]aws.Tag{} + resourceTypeFiltersEC2RDSWithTag["ec2:instance"] = []aws.Tag{ + { + Key: "name", + Value: "test", + }, + } + resourceTypeFiltersEC2RDSWithTag["rds"] = []aws.Tag{ + { + Key: "name", + Value: "test", + }, + } + expectedListMetricWithDetailEC2RDSWithTag := listMetricWithDetail{ + metricsWithStats: []metricsWithStatistics{ + { + cloudwatch.Metric{ + Dimensions: []cloudwatch.Dimension{{ + Name: awssdk.String("InstanceId"), + Value: awssdk.String("i-1"), + }}, + MetricName: awssdk.String("CPUUtilization"), + Namespace: awssdk.String("AWS/EC2"), + }, + []string{"Average"}, + nil, + }, + { + cloudwatch.Metric{ + Dimensions: []cloudwatch.Dimension{{ + Name: awssdk.String("DBClusterIdentifier"), + Value: awssdk.String("test1-cluster"), + }, + { + Name: awssdk.String("Role"), + Value: awssdk.String("READER"), + }}, + MetricName: awssdk.String("CommitThroughput"), + Namespace: awssdk.String("AWS/RDS"), + }, + []string{"Average"}, + nil, + }, + }, + resourceTypeFilters: resourceTypeFiltersEC2RDSWithTag, + } + expectedNamespaceDetailLambda := map[string][]namespaceDetail{} expectedNamespaceDetailLambda["AWS/Lambda"] = []namespaceDetail{ { @@ -269,7 +316,7 @@ func TestReadCloudwatchConfig(t *testing.T) { tags: []aws.Tag{ { Key: "name", - Value: "test-ec2", + Value: "test", }, }, }, @@ -282,7 +329,7 @@ func TestReadCloudwatchConfig(t *testing.T) { tags: []aws.Tag{ { Key: "name", - Value: "test-elb1", + Value: "test", }, }, }, @@ -293,7 +340,7 @@ func TestReadCloudwatchConfig(t *testing.T) { tags: []aws.Tag{ { Key: "name", - Value: "test-elb2", + Value: "test", }, }, }, @@ -303,6 +350,12 @@ func TestReadCloudwatchConfig(t *testing.T) { expectedNamespaceDetailELBLambda["AWS/Lambda"] = []namespaceDetail{ { statistics: defaultStatistics, + tags: []aws.Tag{ + { + Key: "name", + Value: "test", + }, + }, }, } expectedNamespaceDetailELBLambda["AWS/ELB"] = []namespaceDetail{ @@ -313,7 +366,7 @@ func TestReadCloudwatchConfig(t *testing.T) { tags: []aws.Tag{ { Key: "name", - Value: "test-elb1", + Value: "test", }, }, }, @@ -324,7 +377,7 @@ func TestReadCloudwatchConfig(t *testing.T) { tags: []aws.Tag{ { Key: "name", - Value: "test-elb2", + Value: "test", }, }, }, @@ -377,6 +430,7 @@ func TestReadCloudwatchConfig(t *testing.T) { cases := []struct { title string cloudwatchMetricsConfig []Config + tagsFilter []aws.Tag expectedListMetricDetailTotal listMetricWithDetail expectedNamespaceDetailTotal map[string][]namespaceDetail }{ @@ -396,6 +450,7 @@ func TestReadCloudwatchConfig(t *testing.T) { Statistic: []string{"Average"}, }, }, + nil, expectedListMetricWithDetailEC2, map[string][]namespaceDetail{}, }, @@ -418,6 +473,7 @@ func TestReadCloudwatchConfig(t *testing.T) { Namespace: "AWS/S3", }, }, + nil, expectedListMetricWithDetailEC2, expectedNamespaceWithDetailS3, }, @@ -456,6 +512,7 @@ func TestReadCloudwatchConfig(t *testing.T) { ResourceTypeFilter: "rds", }, }, + nil, expectedListMetricWithDetailEC2RDS, expectedNamespaceDetailLambda, }, @@ -472,6 +529,7 @@ func TestReadCloudwatchConfig(t *testing.T) { ResourceTypeFilter: "s3", }, }, + nil, listMetricWithDetail{ resourceTypeFilters: map[string][]aws.Tag{}, }, @@ -485,6 +543,7 @@ func TestReadCloudwatchConfig(t *testing.T) { ResourceTypeFilter: "ec2", }, }, + nil, listMetricWithDetail{ resourceTypeFilters: map[string][]aws.Tag{}, }, @@ -500,6 +559,7 @@ func TestReadCloudwatchConfig(t *testing.T) { Statistic: []string{"Average", "Maximum"}, }, }, + nil, listMetricWithDetail{ resourceTypeFilters: map[string][]aws.Tag{}, }, @@ -513,6 +573,7 @@ func TestReadCloudwatchConfig(t *testing.T) { MetricName: []string{"MemoryUsed"}, }, }, + nil, listMetricWithDetail{ resourceTypeFilters: map[string][]aws.Tag{}, }, @@ -525,36 +586,24 @@ func TestReadCloudwatchConfig(t *testing.T) { Namespace: "AWS/EC2", MetricName: []string{"CPUUtilization"}, ResourceTypeFilter: resourceTypeEC2, - TagsFilter: []aws.Tag{ - { - Key: "name", - Value: "test-ec2", - }, - }, }, { Namespace: "AWS/ELB", MetricName: []string{"BackendConnectionErrors", "HTTPCode_Backend_2XX", "HTTPCode_Backend_3XX"}, Statistic: []string{"Sum"}, ResourceTypeFilter: "elasticloadbalancing", - TagsFilter: []aws.Tag{ - { - Key: "name", - Value: "test-elb1", - }, - }, }, { Namespace: "AWS/ELB", MetricName: []string{"HealthyHostCount", "SurgeQueueLength", "UnHealthyHostCount"}, Statistic: []string{"Maximum"}, ResourceTypeFilter: "elasticloadbalancing", - TagsFilter: []aws.Tag{ - { - Key: "name", - Value: "test-elb2", - }, - }, + }, + }, + []aws.Tag{ + { + Key: "name", + Value: "test", }, }, listMetricWithDetail{ @@ -570,24 +619,12 @@ func TestReadCloudwatchConfig(t *testing.T) { MetricName: []string{"BackendConnectionErrors", "HTTPCode_Backend_2XX", "HTTPCode_Backend_3XX"}, Statistic: []string{"Sum"}, ResourceTypeFilter: "elasticloadbalancing", - TagsFilter: []aws.Tag{ - { - Key: "name", - Value: "test-elb1", - }, - }, }, { Namespace: "AWS/ELB", MetricName: []string{"HealthyHostCount", "SurgeQueueLength", "UnHealthyHostCount"}, Statistic: []string{"Maximum"}, ResourceTypeFilter: "elasticloadbalancing", - TagsFilter: []aws.Tag{ - { - Key: "name", - Value: "test-elb2", - }, - }, }, { Namespace: "AWS/Lambda", @@ -621,7 +658,13 @@ func TestReadCloudwatchConfig(t *testing.T) { ResourceTypeFilter: "rds", }, }, - expectedListMetricWithDetailEC2RDS, + []aws.Tag{ + { + Key: "name", + Value: "test", + }, + }, + expectedListMetricWithDetailEC2RDSWithTag, expectedNamespaceDetailELBLambda, }, { @@ -639,6 +682,7 @@ func TestReadCloudwatchConfig(t *testing.T) { ResourceTypeFilter: "ec2:instance", }, }, + nil, listMetricWithDetail{ resourceTypeFilters: map[string][]aws.Tag{}, }, @@ -660,6 +704,7 @@ func TestReadCloudwatchConfig(t *testing.T) { Statistic: []string{"Average"}, }, }, + nil, expectedListMetricsEC2WithDim, map[string][]namespaceDetail{}, }, @@ -668,6 +713,7 @@ func TestReadCloudwatchConfig(t *testing.T) { for _, c := range cases { t.Run(c.title, func(t *testing.T) { m.CloudwatchConfigs = c.cloudwatchMetricsConfig + m.MetricSet.TagsFilter = c.tagsFilter listMetricDetailTotal, namespaceDetailTotal := m.readCloudwatchConfig() assert.Equal(t, c.expectedListMetricDetailTotal, listMetricDetailTotal) assert.Equal(t, c.expectedNamespaceDetailTotal, namespaceDetailTotal) From 94c7b7df19644d4fe05b873be4d78e9faf6618da Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Tue, 19 May 2020 10:24:54 -0600 Subject: [PATCH 4/4] add changelog --- CHANGELOG.next.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 2517950a478c..7d8aed0cf37c 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -455,7 +455,7 @@ field. You can revert this change by configuring tags for the module and omittin *Journalbeat* *Metricbeat* - +- Deprecate tags config parameter in cloudwatch metricset. {pull}16733[16733] *Packetbeat*