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.

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

Fix acknowledge reminder task #5179

Merged
merged 6 commits into from
Oct 16, 2024
Merged

Fix acknowledge reminder task #5179

merged 6 commits into from
Oct 16, 2024

Conversation

Ferril
Copy link
Member

@Ferril Ferril commented Oct 15, 2024

What this PR does

  • Adds 10 minutes lock for acknowledge reminder task to prevent task duplicates, that causes posting multiple reminder messages and flooding in Slack threads.
  • Adds a new signal for acknowledge reminder task instead of using alert_group_action_triggered_signal since it is used only to post reminder message in Slack thread and it's not needed to be processed by other representatives

Which issue(s) this PR closes

Related to https://github.com/grafana/oncall-private/issues/2953

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

@Ferril Ferril added pr:no public docs Added to a PR that does not require public documentation updates release:ignore PR will not be added to release notes labels Oct 15, 2024
@Ferril Ferril requested a review from a team October 15, 2024 14:00
Copy link
Contributor

@matiasb matiasb left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -41,7 +41,6 @@ def get_handlers_map():
AlertGroupLogRecord.TYPE_AUTO_UN_ACK: "alert_group_action",
AlertGroupLogRecord.TYPE_RESOLVED: "alert_group_action",
AlertGroupLogRecord.TYPE_UN_RESOLVED: "alert_group_action",
AlertGroupLogRecord.TYPE_ACK_REMINDER_TRIGGERED: "alert_group_action",
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have reminders for telegram, do we? (so was this doing nothing?)

Copy link
Member Author

@Ferril Ferril Oct 16, 2024

Choose a reason for hiding this comment

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

Correct, it's a slack-specific feature.
After receiving the signal Telegram representative updates messages related to the alert group (main message and log), but in this case it does it for nothing, since alert group status was not changed and we don't show reminder logs in alert group timeline.

@Ferril Ferril added this pull request to the merge queue Oct 16, 2024
Merged via the queue into dev with commit 8420cfd Oct 16, 2024
25 checks passed
@Ferril Ferril deleted the julia/fix-acknowledge-reminder branch October 16, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates release:ignore PR will not be added to release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants