From e8ed79873a8ee6a072a398fda6467ce4c10d76ad Mon Sep 17 00:00:00 2001 From: Shirley <4163034+fridgepoet@users.noreply.github.com> Date: Wed, 7 Dec 2022 16:37:35 +0100 Subject: [PATCH] CloudWatch: return silently in case of missing log groups or incomplete query (#59701) * Remove defaultLogGroups check in filterQuery --- .../datasource/cloudwatch/datasource.test.ts | 92 +++++++++++++++---- .../CloudWatchLogsQueryRunner.test.ts | 40 +------- .../query-runner/CloudWatchLogsQueryRunner.ts | 37 ++++---- .../plugins/datasource/cloudwatch/types.ts | 3 +- 4 files changed, 96 insertions(+), 76 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index e15fb3da10d..42e1f6cdf62 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts @@ -19,23 +19,83 @@ describe('datasource', () => { jest.clearAllMocks(); }); describe('query', () => { - it('should return error if log query and log groups is not specified', async () => { - const { datasource } = setupMockedDataSource(); - const observable = datasource.query({ - targets: [{ queryMode: 'Logs', id: '', refId: '', region: '' }], - requestId: '', - interval: '', - intervalMs: 0, - range: timeRange, - scopedVars: {}, - timezone: '', - app: '', - startTime: 0, - }); + it('should not run a query if log groups is not specified', async () => { + const { datasource, fetchMock } = setupMockedDataSource(); + await lastValueFrom( + datasource.query({ + targets: [ + { + queryMode: 'Logs', + id: '', + refId: '', + region: '', + expression: 'some query string', // missing logGroups and logGroupNames, this query will be not be run + }, + { + queryMode: 'Logs', + id: '', + refId: '', + region: '', + logGroupNames: ['/some/group'], + expression: 'some query string', + }, + ], + requestId: '', + interval: '', + intervalMs: 0, + range: timeRange, + scopedVars: {}, + timezone: '', + app: '', + startTime: 0, + }) + ); - await expect(observable).toEmitValuesWith((received) => { - const response = received[0]; - expect(response.error?.message).toBe('Log group is required'); + expect(fetchMock.mock.calls[0][0].data.queries).toHaveLength(1); + expect(fetchMock.mock.calls[0][0].data.queries[0]).toMatchObject({ + queryString: 'some query string', + logGroupNames: ['/some/group'], + region: 'us-west-1', + }); + }); + + it('should not run a query if query expression is not specified', async () => { + const { datasource, fetchMock } = setupMockedDataSource(); + await lastValueFrom( + datasource.query({ + targets: [ + { + queryMode: 'Logs', + id: '', + refId: '', + region: '', + logGroupNames: ['/some/group'], // missing query expression, this query will be not be run + }, + { + queryMode: 'Logs', + id: '', + refId: '', + region: '', + logGroupNames: ['/some/group'], + expression: 'some query string', + }, + ], + requestId: '', + interval: '', + intervalMs: 0, + range: timeRange, + scopedVars: {}, + timezone: '', + app: '', + startTime: 0, + }) + ); + + expect(fetchMock.mock.calls[0][0].data.queries).toHaveLength(1); + expect(fetchMock.mock.calls[0][0].data.queries[0]).toMatchObject({ + queryString: 'some query string', + logGroupNames: ['/some/group'], + region: 'us-west-1', }); }); 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 73660dd5a08..80c8e3d59e7 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts @@ -1,18 +1,10 @@ import { interval, lastValueFrom, of } from 'rxjs'; -import { - dataFrameToJSON, - DataQueryErrorType, - DataQueryRequest, - FieldType, - LogLevel, - LogRowModel, - MutableDataFrame, -} from '@grafana/data'; +import { dataFrameToJSON, DataQueryErrorType, FieldType, LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data'; import { genMockFrames, setupMockedLogsQueryRunner } from '../__mocks__/LogsQueryRunner'; import { validLogsQuery } from '../__mocks__/queries'; -import { CloudWatchQuery, LogAction } from '../types'; +import { LogAction } from '../types'; import * as rxjsUtils from '../utils/rxjs/increasingInterval'; import { LOG_IDENTIFIER_INTERNAL, LOGSTREAM_IDENTIFIER_INTERNAL } from './CloudWatchLogsQueryRunner'; @@ -221,32 +213,4 @@ describe('CloudWatchLogsQueryRunner', () => { expect(i).toBe(3); }); }); - - describe('handleLogQueries', () => { - it('should return error message when missing query string', async () => { - const { runner } = setupMockedLogsQueryRunner(); - const response = await lastValueFrom( - runner.handleLogQueries( - [ - { - datasource: { type: 'cloudwatch', uid: 'Zne6OZIVk' }, - id: '', - logGroups: [{ label: '', text: '', value: '' }], - queryMode: 'Logs', - refId: 'A', - region: 'default', - }, - ], - { scopedVars: {} } as DataQueryRequest - ) - ); - - expect(response).toEqual({ - data: [], - error: { - message: 'Query is required', - }, - }); - }); - }); }); diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts index 254f2e32ac2..2516cf85b73 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts @@ -8,7 +8,6 @@ import { map, mergeMap, Observable, - of, repeat, scan, share, @@ -85,7 +84,9 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { logQueries: CloudWatchLogsQuery[], options: DataQueryRequest ): Observable => { - const queryParams = logQueries.map((target: CloudWatchLogsQuery) => ({ + const validLogQueries = logQueries.filter(this.filterQuery); + + const startQueryRequests: StartQueryRequest[] = validLogQueries.map((target: CloudWatchLogsQuery) => ({ queryString: target.expression || '', refId: target.refId, logGroupNames: target.logGroupNames || this.defaultLogGroups, @@ -98,24 +99,6 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { ), })); - const hasQueryWithMissingLogGroupSelection = queryParams.some((qp) => { - const missingLogGroupNames = qp.logGroupNames.length === 0; - const missingLogGroups = qp.logGroups.length === 0; - return missingLogGroupNames && missingLogGroups; - }); - - if (hasQueryWithMissingLogGroupSelection) { - return of({ data: [], error: { message: 'Log group is required' } }); - } - - const hasQueryWithMissingQueryString = queryParams.some((qp) => { - return qp.queryString.length === 0; - }); - - if (hasQueryWithMissingQueryString) { - return of({ data: [], error: { message: 'Query is required' } }); - } - const startTime = new Date(); const timeoutFunc = () => { return Date.now() >= startTime.valueOf() + rangeUtil.intervalToMs(this.logsTimeout); @@ -129,7 +112,7 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { skipCache: true, }); }, - queryParams, + startQueryRequests, timeoutFunc ).pipe( mergeMap(({ frames, error }: { frames: DataFrame[]; error?: DataQueryError }) => @@ -445,6 +428,18 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { return getLogGroupFieldsResponse; } + + private filterQuery(query: CloudWatchLogsQuery) { + const hasMissingLegacyLogGroupNames = !query.logGroupNames?.length; + const hasMissingLogGroups = !query.logGroups?.length; + const hasMissingQueryString = !query.expression?.length; + + if ((hasMissingLogGroups && hasMissingLegacyLogGroupNames) || hasMissingQueryString) { + return false; + } + + return true; + } } function withTeardown(observable: Observable, onUnsubscribe: () => void): Observable { diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index d656913493d..3f17b651ead 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -331,7 +331,8 @@ export interface StartQueryRequest { /** * The list of log groups to be queried. You can include up to 20 log groups. A StartQuery operation must include a logGroupNames or a logGroupName parameter, but not both. */ - logGroupNames?: string[]; + logGroupNames?: string[] /* not quite deprecated yet, but will be soon */; + logGroups?: SelectableResourceValue[]; /** * The query string to use. For more information, see CloudWatch Logs Insights Query Syntax. */