CloudWatch: Move logger to its own package and minor refactoring (#57107)

* Move log to its own package

* Rename test-data to testdata

* Change alias to string type

* Remove parseQueries from a method of cloudWatchExecutor

pkg/tsdb/cloudwatch/cloudwatch.go

@@ -27,9 +27,9 @@ import (
 	"github.com/grafana/grafana-plugin-sdk-go/backend/resource/httpadapter"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
 	"github.com/grafana/grafana/pkg/infra/httpclient"
-	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 )
 
 type datasourceInfo struct {
@@ -81,11 +81,10 @@ const (
 	timeSeriesQuery = "timeSeriesQuery"
 )
 
-var plog = log.New("tsdb.cloudwatch")
 var aliasFormat = regexp.MustCompile(`\{\{\s*(.+?)\s*\}\}`)
 
 func ProvideService(cfg *setting.Cfg, httpClientProvider httpclient.Provider, features featuremgmt.FeatureToggles) *CloudWatchService {
-	plog.Debug("initing")
+	cwlog.Debug("initing")
 
 	executor := newExecutor(datasource.NewInstanceManager(NewInstanceSettings(httpClientProvider)), cfg, awsds.NewSessionCache(), features)
 
@@ -160,9 +159,9 @@ func NewInstanceSettings(httpClientProvider httpclient.Provider) datasource.Inst
 			at = awsds.AuthTypeEC2IAMRole
 		case "arn":
 			at = awsds.AuthTypeDefault
-			plog.Warn("Authentication type \"arn\" is deprecated, falling back to default")
+			cwlog.Warn("Authentication type \"arn\" is deprecated, falling back to default")
 		default:
-			plog.Warn("Unrecognized AWS authentication type", "type", jsonData.AuthType)
+			cwlog.Warn("Unrecognized AWS authentication type", "type", jsonData.AuthType)
 		}
 
 		model.authType = at

pkg/tsdb/cloudwatch/cloudwatch_query.go

@@ -6,6 +6,8 @@ import (
 	"net/url"
 	"strings"
 	"time"
+
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 )
 
 type cloudWatchQuery struct {
@@ -41,7 +43,7 @@ func (q *cloudWatchQuery) getGMDAPIMode() gmdApiMode {
 		return GMDApiModeSQLExpression
 	}
 
-	plog.Warn("Could not resolve CloudWatch metric query type. Falling back to metric stat.", "query", q)
+	cwlog.Warn("Could not resolve CloudWatch metric query type. Falling back to metric stat.", "query", q)
 	return GMDApiModeMetricStat
 }
 

pkg/tsdb/cloudwatch/cwlog/cwlog.go

@@ -0,0 +1,19 @@
+package cwlog
+
+import "github.com/grafana/grafana/pkg/infra/log"
+
+var (
+	cwlog = log.New("tsdb.cloudwatch")
+)
+
+func Warn(msg string, args ...interface{}) {
+	cwlog.Warn(msg, args)
+}
+
+func Debug(msg string, args ...interface{}) {
+	cwlog.Debug(msg, args)
+}
+
+func Error(msg string, args ...interface{}) {
+	cwlog.Error(msg, args)
+}

pkg/tsdb/cloudwatch/log_actions.go

@@ -14,6 +14,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -240,7 +241,7 @@ func (e *cloudWatchExecutor) handleStartQuery(ctx context.Context, logsClient cl
 	if err != nil {
 		var awsErr awserr.Error
 		if errors.As(err, &awsErr) && awsErr.Code() == "LimitExceededException" {
-			plog.Debug("executeStartQuery limit exceeded", "err", awsErr)
+			cwlog.Debug("executeStartQuery limit exceeded", "err", awsErr)
 			return nil, &AWSError{Code: limitExceededException, Message: err.Error()}
 		}
 		return nil, err

pkg/tsdb/cloudwatch/metric_find_query.go

@@ -21,6 +21,7 @@ import (
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/constants"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 )
 
 type suggestData struct {
@@ -76,7 +77,7 @@ func (e *cloudWatchExecutor) handleGetRegions(pluginCtx backend.PluginContext, p
 	r, err := client.DescribeRegions(&ec2.DescribeRegionsInput{})
 	if err != nil {
 		// ignore error for backward compatibility
-		plog.Error("Failed to get regions", "error", err)
+		cwlog.Error("Failed to get regions", "error", err)
 	} else {
 		for _, region := range r.Regions {
 			exists := false
@@ -495,7 +496,7 @@ func (e *cloudWatchExecutor) listMetrics(pluginCtx backend.PluginContext, region
 		return nil, err
 	}
 
-	plog.Debug("Listing metrics pages")
+	cwlog.Debug("Listing metrics pages")
 	var cloudWatchMetrics []*cloudwatch.Metric
 
 	pageNum := 0
@@ -563,7 +564,7 @@ func (e *cloudWatchExecutor) resourceGroupsGetResources(pluginCtx backend.Plugin
 var metricsCacheLock sync.Mutex
 
 func (e *cloudWatchExecutor) getMetricsForCustomMetrics(region, namespace string, pluginCtx backend.PluginContext) ([]string, error) {
-	plog.Debug("Getting metrics for custom metrics", "region", region, "namespace", namespace)
+	cwlog.Debug("Getting metrics for custom metrics", "region", region, "namespace", namespace)
 	metricsCacheLock.Lock()
 	defer metricsCacheLock.Unlock()
 

pkg/tsdb/cloudwatch/request_parser.go

@@ -13,7 +13,7 @@ import (
 
 	"github.com/google/uuid"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 )
 
 var validMetricDataID = regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*$`)
@@ -39,13 +39,13 @@ type QueryJson struct {
 	TimezoneUTCOffset string                 `json:"timezoneUTCOffset,omitempty"`
 	QueryType         string                 `json:"type,omitempty"`
 	Hide              *bool                  `json:"hide,omitempty"`
-	Alias             *string                `json:"alias,omitempty"`
+	Alias             string                 `json:"alias,omitempty"`
 }
 
 // parseQueries parses the json queries and returns a map of cloudWatchQueries by region. The cloudWatchQuery has a 1 to 1 mapping to a query editor row
-func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime time.Time, endTime time.Time) (map[string][]*cloudWatchQuery, error) {
+func parseQueries(queries []backend.DataQuery, startTime time.Time, endTime time.Time, dynamicLabelsEnabled bool) (map[string][]*cloudWatchQuery, error) {
 	requestQueries := make(map[string][]*cloudWatchQuery)
-	migratedQueries, err := migrateLegacyQuery(queries, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels))
+	migratedQueries, err := migrateLegacyQuery(queries, dynamicLabelsEnabled)
 	if err != nil {
 		return nil, err
 	}
@@ -140,11 +140,10 @@ var aliasPatterns = map[string]string{
 var legacyAliasRegexp = regexp.MustCompile(`{{\s*(.+?)\s*}}`)
 
 func migrateAliasToDynamicLabel(queryJson *QueryJson) {
-	fullAliasField := ""
+	fullAliasField := queryJson.Alias
 
-	if queryJson.Alias != nil && *queryJson.Alias != "" {
-		matches := legacyAliasRegexp.FindAllStringSubmatch(*queryJson.Alias, -1)
-		fullAliasField = *queryJson.Alias
+	if fullAliasField != "" {
+		matches := legacyAliasRegexp.FindAllStringSubmatch(fullAliasField, -1)
 
 		for _, groups := range matches {
 			fullMatch := groups[0]
@@ -160,9 +159,9 @@ func migrateAliasToDynamicLabel(queryJson *QueryJson) {
 }
 
 func parseRequestQuery(model QueryJson, refId string, startTime time.Time, endTime time.Time) (*cloudWatchQuery, error) {
-	plog.Debug("Parsing request query", "query", model)
+	cwlog.Debug("Parsing request query", "query", model)
 	cloudWatchQuery := cloudWatchQuery{
-		Alias:             "",
+		Alias:             model.Alias,
 		Label:             "",
 		MatchExact:        true,
 		Statistic:         "",
@@ -256,10 +255,6 @@ func parseRequestQuery(model QueryJson, refId string, startTime time.Time, endTi
 		cloudWatchQuery.MatchExact = *model.MatchExact
 	}
 
-	if model.Alias != nil {
-		cloudWatchQuery.Alias = *model.Alias
-	}
-
 	if model.Label != nil {
 		cloudWatchQuery.Label = *model.Label
 	}

pkg/tsdb/cloudwatch/request_parser_test.go

@@ -350,8 +350,7 @@ func TestRequestParser(t *testing.T) {
 
 	t.Run("parseRequestQuery sets label when label is present in json query", func(t *testing.T) {
 		query := getBaseJsonQuery()
-		alias := "some alias"
-		query.Alias = &alias
+		query.Alias = "some alias"
 
 		label := "some label"
 		query.Label = &label
@@ -401,7 +400,7 @@ func Test_migrateAliasToDynamicLabel_single_query_preserves_old_alias_and_create
 				Region:     "us-east-1",
 				Namespace:  "ec2",
 				MetricName: "CPUUtilization",
-				Alias:      &tc.inputAlias,
+				Alias:      tc.inputAlias,
 				Dimensions: map[string]interface{}{
 					"InstanceId": []interface{}{"test"},
 				},
@@ -413,7 +412,7 @@ func Test_migrateAliasToDynamicLabel_single_query_preserves_old_alias_and_create
 			migrateAliasToDynamicLabel(&queryToMigrate)
 
 			expected := QueryJson{
-				Alias: &tc.inputAlias,
+				Alias: tc.inputAlias,
 				Dimensions: map[string]interface{}{
 					"InstanceId": []interface{}{"test"},
 				},

pkg/tsdb/cloudwatch/resource_handler.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/backend/resource/httpadapter"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 )
 
 func (e *cloudWatchExecutor) newResourceMux() *http.ServeMux {
@@ -47,7 +48,7 @@ func handleResourceReq(handleFunc handleFn) func(rw http.ResponseWriter, req *ht
 		rw.WriteHeader(http.StatusOK)
 		_, err = rw.Write(body)
 		if err != nil {
-			plog.Error("Unable to write HTTP response", "error", err)
+			cwlog.Error("Unable to write HTTP response", "error", err)
 		}
 	}
 }
@@ -56,6 +57,6 @@ func writeResponse(rw http.ResponseWriter, code int, msg string) {
 	rw.WriteHeader(code)
 	_, err := rw.Write([]byte(msg))
 	if err != nil {
-		plog.Error("Unable to write HTTP response", "error", err)
+		cwlog.Error("Unable to write HTTP response", "error", err)
 	}
 }

pkg/tsdb/cloudwatch/response_parser_test.go

@@ -29,7 +29,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
 	startTime := time.Now()
 	endTime := startTime.Add(2 * time.Hour)
 	t.Run("when aggregating multi-outputs response", func(t *testing.T) {
-		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs-query-a.json")
+		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./testdata/multiple-outputs-query-a.json")
 		require.NoError(t, err)
 		aggregatedResponse := aggregateResponse(getMetricDataOutputs)
 		idA := "a"
@@ -59,7 +59,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
 	})
 
 	t.Run("when aggregating multi-outputs response with PartialData and ArithmeticError", func(t *testing.T) {
-		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs-query-b.json")
+		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./testdata/multiple-outputs-query-b.json")
 		require.NoError(t, err)
 		aggregatedResponse := aggregateResponse(getMetricDataOutputs)
 		idB := "b"
@@ -73,7 +73,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
 	})
 
 	t.Run("when aggregating multi-outputs response", func(t *testing.T) {
-		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/single-output-multiple-metric-data-results.json")
+		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./testdata/single-output-multiple-metric-data-results.json")
 		require.NoError(t, err)
 		aggregatedResponse := aggregateResponse(getMetricDataOutputs)
 		idA := "a"
@@ -91,7 +91,7 @@ func TestCloudWatchResponseParser(t *testing.T) {
 	})
 
 	t.Run("when aggregating response and error codes are in first GetMetricDataOutput", func(t *testing.T) {
-		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs2.json")
+		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./testdata/multiple-outputs2.json")
 		require.NoError(t, err)
 		aggregatedResponse := aggregateResponse(getMetricDataOutputs)
 		t.Run("response for id a", func(t *testing.T) {

pkg/tsdb/cloudwatch/time_series_query.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/cwlog"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -15,7 +17,7 @@ type responseWrapper struct {
 }
 
 func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
-	plog.Debug("Executing time series query")
+	cwlog.Debug("Executing time series query")
 	resp := backend.NewQueryDataResponse()
 
 	if len(req.Queries) == 0 {
@@ -28,7 +30,7 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba
 		return nil, fmt.Errorf("invalid time range: start time must be before end time")
 	}
 
-	requestQueriesByRegion, err := e.parseQueries(req.Queries, startTime, endTime)
+	requestQueriesByRegion, err := parseQueries(req.Queries, startTime, endTime, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels))
 	if err != nil {
 		return nil, err
 	}
@@ -45,7 +47,7 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, req *ba
 		eg.Go(func() error {
 			defer func() {
 				if err := recover(); err != nil {
-					plog.Error("Execute Get Metric Data Query Panic", "error", err, "stack", log.Stack(1))
+					cwlog.Error("Execute Get Metric Data Query Panic", "error", err, "stack", log.Stack(1))
 					if theErr, ok := err.(error); ok {
 						resultChan <- &responseWrapper{
 							DataResponse: &backend.DataResponse{