Skip to content

Commit

Permalink
[Search Source] Update search source to use minimal data view spec fo…
Browse files Browse the repository at this point in the history
…r serialization (elastic#169460)

## Summary

Currently we use the full data view spec whenever we serialize fields
for a search source with an ad hoc data view, except when otherwise
specified (currently only when serializing for alert URLs). As a result,
whenever we persist a serialized search source containing an ad hoc data
view within a saved object, we end up persisting the entire data view
field list as well. For data views that target hundreds or thousands of
fields, this can result in huge saved objects that exceed Kibana's max
payload size, causing issues like the one seen in elastic#168573.

Does it ever make sense to persist the full data view spec now that we
have a minimal spec (strips all field list data except popularity
counts, custom labels, etc.)? I don't think it does, so this PR updates
search source to always use the minimal data view spec for ad hoc data
views, limiting the impact of large field lists. Barring a good reason
not to do this, I think we should make this the standard behaviour.

Fixes elastic#168573.

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit f0bf4d2)
  • Loading branch information
davismcphee committed Oct 26, 2023
1 parent bfa39c7 commit 1ed8b11
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ describe('SearchSource', () => {
const localDataView = {
id: 'local-123',
isPersisted: () => false,
toSpec: () => ({ id: 'local-123' }),
toMinimalSpec: () => ({ id: 'local-123' }),
} as DataView;
searchSource.setField('index', localDataView);
const { searchSourceJSON, references } = searchSource.serialize();
Expand Down Expand Up @@ -1029,7 +1029,7 @@ describe('SearchSource', () => {
const indexPattern123 = {
id: '123',
isPersisted: jest.fn(() => true),
toSpec: jest.fn(),
toMinimalSpec: jest.fn(),
} as unknown as DataView;

test('should return serialized fields', () => {
Expand All @@ -1038,7 +1038,7 @@ describe('SearchSource', () => {
return filter;
});
const serializedFields = searchSource.getSerializedFields();
expect(indexPattern123.toSpec).toHaveBeenCalledTimes(0);
expect(indexPattern123.toMinimalSpec).toHaveBeenCalledTimes(0);
expect(serializedFields).toMatchSnapshot();
});

Expand All @@ -1048,18 +1048,18 @@ describe('SearchSource', () => {
const childSearchSource = searchSource.createChild();
childSearchSource.setField('timeout', '100');
const serializedFields = childSearchSource.getSerializedFields(true);
expect(indexPattern123.toSpec).toHaveBeenCalledTimes(0);
expect(indexPattern123.toMinimalSpec).toHaveBeenCalledTimes(0);
expect(serializedFields).toMatchObject({
timeout: '100',
parent: { index: '123', from: 123 },
});
});

test('should use spec', () => {
test('should use minimal spec for ad hoc data view', () => {
indexPattern123.isPersisted = jest.fn(() => false);
searchSource.setField('index', indexPattern123);
searchSource.getSerializedFields(true, false);
expect(indexPattern123.toSpec).toHaveBeenCalledWith(false);
searchSource.getSerializedFields(true);
expect(indexPattern123.toMinimalSpec).toHaveBeenCalledTimes(1);
});
});

Expand Down
6 changes: 2 additions & 4 deletions src/plugins/data/common/search/search_source/search_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ export class SearchSource {
/**
* serializes search source fields (which can later be passed to {@link ISearchStartSearchSource})
*/
public getSerializedFields(recurse = false, includeFields = true): SerializedSearchSourceFields {
public getSerializedFields(recurse = false): SerializedSearchSourceFields {
const {
filter: originalFilters,
aggs: searchSourceAggs,
Expand All @@ -961,9 +961,7 @@ export class SearchSource {
...searchSourceFields,
};
if (index) {
serializedSearchSourceFields.index = index.isPersisted()
? index.id
: index.toSpec(includeFields);
serializedSearchSourceFields.index = index.isPersisted() ? index.id : index.toMinimalSpec();
}
if (sort) {
serializedSearchSourceFields.sort = !Array.isArray(sort) ? [sort] : sort;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/public/utils/get_sharing_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export async function getSharingData(

searchSource.setField('fields', fields);
}
return searchSource.getSerializedFields(true, false);
return searchSource.getSerializedFields(true);
},
columns,
};
Expand Down

0 comments on commit 1ed8b11

Please sign in to comment.