Skip to content

Commit

Permalink
[Discover][ES]QL] Ensure the same time range is being used for docume…
Browse files Browse the repository at this point in the history
…nts and histogram (#204694)

Fixing the case a relative time range is set (for example "Last 15 minutes") and the time range goes out of sync for table and histogram requests on Discover in ES|QL mode.

Fixes a redundant ES|QL request for histogram data, with different timeranges, when the timerange is changed.
  • Loading branch information
kertal authored Jan 10, 2025
1 parent b37ec3c commit df495ce
Show file tree
Hide file tree
Showing 15 changed files with 160 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@ async function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) {
stateContainer.appState.update({
dataSource: createDataViewDataSource({ dataViewId: dataViewMock.id! }),
});
stateContainer.internalState.transitions.setDataRequestParams({
timeRangeRelative: {
from: '2020-05-14T11:05:13.590',
to: '2020-05-14T11:20:13.590',
},
timeRangeAbsolute: {
from: '2020-05-14T11:05:13.590',
to: '2020-05-14T11:20:13.590',
},
});

stateContainer.dataState.data$.documents$ = documents$;

const props = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import useObservable from 'react-use/lib/useObservable';
import type { DocViewFilterFn } from '@kbn/unified-doc-viewer/types';
import { DiscoverGridSettings } from '@kbn/saved-search-plugin/common';
import { useQuerySubscriber } from '@kbn/unified-field-list';
import { map } from 'rxjs';
import { DiscoverGrid } from '../../../../components/discover_grid';
import { getDefaultRowsPerPage } from '../../../../../common/constants';
import { useInternalStateSelector } from '../../state_management/discover_internal_state_container';
Expand Down Expand Up @@ -112,6 +111,7 @@ function DiscoverDocumentsComponent({
const documents$ = stateContainer.dataState.data$.documents$;
const savedSearch = useSavedSearchInitial();
const { dataViews, capabilities, uiSettings, uiActions, ebtManager, fieldsMetadata } = services;
const requestParams = useInternalStateSelector((state) => state.dataRequestParams);
const [
dataSource,
query,
Expand Down Expand Up @@ -269,20 +269,14 @@ function DiscoverDocumentsComponent({
: undefined,
[documentState.esqlQueryColumns]
);

const { filters } = useQuerySubscriber({ data: services.data });

const timeRange = useObservable(
services.timefilter.getTimeUpdate$().pipe(map(() => services.timefilter.getTime())),
services.timefilter.getTime()
);

const cellActionsMetadata = useAdditionalCellActions({
dataSource,
dataView,
query,
filters,
timeRange,
timeRange: requestParams.timeRangeAbsolute,
});

const renderDocumentView = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,25 @@ import { createDataViewDataSource } from '../../../../../common/data_sources';
function getStateContainer(savedSearch?: SavedSearch) {
const stateContainer = getDiscoverStateMock({ isTimeBased: true, savedSearch });
const dataView = savedSearch?.searchSource?.getField('index') as DataView;

stateContainer.appState.update({
const appState = {
dataSource: createDataViewDataSource({ dataViewId: dataView?.id! }),
interval: 'auto',
hideChart: false,
});
};

stateContainer.appState.update(appState);

stateContainer.internalState.transitions.setDataView(dataView);
stateContainer.internalState.transitions.setDataRequestParams({
timeRangeAbsolute: {
from: '2020-05-14T11:05:13.590',
to: '2020-05-14T11:20:13.590',
},
timeRangeRelative: {
from: '2020-05-14T11:05:13.590',
to: '2020-05-14T11:20:13.590',
},
});

return stateContainer;
}
Expand All @@ -65,16 +76,7 @@ const mountComponent = async ({
const dataView = savedSearch?.searchSource?.getField('index') as DataView;

let services = discoverServiceMock;
services.data.query.timefilter.timefilter.getAbsoluteTime = () => {
return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' };
};
services.data.query.timefilter.timefilter.getTime = () => {
return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' };
};
(services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({
language: 'kuery',
query: '',
});

(searchSourceInstanceMock.fetch$ as jest.Mock).mockImplementation(
jest.fn().mockReturnValue(of({ rawResponse: { hits: { total: 2 } } }))
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ async function mountComponent(
query,
});
stateContainer.internalState.transitions.setDataView(dataView);
stateContainer.internalState.transitions.setDataRequestParams({
timeRangeAbsolute: time,
timeRangeRelative: time,
});

const props = {
dataView,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,23 @@ describe('useDiscoverHistogram', () => {
});
expect(hook.result.current.isChartLoading).toBe(true);
});

it('should use timerange + timeRangeRelative + query given by the internalState container', async () => {
const fetch$ = new Subject<void>();
const stateContainer = getStateContainer();
const timeRangeAbs = { from: '2021-05-01T20:00:00Z', to: '2021-05-02T20:00:00Z' };
const timeRangeRel = { from: 'now-15m', to: 'now' };
stateContainer.internalState.transitions.setDataRequestParams({
timeRangeAbsolute: timeRangeAbs,
timeRangeRelative: timeRangeRel,
});
const { hook } = await renderUseDiscoverHistogram({ stateContainer });
act(() => {
fetch$.next();
});
expect(hook.result.current.timeRange).toBe(timeRangeAbs);
expect(hook.result.current.relativeTimeRange).toBe(timeRangeRel);
});
});

describe('refetching', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,9 @@ export const useDiscoverHistogram = ({
* Request params
*/
const { query, filters } = useQuerySubscriber({ data: services.data });
const requestParams = useInternalStateSelector((state) => state.dataRequestParams);
const customFilters = useInternalStateSelector((state) => state.customFilters);
const timefilter = services.data.query.timefilter.timefilter;
const timeRange = timefilter.getAbsoluteTime();
const relativeTimeRange = useObservable(
timefilter.getTimeUpdate$().pipe(map(() => timefilter.getTime())),
timefilter.getTime()
);

const { timeRangeRelative: relativeTimeRange, timeRangeAbsolute: timeRange } = requestParams;
// When in ES|QL mode, update the data view, query, and
// columns only when documents are done fetching so the Lens suggestions
// don't frequently change, such as when the user modifies the table
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ describe('test fetchAll', () => {
rowHeight: false,
breakdownField: false,
},
dataRequestParams: {},
}),
searchSessionId: '123',
initialFetchStatus: FetchStatus.UNINITIALIZED,
Expand Down Expand Up @@ -273,6 +274,7 @@ describe('test fetchAll', () => {
rowHeight: false,
breakdownField: false,
},
dataRequestParams: {},
}),
};
fetchAll(subjects, false, deps);
Expand Down Expand Up @@ -396,6 +398,7 @@ describe('test fetchAll', () => {
rowHeight: false,
breakdownField: false,
},
dataRequestParams: {},
}),
};
fetchAll(subjects, false, deps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export function fetchAll(
services,
sort: getAppState().sort as SortOrder[],
customFilters: getInternalState().customFilters,
inputTimeRange: getInternalState().dataRequestParams.timeRangeAbsolute,
});
}

Expand All @@ -117,6 +118,7 @@ export function fetchAll(
data,
expressions,
profilesManager,
timeRange: getInternalState().dataRequestParams.timeRangeAbsolute,
})
: fetchDocuments(searchSource, fetchDeps);
const fetchType = isEsqlQuery ? 'fetchTextBased' : 'fetchDocuments';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,23 @@ import { RequestAdapter } from '@kbn/inspector-plugin/common';
import { of } from 'rxjs';
import { dataViewWithTimefieldMock } from '../../../__mocks__/data_view_with_timefield';
import { discoverServiceMock } from '../../../__mocks__/services';
import { fetchEsql } from './fetch_esql';
import { fetchEsql, getTextBasedQueryStateToAstProps } from './fetch_esql';
import { TimeRange } from '@kbn/es-query';

describe('fetchEsql', () => {
beforeEach(() => {
jest.clearAllMocks();
});

const fetchEsqlMockProps = {
query: { esql: 'from *' },
dataView: dataViewWithTimefieldMock,
inspectorAdapters: { requests: new RequestAdapter() },
data: discoverServiceMock.data,
expressions: discoverServiceMock.expressions,
profilesManager: discoverServiceMock.profilesManager,
};

it('resolves with returned records', async () => {
const hits = [
{ _id: '1', foo: 'bar' },
Expand All @@ -46,16 +56,7 @@ describe('fetchEsql', () => {
discoverServiceMock.profilesManager,
'resolveDocumentProfile'
);
expect(
await fetchEsql({
query: { esql: 'from *' },
dataView: dataViewWithTimefieldMock,
inspectorAdapters: { requests: new RequestAdapter() },
data: discoverServiceMock.data,
expressions: discoverServiceMock.expressions,
profilesManager: discoverServiceMock.profilesManager,
})
).toEqual({
expect(await fetchEsql(fetchEsqlMockProps)).toEqual({
records,
esqlQueryColumns: ['_id', 'foo'],
esqlHeaderWarning: undefined,
Expand All @@ -64,4 +65,24 @@ describe('fetchEsql', () => {
expect(resolveDocumentProfileSpy).toHaveBeenCalledWith({ record: records[0] });
expect(resolveDocumentProfileSpy).toHaveBeenCalledWith({ record: records[1] });
});

it('should use inputTimeRange if provided', () => {
const timeRange: TimeRange = { from: 'now-15m', to: 'now' };
const result = getTextBasedQueryStateToAstProps({ ...fetchEsqlMockProps, timeRange });
expect(result.time).toEqual(timeRange);
});

it('should use absolute time from data if inputTimeRange is not provided', () => {
const absoluteTimeRange: TimeRange = {
from: '2021-08-31T22:00:00.000Z',
to: '2021-09-01T22:00:00.000Z',
};
jest
.spyOn(discoverServiceMock.data.query.timefilter.timefilter, 'getAbsoluteTime')
.mockReturnValue(absoluteTimeRange);

const result = getTextBasedQueryStateToAstProps(fetchEsqlMockProps);

expect(result.time).toEqual(absoluteTimeRange);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function fetchEsql({
query,
inputQuery,
filters,
inputTimeRange,
timeRange,
dataView,
abortSignal,
inspectorAdapters,
Expand All @@ -43,28 +43,23 @@ export function fetchEsql({
query: Query | AggregateQuery;
inputQuery?: Query;
filters?: Filter[];
inputTimeRange?: TimeRange;
timeRange?: TimeRange;
dataView: DataView;
abortSignal?: AbortSignal;
inspectorAdapters: Adapters;
data: DataPublicPluginStart;
expressions: ExpressionsStart;
profilesManager: ProfilesManager;
}): Promise<RecordsFetchResponse> {
const timeRange = inputTimeRange ?? data.query.timefilter.timefilter.getTime();
return textBasedQueryStateToAstWithValidation({
filters,
const props = getTextBasedQueryStateToAstProps({
query,
time: timeRange,
timeFieldName: dataView.timeFieldName,
inputQuery,
titleForInspector: i18n.translate('discover.inspectorEsqlRequestTitle', {
defaultMessage: 'Table',
}),
descriptionForInspector: i18n.translate('discover.inspectorEsqlRequestDescription', {
defaultMessage: 'This request queries Elasticsearch to fetch results for the table.',
}),
})
filters,
timeRange,
dataView,
data,
});
return textBasedQueryStateToAstWithValidation(props)
.then((ast) => {
if (ast) {
const contract = expressions.execute(ast, null, {
Expand Down Expand Up @@ -118,3 +113,32 @@ export function fetchEsql({
throw new Error(err.message);
});
}
export function getTextBasedQueryStateToAstProps({
query,
inputQuery,
filters,
timeRange,
dataView,
data,
}: {
query: Query | AggregateQuery;
inputQuery?: Query;
filters?: Filter[];
timeRange?: TimeRange;
dataView: DataView;
data: DataPublicPluginStart;
}) {
return {
filters,
query,
time: timeRange ?? data.query.timefilter.timefilter.getAbsoluteTime(),
timeFieldName: dataView.timeFieldName,
inputQuery,
titleForInspector: i18n.translate('discover.inspectorEsqlRequestTitle', {
defaultMessage: 'Table',
}),
descriptionForInspector: i18n.translate('discover.inspectorEsqlRequestDescription', {
defaultMessage: 'This request queries Elasticsearch to fetch results for the table.',
}),
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ISearchSource } from '@kbn/data-plugin/public';
import { DataViewType, DataView } from '@kbn/data-views-plugin/public';
import { Filter } from '@kbn/es-query';
import type { ISearchSource } from '@kbn/data-plugin/public';
import { DataViewType, type DataView } from '@kbn/data-views-plugin/public';
import type { Filter, TimeRange } from '@kbn/es-query';
import type { SortOrder } from '@kbn/saved-search-plugin/public';
import { SORT_DEFAULT_ORDER_SETTING } from '@kbn/discover-utils';
import { DiscoverServices } from '../../../build_services';
Expand All @@ -25,11 +25,13 @@ export function updateVolatileSearchSource(
services,
sort,
customFilters,
inputTimeRange,
}: {
dataView: DataView;
services: DiscoverServices;
sort?: SortOrder[];
customFilters: Filter[];
inputTimeRange?: TimeRange;
}
) {
const { uiSettings, data } = services;
Expand All @@ -48,7 +50,7 @@ export function updateVolatileSearchSource(

if (dataView.type !== DataViewType.ROLLUP) {
// Set the date range filter fields from timeFilter using the absolute format. Search sessions requires that it be converted from a relative range
const timeFilter = data.query.timefilter.timefilter.createFilter(dataView);
const timeFilter = data.query.timefilter.timefilter.createFilter(dataView, inputTimeRange);
filters = timeFilter ? [...filters, timeFilter] : filters;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ export function getDataStateContainer({
return;
}

internalStateContainer.transitions.setDataRequestParams({
timeRangeAbsolute: timefilter.getAbsoluteTime(),
timeRangeRelative: timefilter.getTime(),
});

await profilesManager.resolveDataSourceProfile({
dataSource: appStateContainer.getState().dataSource,
dataView: getSavedSearch().searchSource.getField('index'),
Expand Down
Loading

0 comments on commit df495ce

Please sign in to comment.