-
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
[Stack Monitoring] Adding alerts to react app #114029
[Stack Monitoring] Adding alerts to react app #114029
Conversation
…add-alerts-to-sm-react
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
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.
Are you still working on this? There are missing some features:
- The modal doesn't appear on the overview page
- Alerts needs to be loaded also in other pages, for example in the elasticsearch nodes page like in angular:
(from your description is not clear if you plan to do this in following PRs, so sorry if that's the case, and I'm also not sure if all the views that load alerts are merged to master...)
There is an error when loading the application:
ERROR in ./public/application/setup_mode/setup_mode_renderer.js │ Module not found: Error: Can't resolve '../../application/global_state_context'
because the path just needs to be updated.
I also see an error when clicking on the "Create default rules" link in the dropdown:
But the request seems to work fine and alerts are created. Maybe it's an error parsing the response.
x-pack/plugins/monitoring/public/application/pages/page_template.tsx
Outdated
Show resolved
Hide resolved
@simianhacker Alerts were not added to any of the views as @estermv pointed out. I wonder if that would be too many changes for one PR or if we should go back through each section to make the changes and test? |
I can add them in this PR. I wasn't sure where all the alerts needed to be loaded. |
Pull request was converted to draft
…ving loadAlerts from page template
…add-alerts-to-sm-react
It looks like all the rules are being passed instead of the ones that belong to that page. Nodes should have 6 rules. Open the dropdown by clicking the badge to see too many rules. I think that gets done like this: https://github.com/elastic/kibana/blob/master/x-pack/plugins/monitoring/public/views/elasticsearch/nodes/index.js#L96 |
Code looks good. I'll leave to someone else to approve it. |
@neptunian The error in #114029 (comment) is happening on master too. We should file a bug and fix that in a separate PR. |
I have alerts firing on the logstash nodes but they don't appear in the Logstash nodes listing page. Also the "Alerts" that has the alert count above the table is missing. Use this to get alert firing https://gist.github.com/chrisronline/3b982d95710ef820d11c7443a1e49091 |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Performance Tests.x-pack/test/performance/tests/reporting_dashboard·ts.performance reporting dashbaord downloaded PDF has OK statusStandard Out
Stack Trace
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
💔 Backport failed
To backport manually run: |
* [Stack Monitoring] Adding alerts to react app * Fixing global state context path * adding alerts to pages; adding alerts model to cluster_overview; removing loadAlerts from page template * Fixing request for enable alerts * remove loadAlerts from page template * Adding request error handlers * removing redundent error handling * Changing useRequestErrorHandler function to be async due to error.response.json call * removing old comment * Fixing contexts paths * Converting ajaxRequestErrorHandler to useRequestErrorHandler * Refactoring error handler for page template and setup mode * Removing unnecessary async/await * Removing unnecessary async/await in useClusters * adding alertTypeIds to each page * fixing instance count * Adding alertTypeIds to index page * Adding alert filters for specific pages * Adding alerts to Logstash nodes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* [Stack Monitoring] Adding alerts to react app (#114029) * [Stack Monitoring] Adding alerts to react app * Fixing global state context path * adding alerts to pages; adding alerts model to cluster_overview; removing loadAlerts from page template * Fixing request for enable alerts * remove loadAlerts from page template * Adding request error handlers * removing redundent error handling * Changing useRequestErrorHandler function to be async due to error.response.json call * removing old comment * Fixing contexts paths * Converting ajaxRequestErrorHandler to useRequestErrorHandler * Refactoring error handler for page template and setup mode * Removing unnecessary async/await * Removing unnecessary async/await in useClusters * adding alertTypeIds to each page * fixing instance count * Adding alertTypeIds to index page * Adding alert filters for specific pages * Adding alerts to Logstash nodes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> * Fixing types from merge Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
This PR closes #111774 by adding the alerts dropdown to the header and loading alerts on all the pages that needed alerts. This PR also closes #111842 by adding an error handler for the requests that had
// TODO: handle error
.