From 7f9d894300a0bf07de37587317a9b48bab3eaea9 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 15 Sep 2022 16:12:03 -0600 Subject: [PATCH 01/13] Change max query size for GetMetricData API to 500 --- x-pack/metricbeat/module/aws/utils.go | 37 +++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index bef034013bb..b7162c66cfa 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -67,12 +67,12 @@ func GetListMetricsOutput(namespace string, regionName string, svcCloudwatch clo // GetMetricDataResults function uses MetricDataQueries to get metric data output. func GetMetricDataResults(metricDataQueries []types.MetricDataQuery, svc cloudwatch.GetMetricDataAPIClient, startTime time.Time, endTime time.Time) ([]types.MetricDataResult, error) { - maxQuerySize := 100 + maxQuerySize := 500 getMetricDataOutput := &cloudwatch.GetMetricDataOutput{NextToken: nil} - // Split metricDataQueries into smaller slices that length no longer than 100. - // 100 is defined in maxQuerySize. - // To avoid ValidationError: The collection MetricDataQueries must not have a size greater than 100. + // Split metricDataQueries into smaller slices that length no longer than 500. + // 500 is defined in maxQuerySize. + // To avoid ValidationError: The collection MetricDataQueries must not have a size greater than 500. for i := 0; i < len(metricDataQueries); i += maxQuerySize { metricDataQueriesPartial := metricDataQueries[i:int(math.Min(float64(i+maxQuerySize), float64(len(metricDataQueries))))] if len(metricDataQueriesPartial) == 0 { @@ -111,19 +111,22 @@ func CheckTimestampInArray(timestamp time.Time, timestampArray []time.Time) (boo // FindTimestamp function checks MetricDataResults and find the timestamp to collect metrics from. // For example, MetricDataResults might look like: -// metricDataResults = [{ -// Id: "sqs0", -// Label: "testName SentMessageSize", -// StatusCode: Complete, -// Timestamps: [2019-03-11 17:45:00 +0000 UTC], -// Values: [981] -// } { -// Id: "sqs1", -// Label: "testName NumberOfMessagesSent", -// StatusCode: Complete, -// Timestamps: [2019-03-11 17:45:00 +0000 UTC,2019-03-11 17:40:00 +0000 UTC], -// Values: [0.5,0] -// }] +// +// metricDataResults = [{ +// Id: "sqs0", +// Label: "testName SentMessageSize", +// StatusCode: Complete, +// Timestamps: [2019-03-11 17:45:00 +0000 UTC], +// Values: [981] +// } { +// +// Id: "sqs1", +// Label: "testName NumberOfMessagesSent", +// StatusCode: Complete, +// Timestamps: [2019-03-11 17:45:00 +0000 UTC,2019-03-11 17:40:00 +0000 UTC], +// Values: [0.5,0] +// }] +// // This case, we are collecting values for both metrics from timestamp 2019-03-11 17:45:00 +0000 UTC. func FindTimestamp(getMetricDataResults []types.MetricDataResult) time.Time { timestamp := time.Time{} From 585a83cc29c8a2e37d8810d93855def2c23ec6b9 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 15 Sep 2022 16:15:11 -0600 Subject: [PATCH 02/13] add changelog --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 846af334d3b..bf653ec27be 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -88,6 +88,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff] - in module/windows/perfmon, changed collection method of the second counter value required to create a displayable value {pull}32305[32305] - Fix and improve AWS metric period calculation to avoid zero-length intervals {pull}32724[32724] - Add missing cluster metadata to k8s module metricsets {pull}32979[32979] {pull}33032[33032] +- Change max query size for GetMetricData API to 500 {pull}33105[33105] *Packetbeat* From d2e18278437ccb7e13c589fae25da21f61fadf70 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 19 Sep 2022 13:51:27 -0600 Subject: [PATCH 03/13] change variable name to maxNumberOfMetricsRetrieved --- x-pack/metricbeat/module/aws/utils.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index b7162c66cfa..fb80658885c 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -67,14 +67,14 @@ func GetListMetricsOutput(namespace string, regionName string, svcCloudwatch clo // GetMetricDataResults function uses MetricDataQueries to get metric data output. func GetMetricDataResults(metricDataQueries []types.MetricDataQuery, svc cloudwatch.GetMetricDataAPIClient, startTime time.Time, endTime time.Time) ([]types.MetricDataResult, error) { - maxQuerySize := 500 + maxNumberOfMetricsRetrieved := 500 getMetricDataOutput := &cloudwatch.GetMetricDataOutput{NextToken: nil} // Split metricDataQueries into smaller slices that length no longer than 500. - // 500 is defined in maxQuerySize. + // 500 is defined in maxNumberOfMetricsRetrieved. // To avoid ValidationError: The collection MetricDataQueries must not have a size greater than 500. - for i := 0; i < len(metricDataQueries); i += maxQuerySize { - metricDataQueriesPartial := metricDataQueries[i:int(math.Min(float64(i+maxQuerySize), float64(len(metricDataQueries))))] + for i := 0; i < len(metricDataQueries); i += maxNumberOfMetricsRetrieved { + metricDataQueriesPartial := metricDataQueries[i:int(math.Min(float64(i+maxNumberOfMetricsRetrieved), float64(len(metricDataQueries))))] if len(metricDataQueriesPartial) == 0 { return getMetricDataOutput.MetricDataResults, nil } From 138ac0bff4af11db4c963f6eb18ced2de7162d76 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 22 Sep 2022 12:11:20 -0600 Subject: [PATCH 04/13] try fix ci by adding terraform init into the cleanup script --- .ci/scripts/terraform-cleanup.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ci/scripts/terraform-cleanup.sh b/.ci/scripts/terraform-cleanup.sh index f1051b9b20d..246dcdf6f9c 100755 --- a/.ci/scripts/terraform-cleanup.sh +++ b/.ci/scripts/terraform-cleanup.sh @@ -1,5 +1,7 @@ #!/usr/bin/env bash +terraform init + set -exuo pipefail DIRECTORY=${1:-.} From 4dbb0074171b524286cab2870cffda662c6abf75 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 22 Sep 2022 14:26:37 -0600 Subject: [PATCH 05/13] undo change for cleanup script --- .ci/scripts/terraform-cleanup.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/.ci/scripts/terraform-cleanup.sh b/.ci/scripts/terraform-cleanup.sh index 246dcdf6f9c..f1051b9b20d 100755 --- a/.ci/scripts/terraform-cleanup.sh +++ b/.ci/scripts/terraform-cleanup.sh @@ -1,7 +1,5 @@ #!/usr/bin/env bash -terraform init - set -exuo pipefail DIRECTORY=${1:-.} From 71e3b1cd200dcc5402e4a34467a075ed73b75ce8 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 22 Sep 2022 15:58:12 -0600 Subject: [PATCH 06/13] add RecentlyActive: PT3H for ListMetrics API call --- x-pack/metricbeat/module/aws/utils.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index fb80658885c..cf5196feea3 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -44,7 +44,8 @@ func GetListMetricsOutput(namespace string, regionName string, svcCloudwatch clo var nextToken *string listMetricsInput := &cloudwatch.ListMetricsInput{ - NextToken: nextToken, + NextToken: nextToken, + RecentlyActive: "PT3H", } if namespace != "*" { From 8eaaa451ac8e21e369297671c8fc2ea00dac4127 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 22 Sep 2022 16:01:52 -0600 Subject: [PATCH 07/13] adjust changelog --- CHANGELOG.next.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index c80777739c6..f986d026129 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -88,7 +88,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff] - in module/windows/perfmon, changed collection method of the second counter value required to create a displayable value {pull}32305[32305] - Fix and improve AWS metric period calculation to avoid zero-length intervals {pull}32724[32724] - Add missing cluster metadata to k8s module metricsets {pull}32979[32979] {pull}33032[33032] -- Change max query size for GetMetricData API to 500 {pull}33105[33105] +- Change max query size for GetMetricData API to 500 and add RecentlyActive for ListMetrics API call {pull}33105[33105] - Add GCP CloudSQL region filter {pull}32943[32943] *Packetbeat* From 9c252363ad96d11855e663092f7567579aed6eb7 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 26 Sep 2022 11:07:20 -0600 Subject: [PATCH 08/13] add comment for RecentlyActive parameter --- .../module/aws/billing/billing_integration_test.go | 2 ++ x-pack/metricbeat/module/aws/utils.go | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/metricbeat/module/aws/billing/billing_integration_test.go b/x-pack/metricbeat/module/aws/billing/billing_integration_test.go index d9056ff3816..acdacf27791 100644 --- a/x-pack/metricbeat/module/aws/billing/billing_integration_test.go +++ b/x-pack/metricbeat/module/aws/billing/billing_integration_test.go @@ -21,6 +21,8 @@ import ( ) func TestFetch(t *testing.T) { + t.Skip("Flaky test: https://github.com/elastic/beats/issues/33190") + config := mtest.GetConfigForTest(t, "billing", "24h") metricSet := mbtest.NewReportingMetricSetV2Error(t, config) diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index cf5196feea3..6c7d3633263 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -44,7 +44,10 @@ func GetListMetricsOutput(namespace string, regionName string, svcCloudwatch clo var nextToken *string listMetricsInput := &cloudwatch.ListMetricsInput{ - NextToken: nextToken, + NextToken: nextToken, + // To filter the results to show only metrics that have had data points published + // in the past three hours, specify this parameter with a value of PT3H. + // Please see https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_ListMetrics.html for more details. RecentlyActive: "PT3H", } From 5dce10c4791848165247ed524629d7f5bb51388e Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 26 Sep 2022 12:40:35 -0600 Subject: [PATCH 09/13] add terraform init before terraform destroy --- .ci/scripts/terraform-cleanup.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.ci/scripts/terraform-cleanup.sh b/.ci/scripts/terraform-cleanup.sh index f1051b9b20d..c9107c9b099 100755 --- a/.ci/scripts/terraform-cleanup.sh +++ b/.ci/scripts/terraform-cleanup.sh @@ -7,6 +7,7 @@ DIRECTORY=${1:-.} FAILED=0 for tfstate in $(find $DIRECTORY -name terraform.tfstate); do cd $(dirname $tfstate) + terraform init if ! terraform destroy -auto-approve; then FAILED=1 fi From d19184dcdb2cedc0597d93e80bd72c4af5ee0a10 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 26 Sep 2022 15:34:27 -0600 Subject: [PATCH 10/13] remove flaky test --- .../metricbeat/module/aws/billing/billing_integration_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/metricbeat/module/aws/billing/billing_integration_test.go b/x-pack/metricbeat/module/aws/billing/billing_integration_test.go index acdacf27791..d9056ff3816 100644 --- a/x-pack/metricbeat/module/aws/billing/billing_integration_test.go +++ b/x-pack/metricbeat/module/aws/billing/billing_integration_test.go @@ -21,8 +21,6 @@ import ( ) func TestFetch(t *testing.T) { - t.Skip("Flaky test: https://github.com/elastic/beats/issues/33190") - config := mtest.GetConfigForTest(t, "billing", "24h") metricSet := mbtest.NewReportingMetricSetV2Error(t, config) From be019920078b85e8a23fa52f8424836bb9bb53a6 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 29 Sep 2022 06:14:06 -0600 Subject: [PATCH 11/13] use constant from aws sdk instead --- x-pack/metricbeat/module/aws/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index 6c7d3633263..702fba21674 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -48,7 +48,7 @@ func GetListMetricsOutput(namespace string, regionName string, svcCloudwatch clo // To filter the results to show only metrics that have had data points published // in the past three hours, specify this parameter with a value of PT3H. // Please see https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_ListMetrics.html for more details. - RecentlyActive: "PT3H", + RecentlyActive: types.RecentlyActivePt3h, } if namespace != "*" { From 76c255e9353da1760a5dfa9746f5af4ff431ba6f Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 3 Oct 2022 09:03:04 -0600 Subject: [PATCH 12/13] Add period check before adding RecentlyActive parameter --- x-pack/metricbeat/module/aws/billing/billing.go | 2 +- .../metricbeat/module/aws/cloudwatch/cloudwatch.go | 2 +- x-pack/metricbeat/module/aws/utils.go | 13 ++++++++----- x-pack/metricbeat/module/aws/utils_test.go | 4 ++-- x-pack/metricbeat/module/azure/billing/billing.go | 4 ++-- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/x-pack/metricbeat/module/aws/billing/billing.go b/x-pack/metricbeat/module/aws/billing/billing.go index e8510c4de7c..37f88deccd9 100644 --- a/x-pack/metricbeat/module/aws/billing/billing.go +++ b/x-pack/metricbeat/module/aws/billing/billing.go @@ -171,7 +171,7 @@ func (m *MetricSet) getCloudWatchBillingMetrics( endTime time.Time) []mb.Event { var events []mb.Event namespace := "AWS/Billing" - listMetricsOutput, err := aws.GetListMetricsOutput(namespace, regionName, svcCloudwatch) + listMetricsOutput, err := aws.GetListMetricsOutput(namespace, regionName, m.Period, svcCloudwatch) if err != nil { m.Logger().Error(err.Error()) return nil diff --git a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go index 3ed35ff6576..7f7ac5a2d1d 100644 --- a/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go +++ b/x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go @@ -183,7 +183,7 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { for namespace, namespaceDetails := range namespaceDetailTotal { m.logger.Debugf("Collected metrics from namespace %s", namespace) - listMetricsOutput, err := aws.GetListMetricsOutput(namespace, regionName, svcCloudwatch) + listMetricsOutput, err := aws.GetListMetricsOutput(namespace, regionName, m.Period, svcCloudwatch) if err != nil { m.logger.Info(err.Error()) continue diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index 702fba21674..cd998dd14f2 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -39,16 +39,19 @@ func GetStartTimeEndTime(now time.Time, period time.Duration, latency time.Durat // GetListMetricsOutput function gets listMetrics results from cloudwatch ~~per namespace~~ for each region. // ListMetrics Cloudwatch API is used to list the specified metrics. The returned metrics can be used with GetMetricData // to obtain statistical data. -func GetListMetricsOutput(namespace string, regionName string, svcCloudwatch cloudwatch.ListMetricsAPIClient) ([]types.Metric, error) { +func GetListMetricsOutput(namespace string, regionName string, period time.Duration, svcCloudwatch cloudwatch.ListMetricsAPIClient) ([]types.Metric, error) { var metricsTotal []types.Metric var nextToken *string listMetricsInput := &cloudwatch.ListMetricsInput{ NextToken: nextToken, - // To filter the results to show only metrics that have had data points published - // in the past three hours, specify this parameter with a value of PT3H. - // Please see https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_ListMetrics.html for more details. - RecentlyActive: types.RecentlyActivePt3h, + } + + // To filter the results to show only metrics that have had data points published + // in the past three hours, specify this parameter with a value of PT3H. + // Please see https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_ListMetrics.html for more details. + if period <= time.Hour*3 { + listMetricsInput.RecentlyActive = types.RecentlyActivePt3h } if namespace != "*" { diff --git a/x-pack/metricbeat/module/aws/utils_test.go b/x-pack/metricbeat/module/aws/utils_test.go index 3329ee45e2d..d1dad2c756b 100644 --- a/x-pack/metricbeat/module/aws/utils_test.go +++ b/x-pack/metricbeat/module/aws/utils_test.go @@ -137,7 +137,7 @@ func (m *MockResourceGroupsTaggingClient) GetResources(_ context.Context, _ *res func TestGetListMetricsOutput(t *testing.T) { svcCloudwatch := &MockCloudWatchClient{} - listMetricsOutput, err := GetListMetricsOutput("AWS/EC2", "us-west-1", svcCloudwatch) + listMetricsOutput, err := GetListMetricsOutput("AWS/EC2", "us-west-1", time.Minute*5, svcCloudwatch) assert.NoError(t, err) assert.Equal(t, 1, len(listMetricsOutput)) assert.Equal(t, namespace, *listMetricsOutput[0].Namespace) @@ -149,7 +149,7 @@ func TestGetListMetricsOutput(t *testing.T) { func TestGetListMetricsOutputWithWildcard(t *testing.T) { svcCloudwatch := &MockCloudWatchClient{} - listMetricsOutput, err := GetListMetricsOutput("*", "us-west-1", svcCloudwatch) + listMetricsOutput, err := GetListMetricsOutput("*", "us-west-1", time.Minute*5, svcCloudwatch) assert.NoError(t, err) assert.Equal(t, 1, len(listMetricsOutput)) assert.Equal(t, namespace, *listMetricsOutput[0].Namespace) diff --git a/x-pack/metricbeat/module/azure/billing/billing.go b/x-pack/metricbeat/module/azure/billing/billing.go index e2d349a53f3..a2968d71a6d 100644 --- a/x-pack/metricbeat/module/azure/billing/billing.go +++ b/x-pack/metricbeat/module/azure/billing/billing.go @@ -113,8 +113,8 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { // Currently, the usage period is the start/end time (00:00:00->23:59:59 UTC) of the day before the reference time. // // For example, if the reference time is 2007-01-09 09:41:00Z, the usage period is: -// 2007-01-08 00:00:00Z -> 2007-01-08 23:59:59Z // +// 2007-01-08 00:00:00Z -> 2007-01-08 23:59:59Z func usageIntervalFrom(reference time.Time) (time.Time, time.Time) { beginningOfDay := reference.UTC().Truncate(24 * time.Hour).Add((-24) * time.Hour) endOfDay := beginningOfDay.Add(time.Hour * 24).Add(time.Second * (-1)) @@ -127,8 +127,8 @@ func usageIntervalFrom(reference time.Time) (time.Time, time.Time) { // reference time. // // For example, if the reference time is 2007-01-09 09:41:00Z, the forecast period is: -// 2007-01-01T00:00:00Z -> 2007-01-31:59:59Z // +// 2007-01-01T00:00:00Z -> 2007-01-31:59:59Z func forecastIntervalFrom(reference time.Time) (time.Time, time.Time) { referenceUTC := reference.UTC() beginningOfMonth := time.Date(referenceUTC.Year(), referenceUTC.Month(), 1, 0, 0, 0, 0, time.UTC) From db4a88a758e1ea73fd26b71a070f8542b77fe6ee Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Mon, 10 Oct 2022 13:44:24 -0600 Subject: [PATCH 13/13] add a comment for GetListMetricsOutput --- x-pack/metricbeat/module/aws/utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/metricbeat/module/aws/utils.go b/x-pack/metricbeat/module/aws/utils.go index cd998dd14f2..d9810a8cae6 100644 --- a/x-pack/metricbeat/module/aws/utils.go +++ b/x-pack/metricbeat/module/aws/utils.go @@ -39,6 +39,8 @@ func GetStartTimeEndTime(now time.Time, period time.Duration, latency time.Durat // GetListMetricsOutput function gets listMetrics results from cloudwatch ~~per namespace~~ for each region. // ListMetrics Cloudwatch API is used to list the specified metrics. The returned metrics can be used with GetMetricData // to obtain statistical data. +// Note: We are not using Dimensions and MetricName in ListMetricsInput because with that we will have to make one ListMetrics +// API call per metric name and set of dimensions. This will increase API cost. func GetListMetricsOutput(namespace string, regionName string, period time.Duration, svcCloudwatch cloudwatch.ListMetricsAPIClient) ([]types.Metric, error) { var metricsTotal []types.Metric var nextToken *string