-
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
[AO] Remove default recovery messages for APM rule types #160928
[AO] Remove default recovery messages for APM rule types #160928
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
Pinging @elastic/actionable-observability (Team: Actionable Observability) |
Pinging @elastic/apm-ui (Team:APM) |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
To update your PR or re-run it, just comment with: |
@@ -16,13 +16,9 @@ import { | |||
import { ApmRuleType } from '../../../../common/rules/apm_rule_types'; | |||
import { | |||
anomalyMessage, | |||
anomalyRecoveryMessage, |
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.
Did you leave the messages on purpose ?
export const anomalyRecoveryMessage = i18n.translate( |
if the recovery messages are not used anywhere can we clean it? We can always bring it back later
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.
Yes, I kept them because we will bring them back after #158183 is done.
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.
Here is a ticket for it: #160969
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.
thanks 🙏
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.
LGTM. just left one comment regarding the messages
) Fixes elastic#160926 Partially reverts elastic#159571 ## Summary At the moment, we don't have any context for recovered alerts for APM rule types, and the default messages will be empty. We will bring these messages back after elastic#158183 is done. ## 🧪 How to test - When creating an APM rule, add an action for the recovered state. You should see the default recovery message as Recovered <img src="https://github.com/elastic/kibana/assets/12370520/eb6ef0cc-0dbc-4758-a73e-4647b068f9a1" width="500"/> (cherry picked from commit 3382061)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…) (#160967) # Backport This will backport the following commits from `main` to `8.9`: - [[AO] Remove default recovery messages for APM rule types (#160928)](#160928) <!--- 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-30T09:14:11Z","message":"[AO] Remove default recovery messages for APM rule types (#160928)\n\nFixes #160926\r\nPartially reverts https://github.com/elastic/kibana/pull/159571\r\n\r\n## Summary\r\n\r\nAt the moment, we don't have any context for recovered alerts for APM\r\nrule types, and the default messages will be empty. We will bring these\r\nmessages back after #158183 is\r\ndone.\r\n\r\n## 🧪 How to test\r\n- When creating an APM rule, add an action for the recovered state. You\r\nshould see the default recovery message as Recovered\r\n\r\n<img\r\nsrc=\"https://github.com/elastic/kibana/assets/12370520/eb6ef0cc-0dbc-4758-a73e-4647b068f9a1\"\r\nwidth=\"500\"/>","sha":"3382061db4acd29c57f3b6def3e3008d46765a38","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:APM","release_note:skip","Team: Actionable Observability","backport:prev-minor","v8.10.0"],"number":160928,"url":"https://github.com/elastic/kibana/pull/160928","mergeCommit":{"message":"[AO] Remove default recovery messages for APM rule types (#160928)\n\nFixes #160926\r\nPartially reverts https://github.com/elastic/kibana/pull/159571\r\n\r\n## Summary\r\n\r\nAt the moment, we don't have any context for recovered alerts for APM\r\nrule types, and the default messages will be empty. We will bring these\r\nmessages back after #158183 is\r\ndone.\r\n\r\n## 🧪 How to test\r\n- When creating an APM rule, add an action for the recovered state. You\r\nshould see the default recovery message as Recovered\r\n\r\n<img\r\nsrc=\"https://github.com/elastic/kibana/assets/12370520/eb6ef0cc-0dbc-4758-a73e-4647b068f9a1\"\r\nwidth=\"500\"/>","sha":"3382061db4acd29c57f3b6def3e3008d46765a38"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/160928","number":160928,"mergeCommit":{"message":"[AO] Remove default recovery messages for APM rule types (#160928)\n\nFixes #160926\r\nPartially reverts https://github.com/elastic/kibana/pull/159571\r\n\r\n## Summary\r\n\r\nAt the moment, we don't have any context for recovered alerts for APM\r\nrule types, and the default messages will be empty. We will bring these\r\nmessages back after #158183 is\r\ndone.\r\n\r\n## 🧪 How to test\r\n- When creating an APM rule, add an action for the recovered state. You\r\nshould see the default recovery message as Recovered\r\n\r\n<img\r\nsrc=\"https://github.com/elastic/kibana/assets/12370520/eb6ef0cc-0dbc-4758-a73e-4647b068f9a1\"\r\nwidth=\"500\"/>","sha":"3382061db4acd29c57f3b6def3e3008d46765a38"}}]}] BACKPORT--> Co-authored-by: Maryam Saeidi <maryam.saeidi@elastic.co>
Fixes #160926
Partially reverts #159571
Summary
At the moment, we don't have any context for recovered alerts for APM rule types, and the default messages will be empty. We will bring these messages back after #158183 is done.
🧪 How to test