Skip to content

[ML] Data Visualizer: Remove duplicated geo examples, support 'version' type, add filters for boolean fields, and add sticky header to Discover#136236

Merged
qn895 merged 9 commits intoelastic:mainfrom
qn895:ml-dv-duplicated-geo-bool-filter-version-icon
Jul 14, 2022
Merged

[ML] Data Visualizer: Remove duplicated geo examples, support 'version' type, add filters for boolean fields, and add sticky header to Discover#136236
qn895 merged 9 commits intoelastic:mainfrom
qn895:ml-dv-duplicated-geo-bool-filter-version-icon

Conversation

@qn895
Copy link
Copy Markdown
Member

@qn895 qn895 commented Jul 12, 2022 •
edited
Loading

Summary

This PR is part of #86387. It adds several enhancements:

  1. Version field types are no longer treated as a keyword type as before. The new behavior/icon matches with Discover's.

Screen Shot 2022-07-12 at 16 12 16

  1. Boolean fields now have filters:

Screen Shot 2022-07-12 at 16 12 30

  1. Geo fields no longer show duplicated results.

  2. Field stats in Discover now have a sticky header when table is scrolled (see issue [Discover] Pin header row in Field Statistics when scrolling #136299):

Screen.Recording.2022-07-13.at.16.11.50.mov
  1. Finally, this PR also changes so that the ML Index data visualizer's data view header uses the nice display name instead of by the index pattern.

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@qn895 qn895 self-assigned this Jul 12, 2022
@qn895 qn895 force-pushed the ml-dv-duplicated-geo-bool-filter-version-icon branch from 8629402 to e923e33 Compare July 12, 2022 21:13
@qn895 qn895 force-pushed the ml-dv-duplicated-geo-bool-filter-version-icon branch from e923e33 to 4b64a60 Compare July 12, 2022 21:30
@qn895 qn895 marked this pull request as ready for review July 13, 2022 15:17
@qn895 qn895 requested a review from a team as a code owner July 13, 2022 15:17
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 13, 2022

Pinging @elastic/ml-ui (:ml)

@qn895 qn895 requested review from peteharverson and walterra July 13, 2022 15:18
@qn895 qn895 mentioned this pull request Jul 13, 2022
Closed
35 tasks
@qn895 qn895 force-pushed the ml-dv-duplicated-geo-bool-filter-version-icon branch from 9e8b3c5 to 3dc8912 Compare July 13, 2022 20:16
@qn895 qn895 requested a review from a team as a code owner July 13, 2022 21:25
@qn895 qn895 changed the title [ML] Data Visualizer: Remove duplicated geo examples, support 'version' type, and add filters for boolean fields [ML] Data Visualizer: Remove duplicated geo examples, support 'version' type, add filters for boolean fields, and add sticky header to Discover Jul 13, 2022
jgowdyelastic
jgowdyelastic reviewed Jul 14, 2022
View reviewed changes
x-pack/plugins/data_visualizer/common/constants.ts
TEXT: 'text',
HISTOGRAM: 'histogram',
UNKNOWN: 'unknown',
VERSION: 'version',
Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic Jul 14, 2022

Choose a reason for hiding this comment

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

nit, UNKNOWN should be the last item in this list

Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic Jul 14, 2022

Choose a reason for hiding this comment

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

not for this PR, but JOB_FIELD_TYPES should probably be replaced with ES_FIELD_TYPES

Copy link
Copy Markdown
Member Author

@qn895 qn895 Jul 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Updated here cc5f0fd

peteharverson
peteharverson approved these changes Jul 14, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest changes and LGTM.
Love the sticky table header in Discover!

@qn895 qn895 force-pushed the ml-dv-duplicated-geo-bool-filter-version-icon branch from 1ab83b9 to cc5f0fd Compare July 14, 2022 16:29
mdefazio
mdefazio approved these changes Jul 14, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

Sass changes LGTM.

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Jul 14, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataVisualizer 364 365 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataVisualizer 544.5KB 545.2KB +683.0B

Page load bundle

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

id before after diff
dataVisualizer 16.6KB 16.6KB +18.0B

History

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

cc @qn895

@qn895 qn895 merged commit 2a6f8e8 into elastic:main Jul 14, 2022
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Jul 14, 2022
@qn895 qn895 mentioned this pull request Jul 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jgowdyelastic jgowdyelastic jgowdyelastic left review comments

@mdefazio mdefazio mdefazio approved these changes

@peteharverson peteharverson peteharverson approved these changes

@walterra walterra Awaiting requested review from walterra

Assignees

@qn895 qn895

Labels

backport:skip This PR does not require backporting Feature:File and Index Data Viz ML file and index data visualizer :ml release_note:enhancement v8.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@qn895 @elasticmachine @kibana-ci @mdefazio @peteharverson @jgowdyelastic @kibanamachine