Cloudwatch: Refactor log group model (#60873)

* refactor log group query model

* update deprecated comment

* refactor test

pkg/tsdb/cloudwatch/log_actions.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/tsdb/cloudwatch/models"
 )
 
 const (
@@ -35,27 +36,6 @@ type AWSError struct {
 	Payload map[string]string
 }
 
-type LogQueryJson struct {
-	LogType            string `json:"type"`
-	SubType            string
-	Limit              *int64
-	Time               int64
-	StartTime          *int64
-	EndTime            *int64
-	LogGroupName       string
-	LogGroupNames      []string
-	LogGroups          []suggestData
-	LogGroupNamePrefix string
-	LogStreamName      string
-	StartFromHead      bool
-	Region             string
-	QueryString        string
-	QueryId            string
-	StatsGroups        []string
-	Subtype            string
-	Expression         string
-}
-
 func (e *AWSError) Error() string {
 	return fmt.Sprintf("%s: %s", e.Code, e.Message)
 }
@@ -67,15 +47,15 @@ func (e *cloudWatchExecutor) executeLogActions(ctx context.Context, logger log.L
 	eg, ectx := errgroup.WithContext(ctx)
 
 	for _, query := range req.Queries {
-		var model LogQueryJson
-		err := json.Unmarshal(query.JSON, &model)
+		var logsQuery models.LogsQuery
+		err := json.Unmarshal(query.JSON, &logsQuery)
 		if err != nil {
 			return nil, err
 		}
 
 		query := query
 		eg.Go(func() error {
-			dataframe, err := e.executeLogAction(ectx, logger, model, query, req.PluginContext)
+			dataframe, err := e.executeLogAction(ectx, logger, logsQuery, query, req.PluginContext)
 			if err != nil {
 				var AWSError *AWSError
 				if errors.As(err, &AWSError) {
@@ -87,7 +67,7 @@ func (e *cloudWatchExecutor) executeLogActions(ctx context.Context, logger log.L
 				return err
 			}
 
-			groupedFrames, err := groupResponseFrame(dataframe, model.StatsGroups)
+			groupedFrames, err := groupResponseFrame(dataframe, logsQuery.StatsGroups)
 			if err != nil {
 				return err
 			}
@@ -115,15 +95,15 @@ func (e *cloudWatchExecutor) executeLogActions(ctx context.Context, logger log.L
 	return resp, nil
 }
 
-func (e *cloudWatchExecutor) executeLogAction(ctx context.Context, logger log.Logger, model LogQueryJson, query backend.DataQuery, pluginCtx backend.PluginContext) (*data.Frame, error) {
+func (e *cloudWatchExecutor) executeLogAction(ctx context.Context, logger log.Logger, logsQuery models.LogsQuery, query backend.DataQuery, pluginCtx backend.PluginContext) (*data.Frame, error) {
 	instance, err := e.getInstance(pluginCtx)
 	if err != nil {
 		return nil, err
 	}
 
 	region := instance.Settings.Region
-	if model.Region != "" {
-		region = model.Region
+	if logsQuery.Region != "" {
+		region = logsQuery.Region
 	}
 
 	logsClient, err := e.getCWLogsClient(pluginCtx, region)
@@ -132,53 +112,53 @@ func (e *cloudWatchExecutor) executeLogAction(ctx context.Context, logger log.Lo
 	}
 
 	var data *data.Frame = nil
-	switch model.SubType {
+	switch logsQuery.SubType {
 	case "GetLogGroupFields":
-		data, err = e.handleGetLogGroupFields(ctx, logsClient, model, query.RefID)
+		data, err = e.handleGetLogGroupFields(ctx, logsClient, logsQuery, query.RefID)
 	case "StartQuery":
-		data, err = e.handleStartQuery(ctx, logger, logsClient, model, query.TimeRange, query.RefID)
+		data, err = e.handleStartQuery(ctx, logger, logsClient, logsQuery, query.TimeRange, query.RefID)
 	case "StopQuery":
-		data, err = e.handleStopQuery(ctx, logsClient, model)
+		data, err = e.handleStopQuery(ctx, logsClient, logsQuery)
 	case "GetQueryResults":
-		data, err = e.handleGetQueryResults(ctx, logsClient, model, query.RefID)
+		data, err = e.handleGetQueryResults(ctx, logsClient, logsQuery, query.RefID)
 	case "GetLogEvents":
-		data, err = e.handleGetLogEvents(ctx, logsClient, model)
+		data, err = e.handleGetLogEvents(ctx, logsClient, logsQuery)
 	}
 	if err != nil {
-		return nil, fmt.Errorf("failed to execute log action with subtype: %s: %w", model.SubType, err)
+		return nil, fmt.Errorf("failed to execute log action with subtype: %s: %w", logsQuery.SubType, err)
 	}
 
 	return data, nil
 }
 
 func (e *cloudWatchExecutor) handleGetLogEvents(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson) (*data.Frame, error) {
+	logsQuery models.LogsQuery) (*data.Frame, error) {
 	limit := defaultEventLimit
-	if parameters.Limit != nil && *parameters.Limit > 0 {
-		limit = *parameters.Limit
+	if logsQuery.Limit != nil && *logsQuery.Limit > 0 {
+		limit = *logsQuery.Limit
 	}
 
 	queryRequest := &cloudwatchlogs.GetLogEventsInput{
 		Limit:         aws.Int64(limit),
-		StartFromHead: aws.Bool(parameters.StartFromHead),
+		StartFromHead: aws.Bool(logsQuery.StartFromHead),
 	}
 
-	if parameters.LogGroupName == "" {
+	if logsQuery.LogGroupName == "" {
 		return nil, fmt.Errorf("Error: Parameter 'logGroupName' is required")
 	}
-	queryRequest.SetLogGroupName(parameters.LogGroupName)
+	queryRequest.SetLogGroupName(logsQuery.LogGroupName)
 
-	if parameters.LogStreamName == "" {
+	if logsQuery.LogStreamName == "" {
 		return nil, fmt.Errorf("Error: Parameter 'logStreamName' is required")
 	}
-	queryRequest.SetLogStreamName(parameters.LogStreamName)
+	queryRequest.SetLogStreamName(logsQuery.LogStreamName)
 
-	if parameters.StartTime != nil && *parameters.StartTime != 0 {
-		queryRequest.SetStartTime(*parameters.StartTime)
+	if logsQuery.StartTime != nil && *logsQuery.StartTime != 0 {
+		queryRequest.SetStartTime(*logsQuery.StartTime)
 	}
 
-	if parameters.EndTime != nil && *parameters.EndTime != 0 {
-		queryRequest.SetEndTime(*parameters.EndTime)
+	if logsQuery.EndTime != nil && *logsQuery.EndTime != 0 {
+		queryRequest.SetEndTime(*logsQuery.EndTime)
 	}
 
 	logEvents, err := logsClient.GetLogEventsWithContext(ctx, queryRequest)
@@ -207,7 +187,7 @@ func (e *cloudWatchExecutor) handleGetLogEvents(ctx context.Context, logsClient
 }
 
 func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson, timeRange backend.TimeRange) (*cloudwatchlogs.StartQueryOutput, error) {
+	logsQuery models.LogsQuery, timeRange backend.TimeRange) (*cloudwatchlogs.StartQueryOutput, error) {
 	startTime := timeRange.From
 	endTime := timeRange.To
 
@@ -220,7 +200,7 @@ func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient c
 	// The usage of ltrim around the @log/@logStream fields is a necessary workaround, as without it,
 	// CloudWatch wouldn't consider a query using a non-alised @log/@logStream valid.
 	modifiedQueryString := "fields @timestamp,ltrim(@log) as " + logIdentifierInternal + ",ltrim(@logStream) as " +
-		logStreamIdentifierInternal + "|" + parameters.QueryString
+		logStreamIdentifierInternal + "|" + logsQuery.QueryString
 
 	startQueryInput := &cloudwatchlogs.StartQueryInput{
 		StartTime: aws.Int64(startTime.Unix()),
@@ -234,10 +214,10 @@ func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient c
 	}
 
 	if e.features.IsEnabled(featuremgmt.FlagCloudWatchCrossAccountQuerying) {
-		if parameters.LogGroups != nil && len(parameters.LogGroups) > 0 {
+		if logsQuery.LogGroups != nil && len(logsQuery.LogGroups) > 0 {
 			var logGroupIdentifiers []string
-			for _, lg := range parameters.LogGroups {
-				arn := lg.Value
+			for _, lg := range logsQuery.LogGroups {
+				arn := lg.ARN
 				// due to a bug in the startQuery api, we remove * from the arn, otherwise it throws an error
 				logGroupIdentifiers = append(logGroupIdentifiers, strings.TrimSuffix(arn, "*"))
 			}
@@ -246,11 +226,11 @@ func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient c
 	}
 
 	if startQueryInput.LogGroupIdentifiers == nil {
-		startQueryInput.LogGroupNames = aws.StringSlice(parameters.LogGroupNames)
+		startQueryInput.LogGroupNames = aws.StringSlice(logsQuery.LogGroupNames)
 	}
 
-	if parameters.Limit != nil {
-		startQueryInput.Limit = aws.Int64(*parameters.Limit)
+	if logsQuery.Limit != nil {
+		startQueryInput.Limit = aws.Int64(*logsQuery.Limit)
 	}
 
 	logger.Debug("calling startquery with context with input", "input", startQueryInput)
@@ -258,8 +238,8 @@ func (e *cloudWatchExecutor) executeStartQuery(ctx context.Context, logsClient c
 }
 
 func (e *cloudWatchExecutor) handleStartQuery(ctx context.Context, logger log.Logger, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	model LogQueryJson, timeRange backend.TimeRange, refID string) (*data.Frame, error) {
-	startQueryResponse, err := e.executeStartQuery(ctx, logsClient, model, timeRange)
+	logsQuery models.LogsQuery, timeRange backend.TimeRange, refID string) (*data.Frame, error) {
+	startQueryResponse, err := e.executeStartQuery(ctx, logsClient, logsQuery, timeRange)
 	if err != nil {
 		var awsErr awserr.Error
 		if errors.As(err, &awsErr) && awsErr.Code() == "LimitExceededException" {
@@ -273,8 +253,8 @@ func (e *cloudWatchExecutor) handleStartQuery(ctx context.Context, logger log.Lo
 	dataFrame.RefID = refID
 
 	region := "default"
-	if model.Region != "" {
-		region = model.Region
+	if logsQuery.Region != "" {
+		region = logsQuery.Region
 	}
 
 	dataFrame.Meta = &data.FrameMeta{
@@ -287,9 +267,9 @@ func (e *cloudWatchExecutor) handleStartQuery(ctx context.Context, logger log.Lo
 }
 
 func (e *cloudWatchExecutor) executeStopQuery(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson) (*cloudwatchlogs.StopQueryOutput, error) {
+	logsQuery models.LogsQuery) (*cloudwatchlogs.StopQueryOutput, error) {
 	queryInput := &cloudwatchlogs.StopQueryInput{
-		QueryId: aws.String(parameters.QueryId),
+		QueryId: aws.String(logsQuery.QueryId),
 	}
 
 	response, err := logsClient.StopQueryWithContext(ctx, queryInput)
@@ -308,8 +288,8 @@ func (e *cloudWatchExecutor) executeStopQuery(ctx context.Context, logsClient cl
 }
 
 func (e *cloudWatchExecutor) handleStopQuery(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson) (*data.Frame, error) {
-	response, err := e.executeStopQuery(ctx, logsClient, parameters)
+	logsQuery models.LogsQuery) (*data.Frame, error) {
+	response, err := e.executeStopQuery(ctx, logsClient, logsQuery)
 	if err != nil {
 		return nil, err
 	}
@@ -319,17 +299,17 @@ func (e *cloudWatchExecutor) handleStopQuery(ctx context.Context, logsClient clo
 }
 
 func (e *cloudWatchExecutor) executeGetQueryResults(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson) (*cloudwatchlogs.GetQueryResultsOutput, error) {
+	logsQuery models.LogsQuery) (*cloudwatchlogs.GetQueryResultsOutput, error) {
 	queryInput := &cloudwatchlogs.GetQueryResultsInput{
-		QueryId: aws.String(parameters.QueryId),
+		QueryId: aws.String(logsQuery.QueryId),
 	}
 
 	return logsClient.GetQueryResultsWithContext(ctx, queryInput)
 }
 
 func (e *cloudWatchExecutor) handleGetQueryResults(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson, refID string) (*data.Frame, error) {
-	getQueryResultsOutput, err := e.executeGetQueryResults(ctx, logsClient, parameters)
+	logsQuery models.LogsQuery, refID string) (*data.Frame, error) {
+	getQueryResultsOutput, err := e.executeGetQueryResults(ctx, logsClient, logsQuery)
 	if err != nil {
 		return nil, err
 	}
@@ -346,10 +326,10 @@ func (e *cloudWatchExecutor) handleGetQueryResults(ctx context.Context, logsClie
 }
 
 func (e *cloudWatchExecutor) handleGetLogGroupFields(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	parameters LogQueryJson, refID string) (*data.Frame, error) {
+	logsQuery models.LogsQuery, refID string) (*data.Frame, error) {
 	queryInput := &cloudwatchlogs.GetLogGroupFieldsInput{
-		LogGroupName: aws.String(parameters.LogGroupName),
-		Time:         aws.Int64(parameters.Time),
+		LogGroupName: aws.String(logsQuery.LogGroupName),
+		Time:         aws.Int64(logsQuery.Time),
 	}
 
 	getLogGroupFieldsOutput, err := logsClient.GetLogGroupFieldsWithContext(ctx, queryInput)

pkg/tsdb/cloudwatch/log_actions_test.go

@@ -410,7 +410,7 @@ func Test_executeStartQuery(t *testing.T) {
 						"subtype": "StartQuery",
 						"limit":   12,
 						"queryString":"fields @message",
-						"logGroups":[{"value": "fakeARN"}]
+						"logGroups":[{"arn": "fakeARN"}]
 					}`),
 				},
 			},
@@ -446,7 +446,7 @@ func Test_executeStartQuery(t *testing.T) {
 						"subtype": "StartQuery",
 						"limit":   12,
 						"queryString":"fields @message",
-						"logGroups":[{"value": "*fake**ARN*"}]
+						"logGroups":[{"arn": "*fake**ARN*"}]
 					}`),
 				},
 			},

pkg/tsdb/cloudwatch/log_alert.go

@@ -10,6 +10,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/models"
 )
 
 const (
@@ -21,22 +22,22 @@ func (e *cloudWatchExecutor) executeLogAlertQuery(ctx context.Context, req *back
 	resp := backend.NewQueryDataResponse()
 
 	for _, q := range req.Queries {
-		var model LogQueryJson
-		err := json.Unmarshal(q.JSON, &model)
+		var logsQuery models.LogsQuery
+		err := json.Unmarshal(q.JSON, &logsQuery)
 		if err != nil {
 			continue
 		}
 
-		model.Subtype = "StartQuery"
-		model.QueryString = model.Expression
+		logsQuery.Subtype = "StartQuery"
+		logsQuery.QueryString = logsQuery.Expression
 
-		region := model.Region
-		if model.Region == "" || region == defaultRegion {
+		region := logsQuery.Region
+		if logsQuery.Region == "" || region == defaultRegion {
 			instance, err := e.getInstance(req.PluginContext)
 			if err != nil {
 				return nil, err
 			}
-			model.Region = instance.Settings.Region
+			logsQuery.Region = instance.Settings.Region
 		}
 
 		logsClient, err := e.getCWLogsClient(req.PluginContext, region)
@@ -44,7 +45,7 @@ func (e *cloudWatchExecutor) executeLogAlertQuery(ctx context.Context, req *back
 			return nil, err
 		}
 
-		getQueryResultsOutput, err := e.alertQuery(ctx, logsClient, q, model)
+		getQueryResultsOutput, err := e.alertQuery(ctx, logsClient, q, logsQuery)
 		if err != nil {
 			return nil, err
 		}
@@ -55,8 +56,8 @@ func (e *cloudWatchExecutor) executeLogAlertQuery(ctx context.Context, req *back
 		}
 
 		var frames []*data.Frame
-		if len(model.StatsGroups) > 0 && len(dataframe.Fields) > 0 {
-			frames, err = groupResults(dataframe, model.StatsGroups)
+		if len(logsQuery.StatsGroups) > 0 && len(dataframe.Fields) > 0 {
+			frames, err = groupResults(dataframe, logsQuery.StatsGroups)
 			if err != nil {
 				return nil, err
 			}
@@ -73,14 +74,14 @@ func (e *cloudWatchExecutor) executeLogAlertQuery(ctx context.Context, req *back
 }
 
 func (e *cloudWatchExecutor) alertQuery(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI,
-	queryContext backend.DataQuery, model LogQueryJson) (*cloudwatchlogs.GetQueryResultsOutput, error) {
-	startQueryOutput, err := e.executeStartQuery(ctx, logsClient, model, queryContext.TimeRange)
+	queryContext backend.DataQuery, logsQuery models.LogsQuery) (*cloudwatchlogs.GetQueryResultsOutput, error) {
+	startQueryOutput, err := e.executeStartQuery(ctx, logsClient, logsQuery, queryContext.TimeRange)
 	if err != nil {
 		return nil, err
 	}
 
-	requestParams := LogQueryJson{
-		Region:  model.Region,
+	requestParams := models.LogsQuery{
+		Region:  logsQuery.Region,
 		QueryId: *startQueryOutput.QueryId,
 	}
 

pkg/tsdb/cloudwatch/models/logs_query.go

@@ -0,0 +1,28 @@
+package models
+
+type LogGroup struct {
+	ARN       string `json:"arn"`
+	Name      string `json:"name"`
+	AccountID string `json:"accountId"`
+}
+
+type LogsQuery struct {
+	LogType            string `json:"type"`
+	SubType            string
+	Limit              *int64
+	Time               int64
+	StartTime          *int64
+	EndTime            *int64
+	LogGroupName       string
+	LogGroupNames      []string
+	LogGroups          []LogGroup `json:"logGroups"`
+	LogGroupNamePrefix string
+	LogStreamName      string
+	StartFromHead      bool
+	Region             string
+	QueryString        string
+	QueryId            string
+	StatsGroups        []string
+	Subtype            string
+	Expression         string
+}