Skip to content

[Logs UI] [Alerting] Alerts management page enhancements#64654

Merged
Kerry350 merged 7 commits intoelastic:masterfrom
Kerry350:logs-alerting-phase-1-followups
May 4, 2020
Merged

[Logs UI] [Alerting] Alerts management page enhancements#64654
Kerry350 merged 7 commits intoelastic:masterfrom
Kerry350:logs-alerting-phase-1-followups

Conversation

@Kerry350
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 commented Apr 28, 2020 •
edited
Loading

Summary

This PR:

  • Moves us over to using the new log source configuration hook from [Logs UI] Reimplement log source configuration routes in plain HTTP+JSON #64021.
  • Ensures that fetch is an injected dependency, which also kills off several legacy_singleton uses.
  • Ultimately ensures that log alerts can be added, and edited, from the alerts management page. Without relying on anything specific to rendering within the logs app.

Testing

⚠️ There is currently an issue whereby the alert types are registered in mount, instead of setup, to reduce bundle sizes which breaks editing on the alerts management page. If someone starts reviewing this before this is sorted with Ops you can simply move the line this.registerLogsAlertType(pluginsSetup); from mount to setup

Alerting requires SSL, with a local setup this can be enabled like so:

  • yarn es snapshot --ssl
  • yarn start --ssl
  • ssl.verification_mode: none inside of the Filebeat config.

Things to check:

  • Adding an alert works from within logs
  • Adding and editing an alert works from the alerts management page
  • Nothing source configuration related has broken

@Kerry350 Kerry350 requested a review from a team April 29, 2020 12:08
@Kerry350 Kerry350 added Feature:Alerting Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.8.0 v8.0.0 labels Apr 29, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 29, 2020

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@Kerry350 Kerry350 marked this pull request as ready for review April 29, 2020 12:09
@Kerry350
Copy link
Copy Markdown
Contributor Author

Kerry350 commented Apr 30, 2020

@elasticmachine merge upstream

@afgomez afgomez self-requested a review May 4, 2020 08:15
afgomez
afgomez approved these changes May 4, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@afgomez afgomez left a comment

Choose a reason for hiding this comment

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

Pulled the code. I couldn't find anything not working 🎉

x-pack/plugins/infra/public/components/alerting/logs/expression_editor/editor.tsx
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { IErrorObject } from '../../../../../../triggers_actions_ui/public/types';
import { useSource } from '../../../../containers/source';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
Copy link
Copy Markdown
Contributor

@afgomez afgomez May 4, 2020

Choose a reason for hiding this comment

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

Do you know why this path might be restricted? Maybe it's a bug and it shouldn't

Copy link
Copy Markdown
Contributor Author

@Kerry350 Kerry350 May 4, 2020

Choose a reason for hiding this comment

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

Yeah, I need to follow this up with the Alerting team. There's a few types we're (logs and metrics) using that aren't exported from public or server, and so aren't really supposed to be used. Once they're bumped up to being part of the public or server contract, I'll remove the lint overrides. I'll handle in a followup 👍

@Kerry350
Copy link
Copy Markdown
Contributor Author

Kerry350 commented May 4, 2020

@elasticmachine merge upstream

@Kerry350
Copy link
Copy Markdown
Contributor Author

Kerry350 commented May 4, 2020

@afgomez Thanks for the review 👍

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 4, 2020

💚 Build Succeeded

History

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

@Kerry350 Kerry350 merged commit 5bcf2c8 into elastic:master May 4, 2020
Kerry350 added a commit to Kerry350/kibana that referenced this pull request May 4, 2020
@Kerry350
[Logs UI] [Alerting] Alerts management page enhancements (elastic#64654)
633250b 
* Ensure adding / editing log alerts works from the alerts management page
@Kerry350 Kerry350 mentioned this pull request May 4, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 4, 2020
@gmmorris
Merge branch 'alerting/np-tests-migration' of github.com:gmmorris/kib…
85a6679 
…ana into alerting/np-tests-migration

* 'alerting/np-tests-migration' of github.com:gmmorris/kibana:
  [APM] Agent remote config: validation for Java agent configs (elastic#63956)
  [APM] Fix duplicate index patterns (elastic#64883)
  Drilldowns (elastic#61219)
  [Alerting] fix labels and links in PagerDuty action ui and docs (elastic#64032)
  [Event Log] Ensure sorting tests are less flaky (elastic#64781)
  update endpoint to restrict removing with datasources (elastic#64978)
  [Logs UI] [Alerting] Alerts management page enhancements (elastic#64654)
  Adjust kibana app owning files (elastic#65064)
  Migrate tutorial resources (elastic#64298)
  [Logs UI] Tweak copy in log alerts dialog (elastic#64645)
  [Logs UI] [Alerting] Documentation (elastic#64886)
  [Logs UI] Add dataset filter to ML module setup screen (elastic#64470)
  [TSVB] Fixing memory leak (elastic#64918)
  Bump backport to 5.4.1 (elastic#65041)
Kerry350 added a commit that referenced this pull request May 4, 2020
@Kerry350 @elasticmachine
[Logs UI] [Alerting] Alerts management page enhancements (#64654) (#6…
521ee01 
…5070)

* Ensure adding / editing log alerts works from the alerts management page

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@afgomez afgomez afgomez approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Feature:Alerting Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.8.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Kerry350 @elasticmachine @kibanamachine @afgomez