Skip to content

Change kibana.alert.evaluation.threshold unit to microseconds - #158703

Merged
maryam-saeidi merged 1 commit into
elastic:mainfrom
maryam-saeidi:156255-change-apm-threshold-unit
Jun 1, 2023
Merged

maryam-saeidi merged 1 commit into
elastic:mainfrom
maryam-saeidi:156255-change-apm-threshold-unit

Conversation

@maryam-saeidi

@maryam-saeidi maryam-saeidi commented May 31, 2023 •

Copy link
Copy Markdown
Member

Resolves #156255
Fixes #158204
Partially reverts #154801

RFC Document

Summary

This PR changes kibana.alert.evaluation.threshold unit to microseconds in AAD.

Case Screenshot
Saved value in AAD image
Should show correct information in flyout image
Should show correct information on alert details page image
Should show correct information in action message image
Same unit for value and threshold in the alert table image

🧪 How to test

  • Generate an APM Latency threshold alert
  • Check the threshold in
    • Alert's flyout
    • Alert's table
    • Alert details page (summary and chart)
    • Action generated by this alert

@ghost

ghost commented May 31, 2023

Copy link
Copy Markdown

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@maryam-saeidi maryam-saeidi self-assigned this May 31, 2023
@maryam-saeidi
maryam-saeidi marked this pull request as ready for review May 31, 2023 09:17
@maryam-saeidi
maryam-saeidi requested review from a team May 31, 2023 09:17
@kibana-ci

Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.5MB 3.5MB -46.0B
observability 917.7KB 917.5KB -196.0B
total -242.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
apm 33.5KB 33.4KB -34.0B
observability 46.0KB 45.9KB -70.0B
total -104.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 416 420 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 500 504 +4
total +6

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

cc @maryam-saeidi

@kdelemme kdelemme left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@botelastic botelastic Bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label May 31, 2023
@elasticmachine

Copy link
Copy Markdown
Contributor

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

@fkanout
fkanout self-requested a review May 31, 2023 14:07

@fkanout fkanout left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The latency chart threshold annotation is fixed too. Nice work @maryam-saeidi !
Screenshot 2023-05-31 at 16 31 57

@maryam-saeidi maryam-saeidi added the Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" label May 31, 2023
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

value: formatAlertEvaluationValue(
alert?.fields[ALERT_RULE_TYPE_ID],
toMicroseconds(alert?.fields[ALERT_EVALUATION_THRESHOLD])
alert?.fields[ALERT_EVALUATION_THRESHOLD]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like alert?.fields[ALERT_EVALUATION_THRESHOLD] was previously in milliseconds but going forward it will be in microseconds? Where does this happen?

@maryam-saeidi maryam-saeidi Jun 1, 2023 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here is the change: (next file change in this PR)

[ALERT_EVALUATION_THRESHOLD]: thresholdMicroseconds,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I see. lgtm 👍

@maryam-saeidi
maryam-saeidi merged commit d13f884 into elastic:main Jun 1, 2023
@kibanamachine

Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
❌ 8.8 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 158703

Questions ?

Please refer to the Backport tool documentation

maryam-saeidi added a commit to maryam-saeidi/kibana that referenced this pull request Jun 1, 2023
…c#158703)

Resolves elastic#156255
Fixes elastic#158204
Partially reverts elastic#154801

[RFC
Document](https://docs.google.com/document/d/1-O6-nqOedrjtTF9mHawd_u-vUdXu06KGI8ic5qNeQPQ/edit?usp=sharing)

This PR changes kibana.alert.evaluation.threshold unit to microseconds
in AAD.

|Case|Screenshot|
|----|---|
|Saved value in
AAD|![image](https://github.com/elastic/kibana/assets/12370520/9178031b-a187-4b3e-b6ed-6e67f4e708ce)|
|Should show correct information in
flyout|![image](https://github.com/elastic/kibana/assets/12370520/75ec6664-8e2f-448d-b6d0-dacf01b22ac3)|
|Should show correct information on alert details
page|![image](https://github.com/elastic/kibana/assets/12370520/4fb4b645-b450-48da-bec9-e7064b26258e)|
|Should show correct information in action
message|![image](https://github.com/elastic/kibana/assets/12370520/c103266e-25ef-4c25-a0da-c6ecfd0e4c0a)|
|Same unit for value and threshold in the alert
table|![image](https://github.com/elastic/kibana/assets/12370520/499814bc-9f41-4b94-9b03-18a75a50489f)|

- Generate an APM Latency threshold alert
- Check the threshold in
    - Alert's flyout
    - Alert's table
    - Alert details page (summary and chart)
    - Action generated by this alert

(cherry picked from commit d13f884)
maryam-saeidi added a commit to maryam-saeidi/kibana that referenced this pull request Jun 5, 2023
…c#158703)

Resolves elastic#156255
Fixes elastic#158204
Partially reverts elastic#154801

[RFC
Document](https://docs.google.com/document/d/1-O6-nqOedrjtTF9mHawd_u-vUdXu06KGI8ic5qNeQPQ/edit?usp=sharing)

## Summary

This PR changes kibana.alert.evaluation.threshold unit to microseconds
in AAD.

|Case|Screenshot|
|----|---|
|Saved value in
AAD|![image](https://github.com/elastic/kibana/assets/12370520/9178031b-a187-4b3e-b6ed-6e67f4e708ce)|
|Should show correct information in
flyout|![image](https://github.com/elastic/kibana/assets/12370520/75ec6664-8e2f-448d-b6d0-dacf01b22ac3)|
|Should show correct information on alert details
page|![image](https://github.com/elastic/kibana/assets/12370520/4fb4b645-b450-48da-bec9-e7064b26258e)|
|Should show correct information in action
message|![image](https://github.com/elastic/kibana/assets/12370520/c103266e-25ef-4c25-a0da-c6ecfd0e4c0a)|
|Same unit for value and threshold in the alert
table|![image](https://github.com/elastic/kibana/assets/12370520/499814bc-9f41-4b94-9b03-18a75a50489f)|

## 🧪 How to test
- Generate an APM Latency threshold alert
- Check the threshold in
    - Alert's flyout
    - Alert's table
    - Alert details page (summary and chart)
    - Action generated by this alert

(cherry picked from commit d13f884)

# Conflicts:
#	x-pack/plugins/apm/public/components/alerting/ui_components/alert_details_app_section/index.tsx
#	x-pack/plugins/observability/public/components/alerts_flyout_body.tsx
@maryam-saeidi

Copy link
Copy Markdown
Member Author

💚 All backports created successfully

Status Branch Result
✅ 8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

maryam-saeidi added a commit that referenced this pull request Jun 5, 2023
…158703) (#159013)

# Backport

This will backport the following commits from `main` to `8.8`:
- [Change kibana.alert.evaluation.threshold unit to microseconds
(#158703)](#158703)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maryam
Saeidi","email":"maryam.saeidi@elastic.co"},"sourceCommit":{"committedDate":"2023-06-01T11:12:21Z","message":"Change
kibana.alert.evaluation.threshold unit to microseconds
(#158703)\n\nResolves #156255\r\nFixes #158204\r\nPartially reverts
#154801\r\n\r\n[RFC\r\nDocument](https://docs.google.com/document/d/1-O6-nqOedrjtTF9mHawd_u-vUdXu06KGI8ic5qNeQPQ/edit?usp=sharing)\r\n\r\n##
Summary\r\n\r\nThis PR changes kibana.alert.evaluation.threshold unit to
microseconds\r\nin AAD.\r\n\r\n|Case|Screenshot|\r\n|----|---|\r\n|Saved
value
in\r\nAAD|![image](https://github.com/elastic/kibana/assets/12370520/9178031b-a187-4b3e-b6ed-6e67f4e708ce)|\r\n|Should
show correct information
in\r\nflyout|![image](https://github.com/elastic/kibana/assets/12370520/75ec6664-8e2f-448d-b6d0-dacf01b22ac3)|\r\n|Should
show correct information on alert
details\r\npage|![image](https://github.com/elastic/kibana/assets/12370520/4fb4b645-b450-48da-bec9-e7064b26258e)|\r\n|Should
show correct information in
action\r\nmessage|![image](https://github.com/elastic/kibana/assets/12370520/c103266e-25ef-4c25-a0da-c6ecfd0e4c0a)|\r\n|Same
unit for value and threshold in the
alert\r\ntable|![image](https://github.com/elastic/kibana/assets/12370520/499814bc-9f41-4b94-9b03-18a75a50489f)|\r\n\r\n\r\n##
🧪 How to test\r\n- Generate an APM Latency threshold alert\r\n- Check
the threshold in\r\n - Alert's flyout\r\n - Alert's table\r\n - Alert
details page (summary and chart)\r\n - Action generated by this
alert","sha":"d13f884ec3a4e61fcc23e5594741ec436fa4c4fb","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:APM","Team:
Actionable
Observability","backport:prev-minor","v8.9.0"],"number":158703,"url":"https://github.com/elastic/kibana/pull/158703","mergeCommit":{"message":"Change
kibana.alert.evaluation.threshold unit to microseconds
(#158703)\n\nResolves #156255\r\nFixes #158204\r\nPartially reverts
#154801\r\n\r\n[RFC\r\nDocument](https://docs.google.com/document/d/1-O6-nqOedrjtTF9mHawd_u-vUdXu06KGI8ic5qNeQPQ/edit?usp=sharing)\r\n\r\n##
Summary\r\n\r\nThis PR changes kibana.alert.evaluation.threshold unit to
microseconds\r\nin AAD.\r\n\r\n|Case|Screenshot|\r\n|----|---|\r\n|Saved
value
in\r\nAAD|![image](https://github.com/elastic/kibana/assets/12370520/9178031b-a187-4b3e-b6ed-6e67f4e708ce)|\r\n|Should
show correct information
in\r\nflyout|![image](https://github.com/elastic/kibana/assets/12370520/75ec6664-8e2f-448d-b6d0-dacf01b22ac3)|\r\n|Should
show correct information on alert
details\r\npage|![image](https://github.com/elastic/kibana/assets/12370520/4fb4b645-b450-48da-bec9-e7064b26258e)|\r\n|Should
show correct information in
action\r\nmessage|![image](https://github.com/elastic/kibana/assets/12370520/c103266e-25ef-4c25-a0da-c6ecfd0e4c0a)|\r\n|Same
unit for value and threshold in the
alert\r\ntable|![image](https://github.com/elastic/kibana/assets/12370520/499814bc-9f41-4b94-9b03-18a75a50489f)|\r\n\r\n\r\n##
🧪 How to test\r\n- Generate an APM Latency threshold alert\r\n- Check
the threshold in\r\n - Alert's flyout\r\n - Alert's table\r\n - Alert
details page (summary and chart)\r\n - Action generated by this
alert","sha":"d13f884ec3a4e61fcc23e5594741ec436fa4c4fb"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/158703","number":158703,"mergeCommit":{"message":"Change
kibana.alert.evaluation.threshold unit to microseconds
(#158703)\n\nResolves #156255\r\nFixes #158204\r\nPartially reverts
#154801\r\n\r\n[RFC\r\nDocument](https://docs.google.com/document/d/1-O6-nqOedrjtTF9mHawd_u-vUdXu06KGI8ic5qNeQPQ/edit?usp=sharing)\r\n\r\n##
Summary\r\n\r\nThis PR changes kibana.alert.evaluation.threshold unit to
microseconds\r\nin AAD.\r\n\r\n|Case|Screenshot|\r\n|----|---|\r\n|Saved
value
in\r\nAAD|![image](https://github.com/elastic/kibana/assets/12370520/9178031b-a187-4b3e-b6ed-6e67f4e708ce)|\r\n|Should
show correct information
in\r\nflyout|![image](https://github.com/elastic/kibana/assets/12370520/75ec6664-8e2f-448d-b6d0-dacf01b22ac3)|\r\n|Should
show correct information on alert
details\r\npage|![image](https://github.com/elastic/kibana/assets/12370520/4fb4b645-b450-48da-bec9-e7064b26258e)|\r\n|Should
show correct information in
action\r\nmessage|![image](https://github.com/elastic/kibana/assets/12370520/c103266e-25ef-4c25-a0da-c6ecfd0e4c0a)|\r\n|Same
unit for value and threshold in the
alert\r\ntable|![image](https://github.com/elastic/kibana/assets/12370520/499814bc-9f41-4b94-9b03-18a75a50489f)|\r\n\r\n\r\n##
🧪 How to test\r\n- Generate an APM Latency threshold alert\r\n- Check
the threshold in\r\n - Alert's flyout\r\n - Alert's table\r\n - Alert
details page (summary and chart)\r\n - Action generated by this
alert","sha":"d13f884ec3a4e61fcc23e5594741ec436fa4c4fb"}}]}] BACKPORT-->
@maryam-saeidi
maryam-saeidi deleted the 156255-change-apm-threshold-unit branch June 21, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:fix Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v8.8.1 v8.9.0

Projects

None yet

7 participants