From c8ce20a807819626979fdf3facce1db53ec511d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ida=20=C5=A0tambuk?= Date: Thu, 27 Jun 2024 17:10:28 +0200 Subject: [PATCH] Cloudwatch: Round up endTime in GetMetricData to next minute (#89341) * Add cloudWatchRoundUpEndTime feature toggle --- .../feature-toggles/index.md | 1 + .../src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 7 ++++++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++++ pkg/services/featuremgmt/toggles_gen.json | 12 ++++++++++ pkg/tsdb/cloudwatch/features/features.go | 1 + .../cloudwatch/get_metric_data_executor.go | 7 ++++++ .../get_metric_data_executor_test.go | 24 +++++++++++++++++-- 9 files changed, 56 insertions(+), 2 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index d66777eb8ce..eaa7b073602 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -63,6 +63,7 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `alertingQueryOptimization` | Optimizes eligible queries in order to reduce load on datasources | | | `cloudWatchNewLabelParsing` | Updates CloudWatch label parsing to be more accurate | Yes | | `pluginProxyPreserveTrailingSlash` | Preserve plugin proxy trailing slash. | | +| `cloudWatchRoundUpEndTime` | Round up end time for metric queries to the next minute to avoid missing data | Yes | ## Public preview feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 30fd6997a52..2b56f82d2bb 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -198,4 +198,5 @@ export interface FeatureToggles { passScopeToDashboardApi?: boolean; alertingApiServer?: boolean; dashboardRestoreUI?: boolean; + cloudWatchRoundUpEndTime?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 68c11c6def8..059cceee86b 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1353,6 +1353,13 @@ var ( Owner: grafanaFrontendPlatformSquad, Expression: "false", // enabled by default }, + { + Name: "cloudWatchRoundUpEndTime", + Description: "Round up end time for metric queries to the next minute to avoid missing data", + Stage: FeatureStageGeneralAvailability, + Owner: awsDatasourcesSquad, + Expression: "true", + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index c3fb9ac3a46..3bb0bf05cac 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -179,3 +179,4 @@ zanzana,experimental,@grafana/identity-access-team,false,false,false passScopeToDashboardApi,experimental,@grafana/dashboards-squad,false,false,false alertingApiServer,experimental,@grafana/alerting-squad,false,true,false dashboardRestoreUI,experimental,@grafana/grafana-frontend-platform,false,false,false +cloudWatchRoundUpEndTime,GA,@grafana/aws-datasources,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 3891a5c1441..75ecaa053b5 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -726,4 +726,8 @@ const ( // FlagDashboardRestoreUI // Enables the frontend to be able to restore a recently deleted dashboard FlagDashboardRestoreUI = "dashboardRestoreUI" + + // FlagCloudWatchRoundUpEndTime + // Round up end time for metric queries to the next minute to avoid missing data + FlagCloudWatchRoundUpEndTime = "cloudWatchRoundUpEndTime" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index f9f6320eae2..10c6014ea06 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -563,6 +563,18 @@ "codeowner": "@grafana/aws-datasources" } }, + { + "metadata": { + "name": "cloudWatchRoundUpEndTime", + "resourceVersion": "1719324143210", + "creationTimestamp": "2024-06-25T14:02:23Z" + }, + "spec": { + "description": "Round up end time for metric queries to the next minute to avoid missing data", + "stage": "GA", + "codeowner": "@grafana/aws-datasources" + } + }, { "metadata": { "name": "configurableSchedulerTick", diff --git a/pkg/tsdb/cloudwatch/features/features.go b/pkg/tsdb/cloudwatch/features/features.go index 1ec7975d6e7..aa766c31dd3 100644 --- a/pkg/tsdb/cloudwatch/features/features.go +++ b/pkg/tsdb/cloudwatch/features/features.go @@ -10,6 +10,7 @@ const ( FlagCloudWatchCrossAccountQuerying = "cloudWatchCrossAccountQuerying" FlagCloudWatchBatchQueries = "cloudWatchBatchQueries" FlagCloudWatchNewLabelParsing = "cloudWatchNewLabelParsing" + FlagCloudWatchRoundUpEndTime = "cloudWatchRoundUpEndTime" ) func IsEnabled(ctx context.Context, feature string) bool { diff --git a/pkg/tsdb/cloudwatch/get_metric_data_executor.go b/pkg/tsdb/cloudwatch/get_metric_data_executor.go index de89b89af2d..76c1b40e0e4 100644 --- a/pkg/tsdb/cloudwatch/get_metric_data_executor.go +++ b/pkg/tsdb/cloudwatch/get_metric_data_executor.go @@ -2,10 +2,12 @@ package cloudwatch import ( "context" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/aws/aws-sdk-go/service/cloudwatch/cloudwatchiface" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/features" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" ) @@ -18,6 +20,11 @@ func (e *cloudWatchExecutor) executeRequest(ctx context.Context, client cloudwat if nextToken != "" { metricDataInput.NextToken = aws.String(nextToken) } + // GetMetricData EndTime is exclusive, so we round up to the next minute to get the last data point + if features.IsEnabled(ctx, features.FlagCloudWatchRoundUpEndTime) { + *metricDataInput.EndTime = metricDataInput.EndTime.Truncate(time.Minute).Add(time.Minute) + } + resp, err := client.GetMetricDataWithContext(ctx, metricDataInput) if err != nil { return mdo, err diff --git a/pkg/tsdb/cloudwatch/get_metric_data_executor_test.go b/pkg/tsdb/cloudwatch/get_metric_data_executor_test.go index 4312fa4f364..3485daff5da 100644 --- a/pkg/tsdb/cloudwatch/get_metric_data_executor_test.go +++ b/pkg/tsdb/cloudwatch/get_metric_data_executor_test.go @@ -3,18 +3,38 @@ package cloudwatch import ( "context" "testing" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudwatch" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/features" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -func TestGetMetricDataExecutorTest(t *testing.T) { +func TestGetMetricDataExecutorTestRequest(t *testing.T) { + t.Run("Should round up end time if cloudWatchRoundUpEndTime is enabled", func(t *testing.T) { + executor := &cloudWatchExecutor{} + queryEndTime, _ := time.Parse("2006-01-02T15:04:05Z07:00", "2024-05-01T01:45:04Z") + inputs := &cloudwatch.GetMetricDataInput{EndTime: &queryEndTime, MetricDataQueries: []*cloudwatch.MetricDataQuery{}} + mockMetricClient := &mocks.MetricsAPI{} + mockMetricClient.On("GetMetricDataWithContext", mock.Anything, mock.Anything, mock.Anything).Return( + &cloudwatch.GetMetricDataOutput{ + MetricDataResults: []*cloudwatch.MetricDataResult{{Values: []*float64{}}}, + }, nil).Once() + _, err := executor.executeRequest(contextWithFeaturesEnabled(features.FlagCloudWatchRoundUpEndTime), mockMetricClient, inputs) + require.NoError(t, err) + expectedTime, _ := time.Parse("2006-01-02T15:04:05Z07:00", "2024-05-01T01:46:00Z") + expectedInput := &cloudwatch.GetMetricDataInput{EndTime: &expectedTime, MetricDataQueries: []*cloudwatch.MetricDataQuery{}} + mockMetricClient.AssertCalled(t, "GetMetricDataWithContext", mock.Anything, expectedInput, mock.Anything) + }) +} + +func TestGetMetricDataExecutorTestResponse(t *testing.T) { executor := &cloudWatchExecutor{} - inputs := &cloudwatch.GetMetricDataInput{MetricDataQueries: []*cloudwatch.MetricDataQuery{}} + inputs := &cloudwatch.GetMetricDataInput{EndTime: aws.Time(time.Now()), MetricDataQueries: []*cloudwatch.MetricDataQuery{}} mockMetricClient := &mocks.MetricsAPI{} mockMetricClient.On("GetMetricDataWithContext", mock.Anything, mock.Anything, mock.Anything).Return( &cloudwatch.GetMetricDataOutput{