diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 3ebe95515c0..7272c08836e 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "slices" + "sync" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -54,9 +55,10 @@ type DataSource struct { ProxyOpts *proxy.Options AWSConfigProvider awsauth.ConfigProvider - logger log.Logger - tagValueCache *cache.Cache - resourceHandler backend.CallResourceHandler + logger log.Logger + tagValueCache *cache.Cache + resourceHandler backend.CallResourceHandler + monitoringAccountCache sync.Map } func (ds *DataSource) newAWSConfig(ctx context.Context, region string) (aws.Config, error) { @@ -273,6 +275,33 @@ func (ds *DataSource) getRGTAClient(ctx context.Context, region string) (resourc return NewRGTAClient(cfg), nil } +func (ds *DataSource) isMonitoringAccount(ctx context.Context, region string) (bool, error) { + if value, ok := ds.monitoringAccountCache.Load(region); ok { + cached := value.(bool) + return cached, nil + } + + client, err := ds.GetAccountsService(ctx, region) + if err != nil { + return false, err + } + + accounts, err := client.GetAccountsForCurrentUserOrRole(ctx) + if err != nil { + return false, err + } + + for _, account := range accounts { + if account.Value.IsMonitoringAccount { + ds.monitoringAccountCache.Store(region, true) + return true, nil + } + } + + ds.monitoringAccountCache.Store(region, false) + return false, nil +} + var terminatedStates = []cloudwatchlogstypes.QueryStatus{ cloudwatchlogstypes.QueryStatusComplete, cloudwatchlogstypes.QueryStatusCancelled, diff --git a/pkg/tsdb/cloudwatch/log_actions.go b/pkg/tsdb/cloudwatch/log_actions.go index 8a6022d44fa..60ebad06442 100644 --- a/pkg/tsdb/cloudwatch/log_actions.go +++ b/pkg/tsdb/cloudwatch/log_actions.go @@ -213,17 +213,48 @@ func (ds *DataSource) executeStartQuery(ctx context.Context, logsClient models.C // log group identifiers can be left out if the query is an SQL query if *logsQuery.QueryLanguage != dataquery.LogsQueryLanguageSQL { - if len(logsQuery.LogGroups) > 0 && features.IsEnabled(ctx, features.FlagCloudWatchCrossAccountQuerying) { - var logGroupIdentifiers []string - 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, "*")) + useLogGroupIdentifiers := false + logGroupsFromQuery := len(logsQuery.LogGroups) > 0 + if logGroupsFromQuery && features.IsEnabled(ctx, features.FlagCloudWatchCrossAccountQuerying) { + region := logsQuery.Region + if region == "" || region == defaultRegion { + region = ds.Settings.Region + } + if region != "" { + isMonitoringAccount, err := ds.isMonitoringAccount(ctx, region) + if err != nil { + ds.logger.FromContext(ctx).Debug("failed to determine monitoring account status", "err", err) + } else if isMonitoringAccount { + // monitoring accounts require querying by log group identifiers because log group names are not unique across accounts. + var logGroupIdentifiers []string + for _, lg := range logsQuery.LogGroups { + // due to a bug in the startQuery api, we remove * from the arn, otherwise it throws an error + arn := strings.TrimSuffix(lg.Arn, "*") + logGroupIdentifiers = append(logGroupIdentifiers, arn) + } + startQueryInput.LogGroupIdentifiers = logGroupIdentifiers + useLogGroupIdentifiers = true + } + } + } + + if !useLogGroupIdentifiers { + // even though logsQuery.LogGroupNames is deprecated, we still need to support it for backwards compatibility and alert queries + startQueryInput.LogGroupNames = append([]string(nil), logsQuery.LogGroupNames...) + if len(startQueryInput.LogGroupNames) == 0 && logGroupsFromQuery { + // deduplicate log group names because we only deduplicate log groups by their ARNs instead of their names when the query is created + seenLogGroupNames := make(map[string]struct{}, len(logsQuery.LogGroups)) + for _, lg := range logsQuery.LogGroups { + if lg.Name == "" { + continue + } + if _, exists := seenLogGroupNames[lg.Name]; exists { + continue + } + seenLogGroupNames[lg.Name] = struct{}{} + startQueryInput.LogGroupNames = append(startQueryInput.LogGroupNames, lg.Name) + } } - startQueryInput.LogGroupIdentifiers = logGroupIdentifiers - } else { - // even though log group names are being phased out, we still need to support them for backwards compatibility and alert queries - startQueryInput.LogGroupNames = logsQuery.LogGroupNames } } diff --git a/pkg/tsdb/cloudwatch/log_actions_test.go b/pkg/tsdb/cloudwatch/log_actions_test.go index ae1a79434bd..142b1d0e24d 100644 --- a/pkg/tsdb/cloudwatch/log_actions_test.go +++ b/pkg/tsdb/cloudwatch/log_actions_test.go @@ -445,7 +445,9 @@ func Test_executeStartQuery(t *testing.T) { t.Run("attaches logGroupIdentifiers if the crossAccount feature is enabled", func(t *testing.T) { cli = fakeCWLogsClient{} - ds := newTestDatasource() + ds := newTestDatasource(func(ds *DataSource) { + ds.monitoringAccountCache.Store("us-east-1", true) + }) _, err := ds.QueryData(contextWithFeaturesEnabled(features.FlagCloudWatchCrossAccountQuerying), &backend.QueryDataRequest{ PluginContext: backend.PluginContext{DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}}, @@ -459,7 +461,8 @@ func Test_executeStartQuery(t *testing.T) { "limit": 12, "queryLanguage": "CWLI", "queryString":"fields @message", - "logGroups":[{"arn": "fakeARN"}] + "logGroups":[{"arn": "fakeARN"}], + "region": "us-east-1" }`), }, }, @@ -480,7 +483,9 @@ func Test_executeStartQuery(t *testing.T) { t.Run("attaches logGroupIdentifiers if the crossAccount feature is enabled and strips out trailing *", func(t *testing.T) { cli = fakeCWLogsClient{} - ds := newTestDatasource() + ds := newTestDatasource(func(ds *DataSource) { + ds.monitoringAccountCache.Store("us-east-1", true) + }) _, err := ds.QueryData(contextWithFeaturesEnabled(features.FlagCloudWatchCrossAccountQuerying), &backend.QueryDataRequest{ PluginContext: backend.PluginContext{DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}}, @@ -493,7 +498,8 @@ func Test_executeStartQuery(t *testing.T) { "subtype": "StartQuery", "limit": 12, "queryString":"fields @message", - "logGroups":[{"arn": "*fake**ARN*"}] + "logGroups":[{"arn": "*fake**ARN*"}], + "region": "us-east-1" }`), }, }, @@ -512,6 +518,44 @@ func Test_executeStartQuery(t *testing.T) { }, cli.calls.startQuery) }) + t.Run("queries by LogGroupNames on StartQueryInput when queried region is not a monitoring account region for the data source", func(t *testing.T) { + cli = fakeCWLogsClient{} + ds := newTestDatasource(func(ds *DataSource) { + // note that the query's region is set to us-east-2, but the data source is only a monitoring account in us-east-1 so it should query by LogGroupNames + ds.monitoringAccountCache.Store("us-east-1", true) + }) + + _, err := ds.QueryData(contextWithFeaturesEnabled(features.FlagCloudWatchCrossAccountQuerying), &backend.QueryDataRequest{ + PluginContext: backend.PluginContext{DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}}, + Queries: []backend.DataQuery{ + { + RefID: "A", + TimeRange: backend.TimeRange{From: time.Unix(0, 0), To: time.Unix(1, 0)}, + JSON: json.RawMessage(`{ + "type": "logAction", + "subtype": "StartQuery", + "limit": 12, + "queryString":"fields @message", + "logGroups":[{"arn": "arn:aws:logs:us-east-1:123456789012:log-group:group","name":"/log-group"}], + "region": "us-east-2" + }`), + }, + }, + }) + + assert.NoError(t, err) + assert.Equal(t, []*cloudwatchlogs.StartQueryInput{ + { + StartTime: aws.Int64(0), + EndTime: aws.Int64(1), + Limit: aws.Int32(12), + QueryString: aws.String("fields @timestamp,ltrim(@log) as __log__grafana_internal__,ltrim(@logStream) as __logstream__grafana_internal__|fields @message"), + LogGroupNames: []string{"/log-group"}, + QueryLanguage: cloudwatchlogstypes.QueryLanguageCwli, + }, + }, cli.calls.startQuery) + }) + t.Run("uses LogGroupNames if the cross account feature flag is not enabled, and log group names is present", func(t *testing.T) { cli = fakeCWLogsClient{} ds := newTestDatasource() @@ -545,6 +589,42 @@ func Test_executeStartQuery(t *testing.T) { }, cli.calls.startQuery) }) + t.Run("deduplicates log group names when derived from logGroups", func(t *testing.T) { + cli = fakeCWLogsClient{} + ds := newTestDatasource() + + _, err := ds.QueryData(context.Background(), &backend.QueryDataRequest{ + PluginContext: backend.PluginContext{DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}}, + Queries: []backend.DataQuery{ + { + RefID: "A", + TimeRange: backend.TimeRange{From: time.Unix(0, 0), To: time.Unix(1, 0)}, + JSON: json.RawMessage(`{ + "type": "logAction", + "subtype": "StartQuery", + "limit": 12, + "queryString":"fields @message", + "logGroups":[ + {"arn": "arn:aws:logs:us-east-1:123456789012:log-group:group1","name":"/log-group"}, + {"arn": "arn:aws:logs:us-east-1:123456789012:log-group:group2","name":"/log-group"} + ] + }`), + }, + }, + }) + assert.NoError(t, err) + assert.Equal(t, []*cloudwatchlogs.StartQueryInput{ + { + StartTime: aws.Int64(0), + EndTime: aws.Int64(1), + Limit: aws.Int32(12), + QueryString: aws.String("fields @timestamp,ltrim(@log) as __log__grafana_internal__,ltrim(@logStream) as __logstream__grafana_internal__|fields @message"), + LogGroupNames: []string{"/log-group"}, + QueryLanguage: cloudwatchlogstypes.QueryLanguageCwli, + }, + }, cli.calls.startQuery) + }) + t.Run("ignores logGroups if feature flag is disabled even if logGroupNames is not present", func(t *testing.T) { cli = fakeCWLogsClient{} ds := newTestDatasource() @@ -600,12 +680,12 @@ func Test_executeStartQuery(t *testing.T) { assert.NoError(t, err) assert.Equal(t, []*cloudwatchlogs.StartQueryInput{ { - StartTime: aws.Int64(0), - EndTime: aws.Int64(1), - Limit: aws.Int32(12), - QueryString: aws.String("fields @timestamp,ltrim(@log) as __log__grafana_internal__,ltrim(@logStream) as __logstream__grafana_internal__|fields @message"), - LogGroupIdentifiers: []string{"*fake**ARN"}, - QueryLanguage: cloudwatchlogstypes.QueryLanguageCwli, + StartTime: aws.Int64(0), + EndTime: aws.Int64(1), + Limit: aws.Int32(12), + QueryString: aws.String("fields @timestamp,ltrim(@log) as __log__grafana_internal__,ltrim(@logStream) as __logstream__grafana_internal__|fields @message"), + LogGroupNames: []string{"/log-group"}, + QueryLanguage: cloudwatchlogstypes.QueryLanguageCwli, }, }, cli.calls.startQuery) }) diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index 3186c92121c..9368ed93d5b 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts @@ -263,8 +263,8 @@ describe('datasource', () => { expect(queryMock.mock.calls[0][0].targets[0]).toMatchObject({ queryString: 'fields templatedField', logGroups: [ - { name: 'templatedGroup-arn-1', arn: 'templatedGroup-arn-1' }, - { name: 'templatedGroup-arn-2', arn: 'templatedGroup-arn-2' }, + { name: 'templatedGroup-1', arn: 'templatedGroup-arn-1' }, + { name: 'templatedGroup-2', arn: 'templatedGroup-arn-2' }, ], logGroupNames: ['/some/group'], region: 'templatedRegion', @@ -394,8 +394,9 @@ describe('datasource', () => { expect(templateService.replace).toHaveBeenNthCalledWith(1, '$regionVar', {}); expect(templateService.replace).toHaveBeenNthCalledWith(2, '$groups', {}, 'pipe'); - expect(templateService.replace).toHaveBeenNthCalledWith(3, '$expressionVar', {}, undefined); - expect(templateService.replace).toHaveBeenCalledTimes(3); + expect(templateService.replace).toHaveBeenNthCalledWith(3, '$groups', {}, 'text'); + expect(templateService.replace).toHaveBeenNthCalledWith(4, '$expressionVar', {}, undefined); + expect(templateService.replace).toHaveBeenCalledTimes(4); }); it('should replace correct variables in CloudWatchMetricsQuery', () => { diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts index ec7715dc4df..55d411585a3 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts @@ -10,7 +10,7 @@ import { LogRowModel, } from '@grafana/data'; -import { regionVariable } from '../mocks/CloudWatchDataSource'; +import { logGroupNamesVariable, regionVariable } from '../mocks/CloudWatchDataSource'; import { setupMockedLogsQueryRunner } from '../mocks/LogsQueryRunner'; import { LogsRequestMock } from '../mocks/Request'; import { validLogsQuery } from '../mocks/queries'; @@ -28,6 +28,63 @@ describe('CloudWatchLogsQueryRunner', () => { jest.clearAllMocks(); }); + describe('interpolateLogsQueryVariables', () => { + it('returns logGroups with arn and name values sourced from the log group template variable', () => { + const { runner } = setupMockedLogsQueryRunner({ variables: [logGroupNamesVariable] }); + + const query: CloudWatchLogsQuery = { + ...validLogsQuery, + logGroups: [{ arn: '$groups', name: '$groups' }], + }; + + const { logGroups } = runner.interpolateLogsQueryVariables(query, {}); + + expect(logGroups).toEqual([ + { arn: 'templatedGroup-arn-1', name: 'templatedGroup-1' }, + { arn: 'templatedGroup-arn-2', name: 'templatedGroup-2' }, + ]); + }); + + it('filters out duplicate log group arns when query already includes an expanded value', () => { + const { runner } = setupMockedLogsQueryRunner({ variables: [logGroupNamesVariable] }); + + const query: CloudWatchLogsQuery = { + ...validLogsQuery, + logGroups: [ + { arn: 'templatedGroup-arn-1', name: 'existing-group-name' }, + { arn: '$groups', name: '$groups' }, + ], + }; + + const { logGroups } = runner.interpolateLogsQueryVariables(query, {}); + + expect(logGroups).toEqual([ + { arn: 'templatedGroup-arn-1', name: 'existing-group-name' }, + { arn: 'templatedGroup-arn-2', name: 'templatedGroup-2' }, + ]); + }); + + it('keeps log groups with duplicate names as long as arns are unique', () => { + const { runner } = setupMockedLogsQueryRunner({ variables: [logGroupNamesVariable] }); + + const query: CloudWatchLogsQuery = { + ...validLogsQuery, + logGroups: [ + { arn: 'arn-1', name: 'templatedGroup-1' }, + { arn: '$groups', name: '$groups' }, + ], + }; + + const { logGroups } = runner.interpolateLogsQueryVariables(query, {}); + + expect(logGroups).toEqual([ + { arn: 'arn-1', name: 'templatedGroup-1' }, + { arn: 'templatedGroup-arn-1', name: 'templatedGroup-1' }, + { arn: 'templatedGroup-arn-2', name: 'templatedGroup-2' }, + ]); + }); + }); + describe('getLogRowContext', () => { it('replaces parameters correctly in the query', async () => { const { runner, queryMock } = setupMockedLogsQueryRunner({ variables: [regionVariable] }); diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts index 7f1ba927f47..cd8e8b29f1c 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts @@ -1,4 +1,4 @@ -import { set, uniq } from 'lodash'; +import { set, uniq, uniqBy } from 'lodash'; import { concatMap, finalize, @@ -253,9 +253,19 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { (query.logGroups || this.instanceSettings.jsonData.logGroups || []).map((lg) => lg.arn), scopedVars ); + const interpolatedLogGroupNames = interpolateStringArrayUsingSingleOrMultiValuedVariable( + this.templateSrv, + (query.logGroups || this.instanceSettings.jsonData.logGroups || []).map((lg) => lg.name), + scopedVars, + 'text' + ); + const interpolatedLogGroups = interpolatedLogGroupArns.map((arn, index) => ({ + arn, + name: interpolatedLogGroupNames[index] ?? arn, + })); // need to support legacy format variables too - const interpolatedLogGroupNames = interpolateStringArrayUsingSingleOrMultiValuedVariable( + const interpolatedLegacyLogGroupNames = interpolateStringArrayUsingSingleOrMultiValuedVariable( this.templateSrv, query.logGroupNames || this.instanceSettings.jsonData.defaultLogGroups || [], scopedVars, @@ -264,8 +274,8 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { // if a log group template variable expands to log group that has already been selected in the log group picker, we need to remove duplicates. // Otherwise the StartLogQuery API will return a permission error - const logGroups = uniq(interpolatedLogGroupArns).map((arn) => ({ arn, name: arn })); - const logGroupNames = uniq(interpolatedLogGroupNames); + const logGroups = uniqBy(interpolatedLogGroups, 'arn'); + const logGroupNames = uniq(interpolatedLegacyLogGroupNames); const logsSQLCustomerFormatter = (value: unknown, model: Partial) => { if (