diff --git a/pkg/tsdb/elasticsearch/client/models.go b/pkg/tsdb/elasticsearch/client/models.go index ba0179331d5..097a5a904b3 100644 --- a/pkg/tsdb/elasticsearch/client/models.go +++ b/pkg/tsdb/elasticsearch/client/models.go @@ -280,8 +280,10 @@ type MetricAggregation struct { // MarshalJSON returns the JSON encoding of the metric aggregation func (a *MetricAggregation) MarshalJSON() ([]byte, error) { - root := map[string]interface{}{ - "field": a.Field, + root := map[string]interface{}{} + + if a.Field != "" { + root["field"] = a.Field } for k, v := range a.Settings { diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/BucketAggregationEditor.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/BucketAggregationEditor.tsx index c16f214895f..d9ec69460ff 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/BucketAggregationEditor.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/BucketAggregationEditor.tsx @@ -1,8 +1,8 @@ -import { MetricFindValue, SelectableValue } from '@grafana/data'; +import { SelectableValue } from '@grafana/data'; import { InlineSegmentGroup, Segment, SegmentAsync } from '@grafana/ui'; import React, { FunctionComponent } from 'react'; +import { useFields } from '../../../hooks/useFields'; import { useDispatch } from '../../../hooks/useStatelessReducer'; -import { useDatasource } from '../ElasticsearchQueryContext'; import { segmentStyles } from '../styles'; import { BucketAggregation, BucketAggregationType, isBucketAggregationWithField } from './aggregations'; import { SettingsEditor } from './SettingsEditor'; @@ -17,11 +17,6 @@ const bucketAggOptions: Array> = Object.e }) ); -const toSelectableValue = ({ value, text }: MetricFindValue): SelectableValue => ({ - label: text, - value: `${value || text}`, -}); - const toOption = (bucketAgg: BucketAggregation) => ({ label: bucketAggregationConfig[bucketAgg.type].label, value: bucketAgg.type, @@ -32,24 +27,8 @@ interface QueryMetricEditorProps { } export const BucketAggregationEditor: FunctionComponent = ({ value }) => { - const datasource = useDatasource(); const dispatch = useDispatch(); - - // TODO: Move this in a separate hook (and simplify) - const getFields = async () => { - const get = () => { - switch (value.type) { - case 'date_histogram': - return datasource.getFields('date'); - case 'geohash_grid': - return datasource.getFields('geo_point'); - default: - return datasource.getFields(); - } - }; - - return (await get().toPromise()).map(toSelectableValue); - }; + const getFields = useFields(value.type); return ( <> diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.test.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.test.tsx index fce0ac04548..7409082ff01 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.test.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.test.tsx @@ -1,9 +1,10 @@ import React, { FunctionComponent } from 'react'; import { renderHook } from '@testing-library/react-hooks'; import { render } from '@testing-library/react'; -import { ElasticsearchProvider, useDatasource, useQuery } from './ElasticsearchQueryContext'; +import { ElasticsearchProvider, useQuery } from './ElasticsearchQueryContext'; import { ElasticsearchQuery } from '../../types'; import { ElasticDatasource } from '../../datasource'; +import { getDefaultTimeRange } from '@grafana/data'; const query: ElasticsearchQuery = { refId: 'A', @@ -24,6 +25,7 @@ describe('ElasticsearchQueryContext', () => { onChange={onChange} datasource={datasource} onRunQuery={onRunQuery} + range={getDefaultTimeRange()} /> ); @@ -39,6 +41,7 @@ describe('ElasticsearchQueryContext', () => { expect(onRunQuery).toHaveBeenCalled(); }); + // the following applies to all hooks in ElasticsearchQueryContext as they all share the same code. describe('useQuery Hook', () => { it('Should throw when used outside of ElasticsearchQueryContext', () => { const { result } = renderHook(() => useQuery()); @@ -53,6 +56,7 @@ describe('ElasticsearchQueryContext', () => { query={query} onChange={() => {}} onRunQuery={() => {}} + range={getDefaultTimeRange()} > {children} @@ -65,28 +69,4 @@ describe('ElasticsearchQueryContext', () => { expect(result.current).toBe(query); }); }); - - describe('useDatasource Hook', () => { - it('Should throw when used outside of ElasticsearchQueryContext', () => { - const { result } = renderHook(() => useDatasource()); - - expect(result.error).toBeTruthy(); - }); - - it('Should return the current datasource instance', () => { - const datasource = {} as ElasticDatasource; - - const wrapper: FunctionComponent = ({ children }) => ( - {}} onRunQuery={() => {}}> - {children} - - ); - - const { result } = renderHook(() => useDatasource(), { - wrapper, - }); - - expect(result.current).toBe(datasource); - }); - }); }); diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.tsx index e05a73e8120..f8bcabfd158 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/ElasticsearchQueryContext.tsx @@ -1,4 +1,4 @@ -import React, { createContext, FunctionComponent, useCallback, useContext } from 'react'; +import React, { Context, createContext, FunctionComponent, useCallback, useContext } from 'react'; import { ElasticDatasource } from '../../datasource'; import { combineReducers, useStatelessReducer, DispatchContext } from '../../hooks/useStatelessReducer'; import { ElasticsearchQuery } from '../../types'; @@ -6,15 +6,18 @@ import { ElasticsearchQuery } from '../../types'; import { reducer as metricsReducer } from './MetricAggregationsEditor/state/reducer'; import { reducer as bucketAggsReducer } from './BucketAggregationsEditor/state/reducer'; import { aliasPatternReducer, queryReducer, initQuery } from './state'; +import { TimeRange } from '@grafana/data'; const DatasourceContext = createContext(undefined); const QueryContext = createContext(undefined); +const RangeContext = createContext(undefined); interface Props { query: ElasticsearchQuery; onChange: (query: ElasticsearchQuery) => void; onRunQuery: () => void; datasource: ElasticDatasource; + range: TimeRange; } export const ElasticsearchProvider: FunctionComponent = ({ @@ -23,6 +26,7 @@ export const ElasticsearchProvider: FunctionComponent = ({ onRunQuery, query, datasource, + range, }) => { const onStateChange = useCallback( (query: ElasticsearchQuery) => { @@ -57,27 +61,28 @@ export const ElasticsearchProvider: FunctionComponent = ({ return ( - {children} + + {children} + ); }; -export const useQuery = (): ElasticsearchQuery => { - const query = useContext(QueryContext); - - if (!query) { - throw new Error('use ElasticsearchProvider first.'); - } +interface GetHook { + (context: Context): () => NonNullable; +} - return query; -}; +const getHook: GetHook = (c) => () => { + const contextValue = useContext(c); -export const useDatasource = () => { - const datasource = useContext(DatasourceContext); - if (!datasource) { + if (!contextValue) { throw new Error('use ElasticsearchProvider first.'); } - return datasource; + return contextValue as NonNullable; }; + +export const useQuery = getHook(QueryContext); +export const useDatasource = getHook(DatasourceContext); +export const useRange = getHook(RangeContext); diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.test.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.test.tsx new file mode 100644 index 00000000000..38770aeda5e --- /dev/null +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.test.tsx @@ -0,0 +1,85 @@ +import { act, fireEvent, render, screen } from '@testing-library/react'; +import { ElasticsearchProvider } from '../ElasticsearchQueryContext'; +import { MetricEditor } from './MetricEditor'; +import React, { ReactNode } from 'react'; +import { ElasticDatasource } from '../../../datasource'; +import { getDefaultTimeRange } from '@grafana/data'; +import { ElasticsearchQuery } from '../../../types'; +import { Average, UniqueCount } from './aggregations'; +import { defaultBucketAgg } from '../../../query_def'; +import { from } from 'rxjs'; + +describe('Metric Editor', () => { + it('Should display a "None" option for "field" if the metric supports inline script', async () => { + const avg: Average = { + id: '1', + type: 'avg', + }; + + const query: ElasticsearchQuery = { + refId: 'A', + query: '', + metrics: [avg], + bucketAggs: [defaultBucketAgg('2')], + }; + + const getFields: ElasticDatasource['getFields'] = jest.fn(() => from([[]])); + + const wrapper = ({ children }: { children: ReactNode }) => ( + {}} + onRunQuery={() => {}} + > + {children} + + ); + + render(, { wrapper }); + + act(() => { + fireEvent.click(screen.getByText('Select Field')); + }); + + expect(await screen.findByText('None')).toBeInTheDocument(); + }); + + it('Should not display a "None" option for "field" if the metric does not support inline script', async () => { + const avg: UniqueCount = { + id: '1', + type: 'cardinality', + }; + + const query: ElasticsearchQuery = { + refId: 'A', + query: '', + metrics: [avg], + bucketAggs: [defaultBucketAgg('2')], + }; + + const getFields: ElasticDatasource['getFields'] = jest.fn(() => from([[]])); + + const wrapper = ({ children }: { children: ReactNode }) => ( + {}} + onRunQuery={() => {}} + > + {children} + + ); + + render(, { wrapper }); + + act(() => { + fireEvent.click(screen.getByText('Select Field')); + }); + + expect(await screen.findByText('No options found')).toBeInTheDocument(); + expect(screen.queryByText('None')).not.toBeInTheDocument(); + }); +}); diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx index 06f025bdca2..a112dca1ec9 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/MetricEditor.tsx @@ -1,7 +1,7 @@ -import { MetricFindValue, SelectableValue } from '@grafana/data'; +import { SelectableValue } from '@grafana/data'; import { InlineSegmentGroup, Segment, SegmentAsync, useTheme } from '@grafana/ui'; import { cx } from '@emotion/css'; -import React, { FunctionComponent } from 'react'; +import React, { FunctionComponent, useCallback } from 'react'; import { useDatasource, useQuery } from '../ElasticsearchQueryContext'; import { useDispatch } from '../../../hooks/useStatelessReducer'; import { getStyles } from './styles'; @@ -13,23 +13,20 @@ import { MetricPicker } from '../../MetricPicker'; import { segmentStyles } from '../styles'; import { isMetricAggregationWithField, + isMetricAggregationWithInlineScript, isMetricAggregationWithSettings, isPipelineAggregation, isPipelineAggregationWithMultipleBucketPaths, MetricAggregation, MetricAggregationType, } from './aggregations'; +import { useFields } from '../../../hooks/useFields'; const toOption = (metric: MetricAggregation) => ({ label: metricAggregationConfig[metric.type].label, value: metric.type, }); -const toSelectableValue = ({ value, text }: MetricFindValue): SelectableValue => ({ - label: text, - value: `${value || text}`, -}); - interface Props { value: MetricAggregation; } @@ -68,24 +65,24 @@ export const MetricEditor: FunctionComponent = ({ value }) => { const datasource = useDatasource(); const query = useQuery(); const dispatch = useDispatch(); + const getFields = useFields(value.type); + + const loadOptions = useCallback(async () => { + const remoteFields = await getFields(); + + // Metric aggregations that have inline script support don't require a field to be set. + if (isMetricAggregationWithInlineScript(value)) { + return [{ label: 'None' }, ...remoteFields]; + } + + return remoteFields; + }, [getFields, value]); const previousMetrics = query.metrics!.slice( 0, query.metrics!.findIndex((m) => m.id === value.id) ); - // TODO: This could be common with the one in BucketAggregationEditor - const getFields = async () => { - const get = () => { - if (value.type === 'cardinality') { - return datasource.getFields(); - } - return datasource.getFields('number'); - }; - - return (await get().toPromise()).map(toSelectableValue); - }; - return ( <> @@ -99,7 +96,7 @@ export const MetricEditor: FunctionComponent = ({ value }) => { {isMetricAggregationWithField(value) && !isPipelineAggregation(value) && ( dispatch(changeMetricField(value.id, e.value!))} placeholder="Select Field" value={value.field} diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.test.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.test.tsx index e8046ae5466..71849b4bd78 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.test.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/SettingsEditor/index.test.tsx @@ -4,6 +4,7 @@ import { SettingsEditor } from '.'; import { ElasticsearchProvider } from '../../ElasticsearchQueryContext'; import { ElasticDatasource } from '../../../../datasource'; import { ElasticsearchQuery } from '../../../../types'; +import { getDefaultTimeRange } from '@grafana/data'; describe('Settings Editor', () => { describe('Raw Data', () => { @@ -33,6 +34,7 @@ describe('Settings Editor', () => { datasource={{} as ElasticDatasource} onChange={onChange} onRunQuery={() => {}} + range={getDefaultTimeRange()} > @@ -67,6 +69,7 @@ describe('Settings Editor', () => { datasource={{} as ElasticDatasource} onChange={onChange} onRunQuery={() => {}} + range={getDefaultTimeRange()} > diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx index 6e759d34c4a..119b4d9efa7 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/index.tsx @@ -1,5 +1,5 @@ import React, { FunctionComponent } from 'react'; -import { QueryEditorProps } from '@grafana/data'; +import { getDefaultTimeRange, QueryEditorProps } from '@grafana/data'; import { ElasticDatasource } from '../../datasource'; import { ElasticsearchOptions, ElasticsearchQuery } from '../../types'; import { ElasticsearchProvider } from './ElasticsearchQueryContext'; @@ -17,8 +17,15 @@ export const QueryEditor: FunctionComponent = ({ onChange, onRunQuery, datasource, + range, }) => ( - + ); diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index e24088ed8f8..a6081768ef7 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -619,6 +619,8 @@ export class ElasticDatasource extends DataSourceApi { const configuredEsVersion = this.esVersion; return this.get('/_mapping', range).pipe( diff --git a/public/app/plugins/datasource/elasticsearch/hooks/useFields.test.tsx b/public/app/plugins/datasource/elasticsearch/hooks/useFields.test.tsx new file mode 100644 index 00000000000..7b3fb059465 --- /dev/null +++ b/public/app/plugins/datasource/elasticsearch/hooks/useFields.test.tsx @@ -0,0 +1,73 @@ +import React, { ReactNode } from 'react'; +import { ElasticDatasource } from '../datasource'; +import { from } from 'rxjs'; +import { ElasticsearchProvider } from '../components/QueryEditor/ElasticsearchQueryContext'; +import { getDefaultTimeRange } from '@grafana/data'; +import { ElasticsearchQuery } from '../types'; +import { defaultBucketAgg, defaultMetricAgg } from '../query_def'; +import { renderHook } from '@testing-library/react-hooks'; +import { useFields } from './useFields'; +import { MetricAggregationType } from '../components/QueryEditor/MetricAggregationsEditor/aggregations'; +import { BucketAggregationType } from '../components/QueryEditor/BucketAggregationsEditor/aggregations'; + +describe('useFields hook', () => { + // TODO: If we move the field type to the configuration objects as described in the hook's source + // we can stop testing for getField to be called with the correct parameters. + it("returns a function that calls datasource's getFields with the correct parameters", async () => { + const timeRange = getDefaultTimeRange(); + const query: ElasticsearchQuery = { + refId: 'A', + query: '', + metrics: [defaultMetricAgg()], + bucketAggs: [defaultBucketAgg()], + }; + + const getFields: ElasticDatasource['getFields'] = jest.fn(() => from([[]])); + + const wrapper = ({ children }: { children: ReactNode }) => ( + {}} + onRunQuery={() => {}} + > + {children} + + ); + + // + // METRIC AGGREGATIONS + // + // Cardinality works on every kind of data + const { result, rerender } = renderHook( + (aggregationType: BucketAggregationType | MetricAggregationType) => useFields(aggregationType), + { wrapper, initialProps: 'cardinality' } + ); + result.current(); + expect(getFields).toHaveBeenLastCalledWith(undefined, timeRange); + + // All other metric aggregations only work on numbers + rerender('avg'); + result.current(); + expect(getFields).toHaveBeenLastCalledWith('number', timeRange); + + // + // BUCKET AGGREGATIONS + // + // Date Histrogram only works on dates + rerender('date_histogram'); + result.current(); + expect(getFields).toHaveBeenLastCalledWith('date', timeRange); + + // Geohash Grid only works on geo_point data + rerender('geohash_grid'); + result.current(); + expect(getFields).toHaveBeenLastCalledWith('geo_point', timeRange); + + // All other bucket aggregation work on any kind of data + rerender('terms'); + result.current(); + expect(getFields).toHaveBeenLastCalledWith(undefined, timeRange); + }); +}); diff --git a/public/app/plugins/datasource/elasticsearch/hooks/useFields.ts b/public/app/plugins/datasource/elasticsearch/hooks/useFields.ts new file mode 100644 index 00000000000..f8f3b49643d --- /dev/null +++ b/public/app/plugins/datasource/elasticsearch/hooks/useFields.ts @@ -0,0 +1,53 @@ +import { MetricFindValue, SelectableValue } from '@grafana/data'; +import { BucketAggregationType } from '../components/QueryEditor/BucketAggregationsEditor/aggregations'; +import { useDatasource, useRange } from '../components/QueryEditor/ElasticsearchQueryContext'; +import { + isMetricAggregationType, + MetricAggregationType, +} from '../components/QueryEditor/MetricAggregationsEditor/aggregations'; + +type AggregationType = BucketAggregationType | MetricAggregationType; + +const getFilter = (aggregationType: AggregationType) => { + // For all metric types we want only numbers, except for cardinality + // TODO: To have a more configuration-driven editor, it would be nice to move this logic in + // metricAggregationConfig and bucketAggregationConfig so that each aggregation type can specify on + // which kind of data it operates. + if (isMetricAggregationType(aggregationType)) { + if (aggregationType !== 'cardinality') { + return 'number'; + } + + return void 0; + } + + switch (aggregationType) { + case 'date_histogram': + return 'date'; + case 'geohash_grid': + return 'geo_point'; + default: + return void 0; + } +}; + +const toSelectableValue = ({ text }: MetricFindValue): SelectableValue => ({ + label: text, + value: text, +}); + +/** + * Returns a function to query the configured datasource for autocomplete values for the specified aggregation type. + * Each aggregation can be run on different types, for example avg only operates on numeric fields, geohash_grid only on geo_point fields. + * @param aggregationType the type of aggregation to get fields for + */ +export const useFields = (aggregationType: AggregationType) => { + const datasource = useDatasource(); + const range = useRange(); + const filter = getFilter(aggregationType); + + return async () => { + const rawFields = await datasource.getFields(filter, range).toPromise(); + return rawFields.map(toSelectableValue); + }; +}; diff --git a/public/app/plugins/datasource/elasticsearch/hooks/useNextId.test.tsx b/public/app/plugins/datasource/elasticsearch/hooks/useNextId.test.tsx index 83a74e23e6b..4d2d99ef14e 100644 --- a/public/app/plugins/datasource/elasticsearch/hooks/useNextId.test.tsx +++ b/public/app/plugins/datasource/elasticsearch/hooks/useNextId.test.tsx @@ -3,6 +3,7 @@ import { renderHook } from '@testing-library/react-hooks'; import { ElasticsearchProvider } from '../components/QueryEditor/ElasticsearchQueryContext'; import { useNextId } from './useNextId'; import { ElasticsearchQuery } from '../types'; +import { getDefaultTimeRange } from '@grafana/data'; describe('useNextId', () => { it('Should return the next available id', () => { @@ -14,7 +15,13 @@ describe('useNextId', () => { }; const wrapper: FunctionComponent = ({ children }) => { return ( - {}} onRunQuery={() => {}}> + {}} + onRunQuery={() => {}} + range={getDefaultTimeRange()} + > {children} );