CloudWatch: return silently in case of missing log groups or incomplete query (#59701)

* Remove defaultLogGroups check in filterQuery

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',
       });
     });
 

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<CloudWatchQuery>
-        )
-      );
-
-      expect(response).toEqual({
-        data: [],
-        error: {
-          message: 'Query is required',
-        },
-      });
-    });
-  });
 });

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<CloudWatchQuery>
   ): Observable<DataQueryResponse> => {
-    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<T = DataQueryResponse>(observable: Observable<T>, onUnsubscribe: () => void): Observable<T> {

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.
    */