From fcbe0059c2b173a0c95b33545493026dfa760a22 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 17 Jun 2022 10:24:38 +0200 Subject: [PATCH] CloudWatch: Allow hidden queries to be executed in case an ID is provided (#50987) * allow hidden queries in case an id is provided * cleanup test * name tests properly --- .../cloudwatch/__mocks__/queries.ts | 28 +++ .../datasource/cloudwatch/datasource.test.ts | 191 +++++++++--------- .../datasource/cloudwatch/datasource.ts | 41 ++-- 3 files changed, 144 insertions(+), 116 deletions(-) create mode 100644 public/app/plugins/datasource/cloudwatch/__mocks__/queries.ts diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/queries.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/queries.ts new file mode 100644 index 00000000000..ebd054e6610 --- /dev/null +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/queries.ts @@ -0,0 +1,28 @@ +import { CloudWatchMetricsQuery, MetricQueryType, MetricEditorMode, CloudWatchLogsQuery } from '../types'; + +export const validMetricsQuery: CloudWatchMetricsQuery = { + id: '', + queryMode: 'Metrics', + region: 'us-east-2', + namespace: 'AWS/EC2', + period: '3000', + alias: '', + metricName: 'CPUUtilization', + dimensions: { InstanceId: 'i-123' }, + matchExact: true, + statistic: 'Average', + expression: '', + refId: 'A', + metricQueryType: MetricQueryType.Search, + metricEditorMode: MetricEditorMode.Code, + hide: false, +}; + +export const validLogsQuery: CloudWatchLogsQuery = { + queryMode: 'Logs', + logGroupNames: ['group-A', 'group-B'], + hide: false, + id: '', + region: 'us-east-2', + refId: 'A', +}; diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index 494a54492c6..20940a4ffad 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts @@ -15,10 +15,11 @@ import { setupMockedDataSource, regionVariable, } from './__mocks__/CloudWatchDataSource'; +import { validLogsQuery, validMetricsQuery } from './__mocks__/queries'; import { - CloudWatchLogsQuery, CloudWatchLogsQueryStatus, CloudWatchMetricsQuery, + CloudWatchQuery, MetricEditorMode, MetricQueryType, } from './types'; @@ -45,6 +46,19 @@ describe('datasource', () => { }); }); + const testTable: Array<{ query: CloudWatchQuery; valid: boolean }> = [ + { query: { ...validLogsQuery, hide: true }, valid: false }, + { query: { ...validLogsQuery, hide: false }, valid: true }, + { query: { ...validMetricsQuery, hide: true }, valid: false }, + { query: { ...validMetricsQuery, hide: true, id: 'queryA' }, valid: true }, + { query: { ...validMetricsQuery, hide: false }, valid: true }, + ]; + + test.each(testTable)('should filter out hidden queries unless id is provided', ({ query, valid }) => { + const { datasource } = setupMockedDataSource(); + expect(datasource.filterQuery(query)).toEqual(valid); + }); + it('should interpolate variables in the query', async () => { const { datasource, fetchMock } = setupMockedDataSource(); await lastValueFrom( @@ -180,120 +194,107 @@ describe('datasource', () => { }); }); - describe('filterQuery', () => { + describe('filterMetricsQuery', () => { const datasource = setupMockedDataSource().datasource; - describe('CloudWatchLogsQuery', () => { - const baseQuery: CloudWatchLogsQuery = { - queryMode: 'Logs', + let baseQuery: CloudWatchMetricsQuery; + beforeEach(() => { + baseQuery = { id: '', - region: '', + region: 'us-east-2', + namespace: '', + period: '', + alias: '', + metricName: '', + dimensions: {}, + matchExact: true, + statistic: '', + expression: '', refId: '', - logGroupNames: ['foo', 'bar'], }; - it('should return false if empty logGroupNames', () => { - expect(datasource.filterQuery({ ...baseQuery, logGroupNames: undefined })).toBeFalsy(); - }); - it('should return true if has logGroupNames', () => { - expect(datasource.filterQuery(baseQuery)).toBeTruthy(); - }); }); - describe('CloudWatchMetricsQuery', () => { - let baseQuery: CloudWatchMetricsQuery; + + it('should error if invalid mode', async () => { + expect(() => datasource.filterMetricQuery(baseQuery)).toThrowError('invalid metric editor mode'); + }); + + describe('metric search queries', () => { beforeEach(() => { baseQuery = { - id: '', - region: 'us-east-2', - namespace: '', - period: '', - alias: '', - metricName: '', - dimensions: {}, - matchExact: true, - statistic: '', - expression: '', - refId: '', + ...baseQuery, + namespace: 'AWS/EC2', + metricName: 'CPUUtilization', + statistic: 'Average', + metricQueryType: MetricQueryType.Search, + metricEditorMode: MetricEditorMode.Builder, }; }); - it('should error if invalid mode', async () => { - expect(() => datasource.filterQuery(baseQuery)).toThrowError('invalid metric editor mode'); + it('should not allow builder queries that dont have namespace, metric or statistic', async () => { + expect(datasource.filterMetricQuery({ ...baseQuery, statistic: undefined })).toBeFalsy(); + expect(datasource.filterMetricQuery({ ...baseQuery, metricName: undefined })).toBeFalsy(); + expect(datasource.filterMetricQuery({ ...baseQuery, namespace: '' })).toBeFalsy(); }); - describe('metric search queries', () => { - beforeEach(() => { - baseQuery = { - ...baseQuery, - namespace: 'AWS/EC2', - metricName: 'CPUUtilization', - statistic: 'Average', - metricQueryType: MetricQueryType.Search, - metricEditorMode: MetricEditorMode.Builder, - }; - }); - - it('should not allow builder queries that dont have namespace, metric or statistic', async () => { - expect(datasource.filterQuery({ ...baseQuery, statistic: undefined })).toBeFalsy(); - expect(datasource.filterQuery({ ...baseQuery, metricName: undefined })).toBeFalsy(); - expect(datasource.filterQuery({ ...baseQuery, namespace: '' })).toBeFalsy(); - }); - - it('should allow builder queries that have namespace, metric or statistic', async () => { - expect(datasource.filterQuery(baseQuery)).toBeTruthy(); - }); - - it('should not allow code queries that dont have an expression', async () => { - expect( - datasource.filterQuery({ ...baseQuery, expression: undefined, metricEditorMode: MetricEditorMode.Code }) - ).toBeFalsy(); - }); - - it('should allow code queries that have an expression', async () => { - expect( - datasource.filterQuery({ ...baseQuery, expression: 'x * 2', metricEditorMode: MetricEditorMode.Code }) - ).toBeTruthy(); - }); + it('should allow builder queries that have namespace, metric or statistic', async () => { + expect(datasource.filterMetricQuery(baseQuery)).toBeTruthy(); }); - describe('metric search expression queries', () => { - beforeEach(() => { - baseQuery = { + it('should not allow code queries that dont have an expression', async () => { + expect( + datasource.filterMetricQuery({ ...baseQuery, - metricQueryType: MetricQueryType.Search, + expression: undefined, metricEditorMode: MetricEditorMode.Code, - }; - }); - - it('should not allow queries that dont have an expression', async () => { - const valid = datasource.filterQuery(baseQuery); - expect(valid).toBeFalsy(); - }); - - it('should allow queries that have an expression', async () => { - baseQuery.expression = 'SUM([a,x])'; - const valid = datasource.filterQuery(baseQuery); - expect(valid).toBeTruthy(); - }); + }) + ).toBeFalsy(); }); - describe('metric query queries', () => { - beforeEach(() => { - baseQuery = { - ...baseQuery, - metricQueryType: MetricQueryType.Query, - metricEditorMode: MetricEditorMode.Code, - }; - }); + it('should allow code queries that have an expression', async () => { + expect( + datasource.filterMetricQuery({ ...baseQuery, expression: 'x * 2', metricEditorMode: MetricEditorMode.Code }) + ).toBeTruthy(); + }); + }); - it('should not allow queries that dont have a sql expresssion', async () => { - const valid = datasource.filterQuery(baseQuery); - expect(valid).toBeFalsy(); - }); + describe('metric search expression queries', () => { + beforeEach(() => { + baseQuery = { + ...baseQuery, + metricQueryType: MetricQueryType.Search, + metricEditorMode: MetricEditorMode.Code, + }; + }); - it('should allow queries that have a sql expresssion', async () => { - baseQuery.sqlExpression = 'select SUM(CPUUtilization) from "AWS/EC2"'; - const valid = datasource.filterQuery(baseQuery); - expect(valid).toBeTruthy(); - }); + it('should not allow queries that dont have an expression', async () => { + const valid = datasource.filterMetricQuery(baseQuery); + expect(valid).toBeFalsy(); + }); + + it('should allow queries that have an expression', async () => { + baseQuery.expression = 'SUM([a,x])'; + const valid = datasource.filterMetricQuery(baseQuery); + expect(valid).toBeTruthy(); + }); + }); + + describe('metric query queries', () => { + beforeEach(() => { + baseQuery = { + ...baseQuery, + metricQueryType: MetricQueryType.Query, + metricEditorMode: MetricEditorMode.Code, + }; + }); + + it('should not allow queries that dont have a sql expresssion', async () => { + const valid = datasource.filterMetricQuery(baseQuery); + expect(valid).toBeFalsy(); + }); + + it('should allow queries that have a sql expresssion', async () => { + baseQuery.sqlExpression = 'select SUM(CPUUtilization) from "AWS/EC2"'; + const valid = datasource.filterMetricQuery(baseQuery); + expect(valid).toBeTruthy(); }); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts index 0c80835037a..f227bf70d82 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.ts @@ -133,10 +133,14 @@ export class CloudWatchDatasource this.annotations = CloudWatchAnnotationSupport; } + filterQuery(query: CloudWatchQuery) { + return query.hide !== true || (isCloudWatchMetricsQuery(query) && query.id !== ''); + } + query(options: DataQueryRequest): Observable { options = cloneDeep(options); - let queries = options.targets.filter((item) => item.hide !== true); + let queries = options.targets.filter(this.filterQuery); const { logQueries, metricsQueries, annotationQueries } = this.getTargetsByQueryMode(queries); const dataQueryResponses: Array> = []; @@ -245,14 +249,7 @@ export class CloudWatchDatasource ); }; - filterQuery(query: CloudWatchQuery): boolean { - if (isCloudWatchLogsQuery(query)) { - return !!query.logGroupNames?.length; - } else if (isCloudWatchAnnotationQuery(query)) { - // annotation query validity already checked in annotationSupport - return true; - } - + filterMetricQuery(query: CloudWatchMetricsQuery): boolean { const { region, metricQueryType, metricEditorMode, expression, metricName, namespace, sqlExpression, statistic } = query; if (!region) { @@ -296,19 +293,21 @@ export class CloudWatchDatasource format: 'Z', }).replace(':', ''); - const validMetricsQueries = metricQueries.filter(this.filterQuery).map((q: CloudWatchMetricsQuery): MetricQuery => { - const migratedQuery = migrateMetricQuery(q); - const migratedAndIterpolatedQuery = this.replaceMetricQueryVars(migratedQuery, options); + const validMetricsQueries = metricQueries + .filter(this.filterMetricQuery) + .map((q: CloudWatchMetricsQuery): MetricQuery => { + const migratedQuery = migrateMetricQuery(q); + const migratedAndIterpolatedQuery = this.replaceMetricQueryVars(migratedQuery, options); - return { - timezoneUTCOffset, - intervalMs: options.intervalMs, - maxDataPoints: options.maxDataPoints, - ...migratedAndIterpolatedQuery, - type: 'timeSeriesQuery', - datasource: this.getRef(), - }; - }); + return { + timezoneUTCOffset, + intervalMs: options.intervalMs, + maxDataPoints: options.maxDataPoints, + ...migratedAndIterpolatedQuery, + type: 'timeSeriesQuery', + datasource: this.getRef(), + }; + }); // No valid targets, return the empty result to save a round trip. if (isEmpty(validMetricsQueries)) {