public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
* Set the log group name when executing log queries from the frontend

* Add helper for a data source instance to check if its a monitoring account

* Execute log queries with log group identifiers only for monitoring account queries

* fix cloudwatch datasource.ts tests

* remove unneeded check
This commit is contained in:
Kevin Yu
2025-11-06 12:19:39 -08:00
committed by GitHub
parent 2ac4d0a13e
commit 69060f5437
6 changed files with 240 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/tsdb/cloudwatch/cloudwatch.go
+32 -3
View File
@@ -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,
pkg/tsdb/cloudwatch/log_actions.go
+41 -10
View File
@@ -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
}
}
pkg/tsdb/cloudwatch/log_actions_test.go
+90 -10
View File
@@ -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)
})
public/app/plugins/datasource/cloudwatch/datasource.test.ts
+5 -4
View File
@@ -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', () => {
public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts
+58 -1
View File
@@ -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] });
public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts
+14 -4
View File
@@ -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<CustomFormatterVariable>) => {
if (