Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] [Saved Search] Cleanup saved search mappings #154016

Merged
merged 9 commits into from
Apr 6, 2023

Conversation

davismcphee
Copy link
Contributor

@davismcphee davismcphee commented Mar 30, 2023

Summary

This PR cleans up saved search mappings, removing all fields that aren't being searched or aggregated on.

Continuation of the work started in #153129.

Checklist

For maintainers

@davismcphee davismcphee added chore Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.8.0 labels Mar 30, 2023
@davismcphee davismcphee self-assigned this Mar 30, 2023
@davismcphee davismcphee marked this pull request as ready for review March 31, 2023 03:31
@davismcphee davismcphee requested review from a team as code owners March 31, 2023 03:31
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@davismcphee davismcphee force-pushed the saved-search-mappings-cleanup branch from 40ff51b to 109f0ee Compare April 3, 2023 19:31
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM 👍

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davismcphee davismcphee force-pushed the saved-search-mappings-cleanup branch from 109f0ee to d106a59 Compare April 5, 2023 23:11
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
savedSearch 5.4KB 5.5KB +106.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/saved-objects-service.html#_mappings

id before after diff
search 24 3 -21
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @davismcphee

@davismcphee davismcphee merged commit 25b8f92 into elastic:main Apr 6, 2023
@davismcphee davismcphee deleted the saved-search-mappings-cleanup branch April 6, 2023 15:51
jughosta pushed a commit that referenced this pull request May 5, 2023
#156769)

## Summary

This PR removes the `minSize` restriction added in #154016 from the
saved search schema `sort` property, which was causing AWS integrations
to fail.

Before:

![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)

After:

![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)

Fixes #156724.

### 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)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 5, 2023
elastic#156769)

## Summary

This PR removes the `minSize` restriction added in elastic#154016 from the
saved search schema `sort` property, which was causing AWS integrations
to fail.

Before:

![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)

After:

![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)

Fixes elastic#156724.

### 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)

(cherry picked from commit 2f53207)
kibanamachine added a commit that referenced this pull request May 5, 2023
… arrays (#156769) (#156789)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Saved Search] Update saved search schema to allow empty `sort`
arrays (#156769)](#156769)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2023-05-05T07:16:32Z","message":"[Saved
Search] Update saved search schema to allow empty `sort` arrays
(#156769)\n\n## Summary\r\n\r\nThis PR removes the `minSize` restriction
added in #154016 from the\r\nsaved search schema `sort` property, which
was causing AWS integrations\r\nto
fail.\r\n\r\nBefore:\r\n\r\n![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)\r\n\r\nAfter:\r\n\r\n![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)\r\n\r\nFixes
#156724.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"2f53207727d321363a1664e2a952301fd5b20f7e","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Saved
Objects","release_note:skip","Team:DataDiscovery","v8.8.0","v8.9.0"],"number":156769,"url":"https://github.com/elastic/kibana/pull/156769","mergeCommit":{"message":"[Saved
Search] Update saved search schema to allow empty `sort` arrays
(#156769)\n\n## Summary\r\n\r\nThis PR removes the `minSize` restriction
added in #154016 from the\r\nsaved search schema `sort` property, which
was causing AWS integrations\r\nto
fail.\r\n\r\nBefore:\r\n\r\n![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)\r\n\r\nAfter:\r\n\r\n![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)\r\n\r\nFixes
#156724.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"2f53207727d321363a1664e2a952301fd5b20f7e"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156769","number":156769,"mergeCommit":{"message":"[Saved
Search] Update saved search schema to allow empty `sort` arrays
(#156769)\n\n## Summary\r\n\r\nThis PR removes the `minSize` restriction
added in #154016 from the\r\nsaved search schema `sort` property, which
was causing AWS integrations\r\nto
fail.\r\n\r\nBefore:\r\n\r\n![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)\r\n\r\nAfter:\r\n\r\n![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)\r\n\r\nFixes
#156724.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"2f53207727d321363a1664e2a952301fd5b20f7e"}}]}]
BACKPORT-->

Co-authored-by: Davis McPhee <davis.mcphee@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting chore Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants