Skip to content

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

Merged
cauemarcondes merged 7 commits intoelastic:masterfrom
cauemarcondes:apm-alerts-service
Sep 28, 2020
Merged

[APM] Alerting: Add global option to create all alert types#78151
cauemarcondes merged 7 commits intoelastic:masterfrom
cauemarcondes:apm-alerts-service

Conversation

@cauemarcondes
Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes commented Sep 22, 2020

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 September 22, 2020 13:38
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Sep 22, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 22, 2020

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

@formgeist
Copy link
Copy Markdown
Contributor

formgeist commented Sep 23, 2020

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
Copy Markdown
Contributor

sorenlouv commented Sep 23, 2020

"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

sorenlouv
sorenlouv reviewed Sep 23, 2020
View reviewed changes
sorenlouv
sorenlouv reviewed Sep 23, 2020
View reviewed changes
@cauemarcondes
Copy link
Copy Markdown
Contributor Author

cauemarcondes commented Sep 23, 2020

@elasticmachine merge upstream

@cauemarcondes
Copy link
Copy Markdown
Contributor Author

cauemarcondes commented Sep 24, 2020

"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
Copy Markdown
Contributor Author

cauemarcondes commented Sep 28, 2020

@elasticmachine merge upstream

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

@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
Copy Markdown
Contributor

sorenlouv commented Sep 28, 2020

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

@cauemarcondes
Copy link
Copy Markdown
Contributor Author

cauemarcondes commented Sep 28, 2020

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

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

@cauemarcondes
Copy link
Copy Markdown
Contributor Author

cauemarcondes commented Sep 28, 2020

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

#78573

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Sep 28, 2020

💚 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

2 more reviewers

@dgieselaar dgieselaar dgieselaar left review comments

@sorenlouv sorenlouv sorenlouv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ogupte ogupte

Labels

apm:test-plan-done Pull request that was successfully tested during the test plan release_note:enhancement Team:APM - DEPRECATED Use Team:obs-ux-infra_services. 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