From 237ff2699deb43245eac4c941b84eb819baa10bc Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 26 Oct 2022 15:59:26 +0200 Subject: [PATCH] CloudWatch: Refactor metrics resource request frontend (#57602) * refactor metric resource request frontend * remove dupe interface --- .../datasource/cloudwatch/__mocks__/API.ts | 4 +- .../__mocks__/CloudWatchDataSource.ts | 1 + .../plugins/datasource/cloudwatch/api.test.ts | 50 +++++++++++++++++-- .../app/plugins/datasource/cloudwatch/api.ts | 11 ++-- .../completion/CompletionItemProvider.ts | 12 ++--- .../MetricStatEditor.test.tsx | 16 +++--- .../MetricStatEditor/MetricStatEditor.tsx | 2 +- .../SQLBuilderEditor/SQLBuilderSelectRow.tsx | 4 +- .../VariableQueryEditor.test.tsx | 2 +- .../VariableQueryEditor.tsx | 2 +- .../datasource/cloudwatch/datasource.test.ts | 2 +- .../plugins/datasource/cloudwatch/hooks.ts | 2 +- .../plugins/datasource/cloudwatch/types.ts | 10 ++-- .../datasource/cloudwatch/variables.ts | 2 +- 14 files changed, 81 insertions(+), 39 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/API.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/API.ts index 719bbc97166..662fc010006 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/API.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/API.ts @@ -12,7 +12,7 @@ export function setupMockedAPI({ response, getMock, }: { - getMock?: jest.Func; + getMock?: jest.Mock; response?: Array<{ text: string; label: string; value: string }>; variables?: CustomVariableModel[]; mockGetVariableName?: boolean; @@ -21,7 +21,7 @@ export function setupMockedAPI({ const timeSrv = getTimeSrv(); const api = new CloudWatchAPI(CloudWatchSettings, templateService); - const resourceRequestMock = jest.fn().mockReturnValue(response); + let resourceRequestMock = getMock ? getMock : jest.fn().mockReturnValue(response); setBackendSrv({ ...getBackendSrv(), get: resourceRequestMock, diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts index 734c61690c7..dcc5c5364ef 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts @@ -82,6 +82,7 @@ export function setupMockedDataSource({ datasource.api.getNamespaces = jest.fn().mockResolvedValue([]); datasource.api.getRegions = jest.fn().mockResolvedValue([]); datasource.api.getDimensionKeys = jest.fn().mockResolvedValue([]); + datasource.api.getMetrics = jest.fn().mockResolvedValue([]); datasource.logsQueryRunner.defaultLogGroups = []; const fetchMock = jest.fn().mockReturnValue(of({})); setBackendSrv({ diff --git a/public/app/plugins/datasource/cloudwatch/api.test.ts b/public/app/plugins/datasource/cloudwatch/api.test.ts index 1088ddee9c4..6199876b77e 100644 --- a/public/app/plugins/datasource/cloudwatch/api.test.ts +++ b/public/app/plugins/datasource/cloudwatch/api.test.ts @@ -61,13 +61,53 @@ describe('api', () => { const { api, resourceRequestMock } = setupMockedAPI({ getMock }); resourceRequestMock.mockResolvedValue([]); await Promise.all([ - api.getMetrics('AWS/EC2', 'us-east-1'), - api.getMetrics('AWS/EC2', 'us-east-1'), - api.getMetrics('AWS/EC2', 'us-east-2'), - api.getMetrics('AWS/EC2', 'us-east-2'), - api.getMetrics('AWS/EC2', 'us-east-2'), + api.getMetrics({ namespace: 'AWS/EC2', region: 'us-east-1' }), + api.getMetrics({ namespace: 'AWS/EC2', region: 'us-east-1' }), + api.getMetrics({ namespace: 'AWS/EC2', region: 'us-east-2' }), + api.getMetrics({ namespace: 'AWS/EC2', region: 'us-east-2' }), + api.getMetrics({ namespace: 'AWS/EC2', region: 'us-east-2' }), ]); expect(resourceRequestMock).toHaveBeenCalledTimes(2); }); }); + + describe('should handle backend srv response mapping', () => { + it('when getAllMetrics is called', async () => { + const getMock = jest.fn().mockResolvedValue([ + { + namespace: 'AWS/EC2', + name: 'CPUUtilization', + }, + { + namespace: 'AWS/Redshift', + name: 'CPUPercentage', + }, + ]); + const { api } = setupMockedAPI({ getMock }); + const allMetrics = await api.getAllMetrics({ region: 'us-east-2' }); + expect(allMetrics).toEqual([ + { metricName: 'CPUUtilization', namespace: 'AWS/EC2' }, + { metricName: 'CPUPercentage', namespace: 'AWS/Redshift' }, + ]); + }); + + it('when getMetrics', async () => { + const getMock = jest.fn().mockResolvedValue([ + { + namespace: 'AWS/EC2', + name: 'CPUUtilization', + }, + { + namespace: 'AWS/EC2', + name: 'CPUPercentage', + }, + ]); + const { api } = setupMockedAPI({ getMock }); + const allMetrics = await api.getMetrics({ region: 'us-east-2', namespace: 'AWS/EC2' }); + expect(allMetrics).toEqual([ + { label: 'CPUUtilization', value: 'CPUUtilization' }, + { label: 'CPUPercentage', value: 'CPUPercentage' }, + ]); + }); + }); }); diff --git a/public/app/plugins/datasource/cloudwatch/api.ts b/public/app/plugins/datasource/cloudwatch/api.ts index 6616483ea44..a6342b11499 100644 --- a/public/app/plugins/datasource/cloudwatch/api.ts +++ b/public/app/plugins/datasource/cloudwatch/api.ts @@ -10,6 +10,7 @@ import { DescribeLogGroupsRequest, GetDimensionKeysRequest, GetDimensionValuesRequest, + GetMetricsRequest, MetricResponse, MultiFilters, } from './types'; @@ -59,7 +60,7 @@ export class CloudWatchAPI extends CloudWatchRequest { }); } - async getMetrics(namespace: string | undefined, region?: string): Promise>> { + async getMetrics({ region, namespace }: GetMetricsRequest): Promise>> { if (!namespace) { return []; } @@ -70,12 +71,10 @@ export class CloudWatchAPI extends CloudWatchRequest { }).then((metrics) => metrics.map((m) => ({ label: m.name, value: m.name }))); } - async getAllMetrics(region: string): Promise> { - const values = await this.memoizedGetRequest('all-metrics', { + async getAllMetrics({ region }: GetMetricsRequest): Promise> { + return this.memoizedGetRequest('metrics', { region: this.templateSrv.replace(this.getActualRegion(region)), - }); - - return values.map((v) => ({ metricName: v.name, namespace: v.namespace })); + }).then((metrics) => metrics.map((m) => ({ metricName: m.name, namespace: m.namespace }))); } async getDimensionKeys({ diff --git a/public/app/plugins/datasource/cloudwatch/cloudwatch-sql/completion/CompletionItemProvider.ts b/public/app/plugins/datasource/cloudwatch/cloudwatch-sql/completion/CompletionItemProvider.ts index 0443725cd78..75e2a192c03 100644 --- a/public/app/plugins/datasource/cloudwatch/cloudwatch-sql/completion/CompletionItemProvider.ts +++ b/public/app/plugins/datasource/cloudwatch/cloudwatch-sql/completion/CompletionItemProvider.ts @@ -112,14 +112,14 @@ export class SQLCompletionItemProvider extends CompletionItemProvider { const namespaceToken = getNamespaceToken(currentToken); if (namespaceToken?.value) { // if a namespace is specified, only suggest metrics for the namespace - const metrics = await this.api.getMetrics( - this.templateSrv.replace(namespaceToken?.value.replace(/\"/g, '')), - this.templateSrv.replace(this.region) - ); + const metrics = await this.api.getMetrics({ + namespace: namespaceToken?.value.replace(/\"/g, ''), + region: this.region, + }); metrics.forEach((m) => m.value && addSuggestion(m.value)); } else { // If no namespace is specified in the query, just list all metrics - const metrics = await this.api.getAllMetrics(this.templateSrv.replace(this.region)); + const metrics = await this.api.getAllMetrics({ region: this.region }); uniq(metrics.map((m) => m.metricName)).forEach((m) => m && addSuggestion(m, { insertText: m })); } } @@ -147,7 +147,7 @@ export class SQLCompletionItemProvider extends CompletionItemProvider { let namespaces = []; if (metricNameToken?.value) { // if a metric is specified, only suggest namespaces that actually have that metric - const metrics = await this.api.getAllMetrics(this.region); + const metrics = await this.api.getMetrics({ region: this.region }); const metricName = this.templateSrv.replace(metricNameToken.value); namespaces = metrics.filter((m) => m.metricName === metricName).map((m) => m.namespace); } else { diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx index 499e38fc553..d0e0e7fcc0f 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx @@ -148,15 +148,13 @@ describe('MetricStatEditor', () => { }); it('should remove metricName from metricStat if it does not exist in new namespace', async () => { - propsNamespaceMetrics.datasource.api.getMetrics = jest - .fn() - .mockImplementation((namespace: string, region: string) => { - let mockMetrics = - namespace === 'n1' && region === props.metricStat.region - ? metrics - : [{ value: 'oldNamespaceMetric', label: 'oldNamespaceMetric', text: 'oldNamespaceMetric' }]; - return Promise.resolve(mockMetrics); - }); + propsNamespaceMetrics.datasource.api.getMetrics = jest.fn().mockImplementation(({ namespace, region }) => { + let mockMetrics = + namespace === 'n1' && region === props.metricStat.region + ? metrics + : [{ value: 'oldNamespaceMetric', label: 'oldNamespaceMetric', text: 'oldNamespaceMetric' }]; + return Promise.resolve(mockMetrics); + }); propsNamespaceMetrics.metricStat.metricName = 'oldNamespaceMetric'; propsNamespaceMetrics.metricStat.namespace = 'n2'; diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx index 96a11b7762b..332f62649d1 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx @@ -48,7 +48,7 @@ export function MetricStatEditor({ if (!metricName) { return metricStat; } - await datasource.api.getMetrics(namespace, region).then((result: Array>) => { + await datasource.api.getMetrics({ namespace, region }).then((result: Array>) => { if (!result.find((metric) => metric.value === metricName)) { metricName = ''; } diff --git a/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx b/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx index 180683169c2..b80a3a4c71b 100644 --- a/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx @@ -67,8 +67,8 @@ const SQLBuilderSelectRow: React.FC = ({ datasource, q }; const validateMetricName = async (query: CloudWatchMetricsQuery) => { - let { region, sql } = query; - await datasource.api.getMetrics(query.namespace, region).then((result: Array>) => { + let { region, sql, namespace } = query; + await datasource.api.getMetrics({ namespace, region }).then((result: Array>) => { if (!result.some((metric) => metric.value === metricName)) { sql = removeMetricName(query).sql; } diff --git a/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.test.tsx index 1e4720c58cb..3795f2e31ba 100644 --- a/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.test.tsx @@ -230,7 +230,7 @@ describe('VariableEditor', () => { container: document.body, }); - expect(ds.datasource.api.getMetrics).toHaveBeenCalledWith('z2', 'b1'); + expect(ds.datasource.api.getMetrics).toHaveBeenCalledWith({ namespace: 'z2', region: 'b1' }); expect(ds.datasource.api.getDimensionKeys).toHaveBeenCalledWith({ namespace: 'z2', region: 'b1' }); expect(props.onChange).toHaveBeenCalledWith({ ...defaultQuery, diff --git a/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx index 59e500e34fe..bed581412dd 100644 --- a/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx @@ -65,7 +65,7 @@ export const VariableQueryEditor = ({ query, datasource, onChange }: Props) => { const sanitizeQuery = async (query: VariableQuery) => { let { metricName, dimensionKey, dimensionFilters, namespace, region } = query; if (metricName) { - await datasource.api.getMetrics(namespace, region).then((result: Array>) => { + await datasource.api.getMetrics({ namespace, region }).then((result: Array>) => { if (!result.find((metric) => metric.value === metricName)) { metricName = ''; } diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index 686721fd5ac..d176bfc8b97 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts @@ -215,7 +215,7 @@ describe('datasource', () => { }, ]), }).datasource; - const allMetrics = await datasource.api.getAllMetrics('us-east-2'); + const allMetrics = await datasource.api.getAllMetrics({ region: 'us-east-2' }); expect(allMetrics[0].metricName).toEqual('CPUUtilization'); expect(allMetrics[0].namespace).toEqual('AWS/EC2'); expect(allMetrics[1].metricName).toEqual('CPUPercentage'); diff --git a/public/app/plugins/datasource/cloudwatch/hooks.ts b/public/app/plugins/datasource/cloudwatch/hooks.ts index d2b72fb2a06..845b12913c0 100644 --- a/public/app/plugins/datasource/cloudwatch/hooks.ts +++ b/public/app/plugins/datasource/cloudwatch/hooks.ts @@ -42,7 +42,7 @@ export const useNamespaces = (datasource: CloudWatchDatasource) => { export const useMetrics = (datasource: CloudWatchDatasource, region: string, namespace: string | undefined) => { const [metrics, setMetrics] = useState>>([]); useEffect(() => { - datasource.api.getMetrics(namespace, region).then((result: Array>) => { + datasource.api.getMetrics({ namespace, region }).then((result: Array>) => { setMetrics(appendTemplateVariables(datasource, result)); }); }, [datasource, region, namespace]); diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index 532c94c413b..6347557c126 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -451,6 +451,11 @@ export interface LegacyAnnotationQuery extends MetricStat, DataQuery { type: string; } +export interface MetricResponse { + name: string; + namespace: string; +} + export interface ResourceRequest { region: string; } @@ -468,7 +473,6 @@ export interface GetDimensionValuesRequest extends ResourceRequest { dimensionFilters?: Dimensions; } -export interface MetricResponse { - name: string; - namespace: string; +export interface GetMetricsRequest extends ResourceRequest { + namespace?: string; } diff --git a/public/app/plugins/datasource/cloudwatch/variables.ts b/public/app/plugins/datasource/cloudwatch/variables.ts index 4cb38573e25..54339e27e9e 100644 --- a/public/app/plugins/datasource/cloudwatch/variables.ts +++ b/public/app/plugins/datasource/cloudwatch/variables.ts @@ -84,7 +84,7 @@ export class CloudWatchVariableSupport extends CustomVariableSupport ({ text: s.label, value: s.value,