Check if escalation was skipped in Slack before trying to notify user (#3562)
# What this PR does Updates check if escalation was skipped in Slack before trying to notify user by Slack. ## Which issue(s) this PR fixes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required)
This commit is contained in:
Yulya Artyukhina
GitHub
parent
92fa509d22
commit
2b62da77b7
4 changed files with 81 additions and 4 deletions
|
|
@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
- Support e2e tests in Tilt and Makefile ([#3516](https://github.com/grafana/oncall/pull/3516))
|
||||
|
||||
### Fixed
|
||||
|
||||
- Check reason to skip notification in Slack to avoid task perform_notification retries @Ferril ([#3562](https://github.com/grafana/oncall/pull/3562))
|
||||
|
||||
## v1.3.80 (2023-12-14)
|
||||
|
||||
### Added
|
||||
|
|
|
|||
|
|
@ -489,6 +489,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.
|
|||
AlertGroup.ACCOUNT_INACTIVE,
|
||||
AlertGroup.RATE_LIMITED,
|
||||
AlertGroup.CHANNEL_NOT_SPECIFIED,
|
||||
AlertGroup.RESTRICTED_ACTION,
|
||||
)
|
||||
|
||||
def is_alert_a_resolve_signal(self, alert):
|
||||
|
|
|
|||
|
|
@ -287,19 +287,29 @@ def perform_notification(log_record_pk):
|
|||
# Code below is not consistent.
|
||||
# We check various slack reasons to skip escalation in this task, in send_slack_notification,
|
||||
# before and after posting of slack message.
|
||||
if alert_group.reason_to_skip_escalation == alert_group.RATE_LIMITED:
|
||||
if alert_group.skip_escalation_in_slack:
|
||||
notification_error_code = UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK
|
||||
if alert_group.reason_to_skip_escalation == alert_group.RATE_LIMITED:
|
||||
notification_error_code = UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_RATELIMIT
|
||||
elif alert_group.reason_to_skip_escalation == alert_group.CHANNEL_ARCHIVED:
|
||||
notification_error_code = (
|
||||
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_CHANNEL_IS_ARCHIVED
|
||||
)
|
||||
elif alert_group.reason_to_skip_escalation == alert_group.ACCOUNT_INACTIVE:
|
||||
notification_error_code = UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_TOKEN_ERROR
|
||||
task_logger.debug(
|
||||
f"send_slack_notification for alert_group {alert_group.pk} failed because of slack ratelimit."
|
||||
f"send_slack_notification for alert_group {alert_group.pk} failed because escalation in slack is "
|
||||
f"skipped, reason: '{alert_group.get_reason_to_skip_escalation_display()}'"
|
||||
)
|
||||
UserNotificationPolicyLogRecord(
|
||||
author=user,
|
||||
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED,
|
||||
notification_policy=notification_policy,
|
||||
reason="Slack ratelimit",
|
||||
reason=f"Skipped escalation in Slack, reason: '{alert_group.get_reason_to_skip_escalation_display()}'",
|
||||
alert_group=alert_group,
|
||||
notification_step=notification_policy.step,
|
||||
notification_channel=notification_channel,
|
||||
notification_error_code=UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_RATELIMIT,
|
||||
notification_error_code=notification_error_code,
|
||||
).save()
|
||||
return
|
||||
|
||||
|
|
|
|||
|
|
@ -2,10 +2,12 @@ from unittest.mock import patch
|
|||
|
||||
import pytest
|
||||
|
||||
from apps.alerts.models import AlertGroup
|
||||
from apps.alerts.tasks.notify_user import notify_user_task, perform_notification
|
||||
from apps.api.permissions import LegacyAccessControlRole
|
||||
from apps.base.models.user_notification_policy import UserNotificationPolicy
|
||||
from apps.base.models.user_notification_policy_log_record import UserNotificationPolicyLogRecord
|
||||
from apps.slack.models import SlackMessage
|
||||
|
||||
NOTIFICATION_UNAUTHORIZED_MSG = "notification is not allowed for user"
|
||||
|
||||
|
|
@ -178,3 +180,63 @@ def test_notify_user_error_if_viewer(
|
|||
assert error_log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED
|
||||
assert error_log_record.reason == NOTIFICATION_UNAUTHORIZED_MSG
|
||||
assert error_log_record.notification_error_code == UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_FORBIDDEN
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@pytest.mark.parametrize(
|
||||
"reason_to_skip_escalation,error_code",
|
||||
[
|
||||
(AlertGroup.RATE_LIMITED, UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_RATELIMIT),
|
||||
(AlertGroup.CHANNEL_ARCHIVED, UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_CHANNEL_IS_ARCHIVED),
|
||||
(AlertGroup.ACCOUNT_INACTIVE, UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_TOKEN_ERROR),
|
||||
(AlertGroup.RESTRICTED_ACTION, UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK),
|
||||
(AlertGroup.NO_REASON, None),
|
||||
],
|
||||
)
|
||||
def test_perform_notification_reason_to_skip_escalation_in_slack(
|
||||
reason_to_skip_escalation,
|
||||
error_code,
|
||||
make_organization,
|
||||
make_slack_team_identity,
|
||||
make_user,
|
||||
make_user_notification_policy,
|
||||
make_alert_receive_channel,
|
||||
make_alert_group,
|
||||
make_user_notification_policy_log_record,
|
||||
make_slack_message,
|
||||
):
|
||||
organization = make_organization()
|
||||
slack_team_identity = make_slack_team_identity()
|
||||
organization.slack_team_identity = slack_team_identity
|
||||
organization.save()
|
||||
user = make_user(organization=organization)
|
||||
user_notification_policy = make_user_notification_policy(
|
||||
user=user,
|
||||
step=UserNotificationPolicy.Step.NOTIFY,
|
||||
notify_by=UserNotificationPolicy.NotificationChannel.SLACK,
|
||||
)
|
||||
alert_receive_channel = make_alert_receive_channel(organization=organization)
|
||||
alert_group = make_alert_group(alert_receive_channel=alert_receive_channel)
|
||||
alert_group.reason_to_skip_escalation = reason_to_skip_escalation
|
||||
alert_group.save()
|
||||
log_record = make_user_notification_policy_log_record(
|
||||
author=user,
|
||||
alert_group=alert_group,
|
||||
notification_policy=user_notification_policy,
|
||||
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED,
|
||||
)
|
||||
if not error_code:
|
||||
make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
|
||||
with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification:
|
||||
perform_notification(log_record.pk)
|
||||
last_log_record = UserNotificationPolicyLogRecord.objects.last()
|
||||
|
||||
if error_code:
|
||||
log_reason = f"Skipped escalation in Slack, reason: '{alert_group.get_reason_to_skip_escalation_display()}'"
|
||||
mocked_send_slack_notification.assert_not_called()
|
||||
assert last_log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED
|
||||
assert last_log_record.reason == log_reason
|
||||
assert last_log_record.notification_error_code == error_code
|
||||
else:
|
||||
mocked_send_slack_notification.assert_called()
|
||||
assert last_log_record.type != UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue