Skip to content

Commit

Permalink
[Discover] Use fields API to retrieve fields (#83891)
Browse files Browse the repository at this point in the history
* Add search source to example plugin.

* Add uiSetting for fields API.

* Update SearchSource to support fields API.

* [PoC] reading from the fields API in Discover

* Add N fields as a default column

* Make fields column non-removeable

* Do not add 'fields' to state

* Remove fields from app state and read from source when needed

* Remove fields column if a new column is added

* Add search source to example plugin.

* Add uiSetting for fields API.

* Update SearchSource to support fields API.

* Improve error handling in search examples plugin.

* Add unit tests for legacy behavior.

* Remove uiSettings feature flag; add fieldsFromSource config.

* Rewrite flatten() based on final API design.

* Update example app based on final API design.

* Update maps app to use legacy fieldsFromSource.

* Update Discover to use legacy fieldsFromSource.

* Rename source filters to field filters.

* Address feedback.

* Update generated docs.

* Update maps functional test.

* Formatting fields column similar to _source

* Moving logic for using search API to updating search source

* Fix small merge error

* Move useSource switch to Discover section of advanced settings

* Do not use fields and source at the same time

* Remove unmapped fields switch

* Add basic support for grouping multifields

* Remove output.txt

* Fix some merge leftovers

* Fix some merge leftovers

* Fix merge errors

* Fix typescript errors and update nested fields logic

* Add a unit test

* Fixing field formats

* Fix multifield selection logic

* Request all fields from source

* Fix eslint

* Fix default columns when switching between _source and fields

* More unit tests

* Update API changes

* Add unit test for discover field details footer

* Remove unused file

* Remove fields formatting from index pattern

* Remove unnecessary check

* Addressing design comments

* Fixing fields column display and renaming it to Document

* Adding more unit tests

* Adding a missing check for useNewFieldsAPI; minor fixes

* Fixing typescript error

* Remove unnecessary console statement

* Add missing prop

* Fixing import order

* Adding functional test to test fields API

* [Functional test] Clean up in after

* Fixing context app

* Addressing PR comments

* Updating failed snapshot

* Addressing PR comments

* Fixing i18n translations, updating type

* Addressing PR comments

* Updating a functional test

* Add a separate functional test for fields API

* Read fields from source in a functional test

* Skip buggy test

* Use default behavior in functional tests

* Fixing remaining failing tests

* Fixing date-nanos test

* Updating FLS test

* Fixing yet another functional test

* Skipping non-relevant tests

* Fixing more tests

* Update stub import in test

* Fix import

* Fix invalid import

Co-authored-by: Luke Elmers <luke.elmers@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 15, 2021
1 parent 31a481a commit 9b22789
Show file tree
Hide file tree
Showing 66 changed files with 1,920 additions and 186 deletions.
1 change: 1 addition & 0 deletions src/plugins/discover/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ export const CONTEXT_STEP_SETTING = 'context:step';
export const CONTEXT_TIE_BREAKER_FIELDS_SETTING = 'context:tieBreakerFields';
export const DOC_TABLE_LEGACY = 'doc_table:legacy';
export const MODIFY_COLUMNS_ON_SWITCH = 'discover:modifyColumnsOnSwitch';
export const SEARCH_FIELDS_FROM_SOURCE = 'discover:searchFieldsFromSource';
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

// @ts-expect-error
import stubbedLogstashFields from './logstash_fields';
import stubbedLogstashFields from '../../../../fixtures/logstash_fields';

const mockLogstashFields = stubbedLogstashFields();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function createSearchSourceStub(hits, timeField) {

searchSourceStub.setParent = sinon.spy(() => searchSourceStub);
searchSourceStub.setField = sinon.spy(() => searchSourceStub);
searchSourceStub.removeField = sinon.spy(() => searchSourceStub);

searchSourceStub.getField = sinon.spy((key) => {
const previousSetCall = searchSourceStub.setField.withArgs(key).lastCall;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import _ from 'lodash';
import { i18n } from '@kbn/i18n';

export function fetchAnchorProvider(indexPatterns, searchSource) {
export function fetchAnchorProvider(indexPatterns, searchSource, useNewFieldsApi = false) {
return async function fetchAnchor(indexPatternId, anchorId, sort) {
const indexPattern = await indexPatterns.get(indexPatternId);
searchSource
Expand All @@ -41,7 +41,10 @@ export function fetchAnchorProvider(indexPatterns, searchSource) {
language: 'lucene',
})
.setField('sort', sort);

if (useNewFieldsApi) {
searchSource.removeField('fieldsFromSource');
searchSource.setField('fields', ['*']);
}
const response = await searchSource.fetch();

if (_.get(response, ['hits', 'total'], 0) < 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,29 @@ describe('context app', function () {
});
});
});

describe('useNewFields API', () => {
let fetchAnchor;
let searchSourceStub;

beforeEach(() => {
searchSourceStub = createSearchSourceStub([{ _id: 'hit1' }]);
fetchAnchor = fetchAnchorProvider(createIndexPatternsStub(), searchSourceStub, true);
});

it('should request fields if useNewFieldsApi set', function () {
searchSourceStub._stubHits = [{ property1: 'value1' }, { property2: 'value2' }];

return fetchAnchor('INDEX_PATTERN_ID', 'id', [
{ '@timestamp': 'desc' },
{ _doc: 'desc' },
]).then(() => {
const setFieldsSpy = searchSourceStub.setField.withArgs('fields');
const removeFieldsSpy = searchSourceStub.removeField.withArgs('fieldsFromSource');
expect(setFieldsSpy.calledOnce).toBe(true);
expect(removeFieldsSpy.calledOnce).toBe(true);
expect(setFieldsSpy.firstCall.args[1]).toEqual(['*']);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,81 @@ describe('context app', function () {
});
});
});

describe('function fetchPredecessors with useNewFieldsApi set', function () {
let fetchPredecessors;
let mockSearchSource;

beforeEach(() => {
mockSearchSource = createContextSearchSourceStub([], '@timestamp', MS_PER_DAY * 8);

setServices({
data: {
search: {
searchSource: {
create: jest.fn().mockImplementation(() => mockSearchSource),
},
},
},
});

fetchPredecessors = (
indexPatternId,
timeField,
sortDir,
timeValIso,
timeValNr,
tieBreakerField,
tieBreakerValue,
size
) => {
const anchor = {
_source: {
[timeField]: timeValIso,
},
sort: [timeValNr, tieBreakerValue],
};

return fetchContextProvider(createIndexPatternsStub(), true).fetchSurroundingDocs(
'predecessors',
indexPatternId,
anchor,
timeField,
tieBreakerField,
sortDir,
size,
[]
);
};
});

it('should perform exactly one query when enough hits are returned', function () {
mockSearchSource._stubHits = [
mockSearchSource._createStubHit(MS_PER_DAY * 3000 + 2),
mockSearchSource._createStubHit(MS_PER_DAY * 3000 + 1),
mockSearchSource._createStubHit(MS_PER_DAY * 3000),
mockSearchSource._createStubHit(MS_PER_DAY * 2000),
mockSearchSource._createStubHit(MS_PER_DAY * 1000),
];

return fetchPredecessors(
'INDEX_PATTERN_ID',
'@timestamp',
'desc',
ANCHOR_TIMESTAMP_3000,
MS_PER_DAY * 3000,
'_doc',
0,
3,
[]
).then((hits) => {
const setFieldsSpy = mockSearchSource.setField.withArgs('fields');
const removeFieldsSpy = mockSearchSource.removeField.withArgs('fieldsFromSource');
expect(mockSearchSource.fetch.calledOnce).toBe(true);
expect(removeFieldsSpy.calledOnce).toBe(true);
expect(setFieldsSpy.calledOnce).toBe(true);
expect(hits).toEqual(mockSearchSource._stubHits.slice(0, 3));
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,81 @@ describe('context app', function () {
});
});
});

describe('function fetchSuccessors with useNewFieldsApi set', function () {
let fetchSuccessors;
let mockSearchSource;

beforeEach(() => {
mockSearchSource = createContextSearchSourceStub([], '@timestamp');

setServices({
data: {
search: {
searchSource: {
create: jest.fn().mockImplementation(() => mockSearchSource),
},
},
},
});

fetchSuccessors = (
indexPatternId,
timeField,
sortDir,
timeValIso,
timeValNr,
tieBreakerField,
tieBreakerValue,
size
) => {
const anchor = {
_source: {
[timeField]: timeValIso,
},
sort: [timeValNr, tieBreakerValue],
};

return fetchContextProvider(createIndexPatternsStub(), true).fetchSurroundingDocs(
'successors',
indexPatternId,
anchor,
timeField,
tieBreakerField,
sortDir,
size,
[]
);
};
});

it('should perform exactly one query when enough hits are returned', function () {
mockSearchSource._stubHits = [
mockSearchSource._createStubHit(MS_PER_DAY * 5000),
mockSearchSource._createStubHit(MS_PER_DAY * 4000),
mockSearchSource._createStubHit(MS_PER_DAY * 3000),
mockSearchSource._createStubHit(MS_PER_DAY * 3000 - 1),
mockSearchSource._createStubHit(MS_PER_DAY * 3000 - 2),
];

return fetchSuccessors(
'INDEX_PATTERN_ID',
'@timestamp',
'desc',
ANCHOR_TIMESTAMP_3000,
MS_PER_DAY * 3000,
'_doc',
0,
3,
[]
).then((hits) => {
expect(mockSearchSource.fetch.calledOnce).toBe(true);
expect(hits).toEqual(mockSearchSource._stubHits.slice(-3));
const setFieldsSpy = mockSearchSource.setField.withArgs('fields');
const removeFieldsSpy = mockSearchSource.removeField.withArgs('fieldsFromSource');
expect(removeFieldsSpy.calledOnce).toBe(true);
expect(setFieldsSpy.calledOnce).toBe(true);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const DAY_MILLIS = 24 * 60 * 60 * 1000;
// look from 1 day up to 10000 days into the past and future
const LOOKUP_OFFSETS = [0, 1, 7, 30, 365, 10000].map((days) => days * DAY_MILLIS);

function fetchContextProvider(indexPatterns: IndexPatternsContract) {
function fetchContextProvider(indexPatterns: IndexPatternsContract, useNewFieldsApi?: boolean) {
return {
fetchSurroundingDocs,
};
Expand Down Expand Up @@ -89,7 +89,14 @@ function fetchContextProvider(indexPatterns: IndexPatternsContract) {
break;
}

const searchAfter = getEsQuerySearchAfter(type, documents, timeField, anchor, nanos);
const searchAfter = getEsQuerySearchAfter(
type,
documents,
timeField,
anchor,
nanos,
useNewFieldsApi
);

const sort = getEsQuerySort(timeField, tieBreakerField, sortDirToApply);

Expand All @@ -116,6 +123,10 @@ function fetchContextProvider(indexPatterns: IndexPatternsContract) {
const { data } = getServices();

const searchSource = await data.search.searchSource.create();
if (useNewFieldsApi) {
searchSource.removeField('fieldsFromSource');
searchSource.setField('fields', ['*']);
}
return searchSource
.setParent(undefined)
.setField('index', indexPattern)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,30 @@ export function getEsQuerySearchAfter(
documents: EsHitRecordList,
timeFieldName: string,
anchor: EsHitRecord,
nanoSeconds: string
nanoSeconds: string,
useNewFieldsApi?: boolean
): EsQuerySearchAfter {
if (documents.length) {
// already surrounding docs -> first or last record is used
const afterTimeRecIdx = type === 'successors' && documents.length ? documents.length - 1 : 0;
const afterTimeDoc = documents[afterTimeRecIdx];
const afterTimeValue = nanoSeconds ? afterTimeDoc._source[timeFieldName] : afterTimeDoc.sort[0];
let afterTimeValue = afterTimeDoc.sort[0];
if (nanoSeconds) {
afterTimeValue = useNewFieldsApi
? afterTimeDoc.fields[timeFieldName][0]
: afterTimeDoc._source[timeFieldName];
}
return [afterTimeValue, afterTimeDoc.sort[1]];
}
// if data_nanos adapt timestamp value for sorting, since numeric value was rounded by browser
// ES search_after also works when number is provided as string
return [nanoSeconds ? anchor._source[timeFieldName] : anchor.sort[0], anchor.sort[1]];
const searchAfter = new Array(2) as EsQuerySearchAfter;
searchAfter[0] = anchor.sort[0];
if (nanoSeconds) {
searchAfter[0] = useNewFieldsApi
? anchor.fields[timeFieldName][0]
: anchor._source[timeFieldName];
}
searchAfter[1] = anchor.sort[1];
return searchAfter;
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,17 @@ import { fetchContextProvider } from '../api/context';
import { getQueryParameterActions } from '../query_parameters';
import { FAILURE_REASONS, LOADING_STATUS } from './index';
import { MarkdownSimple } from '../../../../../../kibana_react/public';
import { SEARCH_FIELDS_FROM_SOURCE } from '../../../../../common';

export function QueryActionsProvider(Promise) {
const { filterManager, indexPatterns, data } = getServices();
const fetchAnchor = fetchAnchorProvider(indexPatterns, data.search.searchSource.createEmpty());
const { fetchSurroundingDocs } = fetchContextProvider(indexPatterns);
const { filterManager, indexPatterns, data, uiSettings } = getServices();
const useNewFieldsApi = !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE);
const fetchAnchor = fetchAnchorProvider(
indexPatterns,
data.search.searchSource.createEmpty(),
useNewFieldsApi
);
const { fetchSurroundingDocs } = fetchContextProvider(indexPatterns, useNewFieldsApi);
const { setPredecessorCount, setQueryParameters, setSuccessorCount } = getQueryParameterActions(
filterManager,
indexPatterns
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
successor-available="contextApp.state.rows.successors.length"
successor-status="contextApp.state.loadingStatus.successors.status"
on-change-successor-count="contextApp.actions.fetchGivenSuccessorRows"
use-new-fields-api="contextApp.state.useNewFieldsApi"
top-nav-menu="contextApp.topNavMenu"
></context-app-legacy>
13 changes: 10 additions & 3 deletions src/plugins/discover/public/application/angular/context_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
*/

import _ from 'lodash';
import { CONTEXT_STEP_SETTING, CONTEXT_TIE_BREAKER_FIELDS_SETTING } from '../../../common';
import {
CONTEXT_STEP_SETTING,
CONTEXT_TIE_BREAKER_FIELDS_SETTING,
SEARCH_FIELDS_FROM_SOURCE,
} from '../../../common';
import { getAngularModule, getServices } from '../../kibana_services';
import contextAppTemplate from './context_app.html';
import './context/components/action_bar';
Expand Down Expand Up @@ -59,9 +63,11 @@ function ContextAppController($scope, Private) {
const { filterManager, indexPatterns, uiSettings, navigation } = getServices();
const queryParameterActions = getQueryParameterActions(filterManager, indexPatterns);
const queryActions = Private(QueryActionsProvider);
const useNewFieldsApi = !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE);
this.state = createInitialState(
parseInt(uiSettings.get(CONTEXT_STEP_SETTING), 10),
getFirstSortableField(this.indexPattern, uiSettings.get(CONTEXT_TIE_BREAKER_FIELDS_SETTING))
getFirstSortableField(this.indexPattern, uiSettings.get(CONTEXT_TIE_BREAKER_FIELDS_SETTING)),
useNewFieldsApi
);
this.topNavMenu = navigation.ui.TopNavMenu;

Expand Down Expand Up @@ -127,7 +133,7 @@ function ContextAppController($scope, Private) {
);
}

function createInitialState(defaultStepSize, tieBreakerField) {
function createInitialState(defaultStepSize, tieBreakerField, useNewFieldsApi) {
return {
queryParameters: createInitialQueryParametersState(defaultStepSize, tieBreakerField),
rows: {
Expand All @@ -137,5 +143,6 @@ function createInitialState(defaultStepSize, tieBreakerField) {
successors: [],
},
loadingStatus: createInitialLoadingStatusState(),
useNewFieldsApi,
};
}
Loading

0 comments on commit 9b22789

Please sign in to comment.