-
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
[ML] Shows alerts data in the Anomaly Explorer #167998
Conversation
@elasticmachine merge upstream |
Pinging @elastic/ml-ui (:ml) |
@darnautov Great to see this! If I delete the annotations, that block below disappears along with the row in the swimlane (and the alerts row). Assuming that if there were alerts, a block would show below similar to Annotations? It seems there are some oddities when to show the row in the swimlane and when not to. Question to the team: |
As discussed with @darnautov and @mdefazio some early feedback:
|
@peteharverson Some comments on the comments. The TLDR; of this is that my concern with simply shooting them off to the Alerts table / Stack management is the switch in context. I think it would help greatly if we can provide slightly more information for the user so they can make a more informed decision whether to leave the ML app entirely.
While I'm not suggesting we need to through a full alert table in here, but I think we should consider at least showing enough information to be usable. And perhaps allow the user to customize what is shown here. So we can perhaps turn off blocks if our concern is page density, but let the user turn those on if they desire. Not sure I follow this one.
If I am on an overview page for AD Jobs, I would expect to clearly see any alerts associated with those rules—and only those rules (not alerts from other rules). For example, this O11y screen shows alerts specific to this host:
While this may save some page height, I'd be concerned that it creates a more confusing UI anyway. It woudl be easier to scroll to a clearly visible section rather than find a small icon.
Agreed, but I think we should prioritize some information here. Particularly if we are sending them outside the ML app. That's a big switch in context for little awareness for what I'm going to see.
Yes. this seems like a reasonable middle-ground. We could provide a selection UI before the flyout if there are multiple alerts in a single point. The flyout does currently have the option to cycle through the documents. |
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.
Observability changes LGTM! Did code review only. Will test it locally as well. Do I need some special data to get ml alerts?
thanks for checking @mgiota! your team were assigned as reviewers because you're mentioned as codeowners for this file |
x-pack/plugins/ml/public/alerting/anomaly_detection_alerts_table/use_alerts_flyout.tsx
Show resolved
Hide resolved
@darnautov Yep this change looks good, from Obs-Ux-Management side PR is already approved! Ι know your team is actively testing alerts on the Anomaly timeline, out of curiosity I wanted to test it locally as well. I figured out I can use kibana sample data and create a job to be able to get the Anomaly timeline. Once I find some time, I will checkout your branch locally, generate some alerts and play with the alerts table. I didn't find any Unless I miss something, the only way to create Machine Learning rules is only through cc @grabowskit |
@elasticmachine merge upstream |
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.
Latest changes LGTM.
Hope we can add the 'Add to cases' actions in for 8.13 once that functionality is supported.
Also look forward to seeing options for enhancing the info in the summary tab @mdefazio .
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Async chunks
Canvas Sharable Runtime
Page load bundle
History
To update your PR or re-run it, just comment with: cc @darnautov |
## Summary [This change](https://github.com/darnautov/kibana/blob/b6f20b2219b5ccf22316ee36e0c079b9e3d1327c/x-pack/plugins/ml/public/application/components/collapsible_panel/panel_header_items.tsx#L9) in this PR: #167998 is causing an error in building storybooks: https://buildkite.com/elastic/kibana-on-merge/builds/38046#018bb9d2-7820-43ad-9144-e40d33d28c3b In brief, the import looks like this: ```typescript import { css } from '@emotion/react/dist/emotion-react.cjs'; ``` but it should be like this: ```typescript import { css } from '@emotion/react'; ``` It looks it's a bad import, we should set up a pre-merge check for these accidental auto-imports. cc: @darnautov please review
Summary
With alerts-as-data integration added in #166349, we're enabled to incorporate alerts historical data into views in the ML UI to see how it correlates with the anomaly results.
This PR add alerts data to the Anomaly Explorer page. If selected anomaly detection jobs have associated alerting rules, we show a new "Alerts" panel.
It contains:
A line chart with alerts count over time using the Lens embeddable
It support sync cursor with the Anomaly swim lane making it easier to align anomalous buckets with alerts spikes.
Summary of the alerting rules
Shows an aggregated information for each alerting rule associated with the current job selection:
Rules summary has a descending order based on the following criteria:
Alert details
It contains an alerts table provided by
triggersActionsUI
plugin. For each alert the user can:Alert context menu
When an anomaly swim lane cells are selected, and there are alerts within the chosen time range, a context menu displaying alert details is shown.
Checklist