Cloudwatch: Define and use getDefaultquery instead of calling onChange on mount (#60221)

* Implement getDefaultQuery for cloudwatch logs and metrics

* Fix bug with query reference in queryMigrations and replace tests

* Move migrate functions to /migrations, remove onChange from LogsQueryField

* [EMPTY] Retry CI

* Fix naming
This commit is contained in:
Ida Štambuk 2022-12-14 12:21:36 +01:00 committed by GitHub
parent 6fcea37638
commit cea5e6589d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 178 additions and 162 deletions

View File

@ -5364,9 +5364,6 @@ exports[`better eslint`] = {
[0, 0, 0, "Unexpected any. Specify a different type.", "0"],
[0, 0, 0, "Unexpected any. Specify a different type.", "1"]
],
"public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/usePreparedMetricsQuery.test.ts:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"]
],
"public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.test.tsx:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"],
[0, 0, 0, "Unexpected any. Specify a different type.", "1"],

View File

@ -62,10 +62,12 @@ export function setupMockedDataSource({
variables,
mockGetVariableName = true,
getMock = jest.fn(),
customInstanceSettings = CloudWatchSettings,
}: {
getMock?: jest.Func;
variables?: CustomVariableModel[];
mockGetVariableName?: boolean;
customInstanceSettings?: DataSourceInstanceSettings<CloudWatchJsonData>;
} = {}) {
let templateService = new TemplateSrv();
if (variables) {
@ -76,7 +78,7 @@ export function setupMockedDataSource({
}
const timeSrv = getTimeSrv();
const datasource = new CloudWatchDatasource(CloudWatchSettings, templateService, timeSrv);
const datasource = new CloudWatchDatasource(customInstanceSettings, templateService, timeSrv);
datasource.getVariables = () => ['test'];
datasource.api.describeLogGroups = jest.fn().mockResolvedValue([]);
datasource.api.getNamespaces = jest.fn().mockResolvedValue([]);

View File

@ -75,14 +75,6 @@ class CloudWatchLogsQueryField extends React.PureComponent<CloudWatchLogsQueryFi
];
}
componentDidMount = () => {
const { query, datasource, onChange } = this.props;
if (onChange) {
onChange({ ...query, logGroupNames: query.logGroupNames ?? datasource.logsQueryRunner.defaultLogGroups });
}
};
onChangeQuery = (value: string) => {
// Send text change to parent
const { query, onChange } = this.props;

View File

@ -8,6 +8,7 @@ import { Input } from '@grafana/ui';
import { MathExpressionQueryField, MetricStatEditor, SQLBuilderEditor, SQLCodeEditor } from '../';
import { CloudWatchDatasource } from '../../datasource';
import { isCloudWatchMetricsQuery } from '../../guards';
import useMigratedMetricsQuery from '../../migrations/useMigratedMetricsQuery';
import {
CloudWatchJsonData,
CloudWatchMetricsQuery,
@ -20,7 +21,6 @@ import { DynamicLabelsField } from '../DynamicLabelsField';
import QueryHeader from '../QueryHeader';
import { Alias } from './Alias';
import usePreparedMetricsQuery from './usePreparedMetricsQuery';
export interface Props extends QueryEditorProps<CloudWatchDatasource, CloudWatchQuery, CloudWatchJsonData> {
query: CloudWatchMetricsQuery;
@ -29,7 +29,7 @@ export interface Props extends QueryEditorProps<CloudWatchDatasource, CloudWatch
export const MetricsQueryEditor = (props: Props) => {
const { query, onRunQuery, datasource } = props;
const [sqlCodeEditorIsDirty, setSQLCodeEditorIsDirty] = useState(false);
const preparedQuery = usePreparedMetricsQuery(query, props.onChange);
const migratedQuery = useMigratedMetricsQuery(query, props.onChange);
const onChange = (query: CloudWatchQuery) => {
const { onChange, onRunQuery } = props;
@ -83,7 +83,7 @@ export const MetricsQueryEditor = (props: Props) => {
if (!sqlCodeEditorIsDirty) {
setSQLCodeEditorIsDirty(true);
}
props.onChange({ ...preparedQuery, sqlExpression });
props.onChange({ ...migratedQuery, sqlExpression });
}}
onRunQuery={onRunQuery}
datasource={datasource}
@ -114,7 +114,7 @@ export const MetricsQueryEditor = (props: Props) => {
<Input
id={`${query.refId}-cloudwatch-metric-query-editor-id`}
onBlur={onRunQuery}
onChange={(event: ChangeEvent<HTMLInputElement>) => onChange({ ...preparedQuery, id: event.target.value })}
onChange={(event: ChangeEvent<HTMLInputElement>) => onChange({ ...migratedQuery, id: event.target.value })}
type="text"
value={query.id}
/>
@ -127,7 +127,7 @@ export const MetricsQueryEditor = (props: Props) => {
placeholder="auto"
onBlur={onRunQuery}
onChange={(event: ChangeEvent<HTMLInputElement>) =>
onChange({ ...preparedQuery, period: event.target.value })
onChange({ ...migratedQuery, period: event.target.value })
}
/>
</EditorField>
@ -142,7 +142,7 @@ export const MetricsQueryEditor = (props: Props) => {
<DynamicLabelsField
width={52}
onRunQuery={onRunQuery}
label={preparedQuery.label ?? ''}
label={migratedQuery.label ?? ''}
onChange={(label) => props.onChange({ ...query, label })}
></DynamicLabelsField>
</EditorField>
@ -155,8 +155,8 @@ export const MetricsQueryEditor = (props: Props) => {
>
<Alias
id={`${query.refId}-cloudwatch-metric-query-editor-alias`}
value={preparedQuery.alias ?? ''}
onChange={(value: string) => onChange({ ...preparedQuery, alias: value })}
value={migratedQuery.alias ?? ''}
onChange={(value: string) => onChange({ ...migratedQuery, alias: value })}
/>
</EditorField>
)}

View File

@ -1,76 +0,0 @@
import { renderHook } from '@testing-library/react-hooks';
import { CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType } from '../../types';
import usePreparedMetricsQuery, { DEFAULT_QUERY } from './usePreparedMetricsQuery';
interface TestScenario {
name: string;
query: any;
expectedQuery: CloudWatchMetricsQuery;
}
const baseQuery: CloudWatchMetricsQuery = {
refId: 'A',
id: '',
region: 'us-east-2',
namespace: 'AWS/EC2',
dimensions: { InstanceId: 'x-123' },
};
describe('usePrepareMetricsQuery', () => {
describe('when an incomplete query is provided', () => {
const testTable: TestScenario[] = [
{ name: 'Empty query', query: { refId: 'A' }, expectedQuery: { ...DEFAULT_QUERY, refId: 'A' } },
{
name: 'Match exact is not part of the query',
query: { ...baseQuery },
expectedQuery: { ...DEFAULT_QUERY, ...baseQuery, matchExact: true },
},
{
name: 'Match exact is part of the query',
query: { ...baseQuery, matchExact: false },
expectedQuery: { ...DEFAULT_QUERY, ...baseQuery, matchExact: false },
},
{
name: 'When editor mode and builder mode different from default is specified',
query: { ...baseQuery, metricQueryType: MetricQueryType.Query, metricEditorMode: MetricEditorMode.Code },
expectedQuery: {
...DEFAULT_QUERY,
...baseQuery,
metricQueryType: MetricQueryType.Query,
metricEditorMode: MetricEditorMode.Code,
},
},
];
describe.each(testTable)('scenario %#: $name', (scenario) => {
it('should set the default values and trigger onChangeQuery', async () => {
const onChangeQuery = jest.fn();
const { result } = renderHook(() => usePreparedMetricsQuery(scenario.query, onChangeQuery));
expect(onChangeQuery).toHaveBeenLastCalledWith(result.current);
expect(result.current).toEqual(scenario.expectedQuery);
});
});
});
describe('when a complete query is provided', () => {
it('should not change the query and should not call onChangeQuery', async () => {
const onChangeQuery = jest.fn();
const completeQuery: CloudWatchMetricsQuery = {
...baseQuery,
expression: '',
queryMode: 'Metrics',
metricName: '',
statistic: 'Sum',
period: '300',
metricQueryType: MetricQueryType.Query,
metricEditorMode: MetricEditorMode.Code,
sqlExpression: 'SELECT 1',
matchExact: false,
};
const { result } = renderHook(() => usePreparedMetricsQuery(completeQuery, onChangeQuery));
expect(onChangeQuery).not.toHaveBeenCalled();
expect(result.current).toEqual(completeQuery);
});
});
});

View File

@ -1,50 +0,0 @@
import deepEqual from 'fast-deep-equal';
import { useEffect, useMemo } from 'react';
import { migrateMetricQuery } from '../../migrations/metricQueryMigrations';
import { CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType } from '../../types';
export const DEFAULT_QUERY: Omit<CloudWatchMetricsQuery, 'refId'> = {
queryMode: 'Metrics',
namespace: '',
metricName: '',
expression: '',
dimensions: {},
region: 'default',
id: '',
statistic: 'Average',
period: '',
metricQueryType: MetricQueryType.Search,
metricEditorMode: MetricEditorMode.Builder,
sqlExpression: '',
matchExact: true,
};
const prepareQuery = (query: CloudWatchMetricsQuery) => {
const withDefaults = { ...DEFAULT_QUERY, ...query };
const migratedQuery = migrateMetricQuery(withDefaults);
// If we didn't make any changes to the object, then return the original object to keep the
// identity the same, and not trigger any other useEffects or anything.
return deepEqual(migratedQuery, query) ? query : migratedQuery;
};
/**
* Returns queries with some defaults + migrations, and calls onChange function to notify if it changes
*/
const usePreparedMetricsQuery = (
query: CloudWatchMetricsQuery,
onChangeQuery: (newQuery: CloudWatchMetricsQuery) => void
) => {
const preparedQuery = useMemo(() => prepareQuery(query), [query]);
useEffect(() => {
if (preparedQuery !== query) {
onChangeQuery(preparedQuery);
}
}, [preparedQuery, query, onChangeQuery]);
return preparedQuery;
};
export default usePreparedMetricsQuery;

View File

@ -1,4 +1,3 @@
import { pick } from 'lodash';
import React from 'react';
import { SelectableValue, ExploreMode } from '@grafana/data';
@ -34,9 +33,8 @@ const QueryHeader: React.FC<QueryHeaderProps> = ({ query, sqlCodeEditorIsDirty,
const onQueryModeChange = ({ value }: SelectableValue<CloudWatchQueryMode>) => {
if (value !== queryMode) {
const commonProps = pick(query, 'id', 'region', 'namespace', 'refId', 'hide', 'key', 'queryType', 'datasource');
onChange({
...commonProps,
...query,
queryMode: value,
} as CloudWatchQuery);
}

View File

@ -1,9 +1,10 @@
import { lastValueFrom } from 'rxjs';
import { toArray } from 'rxjs/operators';
import { dateTime, Field } from '@grafana/data';
import { CoreApp, dateTime, Field } from '@grafana/data';
import {
CloudWatchSettings,
fieldsVariable,
logGroupNamesVariable,
regionVariable,
@ -12,7 +13,14 @@ import {
import { setupForLogs } from './__mocks__/logsTestContext';
import { validLogsQuery, validMetricSearchBuilderQuery } from './__mocks__/queries';
import { TimeRangeMock } from './__mocks__/timeRange';
import { CloudWatchLogsQuery, CloudWatchMetricsQuery, CloudWatchQuery } from './types';
import {
CloudWatchLogsQuery,
CloudWatchMetricsQuery,
CloudWatchQuery,
CloudWatchDefaultQuery,
MetricEditorMode,
MetricQueryType,
} from './types';
describe('datasource', () => {
beforeEach(() => {
@ -338,4 +346,34 @@ describe('datasource', () => {
expect(templateService.getVariableName).toHaveBeenCalledTimes(1);
});
});
describe('when setting default query', () => {
it('should set default query to be a Metrics query', () => {
const { datasource } = setupMockedDataSource();
expect(datasource.getDefaultQuery(CoreApp.PanelEditor).queryMode).toEqual('Metrics');
});
it('should set default log groups in default query', () => {
const { datasource } = setupMockedDataSource({
customInstanceSettings: {
...CloudWatchSettings,
jsonData: { ...CloudWatchSettings.jsonData, defaultLogGroups: ['testLogGroup'] },
},
});
expect((datasource.getDefaultQuery(CoreApp.PanelEditor) as CloudWatchDefaultQuery).logGroupNames).toEqual([
'testLogGroup',
]);
});
it('should set default values from metrics query', () => {
const { datasource } = setupMockedDataSource();
expect(datasource.getDefaultQuery(CoreApp.PanelEditor).region).toEqual('default');
expect((datasource.getDefaultQuery(CoreApp.PanelEditor) as CloudWatchDefaultQuery).statistic).toEqual('Average');
expect((datasource.getDefaultQuery(CoreApp.PanelEditor) as CloudWatchDefaultQuery).metricQueryType).toEqual(
MetricQueryType.Search
);
expect((datasource.getDefaultQuery(CoreApp.PanelEditor) as CloudWatchDefaultQuery).metricEditorMode).toEqual(
MetricEditorMode.Builder
);
expect((datasource.getDefaultQuery(CoreApp.PanelEditor) as CloudWatchDefaultQuery).matchExact).toEqual(true);
});
});
});

View File

@ -2,6 +2,7 @@ import { cloneDeep, find, isEmpty } from 'lodash';
import { merge, Observable, of } from 'rxjs';
import {
CoreApp,
DataFrame,
DataQueryRequest,
DataQueryResponse,
@ -20,6 +21,7 @@ import { RowContextOptions } from '../../../features/logs/components/LogRowConte
import { CloudWatchAnnotationSupport } from './annotationSupport';
import { CloudWatchAPI } from './api';
import { SQLCompletionItemProvider } from './cloudwatch-sql/completion/CompletionItemProvider';
import { DEFAULT_METRICS_QUERY, getDefaultLogsQuery } from './defaultQueries';
import { isCloudWatchAnnotationQuery, isCloudWatchLogsQuery, isCloudWatchMetricsQuery } from './guards';
import { CloudWatchLanguageProvider } from './language_provider';
import { MetricMathCompletionItemProvider } from './metric-math/completion/CompletionItemProvider';
@ -43,6 +45,7 @@ export class CloudWatchDatasource
languageProvider: CloudWatchLanguageProvider;
sqlCompletionItemProvider: SQLCompletionItemProvider;
metricMathCompletionItemProvider: MetricMathCompletionItemProvider;
defaultLogGroups?: string[];
type = 'cloudwatch';
@ -67,6 +70,7 @@ export class CloudWatchDatasource
this.annotationQueryRunner = new CloudWatchAnnotationQueryRunner(instanceSettings, templateSrv);
this.variables = new CloudWatchVariableSupport(this.api);
this.annotations = CloudWatchAnnotationSupport;
this.defaultLogGroups = instanceSettings.jsonData.defaultLogGroups;
}
filterQuery(query: CloudWatchQuery) {
@ -173,4 +177,11 @@ export class CloudWatchDatasource
}
return region;
}
getDefaultQuery(_: CoreApp): Partial<CloudWatchQuery> {
return {
...getDefaultLogsQuery(this.defaultLogGroups),
...DEFAULT_METRICS_QUERY,
};
}
}

View File

@ -0,0 +1,24 @@
import { CloudWatchLogsQuery, CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType } from './types';
export const DEFAULT_METRICS_QUERY: Omit<CloudWatchMetricsQuery, 'refId'> = {
queryMode: 'Metrics',
namespace: '',
metricName: '',
expression: '',
dimensions: {},
region: 'default',
id: '',
statistic: 'Average',
period: '',
metricQueryType: MetricQueryType.Search,
metricEditorMode: MetricEditorMode.Builder,
sqlExpression: '',
matchExact: true,
};
export const getDefaultLogsQuery = (defaultLogGroups?: string[]): Omit<CloudWatchLogsQuery, 'refId' | 'queryMode'> => ({
id: '',
region: 'default',
expression: '',
logGroupNames: defaultLogGroups,
});

View File

@ -1,3 +1,5 @@
import deepEqual from 'fast-deep-equal';
import { config } from '@grafana/runtime';
import { CloudWatchMetricsQuery } from '../types';
@ -6,7 +8,7 @@ import { CloudWatchMetricsQuery } from '../types';
export function migrateMetricQuery(query: CloudWatchMetricsQuery): CloudWatchMetricsQuery {
//add metric query migrations here
const migratedQuery = migrateAliasPatterns(query);
return migratedQuery;
return deepEqual(migratedQuery, query) ? query : migratedQuery;
}
const aliasPatterns: Record<string, string> = {
@ -20,16 +22,19 @@ const aliasPatterns: Record<string, string> = {
export function migrateAliasPatterns(query: CloudWatchMetricsQuery): CloudWatchMetricsQuery {
if (config.featureToggles.cloudWatchDynamicLabels && !query.hasOwnProperty('label')) {
const regex = /{{\s*(.+?)\s*}}/g;
query.label =
query.alias?.replace(regex, (_, value) => {
if (aliasPatterns.hasOwnProperty(value)) {
return `\${${aliasPatterns[value]}}`;
}
const newQuery = { ...query };
if (!query.hasOwnProperty('label')) {
const regex = /{{\s*(.+?)\s*}}/g;
newQuery.label =
query.alias?.replace(regex, (_, value) => {
if (aliasPatterns.hasOwnProperty(value)) {
return `\${${aliasPatterns[value]}}`;
}
return `\${PROP('Dim.${value}')}`;
}) ?? '';
return `\${PROP('Dim.${value}')}`;
}) ?? '';
}
return newQuery;
}
return query;
}

View File

@ -0,0 +1,44 @@
import { renderHook } from '@testing-library/react-hooks';
import { config } from '@grafana/runtime';
import { DEFAULT_METRICS_QUERY } from '../defaultQueries';
import { CloudWatchMetricsQuery } from '../types';
import { migrateAliasPatterns } from './metricQueryMigrations';
import useMigratedMetricsQuery from './useMigratedMetricsQuery';
describe('usePrepareMetricsQuery', () => {
const DEFAULT_TEST_QUERY: CloudWatchMetricsQuery = { ...DEFAULT_METRICS_QUERY, refId: 'testId' };
describe('when dynamic labels are true and there is no label', () => {
const testQuery: CloudWatchMetricsQuery = { ...DEFAULT_TEST_QUERY, alias: 'test' };
it('should replace label with alias and trigger onChangeQuery', async () => {
config.featureToggles.cloudWatchDynamicLabels = true;
const expectedQuery: CloudWatchMetricsQuery = migrateAliasPatterns(testQuery);
const onChangeQuery = jest.fn();
const { result } = renderHook(() => useMigratedMetricsQuery(testQuery, onChangeQuery));
expect(onChangeQuery).toHaveBeenLastCalledWith(result.current);
expect(result.current).toEqual(expectedQuery);
});
});
describe('when query has a label', () => {
config.featureToggles.cloudWatchDynamicLabels = true;
const testQuery: CloudWatchMetricsQuery = { ...DEFAULT_TEST_QUERY, label: 'test' };
it('should not replace label or trigger onChange', async () => {
const onChangeQuery = jest.fn();
const { result } = renderHook(() => useMigratedMetricsQuery(testQuery, onChangeQuery));
expect(result.current).toEqual(testQuery);
expect(onChangeQuery).toHaveBeenCalledTimes(0);
});
});
describe('when dynamic labels feature flag is disabled', () => {
const testQuery: CloudWatchMetricsQuery = { ...DEFAULT_TEST_QUERY };
it('should not replace label or trigger onChange', async () => {
config.featureToggles.cloudWatchDynamicLabels = false;
const onChangeQuery = jest.fn();
const { result } = renderHook(() => useMigratedMetricsQuery(testQuery, onChangeQuery));
expect(result.current).toEqual(testQuery);
expect(onChangeQuery).toHaveBeenCalledTimes(0);
});
});
});

View File

@ -0,0 +1,25 @@
import { useEffect, useMemo } from 'react';
import { CloudWatchMetricsQuery } from '../types';
import { migrateMetricQuery } from './metricQueryMigrations';
/**
* Returns queries with migrations, and calls onChange function to notify if it changes
*/
const useMigratedMetricsQuery = (
query: CloudWatchMetricsQuery,
onChangeQuery: (newQuery: CloudWatchMetricsQuery) => void
) => {
const migratedQUery = useMemo(() => migrateMetricQuery(query), [query]);
useEffect(() => {
if (migratedQUery !== query) {
onChangeQuery(migratedQUery);
}
}, [migratedQUery, query, onChangeQuery]);
return migratedQUery;
};
export default useMigratedMetricsQuery;

View File

@ -105,8 +105,14 @@ export interface CloudWatchLogsQuery extends DataQuery {
/* not quite deprecated yet, but will be soon */
logGroupNames?: string[];
}
// We want to allow setting defaults for both Logs and Metrics queries
export type CloudWatchDefaultQuery = Omit<CloudWatchLogsQuery, 'queryMode'> & CloudWatchMetricsQuery;
export type CloudWatchQuery = CloudWatchMetricsQuery | CloudWatchLogsQuery | CloudWatchAnnotationQuery;
export type CloudWatchQuery =
| CloudWatchMetricsQuery
| CloudWatchLogsQuery
| CloudWatchAnnotationQuery
| CloudWatchDefaultQuery;
export interface CloudWatchAnnotationQuery extends MetricStat, DataQuery {
queryMode: 'Annotations';