From 4a07a2dc5d977ca7534bba34f7daa7ceca52941c Mon Sep 17 00:00:00 2001 From: Adam Simpson Date: Thu, 11 May 2023 15:26:51 -0400 Subject: [PATCH] Google Cloud Monitor: Fix mem usage for dropdown (#67683) (#67950) Previously the Metric name dropdown would attempt to load _all_ the available metric names into the Select which would eventually crash the browser if the dataset was large enough. We can fix this by using AsyncSelect and making another query once a Service is selected _and_ the user types a few characters. * fix: update tests for AsyncSelect * fix lint * fix: add subset of metrics on initial load --- .../__mocks__/cloudMonitoringDatasource.ts | 1 + .../VisualMetricQueryEditor.test.tsx | 79 ++++--------------- .../components/VisualMetricQueryEditor.tsx | 59 ++++++++++---- .../datasource/cloud-monitoring/datasource.ts | 11 +++ 4 files changed, 71 insertions(+), 79 deletions(-) diff --git a/public/app/plugins/datasource/cloud-monitoring/__mocks__/cloudMonitoringDatasource.ts b/public/app/plugins/datasource/cloud-monitoring/__mocks__/cloudMonitoringDatasource.ts index 87c377637bf..aff194f3a55 100644 --- a/public/app/plugins/datasource/cloud-monitoring/__mocks__/cloudMonitoringDatasource.ts +++ b/public/app/plugins/datasource/cloud-monitoring/__mocks__/cloudMonitoringDatasource.ts @@ -14,6 +14,7 @@ export const createMockDatasource = (overrides?: Partial) => { getProjects: jest.fn().mockResolvedValue([]), getDefaultProject: jest.fn().mockReturnValue('cloud-monitoring-default-project'), templateSrv, + filterMetricsByType: jest.fn().mockResolvedValue([]), getSLOServices: jest.fn().mockResolvedValue([]), migrateQuery: jest.fn().mockImplementation((query) => query), timeSrv: getTimeSrv(), diff --git a/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.test.tsx b/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.test.tsx index b408714dc49..a2227a23e92 100644 --- a/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.test.tsx +++ b/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.test.tsx @@ -1,4 +1,5 @@ -import { act, render, screen, waitFor, within } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import { openMenu, select } from 'react-select-event'; @@ -60,6 +61,7 @@ describe('VisualMetricQueryEditor', () => { const mockMetricDescriptor = createMockMetricDescriptor({ displayName: 'metricName_test', type: 'test_type' }); const datasource = createMockDatasource({ getMetricTypes: jest.fn().mockResolvedValue([createMockMetricDescriptor(), mockMetricDescriptor]), + filterMetricsByType: jest.fn().mockResolvedValue([createMockMetricDescriptor(), mockMetricDescriptor]), getLabels: jest.fn().mockResolvedValue([]), }); @@ -71,7 +73,8 @@ describe('VisualMetricQueryEditor', () => { await select(service, 'Srv', { container: document.body }); }); const metricName = await screen.findByLabelText('Metric name'); - await openMenu(metricName); + openMenu(metricName); + await userEvent.type(metricName, 'test'); await waitFor(() => expect(document.body).toHaveTextContent('metricName_test')); await act(async () => { await select(metricName, 'metricName_test', { container: document.body }); @@ -81,66 +84,6 @@ describe('VisualMetricQueryEditor', () => { }); }); - it('should render available metric options according to the selected service', async () => { - const onChange = jest.fn(); - const query = createMockTimeSeriesList(); - const datasource = createMockDatasource({ - getMetricTypes: jest.fn().mockResolvedValue([ - createMockMetricDescriptor({ - service: 'service_a', - serviceShortName: 'srv_a', - type: 'metric1', - description: 'description_metric1', - displayName: 'displayName_metric1', - }), - createMockMetricDescriptor({ - service: 'service_b', - serviceShortName: 'srv_b', - type: 'metric2', - description: 'description_metric2', - displayName: 'displayName_metric2', - }), - createMockMetricDescriptor({ - service: 'service_b', - serviceShortName: 'srv_b', - type: 'metric3', - description: 'description_metric3', - displayName: 'displayName_metric3', - }), - ]), - getLabels: jest.fn().mockResolvedValue([]), - }); - - render(); - - const service = await screen.findByLabelText('Service'); - await openMenu(service); - await act(async () => { - await select(service, 'Srv A', { container: document.body }); - }); - const metricName = await screen.findByLabelText('Metric name'); - await openMenu(metricName); - - const metricNameOptions = screen.getByLabelText('Select options menu'); - expect(within(metricNameOptions).getByText('description_metric1')).toBeInTheDocument(); - expect(within(metricNameOptions).getByText('displayName_metric1')).toBeInTheDocument(); - expect(within(metricNameOptions).queryByText('displayName_metric2')).not.toBeInTheDocument(); - expect(within(metricNameOptions).queryByText('description_metric2')).not.toBeInTheDocument(); - expect(within(metricNameOptions).queryByText('displayName_metric3')).not.toBeInTheDocument(); - expect(within(metricNameOptions).queryByText('description_metric3')).not.toBeInTheDocument(); - - await openMenu(service); - await act(async () => { - await select(service, 'Srv B', { container: document.body }); - }); - expect(within(metricNameOptions).queryByText('displayName_metric1')).not.toBeInTheDocument(); - expect(within(metricNameOptions).queryByText('description_metric1')).not.toBeInTheDocument(); - expect(within(metricNameOptions).getByText('displayName_metric2')).toBeInTheDocument(); - expect(within(metricNameOptions).getByText('description_metric2')).toBeInTheDocument(); - expect(within(metricNameOptions).getByText('displayName_metric3')).toBeInTheDocument(); - expect(within(metricNameOptions).getByText('description_metric3')).toBeInTheDocument(); - }); - it('should have a distinct list of services', async () => { const onChange = jest.fn(); const datasource = createMockDatasource({ @@ -210,6 +153,12 @@ describe('VisualMetricQueryEditor', () => { }); const onChange = jest.fn(); const datasource = createMockDatasource({ + filterMetricsByType: jest + .fn() + .mockResolvedValue([ + createMockMetricDescriptor(), + createMockMetricDescriptor({ type: 'type2', displayName: 'metricName2', metricKind: MetricKind.GAUGE }), + ]), getMetricTypes: jest .fn() .mockResolvedValue([ @@ -227,7 +176,11 @@ describe('VisualMetricQueryEditor', () => { expect(await screen.findByText('metric.test_groupby')).toBeInTheDocument(); const metric = await screen.findByLabelText('Metric name'); openMenu(metric); - await select(metric, 'metricName2', { container: document.body }); + await userEvent.type(metric, 'type2'); + await waitFor(() => expect(document.body).toHaveTextContent('metricName2')); + await act(async () => { + await select(metric, 'metricName2', { container: document.body }); + }); expect(onChange).toBeCalledWith( expect.objectContaining({ filters: ['metric.test_label', '=', 'test', 'AND', 'metric.type', '=', 'type2'] }) ); diff --git a/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.tsx b/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.tsx index 5e9976b3526..dd96bc0a0cb 100644 --- a/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.tsx +++ b/public/app/plugins/datasource/cloud-monitoring/components/VisualMetricQueryEditor.tsx @@ -1,10 +1,11 @@ import { css } from '@emotion/css'; +import debounce from 'debounce-promise'; import { startCase, uniqBy } from 'lodash'; import React, { useCallback, useEffect, useState } from 'react'; import { GrafanaTheme2, SelectableValue, TimeRange } from '@grafana/data'; import { EditorField, EditorFieldGroup, EditorRow } from '@grafana/experimental'; -import { getSelectStyles, Select, useStyles2, useTheme2 } from '@grafana/ui'; +import { getSelectStyles, Select, AsyncSelect, useStyles2, useTheme2 } from '@grafana/ui'; import CloudMonitoringDatasource from '../datasource'; import { getAlignmentPickerData, getMetricType, setMetricType } from '../functions'; @@ -159,6 +160,33 @@ export function Editor({ return services.length > 0 ? uniqBy(services, (s) => s.value) : []; }; + const filterMetrics = async (filter: string) => { + const metrics = await datasource.filterMetricsByType(projectName, service); + const filtered = metrics + .filter((m) => m.type.includes(filter.toLowerCase())) + .map((m) => ({ + value: m.type, + label: m.displayName, + component: function optionComponent() { + return ( +
+
{m.type}
+
{m.description}
+
+ ); + }, + })); + return [ + { + label: 'Template Variables', + options: variableOptionGroup.options, + }, + ...filtered, + ]; + }; + + const debounceFilter = debounce(filterMetrics, 400); + const onMetricTypeChange = ({ value }: SelectableValue) => { const metricDescriptor = getSelectedMetricDescriptor(metricDescriptors, value!); setMetricDescriptor(metricDescriptor); @@ -205,6 +233,7 @@ export function Editor({ s.value === metricType)} - options={[ - { - label: 'Template Variables', - options: variableOptionGroup.options, - }, - ...metrics, - ]} - placeholder="Select Metric" - inputId={`${refId}-select-metric`} - /> + + + s.value === metricType)} + loadOptions={debounceFilter} + defaultOptions={metrics.slice(0, 100)} + placeholder="Select Metric" + inputId={`${refId}-select-metric`} + disabled={service === ''} + /> + diff --git a/public/app/plugins/datasource/cloud-monitoring/datasource.ts b/public/app/plugins/datasource/cloud-monitoring/datasource.ts index 0c5a9c70af1..780ef8630b3 100644 --- a/public/app/plugins/datasource/cloud-monitoring/datasource.ts +++ b/public/app/plugins/datasource/cloud-monitoring/datasource.ts @@ -196,6 +196,17 @@ export default class CloudMonitoringDatasource extends DataSourceWithBackend< ) as Promise; } + async filterMetricsByType(projectName: string, filter: string): Promise { + if (!projectName) { + return []; + } + + return this.getResource( + `metricDescriptors/v3/projects/${this.templateSrv.replace(projectName)}/metricDescriptors`, + { filter: `metric.type : "${filter}"` } + ); + } + async getSLOServices(projectName: string): Promise>> { return this.getResource(`services/v3/projects/${this.templateSrv.replace(projectName)}/services?pageSize=1000`); }