CloudWatch/Logs: Add error message when log groups are not selected (#24361)

* Add error message

* Fix empty check

public/app/plugins/datasource/cloudwatch/datasource.test.ts

@@ -1,24 +1,39 @@
 import { CloudWatchDatasource } from './datasource';
 import { TemplateSrv } from '../../../features/templating/template_srv';
 import { setBackendSrv } from '@grafana/runtime';
-import { DefaultTimeRange } from '@grafana/data';
+import { DataQueryResponse, DefaultTimeRange } from '@grafana/data';
 
 describe('datasource', () => {
-  describe('describeLogGroup', () => {
-    it('replaces region correctly in the query', async () => {
-      const datasource = new CloudWatchDatasource(
-        { jsonData: { defaultRegion: 'us-west-1' } } as any,
-        new TemplateSrv(),
-        {
-          timeRange() {
-            return DefaultTimeRange;
+  describe('query', () => {
+    it('should return error if log query and log groups is not specified', async () => {
+      const { datasource } = setup();
+      const response: DataQueryResponse = (await datasource.query({
+        targets: [
+          {
+            queryMode: 'Logs' as 'Logs',
           },
-        } as any
-      );
-      const datasourceRequestMock = jest.fn();
-      datasourceRequestMock.mockResolvedValue({ data: [] });
-      setBackendSrv({ datasourceRequest: datasourceRequestMock } as any);
+        ],
+      } as any)) as any;
+      expect(response.error.message).toBe('Log group is required');
+    });
 
+    it('should return empty response if queries are hidden', async () => {
+      const { datasource } = setup();
+      const response: DataQueryResponse = (await datasource.query({
+        targets: [
+          {
+            queryMode: 'Logs' as 'Logs',
+            hide: true,
+          },
+        ],
+      } as any)) as any;
+      expect(response.data).toEqual([]);
+    });
+  });
+
+  describe('describeLogGroup', () => {
+    it('replaces region correctly in the query', async () => {
+      const { datasource, datasourceRequestMock } = setup();
       await datasource.describeLogGroups({ region: 'default' });
       expect(datasourceRequestMock.mock.calls[0][0].data.queries[0].region).toBe('us-west-1');
 
@@ -27,3 +42,16 @@ describe('datasource', () => {
     });
   });
 });
+
+function setup() {
+  const datasource = new CloudWatchDatasource({ jsonData: { defaultRegion: 'us-west-1' } } as any, new TemplateSrv(), {
+    timeRange() {
+      return DefaultTimeRange;
+    },
+  } as any);
+  const datasourceRequestMock = jest.fn();
+  datasourceRequestMock.mockResolvedValue({ data: [] });
+  setBackendSrv({ datasourceRequest: datasourceRequestMock } as any);
+
+  return { datasource, datasourceRequestMock };
+}

public/app/plugins/datasource/cloudwatch/datasource.ts

@@ -39,6 +39,7 @@ import {
   GetLogGroupFieldsResponse,
   LogAction,
   GetLogEventsRequest,
+  MetricQuery,
 } from './types';
 import { from, empty, Observable } from 'rxjs';
 import { delay, expand, map, mergeMap, tap, finalize, catchError } from 'rxjs/operators';
@@ -97,13 +98,27 @@ export class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery, CloudWa
     this.languageProvider = new CloudWatchLanguageProvider(this);
   }
 
-  query(options: DataQueryRequest<CloudWatchQuery>) {
+  query(options: DataQueryRequest<CloudWatchQuery>): Promise<DataQueryResponse> | Observable<DataQueryResponse> {
     options = angular.copy(options);
 
     const firstTarget = options.targets[0];
 
+    let queries = options.targets.filter(item => item.id !== '' || item.hide !== true);
+
     if (firstTarget.queryMode === 'Logs') {
-      const queryParams = options.targets.map((target: CloudWatchLogsQuery) => ({
+      const logQueries: CloudWatchLogsQuery[] = queries.filter(item => item.queryMode === 'Logs') as any;
+
+      const validLogQueries = logQueries.filter(item => item.logGroupNames?.length);
+      if (logQueries.length > validLogQueries.length) {
+        return Promise.resolve({ data: [], error: { message: 'Log group is required' } });
+      }
+
+      // No valid targets, return the empty result to save a round trip.
+      if (_.isEmpty(validLogQueries)) {
+        return Promise.resolve({ data: [] });
+      }
+
+      const queryParams = validLogQueries.map((target: CloudWatchLogsQuery) => ({
         queryString: target.expression,
         refId: target.refId,
         logGroupNames: target.logGroupNames,
@@ -124,57 +139,58 @@ export class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery, CloudWa
       );
     }
 
-    const queries = options.targets
+    const metricQueries: MetricQuery[] = options.targets
       .filter(
         item =>
-          (item.id !== '' || item.hide !== true) &&
           item.queryMode !== 'Logs' &&
           ((!!item.region && !!item.namespace && !!item.metricName && !_.isEmpty(item.statistics)) ||
             item.expression?.length > 0)
       )
-      .map((item: CloudWatchMetricsQuery) => {
-        item.region = this.replace(this.getActualRegion(item.region), options.scopedVars, true, 'region');
-        item.namespace = this.replace(item.namespace, options.scopedVars, true, 'namespace');
-        item.metricName = this.replace(item.metricName, options.scopedVars, true, 'metric name');
-        item.dimensions = this.convertDimensionFormat(item.dimensions, options.scopedVars);
-        item.statistics = item.statistics.map(stat => this.replace(stat, options.scopedVars, true, 'statistics'));
-        item.period = String(this.getPeriod(item, options)); // use string format for period in graph query, and alerting
-        item.id = this.templateSrv.replace(item.id, options.scopedVars);
-        item.expression = this.templateSrv.replace(item.expression, options.scopedVars);
-
-        // valid ExtendedStatistics is like p90.00, check the pattern
-        const hasInvalidStatistics = item.statistics.some(s => {
-          if (s.indexOf('p') === 0) {
-            const matches = /^p\d{2}(?:\.\d{1,2})?$/.exec(s);
-            return !matches || matches[0] !== s;
-          }
+      .map(
+        (item: CloudWatchMetricsQuery): MetricQuery => {
+          item.region = this.replace(this.getActualRegion(item.region), options.scopedVars, true, 'region');
+          item.namespace = this.replace(item.namespace, options.scopedVars, true, 'namespace');
+          item.metricName = this.replace(item.metricName, options.scopedVars, true, 'metric name');
+          item.dimensions = this.convertDimensionFormat(item.dimensions, options.scopedVars);
+          item.statistics = item.statistics.map(stat => this.replace(stat, options.scopedVars, true, 'statistics'));
+          item.period = String(this.getPeriod(item, options)); // use string format for period in graph query, and alerting
+          item.id = this.templateSrv.replace(item.id, options.scopedVars);
+          item.expression = this.templateSrv.replace(item.expression, options.scopedVars);
+
+          // valid ExtendedStatistics is like p90.00, check the pattern
+          const hasInvalidStatistics = item.statistics.some(s => {
+            if (s.indexOf('p') === 0) {
+              const matches = /^p\d{2}(?:\.\d{1,2})?$/.exec(s);
+              return !matches || matches[0] !== s;
+            }
 
-          return false;
-        });
+            return false;
+          });
 
-        if (hasInvalidStatistics) {
-          throw { message: 'Invalid extended statistics' };
-        }
+          if (hasInvalidStatistics) {
+            throw { message: 'Invalid extended statistics' };
+          }
 
-        return {
-          refId: item.refId,
-          intervalMs: options.intervalMs,
-          maxDataPoints: options.maxDataPoints,
-          datasourceId: this.id,
-          type: 'timeSeriesQuery',
-          ...item,
-        };
-      });
+          return {
+            refId: item.refId,
+            intervalMs: options.intervalMs,
+            maxDataPoints: options.maxDataPoints,
+            datasourceId: this.id,
+            type: 'timeSeriesQuery',
+            ...item,
+          };
+        }
+      );
 
     // No valid targets, return the empty result to save a round trip.
-    if (_.isEmpty(queries)) {
+    if (_.isEmpty(metricQueries)) {
       return Promise.resolve({ data: [] });
     }
 
     const request = {
       from: options?.range?.from.valueOf().toString(),
       to: options?.range?.to.valueOf().toString(),
-      queries: queries,
+      queries: metricQueries,
     };
 
     return this.performTimeSeriesQuery(request, options.range);

public/app/plugins/datasource/cloudwatch/types.ts

@@ -297,7 +297,7 @@ export interface MetricRequest {
   debug?: boolean;
 }
 
-interface MetricQuery {
+export interface MetricQuery {
   [key: string]: any;
   datasourceId: number;
   refId?: string;