[Response Ops][Alerting] Fixing bug with using runSoon on pre-8.x rule
#142505
Conversation
ymao1
changed the title
runSoon on pre-8.x rule
ymao1
added
Feature:Alerting
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
| ? await this.taskManager.get(attributes.scheduledTaskId) | ||
| : null; | ||
| } catch (err) { | ||
| return i18n.translate('xpack.alerting.rulesClient.runSoon.getTaskError', { |
There was a problem hiding this comment.
do we really need to return an error at this point? I am wondering if we can just ignore attributes.scheduledTaskId and just use id
There was a problem hiding this comment.
If there is an error getting the task doc at this point, it likely means the task doc doesn't exist, so trying to run it later will result in an error as well. Returning at this point avoids us checking for the task again, which happens when you call taskManager.runSoon
| @@ -2989,7 +3011,16 @@ export class RulesClient { | |||
| }); | |||
| } | |||
|
|
|||
| await this.taskManager.runSoon(id); | |||
| try { | |||
| await this.taskManager.runSoon(attributes.scheduledTaskId ? attributes.scheduledTaskId : id); | |||
There was a problem hiding this comment.
Is there a case where attributes.scheduledTaskId is falsy but id would actually be valid? It seems like it would work, but wondering how the rule could get in this state.
There was a problem hiding this comment.
Hmm...I guess this could happen previously if something occurred when the rule was getting disabled and the rule SO gets updated with a null scheduledTaskId but then deleting the associated document ends in an error. If that occurs, the user would be seeing a lot of Rule ran after disabled errors.
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @ymao1 |
…ule (elastic#142505) * Running task using scheduled task id. Adding functional test * dont run if rule is disable * Fixing i18n (cherry picked from commit e007ad6)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ule (#142505) (#142550) * Running task using scheduled task id. Adding functional test * dont run if rule is disable * Fixing i18n (cherry picked from commit e007ad6) Co-authored-by: Ying Mao <[email protected]>
…ule (elastic#142505) * Running task using scheduled task id. Adding functional test * dont run if rule is disable * Fixing i18n
…ule (elastic#142505) * Running task using scheduled task id. Adding functional test * dont run if rule is disable * Fixing i18n
Resolves #142293
Summary
Updates
runSoonfunction to use scheduled task id to run rule instead of rule ID. Using the rule ID causes issues for pre-8.x rules where the scheduled task ID was randomly generated and did not match the rule ID. Also added some additional try/catches to provide more graceful error handling in the case of missing tasks.To Verify
Run Rulefrom the menu dropdown. This should return a success toast saying that the rule has been scheduled to run.Checklist