From a30885c523e0ce4a05047c7939df69adc8c2ebbd Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 19 Oct 2022 08:39:18 +0200 Subject: [PATCH] CloudWatch: Make sure adoption tracking is done on valid, migrated queries (#56872) * make sure adoption tracking is done on valid, migrated queries * ignore hidden queries * fix test * remove obsolete test --- .../__mocks__/dashboardOnLoadedEvent.ts | 24 +++++++++++++++++ .../CloudWatchMetricsQueryRunner.test.ts | 4 --- .../CloudWatchMetricsQueryRunner.ts | 19 ++----------- .../datasource/cloudwatch/tracking.test.ts | 8 +++--- .../plugins/datasource/cloudwatch/tracking.ts | 27 ++++++++++++++++--- .../datasource/cloudwatch/utils/utils.ts | 20 ++++++++++++++ 6 files changed, 74 insertions(+), 28 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/dashboardOnLoadedEvent.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/dashboardOnLoadedEvent.ts index 07d87478f93..1161f75b6e5 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/dashboardOnLoadedEvent.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/dashboardOnLoadedEvent.ts @@ -710,6 +710,30 @@ export const CloudWatchDashboardLoadedEvent = new DashboardLoadedEvent({ sqlExpression: '', statistic: 'Average', }, + { + alias: '', + datasource: { + type: 'cloudwatch', + uid: 'abc', + }, + dimensions: { + InstanceId: '*', + }, + expression: 'a / ', + hide: false, + id: '', + matchExact: true, + metricEditorMode: 0, + metricName: 'CPUUtilization', + metricQueryType: 0, + namespace: 'AWS/EC2', + period: '', + queryMode: '', + refId: 'B', + region: 'default', + sqlExpression: '', + statistic: '', + }, ] as CloudWatchQuery[], }, }); diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts index 2393d200805..343f4be4593 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts @@ -726,10 +726,6 @@ describe('CloudWatchMetricsQueryRunner', () => { }; }); - it('should error if invalid mode', async () => { - expect(() => runner.filterMetricQuery(baseQuery)).toThrowError('invalid metric editor mode'); - }); - describe('metric search queries', () => { beforeEach(() => { baseQuery = { diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts index 79f23bf3148..85f30e1c954 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts @@ -28,11 +28,10 @@ import { CloudWatchMetricsQuery, CloudWatchQuery, DataQueryError, - MetricEditorMode, MetricQuery, - MetricQueryType, MetricRequest, } from '../types'; +import { filterMetricsQuery } from '../utils/utils'; import { CloudWatchRequest } from './CloudWatchRequest'; @@ -172,21 +171,7 @@ export class CloudWatchMetricsQueryRunner extends CloudWatchRequest { } filterMetricQuery(query: CloudWatchMetricsQuery): boolean { - const { region, metricQueryType, metricEditorMode, expression, metricName, namespace, sqlExpression, statistic } = - query; - if (!region) { - return false; - } - if (metricQueryType === MetricQueryType.Search && metricEditorMode === MetricEditorMode.Builder) { - return !!namespace && !!metricName && !!statistic; - } else if (metricQueryType === MetricQueryType.Search && metricEditorMode === MetricEditorMode.Code) { - return !!expression; - } else if (metricQueryType === MetricQueryType.Query) { - // still TBD how to validate the visual query builder for SQL - return !!sqlExpression; - } - - throw new Error('invalid metric editor mode'); + return filterMetricsQuery(query); } replaceMetricQueryVars( diff --git a/public/app/plugins/datasource/cloudwatch/tracking.test.ts b/public/app/plugins/datasource/cloudwatch/tracking.test.ts index 1807eb5cf5a..4827233b79a 100644 --- a/public/app/plugins/datasource/cloudwatch/tracking.test.ts +++ b/public/app/plugins/datasource/cloudwatch/tracking.test.ts @@ -26,14 +26,14 @@ describe('onDashboardLoadedHandler', () => { grafana_version: 'v9.0.0', org_id: 1, logs_queries_count: 1, - metrics_queries_count: 21, + metrics_queries_count: 20, metrics_query_builder_count: 3, metrics_query_code_count: 4, metrics_query_count: 7, - metrics_search_builder_count: 9, + metrics_search_builder_count: 8, metrics_search_code_count: 5, - metrics_search_count: 14, - metrics_search_match_exact_count: 9, + metrics_search_count: 13, + metrics_search_match_exact_count: 8, }); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/tracking.ts b/public/app/plugins/datasource/cloudwatch/tracking.ts index 6f5d7e46899..4be3d49d846 100644 --- a/public/app/plugins/datasource/cloudwatch/tracking.ts +++ b/public/app/plugins/datasource/cloudwatch/tracking.ts @@ -2,8 +2,16 @@ import { DashboardLoadedEvent } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; import { isCloudWatchLogsQuery, isCloudWatchMetricsQuery } from './guards'; +import { migrateMetricQuery } from './migrations/metricQueryMigrations'; import pluginJson from './plugin.json'; -import { CloudWatchMetricsQuery, CloudWatchQuery, MetricEditorMode, MetricQueryType } from './types'; +import { + CloudWatchLogsQuery, + CloudWatchMetricsQuery, + CloudWatchQuery, + MetricEditorMode, + MetricQueryType, +} from './types'; +import { filterMetricsQuery } from './utils/utils'; interface CloudWatchOnDashboardLoadedTrackingEvent { grafana_version?: string; @@ -56,8 +64,21 @@ export const onDashboardLoadedHandler = ({ return; } - const logsQueries = cloudWatchQueries.filter(isCloudWatchLogsQuery); - const metricsQueries = cloudWatchQueries.filter(isCloudWatchMetricsQuery); + let logsQueries: CloudWatchLogsQuery[] = []; + let metricsQueries: CloudWatchMetricsQuery[] = []; + + for (const query of cloudWatchQueries) { + if (query.hide) { + continue; + } + + if (isCloudWatchLogsQuery(query)) { + query.logGroupNames?.length && logsQueries.push(query); + } else if (isCloudWatchMetricsQuery(query)) { + const migratedQuery = migrateMetricQuery(query); + filterMetricsQuery(migratedQuery) && metricsQueries.push(query); + } + } const e: CloudWatchOnDashboardLoadedTrackingEvent = { grafana_version: grafanaVersion, diff --git a/public/app/plugins/datasource/cloudwatch/utils/utils.ts b/public/app/plugins/datasource/cloudwatch/utils/utils.ts index d0f626cb2a1..1666c8a3865 100644 --- a/public/app/plugins/datasource/cloudwatch/utils/utils.ts +++ b/public/app/plugins/datasource/cloudwatch/utils/utils.ts @@ -1,5 +1,7 @@ import { SelectableValue } from '@grafana/data'; +import { CloudWatchMetricsQuery, MetricQueryType, MetricEditorMode } from '../types'; + import { CloudWatchDatasource } from './../datasource'; export const toOption = (value: string) => ({ label: value, value }); @@ -8,3 +10,21 @@ export const appendTemplateVariables = (datasource: CloudWatchDatasource, values ...values, { label: 'Template Variables', options: datasource.getVariables().map(toOption) }, ]; + +export const filterMetricsQuery = (query: CloudWatchMetricsQuery): boolean => { + const { region, metricQueryType, metricEditorMode, expression, metricName, namespace, sqlExpression, statistic } = + query; + if (!region) { + return false; + } + if (metricQueryType === MetricQueryType.Search && metricEditorMode === MetricEditorMode.Builder) { + return !!namespace && !!metricName && !!statistic; + } else if (metricQueryType === MetricQueryType.Search && metricEditorMode === MetricEditorMode.Code) { + return !!expression; + } else if (metricQueryType === MetricQueryType.Query) { + // still TBD how to validate the visual query builder for SQL + return !!sqlExpression; + } + + return false; +};