-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Update anomalies tab to display the same quantity of anomalies when navigating from entity analytics page #139910
Conversation
d5864f8
to
a7b5d48
Compare
99982de
to
528f727
Compare
528f727
to
7c9c0d3
Compare
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
x-pack/plugins/security_solution/public/common/components/ml/anomaly/use_anomalies_search.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/network/store/reducer.ts
Outdated
Show resolved
Hide resolved
defaultMessage: 'Job', | ||
}); | ||
|
||
export const INTERVAL_TOOLTIP = i18n.translate('xpack.securitySolution.ml.table.intervalTooltip', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand the auto
option in the interval 😅 maybe we could explain it here. Has this text been agreed with @paulewing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, it is good to have consistency. It's just that what the auto
option does is not explained, maybe in the ML context is more intuitive. I don't know, just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I checked the code I was confused about the use of redux state for the anomalies table filters, later I realized it is needed to set the filters from the entities page, before navigating to the specific anomalies pages. It would be nice to mention that in the description.
I left some NITs and I think we should also wait for Paul's approval (or some designer).
But the code overall LGTM! Tested locally as well.
Great jobId
! 🎉
@semd That is not the only reason why I saved the user selection in the redux store. Most tables on User/Host/Network pages also save the user selection inside the redux store. I guess the reason for that is to preserve the selection while navigating between tabs. But there are many inconsistencies. For example, here is the initial data stored inside host tabs.
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @machadoum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! I also need to check the codeowners because I don't think we should have been pinged on most of these
Summary
Anomalies tab shows the most critical anomalies for a User/Host/Network per hour or day.
When users navigate from the Entity analytics page to the Anomalies tab, we have to ensure they will see the same number of anomalies displayed on the Entity Analytics page. For that, I had to add two fields to the tab.
When the user clicks on the number of anomalies for a job id, I preselect the job id and preselect interval as 'show all'.
Checklist
Delete any items that are not applicable to this PR.