CloudWatch Logs: Limit CloudWatch logs queries to use logGroupIdentifiers only for monitoring accounts (#273)

kevinwcyu · web-flow · commit 56ab6e3f2658 · 2025-11-07T17:59:16.000Z
diff --git a/pkg/cloudwatch/cloudwatch.go b/pkg/cloudwatch/cloudwatch.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"slices"
+	"sync"
 	"time"
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend/resource/httpadapter"
@@ -55,9 +56,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) {
@@ -274,6 +276,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/cloudwatch/log_actions.go b/pkg/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/cloudwatch/log_actions_test.go b/pkg/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/src/datasource.test.ts b/src/datasource.test.ts
@@ -270,8 +270,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',
@@ -401,8 +401,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/src/query-runner/CloudWatchLogsQueryRunner.test.ts b/src/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/src/query-runner/CloudWatchLogsQueryRunner.ts b/src/query-runner/CloudWatchLogsQueryRunner.ts
@@ -1,4 +1,4 @@
-import { set, uniq } from 'lodash';
+import { set, uniq, uniqBy } from 'lodash';
 import {
   concatMap,
   finalize,
@@ -254,9 +254,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,
       // eslint-disable-next-line @typescript-eslint/no-deprecated
       query.logGroupNames || this.instanceSettings.jsonData.defaultLogGroups || [],
@@ -266,8 +276,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<CustomFormatterVariable>) => {
       if (
