[v9.3.x] CloudWatch: fix custom namespace for listing dimension keys, refactor to non-pointer types, add test assertions, rename packages (#59130)

CloudWatch: fix custom namespace for listing dimension keys, refactor to non-pointer types, add test assertions, rename packages (#59106)

Co-authored-by: Erik Sundell <erik.sundell87@gmail.com>
(cherry picked from commit c43e1a721f)

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

pkg/tsdb/cloudwatch/models/api.go

@@ -2,14 +2,14 @@ package models
 
 import (
 	"github.com/aws/aws-sdk-go/service/cloudwatch"
-	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources"
 )
 
 type ListMetricsProvider interface {
-	GetDimensionKeysByDimensionFilter(*request.DimensionKeysRequest) ([]string, error)
+	GetDimensionKeysByDimensionFilter(resources.DimensionKeysRequest) ([]string, error)
 	GetDimensionKeysByNamespace(string) ([]string, error)
-	GetDimensionValuesByDimensionFilter(*request.DimensionValuesRequest) ([]string, error)
-	GetMetricsByNamespace(namespace string) ([]Metric, error)
+	GetDimensionValuesByDimensionFilter(resources.DimensionValuesRequest) ([]string, error)
+	GetMetricsByNamespace(namespace string) ([]resources.Metric, error)
 }
 
 type MetricsClientProvider interface {

pkg/tsdb/cloudwatch/models/request/types.go

@@ -1,6 +0,0 @@
-package request
-
-type Dimension struct {
-	Name  string
-	Value string
-}

pkg/tsdb/cloudwatch/models/resources/dimension_keys_request.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"net/url"
@@ -9,7 +9,6 @@ type DimensionKeysRequestType uint32
 const (
 	StandardDimensionKeysRequest DimensionKeysRequestType = iota
 	FilterDimensionKeysRequest
-	CustomMetricDimensionKeysRequest
 )
 
 type DimensionKeysRequest struct {
@@ -20,24 +19,20 @@ type DimensionKeysRequest struct {
 }
 
 func (q *DimensionKeysRequest) Type() DimensionKeysRequestType {
-	if isCustomNamespace(q.Namespace) {
-		return CustomMetricDimensionKeysRequest
-	}
-
-	if len(q.DimensionFilter) > 0 {
+	if isCustomNamespace(q.Namespace) || len(q.DimensionFilter) > 0 {
 		return FilterDimensionKeysRequest
 	}
 
 	return StandardDimensionKeysRequest
 }
 
-func GetDimensionKeysRequest(parameters url.Values) (*DimensionKeysRequest, error) {
+func GetDimensionKeysRequest(parameters url.Values) (DimensionKeysRequest, error) {
 	resourceRequest, err := getResourceRequest(parameters)
 	if err != nil {
-		return nil, err
+		return DimensionKeysRequest{}, err
 	}
 
-	request := &DimensionKeysRequest{
+	request := DimensionKeysRequest{
 		ResourceRequest: resourceRequest,
 		Namespace:       parameters.Get("namespace"),
 		MetricName:      parameters.Get("metricName"),
@@ -46,7 +41,7 @@ func GetDimensionKeysRequest(parameters url.Values) (*DimensionKeysRequest, erro
 
 	dimensions, err := parseDimensionFilter(parameters.Get("dimensionFilters"))
 	if err != nil {
-		return nil, err
+		return DimensionKeysRequest{}, err
 	}
 
 	request.DimensionFilter = dimensions

pkg/tsdb/cloudwatch/models/resources/dimension_keys_request_test.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"testing"
@@ -69,4 +69,40 @@ func TestDimensionKeyRequest(t *testing.T) {
 		assert.Equal(t, "InstanceId", request.DimensionFilter[0].Name)
 		assert.Equal(t, "", request.DimensionFilter[0].Value)
 	})
+
+	type testCase struct {
+		name                 string
+		dimensionKeysRequest DimensionKeysRequest
+		expectedType         DimensionKeysRequestType
+	}
+	testCases := []testCase{
+		{
+			name: "With custom namespace it should resolve to FilterDimensionKeysRequest",
+			dimensionKeysRequest: DimensionKeysRequest{
+				Namespace: "custom",
+			},
+			expectedType: FilterDimensionKeysRequest,
+		},
+		{
+			name: "With dimension filter it should resolve to FilterDimensionKeysRequest",
+			dimensionKeysRequest: DimensionKeysRequest{
+				Namespace:       "AWS/EC2",
+				DimensionFilter: []*Dimension{{Name: "InstanceId", Value: "i-1234567890abcdef0"}},
+			},
+			expectedType: FilterDimensionKeysRequest,
+		},
+		{
+			name: "With dimension filter and without custom namespace it should resolve to StandardDimensionKeysRequest",
+			dimensionKeysRequest: DimensionKeysRequest{
+				Namespace: "AWS/EC2",
+			},
+			expectedType: StandardDimensionKeysRequest,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expectedType, tc.dimensionKeysRequest.Type())
+		})
+	}
 }

pkg/tsdb/cloudwatch/models/resources/dimension_values_request.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"net/url"
@@ -12,13 +12,13 @@ type DimensionValuesRequest struct {
 	DimensionFilter []*Dimension
 }
 
-func GetDimensionValuesRequest(parameters url.Values) (*DimensionValuesRequest, error) {
+func GetDimensionValuesRequest(parameters url.Values) (DimensionValuesRequest, error) {
 	resourceRequest, err := getResourceRequest(parameters)
 	if err != nil {
-		return nil, err
+		return DimensionValuesRequest{}, err
 	}
 
-	request := &DimensionValuesRequest{
+	request := DimensionValuesRequest{
 		ResourceRequest: resourceRequest,
 		Namespace:       parameters.Get("namespace"),
 		MetricName:      parameters.Get("metricName"),
@@ -28,7 +28,7 @@ func GetDimensionValuesRequest(parameters url.Values) (*DimensionValuesRequest,
 
 	dimensions, err := parseDimensionFilter(parameters.Get("dimensionFilters"))
 	if err != nil {
-		return nil, err
+		return DimensionValuesRequest{}, err
 	}
 
 	request.DimensionFilter = dimensions

pkg/tsdb/cloudwatch/models/resources/dimension_values_rquest_test.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"testing"

pkg/tsdb/cloudwatch/models/resources/metrics_resource_request.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"net/url"

pkg/tsdb/cloudwatch/models/resources/metrics_resource_request_test.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"net/url"

pkg/tsdb/cloudwatch/models/resources/resource_request.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"fmt"

pkg/tsdb/cloudwatch/models/resources/resource_request_test.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"testing"
@@ -10,7 +10,7 @@ import (
 func TestResourceRequest(t *testing.T) {
 	t.Run("Should return an error if region is not provided", func(t *testing.T) {
 		request, err := GetDimensionValuesRequest(map[string][]string{})
-		require.Nil(t, request)
+		require.Empty(t, request)
 		assert.Equal(t, "region is required", err.Error())
 	})
 }

pkg/tsdb/cloudwatch/models/resources/types.go

@@ -0,0 +1,11 @@
+package resources
+
+type Dimension struct {
+	Name  string
+	Value string
+}
+
+type Metric struct {
+	Name      string `json:"name"`
+	Namespace string `json:"namespace"`
+}

pkg/tsdb/cloudwatch/models/resources/utils.go

@@ -1,4 +1,4 @@
-package request
+package resources
 
 import (
 	"encoding/json"

pkg/tsdb/cloudwatch/models/settings.go

@@ -13,11 +13,11 @@ type CloudWatchSettings struct {
 	Namespace string `json:"customMetricsNamespaces"`
 }
 
-func LoadCloudWatchSettings(config backend.DataSourceInstanceSettings) (*CloudWatchSettings, error) {
-	instance := &CloudWatchSettings{}
+func LoadCloudWatchSettings(config backend.DataSourceInstanceSettings) (CloudWatchSettings, error) {
+	instance := CloudWatchSettings{}
 	if config.JSONData != nil && len(config.JSONData) > 1 {
-		if err := json.Unmarshal(config.JSONData, instance); err != nil {
-			return nil, fmt.Errorf("could not unmarshal DatasourceSettings json: %w", err)
+		if err := json.Unmarshal(config.JSONData, &instance); err != nil {
+			return CloudWatchSettings{}, fmt.Errorf("could not unmarshal DatasourceSettings json: %w", err)
 		}
 	}
 

pkg/tsdb/cloudwatch/models/types.go

@@ -8,7 +8,7 @@ import (
 
 type RequestContext struct {
 	MetricsClientProvider MetricsClientProvider
-	Settings              *CloudWatchSettings
+	Settings              CloudWatchSettings
 }
 
 type RequestContextFactoryFunc func(pluginCtx backend.PluginContext, region string) (reqCtx RequestContext, err error)
@@ -35,8 +35,3 @@ type metricStatMeta struct {
 	Period int    `json:"period"`
 	Label  string `json:"label,omitempty"`
 }
-
-type Metric struct {
-	Name      string `json:"name"`
-	Namespace string `json:"namespace"`
-}