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

[APM] Alerting: Add global option to create all alert types #78151

Merged
merged 7 commits into from
Sep 28, 2020
Merged

[APM] Alerting: Add global option to create all alert types #78151

merged 7 commits into from
Sep 28, 2020

Conversation

cauemarcondes
Copy link
Contributor

closes #76365

Screenshot 2020-09-22 at 15 33 37

Screenshot 2020-09-22 at 15 36 23

Screenshot 2020-09-22 at 15 36 33

Screenshot 2020-09-22 at 15 36 42

@cauemarcondes cauemarcondes requested a review from a team as a code owner September 22, 2020 13:38
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Sep 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@formgeist
Copy link
Contributor

Looks good - may I suggest a small naming change; I think we should change "All" to "Any" in the case of the service, transaction type, and environment conditions. The reason is that I might read "all" to mean only what's seen up until now and not what might be added in the future. "Any" implies that it will alert of any service, type, or environment that goes above the thresholds set. Thoughts? cc @sqren

Another minor layout thing; seeing as we're pushing the limits of the flex to the max in the header, can we make sure that the options wrap underneath the APM title when the viewport gets smaller?

Screenshot 2020-09-23 at 09 32 21

@sorenlouv
Copy link
Member

"Any" implies that it will alert of any service, type, or environment that goes above the thresholds set. Thoughts?

sgtm 👍

Another small thing: can we make the order of Service, Type and Environment consistent - in that order.

image

image

@cauemarcondes
Copy link
Contributor Author

@elasticmachine merge upstream

@cauemarcondes
Copy link
Contributor Author

"Any" implies that it will alert of any service, type, or environment that goes above the thresholds set. Thoughts?

@formgeist @sqren maybe we should wait until we remove the All option from the list of environments to change the labels to Any. I don't want to do some workaround to remove the All and add Any in the environment list.

Screenshot 2020-09-24 at 15 54 46

At least for now we don't mix up All and Any.

@cauemarcondes
Copy link
Contributor Author

@elasticmachine merge upstream

sorenlouv
sorenlouv approved these changes Sep 28, 2020
View reviewed changes
Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

lgtm!

@sorenlouv
Copy link
Member

@cauemarcondes Are you going to add defaultActionMessage in a follow-up PR?

@cauemarcondes
Copy link
Contributor Author

@cauemarcondes Are you going to add defaultActionMessage in a follow-up PR?

yes, since it's not part of this issue.

@cauemarcondes
Copy link
Contributor Author

@cauemarcondes Are you going to add defaultActionMessage in a follow-up PR?

#78573

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@cauemarcondes cauemarcondes merged commit 966f00a into elastic:master Sep 28, 2020
@cauemarcondes cauemarcondes deleted the apm-alerts-service branch September 28, 2020 13:13
@cauemarcondes cauemarcondes mentioned this pull request Sep 28, 2020
Merged
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Sep 28, 2020
@cauemarcondes @elasticmachine
[APM] Alerting: Add global option to create all alert types (elastic#…
dc1b60b 
…78151)

* adding alert to service page

* sending on alert per service environment and transaction type

* addressing PR comment

* addressing PR comment

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 28, 2020
@gmmorris
Merge branch 'master' into task-manager/health
92cf5ad 
* master: (226 commits)
  [Enterprise Search] Added Logic for the Credentials View (elastic#77626)
  [CSM] Js errors (elastic#77919)
  Add the @kbn/apm-config-loader package (elastic#77855)
  [Security Solution] Refactor useSelector (elastic#75297)
  Implement tagcloud renderer (elastic#77910)
  [APM] Alerting: Add global option to create all alert types (elastic#78151)
  [Ingest pipelines] Upload indexed document to test a pipeline (elastic#77939)
  TypeScript cleanup in visualizations plugin (elastic#78428)
  Lazy load metric & mardown visualizations (elastic#78391)
  [Detections][EQL] EQL rule execution in detection engine (elastic#77419)
  Update tutorial-full-experience.asciidoc (elastic#75836)
  Update tutorial-define-index.asciidoc (elastic#75754)
  Add support for runtime field types to mappings editor. (elastic#77420)
  [Monitoring] Usage collection (elastic#75878)
  [Docs][Actions] Add docs for Jira and IBM Resilient (elastic#78316)
  [Security Solution][Resolver] Update @timestamp formatting (elastic#78166)
  [Security Solution] Fix app layout (elastic#76668)
  [Security Solution][Resolver] 2 new functions to DAL (elastic#78477)
  Adds new elasticsearch client to telemetry plugin (elastic#78046)
  skip flaky suite (elastic#78512) (elastic#78511) (elastic#78510) (elastic#78509) (elastic#78508) (elastic#78507) (elastic#78506) (elastic#78505) (elastic#78504) (elastic#78503) (elastic#78502) (elastic#78501) (elastic#78500)
  ...
cauemarcondes added a commit that referenced this pull request Sep 29, 2020
@cauemarcondes @elasticmachine
[APM] Alerting: Add global option to create all alert types (#78151) (#…
9b3e54e 
…78579)

* adding alert to service page

* sending on alert per service environment and transaction type

* addressing PR comment

* addressing PR comment

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@ogupte ogupte self-assigned this Oct 20, 2020
@ogupte ogupte added the apm:test-plan-done Pull request that was successfully tested during the test plan label Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgieselaar dgieselaar dgieselaar left review comments

@sorenlouv sorenlouv sorenlouv approved these changes

Assignees

@ogupte ogupte

Labels
apm:test-plan-done Pull request that was successfully tested during the test plan release_note:enhancement Team:APM All issues that need APM UI Team support v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[APM] Alerting: Add global option to create all alert types
7 participants
@cauemarcondes @elasticmachine @formgeist @sorenlouv @kibanamachine @dgieselaar @ogupte