CloudWatch: Fix conditions for fetching wildcards (grafana#98648)

tipee-sa · Jan 9, 2025 · 5690859 · 5690859
1 parent 7cad430
commit 5690859
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 19 deletions.
diff --git 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,15 +33,15 @@ 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
 	queries := copyQueries(origQueries)
 	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
@@ -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
@@ -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
 				}