From d385045d16cae1905f5f46c47e853dbd6b3aac45 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Mon, 11 May 2020 12:53:42 +0200 Subject: [PATCH] CloudWatch/Logs: Add error message when log groups are not selected (#24361) * Add error message * Fix empty check --- .../datasource/cloudwatch/datasource.test.ts | 56 +++++++++--- .../datasource/cloudwatch/datasource.ts | 86 +++++++++++-------- .../plugins/datasource/cloudwatch/types.ts | 2 +- 3 files changed, 94 insertions(+), 50 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index fc5e8bbc8f5..f018d500857 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/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('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)) 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 = 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); - + 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 }; +} diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts index aa00c133f65..dc310b97737 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.ts +++ b/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) { + query(options: DataQueryRequest): Promise | Observable { 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 - (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); + .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; + // 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; + }); + + if (hasInvalidStatistics) { + throw { message: 'Invalid extended statistics' }; } - return false; - }); - - 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); diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index 673bb6e0374..3016917cf91 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/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;