From 55a9c98ae39f0651f56eb5b5d0567032737df42a Mon Sep 17 00:00:00 2001 From: Isabella Siu Date: Tue, 1 Feb 2022 11:36:51 -0500 Subject: [PATCH] CloudWatch: Use metrics query header for logs (#44668) Co-authored-by: Sarah Zinger Co-authored-by: Yaelle Chaudy <42030685+yaelleC@users.noreply.github.com> --- .../cloudwatch/components/LogsQueryEditor.tsx | 4 +- .../components/LogsQueryField.test.tsx | 5 +- .../cloudwatch/components/LogsQueryField.tsx | 71 ++------ .../components/MetricsQueryHeader.test.tsx | 133 +++++++++++++++ .../components/MetricsQueryHeader.tsx | 89 ++++++++++ .../components/PanelQueryEditor.tsx | 38 ----- .../components/QueryHeader.test.tsx | 153 ++++++------------ .../cloudwatch/components/QueryHeader.tsx | 110 +++++-------- 8 files changed, 328 insertions(+), 275 deletions(-) create mode 100644 public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.test.tsx create mode 100644 public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.tsx diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryEditor.tsx index 1dee5d188bc..034c120450d 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogsQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryEditor.tsx @@ -42,9 +42,7 @@ export const CloudWatchLogsQueryEditor = memo(function CloudWatchLogsQueryEditor datasource={datasource} query={query} onBlur={() => {}} - onChange={(val: CloudWatchQuery) => { - onChange({ ...val, queryMode: 'Logs' }); - }} + onChange={onChange} onRunQuery={onRunQuery} history={[]} data={data} diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx index 633988f48d2..172c785c5d2 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx @@ -49,7 +49,6 @@ describe('CloudWatchLogsQueryField', () => { onChange={onChange} /> ); - const getRegionSelect = () => wrapper.find({ label: 'Region' }).props().inputEl; const getLogGroupSelect = () => wrapper.find({ label: 'Log Groups' }).props().inputEl; getLogGroupSelect().props.onChange([{ value: 'log_group_1' }]); @@ -57,12 +56,12 @@ describe('CloudWatchLogsQueryField', () => { expect(getLogGroupSelect().props.value[0].value).toBe('log_group_1'); // We select new region where the selected log group does not exist - await getRegionSelect().props.onChange({ value: 'region2' }); + await (wrapper.instance() as CloudWatchLogsQueryField).onRegionChange('region2'); // We clear the select expect(getLogGroupSelect().props.value.length).toBe(0); // Make sure we correctly updated the upstream state - expect(onChange.mock.calls[onChange.mock.calls.length - 1][0]).toEqual({ region: 'region2', logGroupNames: [] }); + expect(onChange).toHaveBeenLastCalledWith({ logGroupNames: [] }); }); it('should merge results of remote log groups search with existing results', async () => { diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx index ecfe5f2da8b..0bd033b018b 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx @@ -7,7 +7,6 @@ import { LegacyForms, MultiSelect, QueryField, - Select, SlatePrism, TypeaheadInput, TypeaheadOutput, @@ -31,6 +30,7 @@ import { notifyApp } from 'app/core/actions'; import { createErrorNotification } from 'app/core/copy/appNotification'; import { InputActionMeta } from '@grafana/ui/src/components/Select/types'; import { getStatsGroups } from '../utils/query/getStatsGroups'; +import QueryHeader from './QueryHeader'; export interface CloudWatchLogsQueryFieldProps extends QueryEditorProps { @@ -54,8 +54,6 @@ interface State { selectedLogGroups: Array>; availableLogGroups: Array>; loadingLogGroups: boolean; - regions: Array>; - selectedRegion: SelectableValue; invalidLogGroups: boolean; hint: | { @@ -76,15 +74,7 @@ export class CloudWatchLogsQueryField extends React.PureComponent { - const { datasource, query, onChange } = this.props; + const { query, onChange } = this.props; this.setState({ loadingLogGroups: true, @@ -191,25 +181,18 @@ export class CloudWatchLogsQueryField extends React.PureComponent { - this.setState({ - regions, - }); - }); }; onChangeQuery = (value: string) => { // Send text change to parent const { query, onChange } = this.props; - const { selectedLogGroups, selectedRegion } = this.state; + const { selectedLogGroups } = this.state; if (onChange) { const nextQuery = { ...query, expression: value, logGroupNames: selectedLogGroups?.map((logGroupName) => logGroupName.value!) ?? [], - region: selectedRegion.value ?? 'default', statsGroups: getStatsGroups(value), }; onChange(nextQuery); @@ -234,22 +217,17 @@ export class CloudWatchLogsQueryField extends React.PureComponent) => { + onRegionChange = async (v: string) => { this.setState({ - selectedRegion: v, loadingLogGroups: true, }); - - const logGroups = await this.fetchLogGroupOptions(v.value!); - + const logGroups = await this.fetchLogGroupOptions(v); this.setState((state) => { const selectedLogGroups = intersectionBy(state.selectedLogGroups, logGroups, 'value'); - const { onChange, query } = this.props; if (onChange) { const nextQuery = { ...query, - region: v.value ?? 'default', logGroupNames: selectedLogGroups.map((group) => group.value!), }; @@ -307,16 +285,8 @@ export class CloudWatchLogsQueryField extends React.PureComponent +
- this.setSelectedRegion(v)} - width={18} - placeholder="Choose Region" - maxMenuHeight={500} - /> - } - /> - { - this.onLogGroupSearchDebounced(value, selectedRegion.value ?? 'default', actionMeta); + this.onLogGroupSearchDebounced(value, query.region, actionMeta); }} /> } diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.test.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.test.tsx new file mode 100644 index 00000000000..14a5dfc9961 --- /dev/null +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.test.tsx @@ -0,0 +1,133 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { act } from 'react-dom/test-utils'; +import { CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType } from '../types'; +import { setupMockedDataSource } from '../__mocks__/CloudWatchDataSource'; +import MetricsQueryHeader from './MetricsQueryHeader'; + +const ds = setupMockedDataSource({ + variables: [], +}); +ds.datasource.getRegions = jest.fn().mockResolvedValue([]); +const query: CloudWatchMetricsQuery = { + id: '', + region: 'us-east-2', + namespace: '', + period: '', + alias: '', + metricName: '', + dimensions: {}, + matchExact: true, + statistic: '', + expression: '', + refId: '', +}; + +describe('MetricsQueryHeader', () => { + describe('confirm modal', () => { + it('should be shown when moving from code editor to builder when in sql mode', async () => { + const onChange = jest.fn(); + const onRunQuery = jest.fn(); + query.metricEditorMode = MetricEditorMode.Code; + query.metricQueryType = MetricQueryType.Query; + + render( + + ); + + const builderElement = screen.getByLabelText('Builder'); + expect(builderElement).toBeInTheDocument(); + await act(async () => { + await builderElement.click(); + }); + + const modalTitleElem = screen.getByText('Are you sure?'); + expect(modalTitleElem).toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('should not be shown when moving from builder to code when in sql mode', async () => { + const onChange = jest.fn(); + const onRunQuery = jest.fn(); + query.metricEditorMode = MetricEditorMode.Builder; + query.metricQueryType = MetricQueryType.Query; + + render( + + ); + + const builderElement = screen.getByLabelText('Code'); + expect(builderElement).toBeInTheDocument(); + await act(async () => { + await builderElement.click(); + }); + + const modalTitleElem = screen.queryByText('Are you sure?'); + expect(modalTitleElem).toBeNull(); + expect(onChange).toHaveBeenCalled(); + }); + + it('should not be shown when moving from code to builder when in standard mode', async () => { + const onChange = jest.fn(); + const onRunQuery = jest.fn(); + query.metricEditorMode = MetricEditorMode.Code; + query.metricQueryType = MetricQueryType.Search; + + render( + + ); + + const builderElement = screen.getByLabelText('Builder'); + expect(builderElement).toBeInTheDocument(); + await act(async () => { + await builderElement.click(); + }); + + const modalTitleElem = screen.queryByText('Are you sure?'); + expect(modalTitleElem).toBeNull(); + expect(onChange).toHaveBeenCalled(); + }); + }); + + it('should call run query when run button is clicked when in metric query mode', async () => { + const onChange = jest.fn(); + const onRunQuery = jest.fn(); + query.metricEditorMode = MetricEditorMode.Code; + query.metricQueryType = MetricQueryType.Query; + + render( + + ); + + const runQueryButton = screen.getByText('Run query'); + expect(runQueryButton).toBeInTheDocument(); + await act(async () => { + await runQueryButton.click(); + }); + expect(onRunQuery).toHaveBeenCalled(); + }); +}); diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.tsx new file mode 100644 index 00000000000..1523c7ad4b9 --- /dev/null +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryHeader.tsx @@ -0,0 +1,89 @@ +import React, { useCallback, useState } from 'react'; + +import { SelectableValue } from '@grafana/data'; +import { Button, ConfirmModal, RadioButtonGroup } from '@grafana/ui'; +import { InlineSelect, FlexItem } from '@grafana/experimental'; + +import { CloudWatchDatasource } from '../datasource'; +import { CloudWatchMetricsQuery, CloudWatchQuery, MetricEditorMode, MetricQueryType } from '../types'; + +interface MetricsQueryHeaderProps { + query: CloudWatchMetricsQuery; + datasource: CloudWatchDatasource; + onChange: (query: CloudWatchQuery) => void; + onRunQuery: () => void; + sqlCodeEditorIsDirty: boolean; +} + +const metricEditorModes: Array> = [ + { label: 'Metric Search', value: MetricQueryType.Search }, + { label: 'Metric Query', value: MetricQueryType.Query }, +]; + +const editorModes = [ + { label: 'Builder', value: MetricEditorMode.Builder }, + { label: 'Code', value: MetricEditorMode.Code }, +]; + +const MetricsQueryHeader: React.FC = ({ + query, + sqlCodeEditorIsDirty, + onChange, + onRunQuery, +}) => { + const { metricEditorMode, metricQueryType } = query; + const [showConfirm, setShowConfirm] = useState(false); + + const onEditorModeChange = useCallback( + (newMetricEditorMode: MetricEditorMode) => { + if ( + sqlCodeEditorIsDirty && + metricQueryType === MetricQueryType.Query && + metricEditorMode === MetricEditorMode.Code + ) { + setShowConfirm(true); + return; + } + onChange({ ...query, metricEditorMode: newMetricEditorMode }); + }, + [setShowConfirm, onChange, sqlCodeEditorIsDirty, query, metricEditorMode, metricQueryType] + ); + + return ( + <> + m.value === metricQueryType)} + options={metricEditorModes} + onChange={({ value }) => { + onChange({ ...query, metricQueryType: value }); + }} + /> + + + + + {query.metricQueryType === MetricQueryType.Query && query.metricEditorMode === MetricEditorMode.Code && ( + + )} + + { + setShowConfirm(false); + onChange({ ...query, metricEditorMode: MetricEditorMode.Builder }); + }} + onDismiss={() => setShowConfirm(false)} + /> + + ); +}; + +export default MetricsQueryHeader; diff --git a/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.tsx index 27d5a6ebcae..811f342f18d 100644 --- a/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.tsx @@ -1,20 +1,12 @@ import React, { PureComponent } from 'react'; -import { pick } from 'lodash'; import { QueryEditorProps, ExploreMode } from '@grafana/data'; -import { Segment } from '@grafana/ui'; import { CloudWatchJsonData, CloudWatchQuery } from '../types'; import { CloudWatchDatasource } from '../datasource'; -import { QueryInlineField } from './'; import { MetricsQueryEditor } from './MetricsQueryEditor'; import LogsQueryEditor from './LogsQueryEditor'; export type Props = QueryEditorProps; -const apiModes = { - Metrics: { label: 'CloudWatch Metrics', value: 'Metrics' }, - Logs: { label: 'CloudWatch Logs', value: 'Logs' }, -}; - export class PanelQueryEditor extends PureComponent { render() { const { query } = this.props; @@ -22,36 +14,6 @@ export class PanelQueryEditor extends PureComponent { return ( <> - {/* TODO: Remove this in favor of the QueryHeader */} - {apiMode === ExploreMode.Logs && ( - - { - const newMode = (value as 'Metrics' | 'Logs') ?? 'Metrics'; - if (newMode !== apiModes[apiMode].value) { - const commonProps = pick( - query, - 'id', - 'region', - 'namespace', - 'refId', - 'hide', - 'key', - 'queryType', - 'datasource' - ); - - this.props.onChange({ - ...commonProps, - queryMode: newMode, - } as CloudWatchQuery); - } - }} - /> - - )} {apiMode === ExploreMode.Logs ? ( ) : ( diff --git a/public/app/plugins/datasource/cloudwatch/components/QueryHeader.test.tsx b/public/app/plugins/datasource/cloudwatch/components/QueryHeader.test.tsx index d66d70a3ea0..81b1263301e 100644 --- a/public/app/plugins/datasource/cloudwatch/components/QueryHeader.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/QueryHeader.test.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import { act } from 'react-dom/test-utils'; -import { CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType } from '../types'; +import { CloudWatchLogsQuery, CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType } from '../types'; import { setupMockedDataSource } from '../__mocks__/CloudWatchDataSource'; import QueryHeader from './QueryHeader'; @@ -9,105 +9,23 @@ const ds = setupMockedDataSource({ variables: [], }); ds.datasource.getRegions = jest.fn().mockResolvedValue([]); -const query: CloudWatchMetricsQuery = { - id: '', - region: 'us-east-2', - namespace: '', - period: '', - alias: '', - metricName: '', - dimensions: {}, - matchExact: true, - statistic: '', - expression: '', - refId: '', -}; describe('QueryHeader', () => { - describe('confirm modal', () => { - it('should be shown when moving from code editor to builder when in sql mode', async () => { - const onChange = jest.fn(); - const onRunQuery = jest.fn(); - query.metricEditorMode = MetricEditorMode.Code; - query.metricQueryType = MetricQueryType.Query; - - render( - - ); - - const builderElement = screen.getByLabelText('Builder'); - expect(builderElement).toBeInTheDocument(); - await act(async () => { - await builderElement.click(); - }); - - const modalTitleElem = screen.getByText('Are you sure?'); - expect(modalTitleElem).toBeInTheDocument(); - expect(onChange).not.toHaveBeenCalled(); - }); - - it('should not be shown when moving from builder to code when in sql mode', async () => { - const onChange = jest.fn(); - const onRunQuery = jest.fn(); - query.metricEditorMode = MetricEditorMode.Builder; - query.metricQueryType = MetricQueryType.Query; - - render( - - ); - - const builderElement = screen.getByLabelText('Code'); - expect(builderElement).toBeInTheDocument(); - await act(async () => { - await builderElement.click(); - }); - - const modalTitleElem = screen.queryByText('Are you sure?'); - expect(modalTitleElem).toBeNull(); - expect(onChange).toHaveBeenCalled(); - }); - - it('should not be shown when moving from code to builder when in standard mode', async () => { - const onChange = jest.fn(); - const onRunQuery = jest.fn(); - query.metricEditorMode = MetricEditorMode.Code; - query.metricQueryType = MetricQueryType.Search; - - render( - - ); - - const builderElement = screen.getByLabelText('Builder'); - expect(builderElement).toBeInTheDocument(); - await act(async () => { - await builderElement.click(); - }); - - const modalTitleElem = screen.queryByText('Are you sure?'); - expect(modalTitleElem).toBeNull(); - expect(onChange).toHaveBeenCalled(); - }); - }); - - it('run button should be displayed in code editor in metric query mode', async () => { + it('should display metric options for metrics', async () => { + const query: CloudWatchMetricsQuery = { + queryType: 'Metrics', + id: '', + region: 'us-east-2', + namespace: '', + period: '', + alias: '', + metricName: '', + dimensions: {}, + matchExact: true, + statistic: '', + expression: '', + refId: '', + }; const onChange = jest.fn(); const onRunQuery = jest.fn(); query.metricEditorMode = MetricEditorMode.Code; @@ -123,11 +41,42 @@ describe('QueryHeader', () => { /> ); - const runQueryButton = screen.getByText('Run query'); - expect(runQueryButton).toBeInTheDocument(); + const builderElement = screen.getByLabelText('Builder'); + expect(builderElement).toBeInTheDocument(); await act(async () => { - await runQueryButton.click(); + await builderElement.click(); }); - expect(onRunQuery).toHaveBeenCalled(); + + const modalTitleElem = screen.getByText('Are you sure?'); + expect(modalTitleElem).toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('should not display metric options for logs', async () => { + const onChange = jest.fn(); + const onRunQuery = jest.fn(); + const query: CloudWatchLogsQuery = { + queryType: 'Metrics', + id: '', + region: 'us-east-2', + expression: '', + refId: '', + queryMode: 'Logs', + }; + + render( + + ); + + const builderElement = screen.queryByLabelText('Builder'); + expect(builderElement).toBeNull(); + const codeElement = screen.queryByLabelText('Code'); + expect(codeElement).toBeNull(); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx b/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx index 9c7a946972b..def9d666832 100644 --- a/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx @@ -1,26 +1,21 @@ -import React, { useCallback, useState } from 'react'; +import React from 'react'; import { pick } from 'lodash'; -import { SelectableValue } from '@grafana/data'; -import { Button, ConfirmModal, RadioButtonGroup } from '@grafana/ui'; -import { EditorHeader, InlineSelect, FlexItem } from '@grafana/experimental'; +import { ExploreMode, SelectableValue } from '@grafana/data'; +import { EditorHeader, InlineSelect } from '@grafana/experimental'; import { CloudWatchDatasource } from '../datasource'; -import { - CloudWatchMetricsQuery, - CloudWatchQuery, - CloudWatchQueryMode, - MetricEditorMode, - MetricQueryType, -} from '../types'; +import { CloudWatchQuery, CloudWatchQueryMode } from '../types'; import { useRegions } from '../hooks'; +import MetricsQueryHeader from './MetricsQueryHeader'; interface QueryHeaderProps { - query: CloudWatchMetricsQuery; + query: CloudWatchQuery; datasource: CloudWatchDatasource; onChange: (query: CloudWatchQuery) => void; onRunQuery: () => void; sqlCodeEditorIsDirty: boolean; + onRegionChange?: (region: string) => Promise; } const apiModes: Array> = [ @@ -28,48 +23,38 @@ const apiModes: Array> = [ { label: 'CloudWatch Logs', value: 'Logs' }, ]; -const metricEditorModes: Array> = [ - { label: 'Metric Search', value: MetricQueryType.Search }, - { label: 'Metric Query', value: MetricQueryType.Query }, -]; - -const editorModes = [ - { label: 'Builder', value: MetricEditorMode.Builder }, - { label: 'Code', value: MetricEditorMode.Code }, -]; - -const QueryHeader: React.FC = ({ query, sqlCodeEditorIsDirty, datasource, onChange, onRunQuery }) => { - const { metricEditorMode, metricQueryType, queryMode, region } = query; - const [showConfirm, setShowConfirm] = useState(false); +const QueryHeader: React.FC = ({ + query, + sqlCodeEditorIsDirty, + datasource, + onChange, + onRunQuery, + onRegionChange, +}) => { + const { queryMode, region } = query; const [regions, regionIsLoading] = useRegions(datasource); - const onEditorModeChange = useCallback( - (newMetricEditorMode: MetricEditorMode) => { - if ( - sqlCodeEditorIsDirty && - metricQueryType === MetricQueryType.Query && - metricEditorMode === MetricEditorMode.Code - ) { - setShowConfirm(true); - return; - } - onChange({ ...query, metricEditorMode: newMetricEditorMode }); - }, - [setShowConfirm, onChange, sqlCodeEditorIsDirty, query, metricEditorMode, metricQueryType] - ); - const onQueryModeChange = ({ value }: SelectableValue) => { if (value !== queryMode) { const commonProps = pick(query, 'id', 'region', 'namespace', 'refId', 'hide', 'key', 'queryType', 'datasource'); - onChange({ ...commonProps, queryMode: value, - }); + } as CloudWatchQuery); } }; + const onRegion = async ({ value }: SelectableValue) => { + if (onRegionChange) { + await onRegionChange(value ?? 'default'); + } + onChange({ + ...query, + region: value, + } as CloudWatchQuery); + }; + return ( = ({ query, sqlCodeEditorIsDirty, value={regions.find((v) => v.value === region)} placeholder="Select region" allowCustomValue - onChange={({ value: region }) => region && onChange({ ...query, region: region })} + onChange={({ value: region }) => region && onRegion({ value: region })} options={regions} isLoading={regionIsLoading} /> - m.value === metricQueryType)} - options={metricEditorModes} - onChange={({ value }) => { - onChange({ ...query, metricQueryType: value }); - }} - /> - - - - - - {query.metricQueryType === MetricQueryType.Query && query.metricEditorMode === MetricEditorMode.Code && ( - + {queryMode !== ExploreMode.Logs && ( + )} - - { - setShowConfirm(false); - onChange({ ...query, metricEditorMode: MetricEditorMode.Builder }); - }} - onDismiss={() => setShowConfirm(false)} - /> ); };