[ML] Match Data Visualizer/Field stats table content with the popover #140667

qn895 · 2022-09-13T22:15:20Z

Summary

Follow up of #139072. This PR matches the content of the Top Values expanded rows with the new Field stats popovers in Discover. Changes include:

Chang the wording to reflect sampled vs not sampled
Add Other bar if top 10 values don't add up to 100%

For reviewers:

kibana-data-discovery: Input to Field stats table was updated to get the total doc count. This is used to show the Calculated from xyz records.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests 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)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

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

This was checked for breaking API changes and was labeled appropriately

jughosta

Great! Thanks for making these updates!

elasticmachine · 2022-09-14T17:37:21Z

Pinging @elastic/ml-ui (:ml)

ninoslavmiskovic

Great that we are aligning the worlds of fields statistics, discover and lens. Looks good.

…hing-to-popover-2

x-pack/plugins/data_visualizer/public/application/common/components/top_values/top_values.tsx

…hing-to-popover-2

elasticmachine · 2022-09-15T16:54:31Z

Pinging @elastic/fleet (Team:Fleet)

peteharverson

Latest changes LGTM

kibana-ci · 2022-09-19T17:10:38Z

💚 Build Succeeded

Buildkite Build
Commit: d3a6a2b

Metrics [docs]

Async chunks

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

id	before	after	diff
`dataVisualizer`	566.7KB	568.4KB	+1.7KB
`discover`	477.1KB	477.3KB	+151.0B
total			+1.9KB

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`dataVisualizer`	2	3	+1

History

💔 Build #72849 failed 3c0d7e27a8d31a76b27698227ada685ba65fb68e
💔 Build #72726 failed 11607ec
💚 Build #72322 succeeded 6bb7851
💔 Build #72237 failed fa6f596e1d591d2a22ceca1cd4f5ed73f251ba09
💔 Build #71991 failed 29332c8

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

cc @qn895

## Summary This PR removes the beta badge for the Field statistics table. <img width="1791" alt="Screen Shot 2022-09-19 at 12 22 30" src="https://user-images.githubusercontent.com/43350163/191076625-9489eaa0-2488-4a5a-b737-e32724d3bffc.png"> Points of consideration for keeping the beta badge: - Easier for us to keep collecting more user feedback. - Potentially switching to [using the new random sampler for aggregation for the field statistics table](#138953) in the next release. Currently, we are pausing this work to match up with the popover (#139072 and #140667) and to fine-tune the user experience/performance. Points of consideration for removing the beta badge: - The field stats table has been available to users since 8.1, and has been in use within ML since 7.x. We should be defining clear criterias for when it can be moved to GA. ### Checklist Delete any items that are not applicable to this PR. - [ ] 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 - [ ] [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) ### 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](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### 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)

qn895 added 2 commits September 13, 2022 17:07

Add matching sample vs not sample messages

baf6a8f

Add other bar to the top values

29332c8

qn895 requested a review from jughosta September 13, 2022 22:16

jughosta approved these changes Sep 14, 2022

View reviewed changes

Fix types, tests

6bb7851

qn895 force-pushed the ml-field-stats-matching-to-popover-2 branch from fa6f596 to 6bb7851 Compare September 14, 2022 16:36

qn895 self-assigned this Sep 14, 2022

qn895 added the :ml label Sep 14, 2022

qn895 requested a review from alvarezmelissa87 September 14, 2022 17:35

qn895 added release_note:enhancement v8.5.0 Feature:File and Index Data Viz ML file and index data visualizer labels Sep 14, 2022

qn895 marked this pull request as ready for review September 14, 2022 17:37

qn895 requested review from a team as code owners September 14, 2022 17:37

ninoslavmiskovic approved these changes Sep 15, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/main' into ml-field-stats-matc…

11607ec

…hing-to-popover-2

peteharverson reviewed Sep 15, 2022

View reviewed changes

x-pack/plugins/data_visualizer/public/application/common/components/top_values/top_values.tsx Show resolved Hide resolved

qn895 added 2 commits September 15, 2022 11:46

Align top values for file upload visualizer

8efe008

Merge remote-tracking branch 'upstream/main' into ml-field-stats-matc…

b97042a

…hing-to-popover-2

kibanamachine requested a review from a team as a code owner September 15, 2022 16:54

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 15, 2022

qn895 force-pushed the ml-field-stats-matching-to-popover-2 branch from a97a7b7 to b97042a Compare September 15, 2022 16:59

peteharverson approved these changes Sep 15, 2022

View reviewed changes

qn895 removed the request for review from a team September 15, 2022 17:00

Merge upstream/main into branch

d3a6a2b

qn895 force-pushed the ml-field-stats-matching-to-popover-2 branch from 3c0d7e2 to d3a6a2b Compare September 19, 2022 16:12

qn895 enabled auto-merge (squash) September 19, 2022 16:12

qn895 merged commit 7b36f54 into elastic:main Sep 19, 2022

kibanamachine added the backport:skip This commit does not require backporting label Sep 19, 2022

qn895 mentioned this pull request Sep 19, 2022

[ML] Remove beta badge for Field statistics table in Discover #140991

Merged

9 tasks

qn895 deleted the ml-field-stats-matching-to-popover-2 branch September 21, 2022 16:33

jughosta mentioned this pull request Sep 22, 2022

[Discover][Lens] Meta - Unified field list #137779

Closed

31 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Match Data Visualizer/Field stats table content with the popover #140667

[ML] Match Data Visualizer/Field stats table content with the popover #140667

qn895 commented Sep 13, 2022 •

edited

Loading

jughosta left a comment

elasticmachine commented Sep 14, 2022

ninoslavmiskovic left a comment

elasticmachine commented Sep 15, 2022

peteharverson left a comment

kibana-ci commented Sep 19, 2022

References to deprecated APIs

[ML] Match Data Visualizer/Field stats table content with the popover #140667

[ML] Match Data Visualizer/Field stats table content with the popover #140667

Conversation

qn895 commented Sep 13, 2022 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

jughosta left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 14, 2022

ninoslavmiskovic left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 15, 2022

peteharverson left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 19, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

References to deprecated APIs

History

qn895 commented Sep 13, 2022 •

edited

Loading