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
2025-02-25 18:55:37 -06:00 · 2022-06-17 10:24:38 +02:00 · 2022-06-17 10:24:38 +02:00 · fcbe0059c2
commit fcbe0059c2
parent 1f0c951bce
3 changed files with 144 additions and 116 deletions
--- a/public/app/plugins/datasource/cloudwatch/mocks/queries.ts
+++ 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',
+};
--- 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();
      });
    });
  });
--- 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<CloudWatchQuery>): Observable<DataQueryResponse> {
    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<Observable<DataQueryResponse>> = [];
@ -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)) {