Skip to content

Commit

Permalink
[v9.5.x] Google Cloud Monitor: Fix mem usage for dropdown (#67949)
Browse files Browse the repository at this point in the history
Google Cloud Monitor: Fix mem usage for dropdown (#67683)

* Google Cloud Monitor: Fix mem usage for dropdown

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

(cherry picked from commit b2e1b3a)

Co-authored-by: Adam Simpson <adam@adamsimpson.net>
  • Loading branch information
grafanabot and asimpson committed May 5, 2023
1 parent ac5bcf3 commit 5bfb970
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 78 deletions.
Expand Up @@ -14,6 +14,7 @@ export const createMockDatasource = (overrides?: Partial<Datasource>) => {
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(),
Expand Down
@@ -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';

Expand Down Expand Up @@ -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([]),
});

Expand All @@ -72,6 +74,7 @@ describe('VisualMetricQueryEditor', () => {
});
const metricName = await screen.findByLabelText('Metric name');
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 });
Expand All @@ -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(<VisualMetricQueryEditor {...defaultProps} onChange={onChange} datasource={datasource} query={query} />);

const service = await screen.findByLabelText('Service');
openMenu(service);
await act(async () => {
await select(service, 'Srv A', { container: document.body });
});
const metricName = await screen.findByLabelText('Metric name');
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();

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({
Expand Down Expand Up @@ -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([
Expand All @@ -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'] })
);
Expand Down
@@ -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';
Expand Down Expand Up @@ -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 (
<div>
<div className={customStyle}>{m.type}</div>
<div className={selectStyles.optionDescription}>{m.description}</div>
</div>
);
},
}));
return [
{
label: 'Template Variables',
options: variableOptionGroup.options,
},
...filtered,
];
};

const debounceFilter = debounce(filterMetrics, 400);

const onMetricTypeChange = ({ value }: SelectableValue<string>) => {
const metricDescriptor = getSelectedMetricDescriptor(metricDescriptors, value!);
setMetricDescriptor(metricDescriptor);
Expand Down Expand Up @@ -205,6 +233,7 @@ export function Editor({
<Select
width="auto"
onChange={onServiceChange}
isLoading={services.length === 0}
value={[...services, ...variableOptionGroup.options].find((s) => s.value === service)}
options={[
{
Expand All @@ -217,21 +246,19 @@ export function Editor({
inputId={`${refId}-service`}
/>
</EditorField>
<EditorField label="Metric name" width="auto">
<Select
width="auto"
onChange={onMetricTypeChange}
value={[...metrics, ...variableOptionGroup.options].find((s) => s.value === metricType)}
options={[
{
label: 'Template Variables',
options: variableOptionGroup.options,
},
...metrics,
]}
placeholder="Select Metric"
inputId={`${refId}-select-metric`}
/>
<EditorField label="Metric name" width="auto" htmlFor={`${refId}-select-metric`}>
<span title={service === '' ? 'Select a service first' : 'Type to search metrics'}>
<AsyncSelect
width="auto"
onChange={onMetricTypeChange}
value={[...metrics, ...variableOptionGroup.options].find((s) => s.value === metricType)}
loadOptions={debounceFilter}
defaultOptions={metrics.slice(0, 100)}
placeholder="Select Metric"
inputId={`${refId}-select-metric`}
disabled={service === ''}
/>
</span>
</EditorField>
</EditorFieldGroup>
</EditorRow>
Expand Down
11 changes: 11 additions & 0 deletions public/app/plugins/datasource/cloud-monitoring/datasource.ts
Expand Up @@ -196,6 +196,17 @@ export default class CloudMonitoringDatasource extends DataSourceWithBackend<
) as Promise<MetricDescriptor[]>;
}

async filterMetricsByType(projectName: string, filter: string): Promise<MetricDescriptor[]> {
if (!projectName) {
return [];
}

return this.getResource(
`metricDescriptors/v3/projects/${this.templateSrv.replace(projectName)}/metricDescriptors`,
{ filter: `metric.type : "${filter}"` }
);
}

async getSLOServices(projectName: string): Promise<Array<SelectableValue<string>>> {
return this.getResource(`services/v3/projects/${this.templateSrv.replace(projectName)}/services?pageSize=1000`);
}
Expand Down

0 comments on commit 5bfb970

Please sign in to comment.