Elasticsearch: Fix broken alerting when using pipeline aggregations (#29903)

* Elasticsearch: Fix broken alerting when using pipeline aggregations

* call onRunQuery after every query change
This commit is contained in:
Giordano Ricci
2021-01-06 15:51:21 +00:00
committed by GitHub
parent 52f79c3625
commit c77fb9fa13
7 changed files with 71 additions and 14 deletions

View File

@@ -12,11 +12,19 @@ const query: ElasticsearchQuery = {
}; };
describe('ElasticsearchQueryContext', () => { describe('ElasticsearchQueryContext', () => {
it('Should call onChange with the default query when the query is empty', () => { it('Should call onChange and onRunQuery with the default query when the query is empty', () => {
const datasource = { timeField: 'TIMEFIELD' } as ElasticDatasource; const datasource = { timeField: 'TIMEFIELD' } as ElasticDatasource;
const onChange = jest.fn(); const onChange = jest.fn();
const onRunQuery = jest.fn();
render(<ElasticsearchProvider query={{ refId: 'A' }} onChange={onChange} datasource={datasource} />); render(
<ElasticsearchProvider
query={{ refId: 'A' }}
onChange={onChange}
datasource={datasource}
onRunQuery={onRunQuery}
/>
);
const changedQuery: ElasticsearchQuery = onChange.mock.calls[0][0]; const changedQuery: ElasticsearchQuery = onChange.mock.calls[0][0];
expect(changedQuery.query).toBeDefined(); expect(changedQuery.query).toBeDefined();
@@ -26,6 +34,8 @@ describe('ElasticsearchQueryContext', () => {
// Should also set timeField to the configured `timeField` option in datasource configuration // Should also set timeField to the configured `timeField` option in datasource configuration
expect(changedQuery.timeField).toBe(datasource.timeField); expect(changedQuery.timeField).toBe(datasource.timeField);
expect(onRunQuery).toHaveBeenCalled();
}); });
describe('useQuery Hook', () => { describe('useQuery Hook', () => {
@@ -37,7 +47,12 @@ describe('ElasticsearchQueryContext', () => {
it('Should return the current query object', () => { it('Should return the current query object', () => {
const wrapper: FunctionComponent = ({ children }) => ( const wrapper: FunctionComponent = ({ children }) => (
<ElasticsearchProvider datasource={{} as ElasticDatasource} query={query} onChange={() => {}}> <ElasticsearchProvider
datasource={{} as ElasticDatasource}
query={query}
onChange={() => {}}
onRunQuery={() => {}}
>
{children} {children}
</ElasticsearchProvider> </ElasticsearchProvider>
); );
@@ -61,7 +76,7 @@ describe('ElasticsearchQueryContext', () => {
const datasource = {} as ElasticDatasource; const datasource = {} as ElasticDatasource;
const wrapper: FunctionComponent = ({ children }) => ( const wrapper: FunctionComponent = ({ children }) => (
<ElasticsearchProvider datasource={datasource} query={query} onChange={() => {}}> <ElasticsearchProvider datasource={datasource} query={query} onChange={() => {}} onRunQuery={() => {}}>
{children} {children}
</ElasticsearchProvider> </ElasticsearchProvider>
); );

View File

@@ -1,4 +1,4 @@
import React, { createContext, FunctionComponent, useContext } from 'react'; import React, { createContext, FunctionComponent, useCallback, useContext } from 'react';
import { ElasticDatasource } from '../../datasource'; import { ElasticDatasource } from '../../datasource';
import { combineReducers, useStatelessReducer, DispatchContext } from '../../hooks/useStatelessReducer'; import { combineReducers, useStatelessReducer, DispatchContext } from '../../hooks/useStatelessReducer';
import { ElasticsearchQuery } from '../../types'; import { ElasticsearchQuery } from '../../types';
@@ -13,10 +13,25 @@ const QueryContext = createContext<ElasticsearchQuery | undefined>(undefined);
interface Props { interface Props {
query: ElasticsearchQuery; query: ElasticsearchQuery;
onChange: (query: ElasticsearchQuery) => void; onChange: (query: ElasticsearchQuery) => void;
onRunQuery: () => void;
datasource: ElasticDatasource; datasource: ElasticDatasource;
} }
export const ElasticsearchProvider: FunctionComponent<Props> = ({ children, onChange, query, datasource }) => { export const ElasticsearchProvider: FunctionComponent<Props> = ({
children,
onChange,
onRunQuery,
query,
datasource,
}) => {
const onStateChange = useCallback(
(query: ElasticsearchQuery) => {
onChange(query);
onRunQuery();
},
[onChange, onRunQuery]
);
const reducer = combineReducers({ const reducer = combineReducers({
query: queryReducer, query: queryReducer,
alias: aliasPatternReducer, alias: aliasPatternReducer,
@@ -26,7 +41,7 @@ export const ElasticsearchProvider: FunctionComponent<Props> = ({ children, onCh
const dispatch = useStatelessReducer( const dispatch = useStatelessReducer(
// timeField is part of the query model, but its value is always set to be the one from datasource settings. // timeField is part of the query model, but its value is always set to be the one from datasource settings.
newState => onChange({ ...query, ...newState, timeField: datasource.timeField }), newState => onStateChange({ ...query, ...newState, timeField: datasource.timeField }),
query, query,
reducer reducer
); );

View File

@@ -150,6 +150,7 @@ interface Logs extends BaseMetricAggregation {
export interface BasePipelineMetricAggregation extends MetricAggregationWithField { export interface BasePipelineMetricAggregation extends MetricAggregationWithField {
type: PipelineMetricAggregationType; type: PipelineMetricAggregationType;
pipelineAgg?: string;
} }
interface PipelineMetricAggregationWithMultipleBucketPaths extends BaseMetricAggregation { interface PipelineMetricAggregationWithMultipleBucketPaths extends BaseMetricAggregation {

View File

@@ -106,7 +106,7 @@ describe('Metric Aggregations Reducer', () => {
it("Should correctly change aggregation's field", () => { it("Should correctly change aggregation's field", () => {
const firstAggregation: MetricAggregation = { const firstAggregation: MetricAggregation = {
id: '1', id: '1',
type: 'count', type: 'min',
}; };
const secondAggregation: MetricAggregation = { const secondAggregation: MetricAggregation = {
id: '2', id: '2',
@@ -116,12 +116,22 @@ describe('Metric Aggregations Reducer', () => {
const expectedSecondAggregation = { const expectedSecondAggregation = {
...secondAggregation, ...secondAggregation,
field: 'new field', field: 'new field',
pipelineAgg: 'new field',
};
const expectedFirstAggregation = {
...firstAggregation,
field: 'new field',
}; };
reducerTester() reducerTester()
.givenReducer(reducer, [firstAggregation, secondAggregation]) .givenReducer(reducer, [firstAggregation, secondAggregation])
// When changing a a pipelineAggregation field we set both pipelineAgg and field
.whenActionIsDispatched(changeMetricField(secondAggregation.id, expectedSecondAggregation.field)) .whenActionIsDispatched(changeMetricField(secondAggregation.id, expectedSecondAggregation.field))
.thenStateShouldEqual([firstAggregation, expectedSecondAggregation]); .thenStateShouldEqual([firstAggregation, expectedSecondAggregation])
// otherwhise only field
.whenActionIsDispatched(changeMetricField(firstAggregation.id, expectedFirstAggregation.field))
.thenStateShouldEqual([expectedFirstAggregation, expectedSecondAggregation]);
}); });
it('Should correctly toggle `hide` field', () => { it('Should correctly toggle `hide` field', () => {

View File

@@ -2,7 +2,12 @@ import { defaultMetricAgg } from '../../../../query_def';
import { ElasticsearchQuery } from '../../../../types'; import { ElasticsearchQuery } from '../../../../types';
import { removeEmpty } from '../../../../utils'; import { removeEmpty } from '../../../../utils';
import { INIT, InitAction } from '../../state'; import { INIT, InitAction } from '../../state';
import { isMetricAggregationWithMeta, isMetricAggregationWithSettings, MetricAggregation } from '../aggregations'; import {
isMetricAggregationWithMeta,
isMetricAggregationWithSettings,
isPipelineAggregation,
MetricAggregation,
} from '../aggregations';
import { getChildren, metricAggregationConfig } from '../utils'; import { getChildren, metricAggregationConfig } from '../utils';
import { import {
ADD_METRIC, ADD_METRIC,
@@ -64,10 +69,16 @@ export const reducer = (
return metric; return metric;
} }
return { const newMetric = {
...metric, ...metric,
field: action.payload.field, field: action.payload.field,
}; };
if (isPipelineAggregation(metric)) {
return { ...newMetric, pipelineAgg: action.payload.field };
}
return newMetric;
}); });
case TOGGLE_METRIC_VISIBILITY: case TOGGLE_METRIC_VISIBILITY:

View File

@@ -12,8 +12,13 @@ import { useNextId } from '../../hooks/useNextId';
export type ElasticQueryEditorProps = QueryEditorProps<ElasticDatasource, ElasticsearchQuery, ElasticsearchOptions>; export type ElasticQueryEditorProps = QueryEditorProps<ElasticDatasource, ElasticsearchQuery, ElasticsearchOptions>;
export const QueryEditor: FunctionComponent<ElasticQueryEditorProps> = ({ query, onChange, datasource }) => ( export const QueryEditor: FunctionComponent<ElasticQueryEditorProps> = ({
<ElasticsearchProvider datasource={datasource} onChange={onChange} query={query}> query,
onChange,
onRunQuery,
datasource,
}) => (
<ElasticsearchProvider datasource={datasource} onChange={onChange} onRunQuery={onRunQuery} query={query}>
<QueryEditorForm value={query} /> <QueryEditorForm value={query} />
</ElasticsearchProvider> </ElasticsearchProvider>
); );

View File

@@ -13,7 +13,7 @@ describe('useNextId', () => {
}; };
const wrapper: FunctionComponent = ({ children }) => { const wrapper: FunctionComponent = ({ children }) => {
return ( return (
<ElasticsearchProvider query={query} datasource={{} as any} onChange={() => {}}> <ElasticsearchProvider query={query} datasource={{} as any} onChange={() => {}} onRunQuery={() => {}}>
{children} {children}
</ElasticsearchProvider> </ElasticsearchProvider>
); );