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] Use fields API to retrieve fields #83891

Merged
merged 101 commits into from
Jan 15, 2021

Conversation

majagrubic
Copy link
Contributor

@majagrubic majagrubic commented Nov 20, 2020

Summary

Release Notes: Support mapping defined runtime fields in Discover

This change implements the usage of the fields API in Discover. Changes are hidden behind a UI setting; if the value is off, everything is (supposed to be) working as previously.

Some notable changes:

  • fields column is formatted similarly to _source and in many ways behaves exactly like _source (ie. it's removed when a new column is added and cannot be removed if only column left)
  • support for multi-fields in the sidebar
  • using fields API will retrieve runtime fields 🎉 (follow the instructions here to create runtime fields for testing)

Current limitations of this PR:

  • expanded doc view hasn't been formatted when using the fields API (fields are not sorted and multifields are not grouped together) -> will be added in a follow-up PR
  • no support for unmapped fields, ie. Discover doesn't show unmapped fields -> will be added in a follow-up PR
  • tackling _source as a default column in the existing saved searches

Checklist

For maintainers

@majagrubic majagrubic changed the title Discover ssource fields [PoC] Discover uses fields API Nov 20, 2020
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Hurray for Clarice Salazar, who finally got a name also when using multi fields 🥳

Bildschirmfoto 2020-11-20 um 12 21 02

@timroes
Copy link
Contributor

timroes commented Nov 24, 2020

I just synced with Stacey about the backwardscompatibility mode again and wanted to leave some notes:

We still need to evaluate how include_unmapped works with * as a field, so can we simply set { field: '*', include_unmapped: true } or do we need to list them more specifically, since all the ES docs never show them used with a pure wildcard.

@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested locally in Firefox, Chrome, Safari. Works! Great work 🥇!.keyword, runtime fields, alias fields they suddenly make sense. Could you open an issue to track remaining stuff that needs to be done? many thx!

@majagrubic
Copy link
Contributor Author

@kertal I opened blocker issues and they're referenced above. will create another one for non-blocking issues

@majagrubic
Copy link
Contributor Author

Tracking issue here: #83891

@majagrubic majagrubic merged commit 9b22789 into elastic:master Jan 15, 2021
@majagrubic majagrubic deleted the discover-ssource-fields branch January 15, 2021 14:51
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jan 19, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jan 19, 2021
majagrubic pushed a commit that referenced this pull request Jan 19, 2021
* [Discover] Use fields API to retrieve fields (#83891)

* 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>
# Conflicts:
#	x-pack/test/functional/apps/security/doc_level_security_roles.js
#	x-pack/test/functional/apps/security/field_level_security.js

* Fixing failing functional tests

* Fixing failing functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kennethlwk
Copy link

Hi, just wondering how do we get the fixed for this issue? We are using 7.3.1 Kibana, and we also see the same problem which is alias in Discover is not showing any value on the alias field, but only have value in actual text field.

Would be good if someone can tell us how do we fetch the changes, is it we just need to clone the latest code to the repo, then we will get the soluton.

@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 16, 2021

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [82c30a3]

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:feature Makes this part of the condensed release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.