diff --git a/.betterer.results b/.betterer.results index c16b06a4d69..17a6ea61e05 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5159,8 +5159,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], "public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] + [0, 0, 0, "Do not use any type assertions.", "0"] ], "public/app/plugins/datasource/cloudwatch/types.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/pkg/tsdb/cloudwatch/cloudwatch_test.go b/pkg/tsdb/cloudwatch/cloudwatch_test.go index 77163253dc7..7477922aa9b 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch_test.go +++ b/pkg/tsdb/cloudwatch/cloudwatch_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "net/http" "testing" "time" @@ -257,152 +256,6 @@ func Test_executeLogAlertQuery(t *testing.T) { }) } -func TestQuery_ResourceRequest_DescribeAllLogGroups(t *testing.T) { - origNewCWLogsClient := NewCWLogsClient - t.Cleanup(func() { - NewCWLogsClient = origNewCWLogsClient - }) - - var cli fakeCWLogsClient - - NewCWLogsClient = func(sess *session.Session) cloudwatchlogsiface.CloudWatchLogsAPI { - return &cli - } - - im := datasource.NewInstanceManager(func(s backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) { - return DataSource{Settings: models.CloudWatchSettings{}}, nil - }) - - executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) - sender := &mockedCallResourceResponseSenderForOauth{} - - t.Run("multiple batches", func(t *testing.T) { - token := "foo" - cli = fakeCWLogsClient{ - logGroups: []cloudwatchlogs.DescribeLogGroupsOutput{ - { - LogGroups: []*cloudwatchlogs.LogGroup{ - { - LogGroupName: aws.String("group_a"), - }, - { - LogGroupName: aws.String("group_b"), - }, - { - LogGroupName: aws.String("group_c"), - }, - }, - NextToken: &token, - }, - { - LogGroups: []*cloudwatchlogs.LogGroup{ - { - LogGroupName: aws.String("group_x"), - }, - { - LogGroupName: aws.String("group_y"), - }, - { - LogGroupName: aws.String("group_z"), - }, - }, - }, - }, - } - - req := &backend.CallResourceRequest{ - Method: "GET", - Path: "/all-log-groups?limit=50", - PluginContext: backend.PluginContext{ - DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ - ID: 0, - }, - PluginID: "cloudwatch", - }, - } - err := executor.CallResource(context.Background(), req, sender) - require.NoError(t, err) - sent := sender.Response - require.NotNil(t, sent) - require.Equal(t, http.StatusOK, sent.Status) - - suggestDataResponse := []suggestData{} - err = json.Unmarshal(sent.Body, &suggestDataResponse) - require.Nil(t, err) - - assert.Equal(t, stringsToSuggestData([]string{ - "group_a", "group_b", "group_c", "group_x", "group_y", "group_z", - }), suggestDataResponse) - }) - - t.Run("Should call api with LogGroupNamePrefix if passed in resource call", func(t *testing.T) { - cli = fakeCWLogsClient{ - logGroups: []cloudwatchlogs.DescribeLogGroupsOutput{ - {LogGroups: []*cloudwatchlogs.LogGroup{}}, - }, - } - - executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) - - req := &backend.CallResourceRequest{ - Method: "GET", - Path: "/all-log-groups?logGroupNamePrefix=test", - PluginContext: backend.PluginContext{ - DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ - ID: 0, - }, - PluginID: "cloudwatch", - }, - } - err := executor.CallResource(context.Background(), req, sender) - - require.NoError(t, err) - sent := sender.Response - require.NotNil(t, sent) - require.Equal(t, http.StatusOK, sent.Status) - - assert.Equal(t, []*cloudwatchlogs.DescribeLogGroupsInput{ - { - Limit: aws.Int64(defaultLogGroupLimit), - LogGroupNamePrefix: aws.String("test"), - }, - }, cli.calls.describeLogGroups) - }) - - t.Run("Should call api without LogGroupNamePrefix when an empty string is passed in resource call", func(t *testing.T) { - cli = fakeCWLogsClient{ - logGroups: []cloudwatchlogs.DescribeLogGroupsOutput{ - {LogGroups: []*cloudwatchlogs.LogGroup{}}, - }, - } - - executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) - - req := &backend.CallResourceRequest{ - Method: "GET", - Path: "/all-log-groups?logGroupNamePrefix=", - PluginContext: backend.PluginContext{ - DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ - ID: 0, - }, - PluginID: "cloudwatch", - }, - } - err := executor.CallResource(context.Background(), req, sender) - - require.NoError(t, err) - sent := sender.Response - require.NotNil(t, sent) - require.Equal(t, http.StatusOK, sent.Status) - - assert.Equal(t, []*cloudwatchlogs.DescribeLogGroupsInput{ - { - Limit: aws.Int64(50), - }, - }, cli.calls.describeLogGroups) - }) -} - func TestQuery_ResourceRequest_DescribeLogGroups_with_CrossAccountQuerying(t *testing.T) { sender := &mockedCallResourceResponseSenderForOauth{} origNewMetricsAPI := NewMetricsAPI @@ -425,7 +278,7 @@ func TestQuery_ResourceRequest_DescribeLogGroups_with_CrossAccountQuerying(t *te return DataSource{Settings: models.CloudWatchSettings{}}, nil }) - t.Run("maps log group api response to resource response of describe-log-groups", func(t *testing.T) { + t.Run("maps log group api response to resource response of log-groups", func(t *testing.T) { logsApi = mocks.LogsAPI{} logsApi.On("DescribeLogGroups", mock.Anything).Return(&cloudwatchlogs.DescribeLogGroupsOutput{ LogGroups: []*cloudwatchlogs.LogGroup{ @@ -434,7 +287,7 @@ func TestQuery_ResourceRequest_DescribeLogGroups_with_CrossAccountQuerying(t *te }, nil) req := &backend.CallResourceRequest{ Method: "GET", - Path: `/describe-log-groups?logGroupPattern=some-pattern&accountId=some-account-id`, + Path: `/log-groups?logGroupPattern=some-pattern&accountId=some-account-id`, PluginContext: backend.PluginContext{ DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ID: 0}, PluginID: "cloudwatch", @@ -464,11 +317,3 @@ func TestQuery_ResourceRequest_DescribeLogGroups_with_CrossAccountQuerying(t *te }) }) } - -func stringsToSuggestData(values []string) []suggestData { - suggestDataArray := make([]suggestData, 0) - for _, v := range values { - suggestDataArray = append(suggestDataArray, suggestData{Text: v, Value: v, Label: v}) - } - return suggestDataArray -} diff --git a/pkg/tsdb/cloudwatch/metric_find_query.go b/pkg/tsdb/cloudwatch/metric_find_query.go index 237f0d4dcae..29387858e5f 100644 --- a/pkg/tsdb/cloudwatch/metric_find_query.go +++ b/pkg/tsdb/cloudwatch/metric_find_query.go @@ -12,7 +12,6 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/cloudwatchlogs" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -288,44 +287,3 @@ func (e *cloudWatchExecutor) resourceGroupsGetResources(pluginCtx backend.Plugin return &resp, nil } - -func (e *cloudWatchExecutor) handleGetAllLogGroups(pluginCtx backend.PluginContext, parameters url.Values) ([]suggestData, error) { - var nextToken *string - - logGroupNamePrefix := parameters.Get("logGroupNamePrefix") - - var err error - logsClient, err := e.getCWLogsClient(pluginCtx, parameters.Get("region")) - if err != nil { - return nil, err - } - - var response *cloudwatchlogs.DescribeLogGroupsOutput - result := make([]suggestData, 0) - for { - input := &cloudwatchlogs.DescribeLogGroupsInput{ - Limit: aws.Int64(defaultLogGroupLimit), - NextToken: nextToken, - } - if len(logGroupNamePrefix) > 0 { - input.LogGroupNamePrefix = aws.String(logGroupNamePrefix) - } - response, err = logsClient.DescribeLogGroups(input) - - if err != nil || response == nil { - return nil, err - } - - for _, logGroup := range response.LogGroups { - logGroupName := *logGroup.LogGroupName - result = append(result, suggestData{Text: logGroupName, Value: logGroupName, Label: logGroupName}) - } - - if response.NextToken == nil { - break - } - nextToken = response.NextToken - } - - return result, nil -} diff --git a/pkg/tsdb/cloudwatch/models/resources/log_groups_resource_request.go b/pkg/tsdb/cloudwatch/models/resources/log_groups_resource_request.go index bf1502c75d0..80843c9d74e 100644 --- a/pkg/tsdb/cloudwatch/models/resources/log_groups_resource_request.go +++ b/pkg/tsdb/cloudwatch/models/resources/log_groups_resource_request.go @@ -12,6 +12,7 @@ type LogGroupsRequest struct { ResourceRequest Limit int64 LogGroupNamePrefix, LogGroupNamePattern *string + ListAllLogGroups bool } func (r LogGroupsRequest) IsTargetingAllAccounts() bool { @@ -33,6 +34,7 @@ func ParseLogGroupsRequest(parameters url.Values) (LogGroupsRequest, error) { }, LogGroupNamePrefix: logGroupNamePrefix, LogGroupNamePattern: logGroupPattern, + ListAllLogGroups: parameters.Get("listAllLogGroups") == "true", }, nil } diff --git a/pkg/tsdb/cloudwatch/resource_handler.go b/pkg/tsdb/cloudwatch/resource_handler.go index 2960c7bc77d..d1994c03801 100644 --- a/pkg/tsdb/cloudwatch/resource_handler.go +++ b/pkg/tsdb/cloudwatch/resource_handler.go @@ -18,8 +18,7 @@ func (e *cloudWatchExecutor) newResourceMux() *http.ServeMux { mux.HandleFunc("/ebs-volume-ids", handleResourceReq(e.handleGetEbsVolumeIds)) mux.HandleFunc("/ec2-instance-attribute", handleResourceReq(e.handleGetEc2InstanceAttribute)) mux.HandleFunc("/resource-arns", handleResourceReq(e.handleGetResourceArns)) - mux.HandleFunc("/describe-log-groups", routes.ResourceRequestMiddleware(routes.LogGroupsHandler, logger, e.getRequestContext)) - mux.HandleFunc("/all-log-groups", handleResourceReq(e.handleGetAllLogGroups)) + mux.HandleFunc("/log-groups", routes.ResourceRequestMiddleware(routes.LogGroupsHandler, logger, e.getRequestContext)) mux.HandleFunc("/metrics", routes.ResourceRequestMiddleware(routes.MetricsHandler, logger, e.getRequestContext)) mux.HandleFunc("/dimension-values", routes.ResourceRequestMiddleware(routes.DimensionValuesHandler, logger, e.getRequestContext)) mux.HandleFunc("/dimension-keys", routes.ResourceRequestMiddleware(routes.DimensionKeysHandler, logger, e.getRequestContext)) diff --git a/pkg/tsdb/cloudwatch/services/log_groups.go b/pkg/tsdb/cloudwatch/services/log_groups.go index 56ef087ad86..54d2e5ada5d 100644 --- a/pkg/tsdb/cloudwatch/services/log_groups.go +++ b/pkg/tsdb/cloudwatch/services/log_groups.go @@ -33,20 +33,28 @@ func (s *LogGroupsService) GetLogGroups(req resources.LogGroupsRequest) ([]resou input.AccountIdentifiers = []*string{req.AccountId} } } - response, err := s.logGroupsAPI.DescribeLogGroups(input) - if err != nil || response == nil { - return nil, err - } + result := []resources.ResourceResponse[resources.LogGroup]{} - var result []resources.ResourceResponse[resources.LogGroup] - for _, logGroup := range response.LogGroups { - result = append(result, resources.ResourceResponse[resources.LogGroup]{ - Value: resources.LogGroup{ - Arn: *logGroup.Arn, - Name: *logGroup.LogGroupName, - }, - AccountId: utils.Pointer(getAccountId(*logGroup.Arn)), - }) + for { + response, err := s.logGroupsAPI.DescribeLogGroups(input) + if err != nil || response == nil { + return nil, err + } + + for _, logGroup := range response.LogGroups { + result = append(result, resources.ResourceResponse[resources.LogGroup]{ + Value: resources.LogGroup{ + Arn: *logGroup.Arn, + Name: *logGroup.LogGroupName, + }, + AccountId: utils.Pointer(getAccountId(*logGroup.Arn)), + }) + } + + if !req.ListAllLogGroups || response.NextToken == nil { + break + } + input.NextToken = response.NextToken } return result, nil diff --git a/pkg/tsdb/cloudwatch/services/log_groups_test.go b/pkg/tsdb/cloudwatch/services/log_groups_test.go index 5bead8d9845..80dcaa32e6c 100644 --- a/pkg/tsdb/cloudwatch/services/log_groups_test.go +++ b/pkg/tsdb/cloudwatch/services/log_groups_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudwatchlogs" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" @@ -44,6 +45,17 @@ func TestGetLogGroups(t *testing.T) { }, resp) }) + t.Run("Should return an empty error if api doesn't return any data", func(t *testing.T) { + mockLogsAPI := &mocks.LogsAPI{} + mockLogsAPI.On("DescribeLogGroups", mock.Anything).Return(&cloudwatchlogs.DescribeLogGroupsOutput{}, nil) + service := NewLogGroupsService(mockLogsAPI, false) + + resp, err := service.GetLogGroups(resources.LogGroupsRequest{}) + + assert.NoError(t, err) + assert.Equal(t, []resources.ResourceResponse[resources.LogGroup]{}, resp) + }) + t.Run("Should only use LogGroupNamePrefix even if LogGroupNamePattern passed in resource call", func(t *testing.T) { // TODO: use LogGroupNamePattern when we have accounted for its behavior, still a little unexpected at the moment mockLogsAPI := &mocks.LogsAPI{} @@ -86,6 +98,82 @@ func TestGetLogGroups(t *testing.T) { assert.Error(t, err) assert.Equal(t, "some error", err.Error()) }) + + t.Run("Should only call the api once in case ListAllLogGroups is set to false", func(t *testing.T) { + mockLogsAPI := &mocks.LogsAPI{} + req := resources.LogGroupsRequest{ + Limit: 2, + LogGroupNamePrefix: utils.Pointer("test"), + ListAllLogGroups: false, + } + + mockLogsAPI.On("DescribeLogGroups", &cloudwatchlogs.DescribeLogGroupsInput{ + Limit: aws.Int64(req.Limit), + LogGroupNamePrefix: req.LogGroupNamePrefix, + }).Return(&cloudwatchlogs.DescribeLogGroupsOutput{ + LogGroups: []*cloudwatchlogs.LogGroup{ + {Arn: utils.Pointer("arn:aws:logs:us-east-1:111:log-group:group_a"), LogGroupName: utils.Pointer("group_a")}, + }, + NextToken: aws.String("next_token"), + }, nil) + + service := NewLogGroupsService(mockLogsAPI, false) + resp, err := service.GetLogGroups(req) + + assert.NoError(t, err) + mockLogsAPI.AssertNumberOfCalls(t, "DescribeLogGroups", 1) + assert.Equal(t, []resources.ResourceResponse[resources.LogGroup]{ + { + AccountId: utils.Pointer("111"), + Value: resources.LogGroup{Arn: "arn:aws:logs:us-east-1:111:log-group:group_a", Name: "group_a"}, + }, + }, resp) + }) + + t.Run("Should keep on calling the api until NextToken is empty in case ListAllLogGroups is set to true", func(t *testing.T) { + mockLogsAPI := &mocks.LogsAPI{} + req := resources.LogGroupsRequest{ + Limit: 2, + LogGroupNamePrefix: utils.Pointer("test"), + ListAllLogGroups: true, + } + + // first call + mockLogsAPI.On("DescribeLogGroups", &cloudwatchlogs.DescribeLogGroupsInput{ + Limit: aws.Int64(req.Limit), + LogGroupNamePrefix: req.LogGroupNamePrefix, + }).Return(&cloudwatchlogs.DescribeLogGroupsOutput{ + LogGroups: []*cloudwatchlogs.LogGroup{ + {Arn: utils.Pointer("arn:aws:logs:us-east-1:111:log-group:group_a"), LogGroupName: utils.Pointer("group_a")}, + }, + NextToken: utils.Pointer("token"), + }, nil) + + // second call + mockLogsAPI.On("DescribeLogGroups", &cloudwatchlogs.DescribeLogGroupsInput{ + Limit: aws.Int64(req.Limit), + LogGroupNamePrefix: req.LogGroupNamePrefix, + NextToken: utils.Pointer("token"), + }).Return(&cloudwatchlogs.DescribeLogGroupsOutput{ + LogGroups: []*cloudwatchlogs.LogGroup{ + {Arn: utils.Pointer("arn:aws:logs:us-east-1:222:log-group:group_b"), LogGroupName: utils.Pointer("group_b")}, + }, + }, nil) + service := NewLogGroupsService(mockLogsAPI, false) + resp, err := service.GetLogGroups(req) + assert.NoError(t, err) + mockLogsAPI.AssertNumberOfCalls(t, "DescribeLogGroups", 2) + assert.Equal(t, []resources.ResourceResponse[resources.LogGroup]{ + { + AccountId: utils.Pointer("111"), + Value: resources.LogGroup{Arn: "arn:aws:logs:us-east-1:111:log-group:group_a", Name: "group_a"}, + }, + { + AccountId: utils.Pointer("222"), + Value: resources.LogGroup{Arn: "arn:aws:logs:us-east-1:222:log-group:group_b", Name: "group_b"}, + }, + }, resp) + }) } func TestGetLogGroupsCrossAccountQuerying(t *testing.T) { diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts index 5907a154e75..52f31219faf 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts @@ -85,7 +85,7 @@ export function setupMockedDataSource({ datasource.api.getDimensionKeys = jest.fn().mockResolvedValue([]); datasource.api.getMetrics = jest.fn().mockResolvedValue([]); datasource.api.getAccounts = jest.fn().mockResolvedValue([]); - datasource.api.describeLogGroups = jest.fn().mockResolvedValue([]); + datasource.api.getLogGroups = jest.fn().mockResolvedValue([]); const fetchMock = jest.fn().mockReturnValue(of({})); setBackendSrv({ ...getBackendSrv(), @@ -193,7 +193,7 @@ export const logGroupNamesVariable: CustomVariableModel = { id: 'groups', name: 'groups', current: { - value: ['templatedGroup-1', 'templatedGroup-2'], + value: ['templatedGroup-arn-1', 'templatedGroup-arn-2'], text: ['templatedGroup-1', 'templatedGroup-2'], selected: true, }, diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts index 668dd3f0253..2b13ce17ce4 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts @@ -1,12 +1,12 @@ import { of } from 'rxjs'; -import { CustomVariableModel, DataFrame } from '@grafana/data'; +import { CustomVariableModel, DataFrame, DataSourceInstanceSettings } from '@grafana/data'; import { BackendDataSourceResponse, getBackendSrv, setBackendSrv } from '@grafana/runtime'; import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { TemplateSrv } from 'app/features/templating/template_srv'; import { CloudWatchLogsQueryRunner } from '../query-runner/CloudWatchLogsQueryRunner'; -import { CloudWatchLogsQueryStatus } from '../types'; +import { CloudWatchJsonData, CloudWatchLogsQueryStatus } from '../types'; import { CloudWatchSettings, setupMockedTemplateService } from './CloudWatchDataSource'; @@ -16,7 +16,13 @@ export function setupMockedLogsQueryRunner({ }, variables, mockGetVariableName = true, -}: { data?: BackendDataSourceResponse; variables?: CustomVariableModel[]; mockGetVariableName?: boolean } = {}) { + settings = CloudWatchSettings, +}: { + data?: BackendDataSourceResponse; + variables?: CustomVariableModel[]; + mockGetVariableName?: boolean; + settings?: DataSourceInstanceSettings; +} = {}) { let templateService = new TemplateSrv(); if (variables) { templateService = setupMockedTemplateService(variables); @@ -25,7 +31,7 @@ export function setupMockedLogsQueryRunner({ } } - const runner = new CloudWatchLogsQueryRunner(CloudWatchSettings, templateService, getTimeSrv()); + const runner = new CloudWatchLogsQueryRunner(settings, templateService, getTimeSrv()); const fetchMock = jest.fn().mockReturnValue(of({ data })); setBackendSrv({ ...getBackendSrv(), diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/Request.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/Request.ts index efb8420b351..3366db848c1 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/Request.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/Request.ts @@ -1,6 +1,6 @@ import { DataQueryRequest } from '@grafana/data'; -import { CloudWatchQuery } from '../types'; +import { CloudWatchQuery, CloudWatchLogsQuery } from '../types'; import { TimeRangeMock } from './timeRange'; @@ -16,3 +16,16 @@ export const RequestMock: DataQueryRequest = { app: '', startTime: 0, }; + +export const LogsRequestMock: DataQueryRequest = { + range: TimeRangeMock, + rangeRaw: { from: TimeRangeMock.from, to: TimeRangeMock.to }, + targets: [], + requestId: '', + interval: '', + intervalMs: 0, + scopedVars: {}, + timezone: '', + app: '', + startTime: 0, +}; diff --git a/public/app/plugins/datasource/cloudwatch/api.test.ts b/public/app/plugins/datasource/cloudwatch/api.test.ts index 99fbbcc32f2..e786a3f0b4d 100644 --- a/public/app/plugins/datasource/cloudwatch/api.test.ts +++ b/public/app/plugins/datasource/cloudwatch/api.test.ts @@ -4,10 +4,10 @@ describe('api', () => { describe('describeLogGroup', () => { it('replaces region correctly in the query', async () => { const { api, resourceRequestMock } = setupMockedAPI(); - await api.describeLogGroups({ region: 'default' }); + await api.getLogGroups({ region: 'default' }); expect(resourceRequestMock.mock.calls[0][1].region).toBe('us-west-1'); - await api.describeLogGroups({ region: 'eu-east' }); + await api.getLogGroups({ region: 'eu-east' }); expect(resourceRequestMock.mock.calls[1][1].region).toBe('eu-east'); }); @@ -49,7 +49,7 @@ describe('api', () => { }, ]; - const logGroups = await api.describeLogGroups({ region: 'default' }); + const logGroups = await api.getLogGroups({ region: 'default' }); expect(logGroups).toEqual(expectedLogGroups); }); diff --git a/public/app/plugins/datasource/cloudwatch/api.ts b/public/app/plugins/datasource/cloudwatch/api.ts index 7b7e4b97037..719d32c30b0 100644 --- a/public/app/plugins/datasource/cloudwatch/api.ts +++ b/public/app/plugins/datasource/cloudwatch/api.ts @@ -64,15 +64,16 @@ export class CloudWatchAPI extends CloudWatchRequest { ); } - async describeLogGroups(params: DescribeLogGroupsRequest): Promise>> { - return this.memoizedGetRequest>>('describe-log-groups', { + getLogGroups(params: DescribeLogGroupsRequest): Promise>> { + return this.memoizedGetRequest>>('log-groups', { ...params, region: this.templateSrv.replace(this.getActualRegion(params.region)), accountId: this.templateSrv.replace(params.accountId), + listAllLogGroups: params.listAllLogGroups ? 'true' : 'false', }); } - async getLogGroupFields({ + getLogGroupFields({ region, arn, logGroupName, @@ -84,16 +85,9 @@ export class CloudWatchAPI extends CloudWatchRequest { }); } - async describeAllLogGroups(params: DescribeLogGroupsRequest) { - return this.memoizedGetRequest('all-log-groups', { - ...params, - region: this.templateSrv.replace(this.getActualRegion(params.region)), - }); - } - - async getMetrics({ region, namespace, accountId }: GetMetricsRequest): Promise>> { + getMetrics({ region, namespace, accountId }: GetMetricsRequest): Promise>> { if (!namespace) { - return []; + return Promise.resolve([]); } return this.memoizedGetRequest>>('metrics', { @@ -103,17 +97,14 @@ export class CloudWatchAPI extends CloudWatchRequest { }).then((metrics) => metrics.map((m) => ({ label: m.value.name, value: m.value.name }))); } - async getAllMetrics({ - region, - accountId, - }: GetMetricsRequest): Promise> { + getAllMetrics({ region, accountId }: GetMetricsRequest): Promise> { return this.memoizedGetRequest>>('metrics', { region: this.templateSrv.replace(this.getActualRegion(region)), accountId: this.templateSrv.replace(accountId), }).then((metrics) => metrics.map((m) => ({ metricName: m.value.name, namespace: m.value.namespace }))); } - async getDimensionKeys({ + getDimensionKeys({ region, namespace = '', dimensionFilters = {}, @@ -129,7 +120,7 @@ export class CloudWatchAPI extends CloudWatchRequest { }).then((r) => r.map((r) => ({ label: r.value, value: r.value }))); } - async getDimensionValues({ + getDimensionValues({ dimensionKey, region, namespace, @@ -138,10 +129,10 @@ export class CloudWatchAPI extends CloudWatchRequest { accountId, }: GetDimensionValuesRequest) { if (!namespace || !metricName) { - return []; + return Promise.resolve([]); } - const values = await this.memoizedGetRequest>>('dimension-values', { + return this.memoizedGetRequest>>('dimension-values', { region: this.templateSrv.replace(this.getActualRegion(region)), namespace: this.templateSrv.replace(namespace), metricName: this.templateSrv.replace(metricName.trim()), @@ -149,7 +140,6 @@ export class CloudWatchAPI extends CloudWatchRequest { dimensionFilters: JSON.stringify(this.convertDimensionFormat(dimensionFilters, {})), accountId: this.templateSrv.replace(accountId), }).then((r) => r.map((r) => ({ label: r.value, value: r.value }))); - return values; } getEbsVolumeIds(region: string, instanceId: string) { diff --git a/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.test.tsx b/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.test.tsx index f15b864520b..4e8d6684a76 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.test.tsx @@ -5,7 +5,7 @@ import React from 'react'; import { config } from '@grafana/runtime'; -import { setupMockedDataSource } from '../../__mocks__/CloudWatchDataSource'; +import { logGroupNamesVariable, setupMockedDataSource } from '../../__mocks__/CloudWatchDataSource'; import { LogGroupsField } from './LogGroupsField'; @@ -30,29 +30,49 @@ describe('LogGroupSelection', () => { lodash.debounce = originalDebounce; }); - it('call describeCrossAccountLogGroups to get associated log group arns and then update props if rendered with legacy log group names', async () => { + it('should call getLogGroups to get associated log group arns and then update props if rendered with legacy log group names', async () => { config.featureToggles.cloudWatchCrossAccountQuerying = true; - defaultProps.datasource.api.describeLogGroups = jest + defaultProps.datasource.api.getLogGroups = jest .fn() .mockResolvedValue([{ value: { arn: 'arn', name: 'loggroupname' } }]); render(); await waitFor(async () => expect(screen.getByText('Select Log Groups')).toBeInTheDocument()); - expect(defaultProps.datasource.api.describeLogGroups).toHaveBeenCalledWith({ + expect(defaultProps.datasource.api.getLogGroups).toHaveBeenCalledWith({ region: defaultProps.region, logGroupNamePrefix: 'loggroupname', }); expect(defaultProps.onChange).toHaveBeenCalledWith([{ arn: 'arn', name: 'loggroupname' }]); }); - it('should not call describeCrossAccountLogGroups and update props if rendered with log groups', async () => { + it('should not call getLogGroups to get associated log group arns for template variables that were part of the legacy log group names array, only include them in the call to onChange', async () => { config.featureToggles.cloudWatchCrossAccountQuerying = true; - defaultProps.datasource.api.describeLogGroups = jest + defaultProps.datasource = setupMockedDataSource({ variables: [logGroupNamesVariable] }).datasource; + defaultProps.datasource.api.getLogGroups = jest + .fn() + .mockResolvedValue([{ value: { arn: 'arn', name: 'loggroupname' } }]); + render(); + + await waitFor(async () => expect(screen.getByText('Select Log Groups')).toBeInTheDocument()); + expect(defaultProps.datasource.api.getLogGroups).toHaveBeenCalledTimes(1); + expect(defaultProps.datasource.api.getLogGroups).toHaveBeenCalledWith({ + region: defaultProps.region, + logGroupNamePrefix: 'loggroupname', + }); + expect(defaultProps.onChange).toHaveBeenCalledWith([ + { arn: 'arn', name: 'loggroupname' }, + { arn: logGroupNamesVariable.name, name: logGroupNamesVariable.name }, + ]); + }); + + it('should not call getLogGroups and update props if rendered with log groups', async () => { + config.featureToggles.cloudWatchCrossAccountQuerying = true; + defaultProps.datasource.api.getLogGroups = jest .fn() .mockResolvedValue([{ value: { arn: 'arn', name: 'loggroupname' } }]); render(); await waitFor(() => expect(screen.getByText('Select Log Groups')).toBeInTheDocument()); - expect(defaultProps.datasource.api.describeLogGroups).not.toHaveBeenCalled(); + expect(defaultProps.datasource.api.getLogGroups).not.toHaveBeenCalled(); expect(defaultProps.onChange).not.toHaveBeenCalled(); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.tsx b/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.tsx index 3964a48fe3b..1ec17498541 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.tsx @@ -4,6 +4,7 @@ import React, { useEffect, useState } from 'react'; import { CloudWatchDatasource } from '../../datasource'; import { useAccountOptions } from '../../hooks'; import { DescribeLogGroupsRequest, LogGroup } from '../../types'; +import { isTemplateVariable } from '../../utils/templateVariableUtils'; import { LogGroupsSelector } from './LogGroupsSelector'; import { SelectedLogGroups } from './SelectedLogGroups'; @@ -38,11 +39,18 @@ export const LogGroupsField = ({ // If log group names are stored in the query model, make a new DescribeLogGroups request for each log group to load the arn. Then update the query model. if (datasource && !loadingLogGroupsStarted && !logGroups?.length && legacyLogGroupNames?.length) { setLoadingLogGroupsStarted(true); + + // there's no need to migrate variables, they will be taken care of in the logs query runner + const variables = legacyLogGroupNames.filter((lgn) => isTemplateVariable(datasource.api.templateSrv, lgn)); + const legacyLogGroupNameValues = legacyLogGroupNames.filter( + (lgn) => !isTemplateVariable(datasource.api.templateSrv, lgn) + ); + Promise.all( - legacyLogGroupNames.map((lg) => datasource.api.describeLogGroups({ region: region, logGroupNamePrefix: lg })) + legacyLogGroupNameValues.map((lg) => datasource.api.getLogGroups({ region: region, logGroupNamePrefix: lg })) ) .then((results) => { - const a = results.flatMap((r) => + const logGroups = results.flatMap((r) => r.map((lg) => ({ arn: lg.value.arn, name: lg.value.name, @@ -50,9 +58,11 @@ export const LogGroupsField = ({ })) ); - onChange(a); + onChange([...logGroups, ...variables.map((v) => ({ name: v, arn: v }))]); }) - .catch(console.error); + .catch((err) => { + console.error(err); + }); } }, [datasource, legacyLogGroupNames, logGroups, onChange, region, loadingLogGroupsStarted]); @@ -60,12 +70,13 @@ export const LogGroupsField = ({
) => - datasource?.api.describeLogGroups({ region: region, ...params }) ?? [] + datasource?.api.getLogGroups({ region: region, ...params }) ?? [] } onChange={onChange} accountOptions={accountState.value} selectedLogGroups={logGroups} onBeforeOpen={onBeforeOpen} + variables={datasource?.getVariables()} /> { +describe('LogGroupsSelector', () => { beforeEach(() => { lodash.debounce = jest.fn().mockImplementation((fn) => { fn.cancel = () => {}; @@ -152,6 +153,7 @@ describe('CrossAccountLogsQueryField', () => { await userEvent.click(screen.getByLabelText('logGroup2')); expect(screen.getByLabelText('logGroup2')).toBeChecked(); }); + it('calls onChange with the selected log group when checked and the user clicks the Add button', async () => { const onChange = jest.fn(); render(); @@ -226,4 +228,86 @@ describe('CrossAccountLogsQueryField', () => { await userEvent.click(screen.getByLabelText('logGroup2')); await waitFor(() => expect(screen.getByText('1 log group selected')).toBeInTheDocument()); }); + + it('should not include selected template variables in the counter label', async () => { + render( + + ); + await userEvent.click(screen.getByText('Select Log Groups')); + await waitFor(() => expect(screen.getByText('1 log group selected')).toBeInTheDocument()); + }); + + it('should be possible to select a template variable and add it to selected log groups when the user clicks the Add button', async () => { + const onChange = jest.fn(); + render( + + ); + await userEvent.click(screen.getByText('Select Log Groups')); + await selectEvent.select(screen.getByLabelText('Template variable'), '$logGroupVariable', { + container: document.body, + }); + await userEvent.click(screen.getByText('Add log groups')); + expect(onChange).toHaveBeenCalledWith([ + { + arn: 'arn:partition:service:region:account-id456:loggroup:someotherloggroup', + name: 'logGroup1', + }, + { + arn: '$logGroupVariable', + name: '$logGroupVariable', + }, + ]); + }); + + it('should be possible to remove template variable from selected log groups', async () => { + const onChange = jest.fn(); + render( + + ); + await userEvent.click(screen.getByText('Select Log Groups')); + await screen.getByRole('button', { name: 'select-clear-value' }).click(); + await userEvent.click(screen.getByText('Add log groups')); + expect(onChange).toHaveBeenCalledWith([ + { + arn: 'arn:partition:service:region:account-id456:loggroup:someotherloggroup', + name: 'logGroup1', + }, + ]); + }); + + it('should display account label if account options prop has values', async () => { + render(); + await userEvent.click(screen.getByText('Select Log Groups')); + expect(screen.getByText('Log group name prefix')).toBeInTheDocument(); + expect(screen.getByText('Account label')).toBeInTheDocument(); + waitFor(() => expect(screen.getByText('Account Name 123')).toBeInTheDocument()); + }); + + it('should not display account label if account options prop doesnt has values', async () => { + render(); + await userEvent.click(screen.getByText('Select Log Groups')); + expect(screen.getByText('Log group name prefix')).toBeInTheDocument(); + expect(screen.queryByText('Account label')).not.toBeInTheDocument(); + waitFor(() => expect(screen.queryByText('Account Name 123')).not.toBeInTheDocument()); + }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsSelector.tsx b/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsSelector.tsx index 5ebcb1b8163..03b471beeca 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsSelector.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsSelector.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useMemo, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { EditorField, Space } from '@grafana/experimental'; -import { Button, Checkbox, Icon, Label, LoadingPlaceholder, Modal, useStyles2 } from '@grafana/ui'; +import { Button, Checkbox, Icon, Label, LoadingPlaceholder, Modal, Select, useStyles2 } from '@grafana/ui'; import Search from '../../Search'; import { DescribeLogGroupsRequest, LogGroup, LogGroupResponse, ResourceResponse } from '../../types'; @@ -13,12 +13,14 @@ type CrossAccountLogsQueryProps = { selectedLogGroups?: LogGroup[]; accountOptions?: Array>; fetchLogGroups: (params: Partial) => Promise>>; + variables?: string[]; onChange: (selectedLogGroups: LogGroup[]) => void; onBeforeOpen?: () => void; }; export const LogGroupsSelector = ({ accountOptions = [], + variables = [], fetchLogGroups, onChange, onBeforeOpen, @@ -31,6 +33,19 @@ export const LogGroupsSelector = ({ const [searchAccountId, setSearchAccountId] = useState(ALL_ACCOUNTS_OPTION.value); const [isLoading, setIsLoading] = useState(false); const styles = useStyles2(getStyles); + const selectedLogGroupsCounter = useMemo( + () => selectedLogGroups.filter((lg) => !lg.name?.startsWith('$')).length, + [selectedLogGroups] + ); + const variableOptions = useMemo(() => variables.map((v) => ({ label: v, value: v })), [variables]); + const selectedVariable = useMemo( + () => selectedLogGroups.find((lg) => lg.name?.startsWith('$'))?.name, + [selectedLogGroups] + ); + const currentVariableOption = { + label: selectedVariable, + value: selectedVariable, + }; useEffect(() => { setSelectedLogGroups(props.selectedLogGroups ?? []); @@ -136,7 +151,7 @@ export const LogGroupsSelector = ({ Log Group - Account name + {accountOptions.length > 0 && Account label} Account ID @@ -169,7 +184,7 @@ export const LogGroupsSelector = ({
- {row.accountLabel} + {accountOptions.length > 0 && {row.accountLabel}} {row.accountId} ))} @@ -179,9 +194,30 @@ export const LogGroupsSelector = ({ - + + +