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.

Sign up for GitHub

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

Jump to bottom

[ML] Explain Log Rate Spikes: Support to filter fields from grouping #153864

Merged
merged 23 commits into from
Apr 11, 2023
Merged

[ML] Explain Log Rate Spikes: Support to filter fields from grouping #153864

merged 23 commits into from
Apr 11, 2023

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Mar 28, 2023 •
edited
Loading

Summary

Part of #153753.

  • Adds a Filter fields popover selector inspired by EUI's data grid column picker to toggle inclusion of fields into grouping.
  • Moves the Group results switch and the Filter fields popover on the same level as the progress controls.
  • Adapts the explain_log_rate_spikes API endpoint to support retrieving a grouping update only.
  • Hides the pagination footer for the results tables if there's less results than the current page size.

aiops-grouping-skip-fields-0002

Checklist

@walterra walterra self-assigned this Mar 28, 2023
@walterra walterra force-pushed the ml-153753-field-selection branch from e884228 to 5256ed0 Compare March 29, 2023 08:54
@walterra walterra mentioned this pull request Mar 29, 2023
Closed
8 tasks
@walterra walterra force-pushed the ml-153753-field-selection branch from 5256ed0 to 19449fa Compare March 30, 2023 08:30
@walterra walterra marked this pull request as ready for review March 30, 2023 09:41
@walterra walterra requested a review from a team as a code owner March 30, 2023 09:41
@walterra walterra added the :ml label Mar 30, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra added Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v8.8.0 release_note:enhancement labels Mar 30, 2023
@walterra
Copy link
Contributor Author

Added a help text to the field filter popover. It's now also possible to select/deselect all fields with no search filter active.

image

alvarezmelissa87
alvarezmelissa87 reviewed Apr 4, 2023
View reviewed changes
x-pack/plugins/aiops/public/components/explain_log_rate_spikes/field_filter_popover.tsx
defaultMessage: 'Filter fields',
})}
value={fieldSearchText}
onChange={(e: ChangeEvent<HTMLInputElement>) => setFieldSearchText(e.currentTarget.value)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be helpful to debounce here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll skip doing this for now since there's no immediate remote or async action involved. (The EUI component I took this from works the same way)

alvarezmelissa87
alvarezmelissa87 approved these changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

Left a small comment but other than that LGTM

szabosteve
szabosteve approved these changes Apr 5, 2023
View reviewed changes
Copy link
Contributor

@szabosteve szabosteve left a comment

Choose a reason for hiding this comment

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

Left one suggestion that might make the help text tighter. Otherwise, UI text LGTM!

x-pack/plugins/aiops/public/components/explain_log_rate_spikes/field_filter_popover.tsx Outdated
<EuiText size="xs" color="subdued" style={{ maxWidth: '400px' }}>
<FormattedMessage
id="xpack.aiops.explainLogRateSpikesPage.fieldFilterHelpText"
defaultMessage="To remove non-relevant fields from groups, deselect them below and click the Apply button to rerun the grouping. The search bar can be used to filter the list of fields and is useful to select/deselect in bulk with the actions below the list."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage="To remove non-relevant fields from groups, deselect them below and click the Apply button to rerun the grouping. The search bar can be used to filter the list of fields and is useful to select/deselect in bulk with the actions below the list."
defaultMessage="Deselect non-relevant fields to remove them from groups and click the Apply button to rerun the grouping. Use the search bar to filter the list, then select/deselect multiple fields with the actions below."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 16a3325.

peteharverson
peteharverson reviewed Apr 6, 2023
View reviewed changes
Copy link
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 and overall looks good. Just left a few comments.

x-pack/plugins/aiops/public/components/explain_log_rate_spikes/field_filter_popover.tsx Show resolved Hide resolved
x-pack/plugins/aiops/public/components/explain_log_rate_spikes/field_filter_popover.tsx Outdated Show resolved Hide resolved
x-pack/plugins/aiops/public/components/explain_log_rate_spikes/field_filter_popover.tsx Show resolved Hide resolved
peteharverson
peteharverson approved these changes Apr 11, 2023
View reviewed changes
Copy link
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 edits and LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 472 474 +2

Async chunks

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

id before after diff
aiops 754.7KB 759.6KB +4.9KB
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 @walterra

@walterra walterra merged commit ab277e4 into elastic:main Apr 11, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 11, 2023
@walterra walterra deleted the ml-153753-field-selection branch April 11, 2023 18:24
@peteharverson peteharverson added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Apr 21, 2023
@walterra walterra mentioned this pull request Apr 25, 2023
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szabosteve szabosteve szabosteve approved these changes

@peteharverson peteharverson peteharverson approved these changes

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

Assignees

@walterra walterra

Labels
backport:skip This commit does not require backporting Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis :ml release_note:feature Makes this part of the condensed release notes v8.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@walterra @elasticmachine @kibana-ci @alvarezmelissa87 @peteharverson @szabosteve @kibanamachine