CloudWatch: Correctly add dimension values to labels (#74847)

Co-authored-by: Shirley <4163034+fridgepoet@users.noreply.github.com>

docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md

@@ -48,6 +48,7 @@ Some features are enabled by default. You can disable these feature by setting t
 | `toggleLabelsInLogsUI`                           | Enable toggleable filters in log details view                                                                                                                                                       | Yes                |
 | `azureMonitorDataplane`                          | Adds dataplane compliant frame metadata in the Azure Monitor datasource                                                                                                                             | Yes                |
 | `prometheusConfigOverhaulAuth`                   | Update the Prometheus configuration page with the new auth component                                                                                                                                | Yes                |
+| `cloudWatchWildCardDimensionValues`              | Fetches dimension values from CloudWatch to correctly label wildcard dimensions                                                                                                                     | Yes                |
 
 ## Preview feature toggles
 

packages/grafana-data/src/types/featureToggles.gen.ts

@@ -132,4 +132,5 @@ export interface FeatureToggles {
   pluginsAPIMetrics?: boolean;
   httpSLOLevels?: boolean;
   idForwarding?: boolean;
+  cloudWatchWildCardDimensionValues?: boolean;
 }

pkg/services/featuremgmt/registry.go

@@ -794,5 +794,12 @@ var (
 			Owner:           grafanaAuthnzSquad,
 			RequiresDevMode: true,
 		},
+		{
+			Name:        "cloudWatchWildCardDimensionValues",
+			Description: "Fetches dimension values from CloudWatch to correctly label wildcard dimensions",
+			Stage:       FeatureStageGeneralAvailability,
+			Expression:  "true", // enabled by default
+			Owner:       awsDatasourcesSquad,
+		},
 	}
 )

pkg/services/featuremgmt/toggles_gen.csv

@@ -113,3 +113,4 @@ externalCorePlugins,experimental,@grafana/plugins-platform-backend,false,false,f
 pluginsAPIMetrics,experimental,@grafana/plugins-platform-backend,false,false,false,true
 httpSLOLevels,experimental,@grafana/hosted-grafana-team,false,false,true,false
 idForwarding,experimental,@grafana/grafana-authnz-team,true,false,false,false
+cloudWatchWildCardDimensionValues,GA,@grafana/aws-datasources,false,false,false,false

pkg/services/featuremgmt/toggles_gen.go

@@ -462,4 +462,8 @@ const (
 	// FlagIdForwarding
 	// Generate signed id token for identity that can be forwarded to plugins and external services
 	FlagIdForwarding = "idForwarding"
+
+	// FlagCloudWatchWildCardDimensionValues
+	// Fetches dimension values from CloudWatch to correctly label wildcard dimensions
+	FlagCloudWatchWildCardDimensionValues = "cloudWatchWildCardDimensionValues"
 )

pkg/tsdb/cloudwatch/cloudwatch.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"sync"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/session"
@@ -28,16 +29,20 @@ import (
 	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/clients"
 	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/kinds/dataquery"
 	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models"
+	"github.com/patrickmn/go-cache"
 )
 
+const tagValueCacheExpiration = time.Hour * 24
+
 type DataQueryJson struct {
 	dataquery.CloudWatchAnnotationQuery
 	Type string `json:"type,omitempty"`
 }
 
 type DataSource struct {
-	Settings   models.CloudWatchSettings
+	Settings      models.CloudWatchSettings
-	HTTPClient *http.Client
+	HTTPClient    *http.Client
+	tagValueCache *cache.Cache
 }
 
 const (
@@ -101,8 +106,9 @@ func NewInstanceSettings(httpClientProvider httpclient.Provider) datasource.Inst
 		}
 
 		return DataSource{
-			Settings:   instanceSettings,
+			Settings:      instanceSettings,
-			HTTPClient: httpClient,
+			HTTPClient:    httpClient,
+			tagValueCache: cache.New(tagValueCacheExpiration, tagValueCacheExpiration*5),
 		}, nil
 	}
 }

pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards.go

@@ -0,0 +1,88 @@
+package cloudwatch
+
+import (
+	"fmt"
+
+	"github.com/grafana/grafana-plugin-sdk-go/backend"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/clients"
+	"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"
+)
+
+// getDimensionValues gets the actual dimension values for dimensions with a wildcard
+func (e *cloudWatchExecutor) getDimensionValuesForWildcards(pluginCtx backend.PluginContext, region string,
+	client models.CloudWatchMetricsAPIProvider, origQueries []*models.CloudWatchQuery, tagValueCache *cache.Cache, logger log.Logger) ([]*models.CloudWatchQuery, error) {
+	metricsClient := clients.NewMetricsClient(client, e.cfg)
+	service := services.NewListMetricsService(metricsClient)
+	// create copies of the original query. All the fields besides Dimensions are primitives
+	queries := copyQueries(origQueries)
+
+	for _, query := range queries {
+		for dimensionKey, values := range query.Dimensions {
+			// if the dimension is not a wildcard, skip it
+			if len(values) != 1 || query.MatchExact || (len(values) == 1 && values[0] != "*") {
+				continue
+			}
+
+			accountID := ""
+			if query.AccountId != nil {
+				accountID = *query.AccountId
+			}
+			cacheKey := fmt.Sprintf("%s-%s-%s-%s-%s", region, accountID, query.Namespace, query.MetricName, dimensionKey)
+			cachedDimensions, found := tagValueCache.Get(cacheKey)
+			if found {
+				logger.Debug("Fetching dimension values from cache")
+				query.Dimensions[dimensionKey] = cachedDimensions.([]string)
+				continue
+			}
+
+			logger.Debug("Cache miss, fetching dimension values from AWS")
+			request := resources.DimensionValuesRequest{
+				ResourceRequest: &resources.ResourceRequest{
+					Region:    region,
+					AccountId: query.AccountId,
+				},
+				Namespace:    query.Namespace,
+				MetricName:   query.MetricName,
+				DimensionKey: dimensionKey,
+			}
+
+			dimensions, err := service.GetDimensionValuesByDimensionFilter(request)
+			if err != nil {
+				return nil, err
+			}
+			newDimensions := make([]string, 0, len(dimensions))
+			for _, resp := range dimensions {
+				newDimensions = append(newDimensions, resp.Value)
+			}
+
+			query.Dimensions[dimensionKey] = newDimensions
+			if len(newDimensions) > 0 {
+				tagValueCache.Set(cacheKey, newDimensions, cache.DefaultExpiration)
+			}
+		}
+	}
+
+	return queries, nil
+}
+
+// copyQueries returns a deep copy of the passed in queries
+func copyQueries(origQueries []*models.CloudWatchQuery) []*models.CloudWatchQuery {
+	newQueries := []*models.CloudWatchQuery{}
+	for _, origQuery := range origQueries {
+		if origQuery == nil {
+			newQueries = append(newQueries, nil)
+			continue
+		}
+		newQuery := *origQuery
+		newQuery.Dimensions = map[string][]string{}
+		for key, val := range origQuery.Dimensions {
+			newQuery.Dimensions[key] = append([]string{}, val...)
+		}
+		newQueries = append(newQueries, &newQuery)
+	}
+	return newQueries
+}

pkg/tsdb/cloudwatch/get_dimension_values_for_wildcards_test.go

@@ -0,0 +1,112 @@
+package cloudwatch
+
+import (
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/service/cloudwatch"
+	"github.com/grafana/grafana-plugin-sdk-go/backend"
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils"
+	"github.com/patrickmn/go-cache"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetDimensionValuesForWildcards(t *testing.T) {
+	logger := &logtest.Fake{}
+	executor := &cloudWatchExecutor{}
+	pluginCtx := backend.PluginContext{
+		DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ID: 1, Updated: time.Now()},
+	}
+	tagValueCache := cache.New(0, 0)
+
+	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"}}
+		queries, err := executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", nil, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		assert.Len(t, queries, 1)
+		assert.NotNil(t, queries[0].Dimensions["Test_DimensionName1"], 1)
+		assert.Equal(t, []string{"Value1"}, queries[0].Dimensions["Test_DimensionName1"])
+	})
+
+	t.Run("Should not change exact dimension value", func(t *testing.T) {
+		query := getBaseQuery()
+		query.MetricName = "Test_MetricName1"
+		query.Dimensions = map[string][]string{"Test_DimensionName1": {"*"}}
+		queries, err := executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", nil, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		assert.Len(t, queries, 1)
+		assert.NotNil(t, queries[0].Dimensions["Test_DimensionName1"])
+		assert.Equal(t, []string{"*"}, queries[0].Dimensions["Test_DimensionName1"])
+	})
+
+	t.Run("Should change wildcard dimension value", func(t *testing.T) {
+		query := getBaseQuery()
+		query.MetricName = "Test_MetricName1"
+		query.Dimensions = map[string][]string{"Test_DimensionName1": {"*"}}
+		query.MatchExact = false
+		api := &mocks.MetricsAPI{Metrics: []*cloudwatch.Metric{
+			{MetricName: utils.Pointer("Test_MetricName1"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName1"), Value: utils.Pointer("Value1")}, {Name: utils.Pointer("Test_DimensionName2"), Value: utils.Pointer("Value2")}}},
+			{MetricName: utils.Pointer("Test_MetricName2"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName1"), Value: utils.Pointer("Value3")}}},
+			{MetricName: utils.Pointer("Test_MetricName3"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName1"), Value: utils.Pointer("Value4")}}},
+			{MetricName: utils.Pointer("Test_MetricName4"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName1"), Value: utils.Pointer("Value2")}}},
+		}}
+		api.On("ListMetricsPages").Return(nil)
+		queries, err := executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", api, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		assert.Len(t, queries, 1)
+		assert.Equal(t, map[string][]string{"Test_DimensionName1": {"Value1", "Value2", "Value3", "Value4"}}, queries[0].Dimensions)
+		api.AssertExpectations(t)
+	})
+
+	t.Run("Should use cache for previously fetched value", func(t *testing.T) {
+		query := getBaseQuery()
+		query.MetricName = "Test_MetricName"
+		query.Dimensions = map[string][]string{"Test_DimensionName": {"*"}}
+		query.MatchExact = false
+		api := &mocks.MetricsAPI{Metrics: []*cloudwatch.Metric{
+			{MetricName: utils.Pointer("Test_MetricName"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName"), Value: utils.Pointer("Value")}}},
+		}}
+		api.On("ListMetricsPages").Return(nil)
+		_, err := executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", api, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		// make sure the original query wasn't altered
+		assert.Equal(t, map[string][]string{"Test_DimensionName": {"*"}}, query.Dimensions)
+
+		//setting the api to nil confirms that it's using the cached value
+		queries, err := executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", nil, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		assert.Len(t, queries, 1)
+		assert.Equal(t, map[string][]string{"Test_DimensionName": {"Value"}}, queries[0].Dimensions)
+		api.AssertExpectations(t)
+	})
+
+	t.Run("Should not cache when no values are returned", func(t *testing.T) {
+		query := getBaseQuery()
+		query.MetricName = "Test_MetricName"
+		query.Dimensions = map[string][]string{"Test_DimensionName2": {"*"}}
+		query.MatchExact = false
+		api := &mocks.MetricsAPI{Metrics: []*cloudwatch.Metric{}}
+		api.On("ListMetricsPages").Return(nil)
+		queries, err := executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", api, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		assert.Len(t, queries, 1)
+		// assert that the values was set to an empty array
+		assert.Equal(t, map[string][]string{"Test_DimensionName2": {}}, queries[0].Dimensions)
+
+		// Confirm that it calls the api again if the last call did not return any values
+		api.Metrics = []*cloudwatch.Metric{
+			{MetricName: utils.Pointer("Test_MetricName"), Dimensions: []*cloudwatch.Dimension{{Name: utils.Pointer("Test_DimensionName2"), Value: utils.Pointer("Value")}}},
+		}
+		api.On("ListMetricsPages").Return(nil)
+		queries, err = executor.getDimensionValuesForWildcards(pluginCtx, "us-east-1", api, []*models.CloudWatchQuery{query}, tagValueCache, logger)
+		assert.Nil(t, err)
+		assert.Len(t, queries, 1)
+		assert.Equal(t, map[string][]string{"Test_DimensionName2": {"Value"}}, queries[0].Dimensions)
+		api.AssertExpectations(t)
+	})
+}

pkg/tsdb/cloudwatch/mocks/cloudwatch_metric_api.go

@@ -52,6 +52,8 @@ func chunkSlice(slice []*cloudwatch.Metric, chunkSize int) [][]*cloudwatch.Metri
 type MetricsAPI struct {
 	cloudwatchiface.CloudWatchAPI
 	mock.Mock
+
+	Metrics []*cloudwatch.Metric
 }
 
 func (m *MetricsAPI) GetMetricDataWithContext(ctx aws.Context, input *cloudwatch.GetMetricDataInput, opts ...request.Option) (*cloudwatch.GetMetricDataOutput, error) {
@@ -59,3 +61,11 @@ func (m *MetricsAPI) GetMetricDataWithContext(ctx aws.Context, input *cloudwatch
 
 	return args.Get(0).(*cloudwatch.GetMetricDataOutput), args.Error(1)
 }
+
+func (m *MetricsAPI) ListMetricsPages(input *cloudwatch.ListMetricsInput, fn func(*cloudwatch.ListMetricsOutput, bool) bool) error {
+	fn(&cloudwatch.ListMetricsOutput{
+		Metrics: m.Metrics,
+	}, true)
+
+	return m.Called().Error(0)
+}

pkg/tsdb/cloudwatch/time_series_query.go

@@ -88,6 +88,13 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, logger
 				return err
 			}
 
+			if e.features.IsEnabled(featuremgmt.FlagCloudWatchWildCardDimensionValues) {
+				requestQueries, err = e.getDimensionValuesForWildcards(req.PluginContext, region, client, requestQueries, instance.tagValueCache, logger)
+				if err != nil {
+					return err
+				}
+			}
+
 			res, err := e.parseResponse(startTime, endTime, mdo, requestQueries)
 			if err != nil {
 				return err

pkg/tsdb/cloudwatch/time_series_query_test.go

@@ -415,7 +415,11 @@ func Test_QueryData_response_data_frame_name_is_always_response_label(t *testing
 	t.Cleanup(func() {
 		NewCWClient = origNewCWClient
 	})
-	var api mocks.MetricsAPI
+
+	api := mocks.MetricsAPI{Metrics: []*cloudwatch.Metric{
+		{MetricName: aws.String(""), Dimensions: []*cloudwatch.Dimension{{Name: aws.String("InstanceId"), Value: aws.String("i-00645d91ed77d87ac")}}},
+	}}
+	api.On("ListMetricsPages").Return(nil)
 
 	NewCWClient = func(sess *session.Session) cloudwatchiface.CloudWatchAPI {
 		return &api