From 5690859f5fcca9d174578ee8fe594285219b8a79 Mon Sep 17 00:00:00 2001 From: Isabella Siu Date: Thu, 9 Jan 2025 14:42:06 -0500 Subject: [PATCH] CloudWatch: Fix conditions for fetching wildcards (#98648) --- .../get_dimension_values_for_wildcards.go | 18 ++++++++- ...get_dimension_values_for_wildcards_test.go | 37 ++++++++++++++++--- pkg/tsdb/cloudwatch/time_series_query.go | 13 +------ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards.go b/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards.go index 96260038064..2919304d84a 100644 --- a/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards.go +++ b/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards.go @@ -5,12 +5,26 @@ import ( "fmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/clients" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/features" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/services" "github.com/patrickmn/go-cache" ) +func shouldSkipFetchingWildcards(ctx context.Context, q *models.CloudWatchQuery) bool { + newLabelParsingEnabled := features.IsEnabled(ctx, features.FlagCloudWatchNewLabelParsing) + if q.MetricQueryType == models.MetricQueryTypeSearch && (q.MatchExact || newLabelParsingEnabled) { + return true + } + + if q.MetricQueryType == models.MetricQueryTypeQuery && q.MetricEditorMode == models.MetricEditorModeRaw { + return true + } + + return false +} + // getDimensionValues gets the actual dimension values for dimensions with a wildcard func (e *cloudWatchExecutor) getDimensionValuesForWildcards( ctx context.Context, @@ -19,7 +33,7 @@ func (e *cloudWatchExecutor) getDimensionValuesForWildcards( origQueries []*models.CloudWatchQuery, tagValueCache *cache.Cache, listMetricsPageLimit int, - shouldSkip func(*models.CloudWatchQuery) bool) ([]*models.CloudWatchQuery, error) { + shouldSkip func(ctx context.Context, query *models.CloudWatchQuery) bool) ([]*models.CloudWatchQuery, error) { metricsClient := clients.NewMetricsClient(client, listMetricsPageLimit) service := services.NewListMetricsService(metricsClient) // create copies of the original query. All the fields besides Dimensions are primitives @@ -27,7 +41,7 @@ func (e *cloudWatchExecutor) getDimensionValuesForWildcards( queries = addWildcardDimensionsForMetricQueryTypeQueries(queries) for _, query := range queries { - if shouldSkip(query) { + if shouldSkip(ctx, query) || query.Namespace == "" || query.MetricName == "" { continue } for dimensionKey, values := range query.Dimensions { diff --git a/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards_test.go b/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards_test.go index 2a3f5c36e78..5549bc3fc5b 100644 --- a/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards_test.go +++ b/pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards_test.go @@ -14,8 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) -func noSkip(q *models.CloudWatchQuery) bool { return false } -func skip(q *models.CloudWatchQuery) bool { return true } +func noSkip(ctx context.Context, q *models.CloudWatchQuery) bool { return false } func TestGetDimensionValuesForWildcards(t *testing.T) { executor := &cloudWatchExecutor{im: defaultTestInstanceManager(), logger: log.NewNullLogger()} @@ -74,13 +73,41 @@ func TestGetDimensionValuesForWildcards(t *testing.T) { }) }) + t.Run("Should skip queries", func(t *testing.T) { + t.Run("when namespace not set", func(t *testing.T) { + query := getBaseQuery() + query.Namespace = "" + query.MetricName = "Test_MetricName" + query.Dimensions = map[string][]string{"Test_DimensionName1": {"*"}} + query.MetricQueryType = models.MetricQueryTypeSearch + + queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", nil, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, noSkip) + assert.Nil(t, err) + assert.Len(t, queries, 1) + assert.Equal(t, []string{"*"}, queries[0].Dimensions["Test_DimensionName1"]) + }) + + t.Run("when metricName not set", func(t *testing.T) { + query := getBaseQuery() + query.MetricName = "" + query.Dimensions = map[string][]string{"Test_DimensionName1": {"*"}} + query.MetricQueryType = models.MetricQueryTypeSearch + + queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", nil, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, noSkip) + assert.Nil(t, err) + assert.Len(t, queries, 1) + assert.Equal(t, []string{"*"}, queries[0].Dimensions["Test_DimensionName1"]) + }) + }) + t.Run("MetricSearch query type", func(t *testing.T) { t.Run("Should not change non-wildcard dimension value", func(t *testing.T) { query := getBaseQuery() query.MetricName = "Test_MetricName1" query.Dimensions = map[string][]string{"Test_DimensionName1": {"Value1"}} query.MetricQueryType = models.MetricQueryTypeSearch - queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", nil, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, skip) + query.MatchExact = false + queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", nil, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, shouldSkipFetchingWildcards) assert.Nil(t, err) assert.Len(t, queries, 1) assert.NotNil(t, queries[0].Dimensions["Test_DimensionName1"], 1) @@ -92,7 +119,7 @@ func TestGetDimensionValuesForWildcards(t *testing.T) { query.MetricName = "Test_MetricName1" query.Dimensions = map[string][]string{"Test_DimensionName1": {"*"}} query.MetricQueryType = models.MetricQueryTypeSearch - queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", nil, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, skip) + queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", nil, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, shouldSkipFetchingWildcards) assert.Nil(t, err) assert.Len(t, queries, 1) assert.NotNil(t, queries[0].Dimensions["Test_DimensionName1"]) @@ -112,7 +139,7 @@ func TestGetDimensionValuesForWildcards(t *testing.T) { {MetricName: utils.Pointer("Test_MetricName4"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName1"), Value: utils.Pointer("Value2")}}}, }} api.On("ListMetricsPagesWithContext").Return(nil) - queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", api, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, noSkip) + queries, err := executor.getDimensionValuesForWildcards(ctx, "us-east-1", api, []*models.CloudWatchQuery{query}, cache.New(0, 0), 50, shouldSkipFetchingWildcards) assert.Nil(t, err) assert.Len(t, queries, 1) assert.Equal(t, map[string][]string{"Test_DimensionName1": {"Value1", "Value2", "Value3", "Value4"}}, queries[0].Dimensions) diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go index 97a14db8daf..6db50c11d0c 100644 --- a/pkg/tsdb/cloudwatch/time_series_query.go +++ b/pkg/tsdb/cloudwatch/time_series_query.go @@ -102,18 +102,7 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba return err } - newLabelParsingEnabled := features.IsEnabled(ctx, features.FlagCloudWatchNewLabelParsing) - requestQueries, err = e.getDimensionValuesForWildcards(ctx, region, client, requestQueries, instance.tagValueCache, instance.Settings.GrafanaSettings.ListMetricsPageLimit, func(q *models.CloudWatchQuery) bool { - if q.MetricQueryType == models.MetricQueryTypeSearch && (q.MatchExact || newLabelParsingEnabled) { - return true - } - - if q.MetricQueryType == models.MetricQueryTypeQuery && q.MetricEditorMode == models.MetricEditorModeRaw { - return true - } - - return false - }) + requestQueries, err = e.getDimensionValuesForWildcards(ctx, region, client, requestQueries, instance.tagValueCache, instance.Settings.GrafanaSettings.ListMetricsPageLimit, shouldSkipFetchingWildcards) if err != nil { return err }