Merge pull request #3574 from grafana/dev

Dev to main
This commit is contained in:
Michael Derynck 2023-12-14 15:34:22 -07:00 committed by GitHub
commit 2339b6096a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 239 additions and 21 deletions

View file

@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## v1.3.80 (2023-12-14)
### Added
- Create success log records for delivered personal notifications ([3557](https://github.com/grafana/oncall/pull/3557))
## v1.3.79 (2023-12-14)
### Added

View file

@ -175,6 +175,13 @@ You can read more about AlertManager Data model [here](https://prometheus.io/doc
<img width="1644" alt="Screenshot 2023-12-14 at 1 14 32PM" src="https://github.com/grafana/oncall/assets/85312870/b62cfa1d-2ff6-4b46-9cec-459b14cd1996">
{{% docs/reference %}}
[user-and-team-management]: "/docs/oncall/ -> /docs/oncall/<ONCALL VERSION>/user-and-team-management"
[user-and-team-management]: "/docs/grafana-cloud/ -> /docs/grafana-cloud/alerting-and-irm/oncall/user-and-team-management"
[complete-the-integration-configuration]: "/docs/oncall/ -> /docs/oncall/<ONCALL VERSION>/integrations#complete-the-integration-configuration"
[complete-the-integration-configuration]: "/docs/grafana-cloud/ -> /docs/grafana-cloud/alerting-and-irm/oncall/integrations#complete-the-integration-configuration"
[migration]: "/docs/oncall/ -> /docs/oncall/<ONCALL VERSION>/integrations/alertmanager#migrating-from-legacy-integration"
[migration]: "/docs/grafana-cloud/ -> /docs/grafana-cloud/alerting-and-irm/oncall/integrations/alertmanager#migrating-from-legacy-integration"
{{% /docs/reference %}}

View file

@ -106,7 +106,7 @@ def check_escalation_finished_task() -> None:
)
total_alert_groups_count = alert_groups.count()
creation_deltas = alert_groups.filter(received_at__isnull=False).aggregate(
creation_deltas = alert_groups.aggregate(
avg_delta=Avg(F("started_at") - F("received_at")),
max_delta=Max(F("started_at") - F("received_at")),
)

View file

@ -161,7 +161,7 @@ def notify_user_task(
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED,
notification_policy=notification_policy,
alert_group=alert_group,
reason=reason,
reason="Alert group slack notifications are disabled",
slack_prevent_posting=prevent_posting_to_thread,
notification_step=notification_policy.step,
notification_channel=notification_policy.notify_by,

View file

@ -25,7 +25,8 @@ def make_alert_group_that_started_at_specific_date(make_alert_group):
):
# we can't simply pass started_at to the fixture because started_at is being "auto-set" on the Model
alert_group = make_alert_group(alert_receive_channel, **kwargs)
alert_group.received_at = started_at - timezone.timedelta(seconds=received_delta)
if received_delta is not None:
alert_group.received_at = started_at - timezone.timedelta(seconds=received_delta)
alert_group.started_at = started_at
alert_group.save()
@ -358,6 +359,7 @@ def test_check_escalation_finished_task_calls_audit_alert_group_escalation_for_e
alert_group1 = make_alert_group_that_started_at_specific_date(alert_receive_channel, received_delta=1)
alert_group2 = make_alert_group_that_started_at_specific_date(alert_receive_channel, received_delta=5)
alert_group3 = make_alert_group_that_started_at_specific_date(alert_receive_channel, received_delta=12)
alert_group3 = make_alert_group_that_started_at_specific_date(alert_receive_channel, received_delta=None)
def _mocked_audit_alert_group_escalation(alert_group):
if not alert_group.id == alert_group3.id:
@ -376,7 +378,7 @@ def test_check_escalation_finished_task_calls_audit_alert_group_escalation_for_e
assert "Alert group ingestion/creation avg delta seconds: 6" in caplog.text
assert "Alert group ingestion/creation max delta seconds: 12" in caplog.text
assert "Alert group notifications success ratio: 33.33" in caplog.text
assert "Alert group notifications success ratio: 25.00" in caplog.text
mocked_audit_alert_group_escalation.assert_any_call(alert_group1)
mocked_audit_alert_group_escalation.assert_any_call(alert_group2)

View file

@ -171,7 +171,7 @@ class UserNotificationPolicyLogRecord(models.Model):
result += f"SMS to {user_verbal} was delivered successfully"
elif notification_channel == UserNotificationPolicy.NotificationChannel.PHONE_CALL:
result += f"phone call to {user_verbal} was successful"
elif notification_channel is None:
else:
result += f"notification to {user_verbal} was delivered successfully"
elif self.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED:
if self.notification_error_code == UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_SMS_LIMIT_EXCEEDED:

View file

@ -121,3 +121,13 @@ def notify_user_async(user_pk, alert_group_pk, notification_policy_pk):
)
logger.error(f"Error while sending email: {e}")
return
# record success log
UserNotificationPolicyLogRecord.objects.create(
author=user,
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS,
notification_policy=notification_policy,
alert_group=alert_group,
notification_step=notification_policy.step,
notification_channel=notification_policy.notify_by,
)

View file

@ -45,6 +45,9 @@ def test_notify_user(
notify_user_async(user.pk, alert_group.pk, notification_policy.pk)
assert len(mail.outbox) == 1
log_record = notification_policy.personal_log_records.last()
assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS
@pytest.mark.django_db
def test_notify_empty_email_host(

View file

@ -142,4 +142,16 @@ def notify_user_about_new_alert_group(user_pk, alert_group_pk, notification_poli
return
message = _get_fcm_message(alert_group, user, device_to_notify, critical)
send_push_notification(device_to_notify, message, _create_error_log_record)
succeeded = send_push_notification(device_to_notify, message, _create_error_log_record)
if succeeded:
# record success log
# (note: send_push_notification should have created a failed log entry if there was an error)
UserNotificationPolicyLogRecord.objects.create(
author=user,
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS,
notification_policy=notification_policy,
alert_group=alert_group,
notification_step=notification_policy.step,
notification_channel=notification_policy.notify_by,
)

View file

@ -47,6 +47,8 @@ def test_notify_user_about_new_alert_group(
)
mock_send_push_notification.assert_called_once()
log_record = notification_policy.personal_log_records.last()
assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS
@patch("apps.mobile_app.tasks.new_alert_group.send_push_notification")

View file

@ -30,7 +30,8 @@ def test_send_push_notification_cloud(
settings.LICENSE = CLOUD_LICENSE_NAME
settings.IS_OPEN_SOURCE = False
utils.send_push_notification(device, mock_message)
succeeded = utils.send_push_notification(device, mock_message)
assert succeeded
mock_send_message.assert_called_once_with(mock_message)
@ -77,8 +78,8 @@ def test_send_push_notification_oss(
device = FCMDevice.objects.create(user=user, registration_id="test_device_id")
mock_message = {"foo": "bar"}
utils.send_push_notification(device, mock_message, mock_error_cb)
succeeded = utils.send_push_notification(device, mock_message, mock_error_cb)
assert succeeded
mock_error_cb.assert_not_called()
mock_send_push_notification_to_fcm_relay.assert_called_once_with(mock_message)
@ -99,8 +100,9 @@ def test_send_push_notification_oss_no_cloud_connector(
device = FCMDevice.objects.create(user=user, registration_id="test_device_id")
mock_message = {"foo": "bar"}
utils.send_push_notification(device, mock_message, mock_error_cb)
succeeded = utils.send_push_notification(device, mock_message, mock_error_cb)
assert not succeeded
mock_error_cb.assert_called_once_with()
mock_send_push_notification_to_fcm_relay.assert_not_called()
@ -128,7 +130,8 @@ def test_send_push_notification_oss_fcm_relay_returns_client_error(
device = FCMDevice.objects.create(user=user, registration_id="test_device_id")
mock_message = {"foo": "bar"}
utils.send_push_notification(device, mock_message, mock_error_cb)
succeeded = utils.send_push_notification(device, mock_message, mock_error_cb)
assert not succeeded
mock_send_push_notification_to_fcm_relay.assert_called_once_with(mock_message)

View file

@ -58,7 +58,7 @@ def send_message_to_fcm_device(device: "FCMDevice", message: Message) -> None:
def send_push_notification(
device_to_notify: "FCMDevice", message: Message, error_cb: typing.Optional[typing.Callable[..., None]] = None
) -> None:
) -> bool:
logger.debug(f"Sending push notification to device type {device_to_notify.type} with message: {message}")
def _error_cb():
@ -72,7 +72,7 @@ def send_push_notification(
if not CloudConnector.objects.exists():
_error_cb()
logger.error("Error while sending a mobile push notification: not connected to cloud")
return
return False
try:
response = _send_push_notification_to_fcm_relay(message)
@ -84,12 +84,15 @@ def send_push_notification(
logger.error(
f"Error while sending a mobile push notification: HTTP client error {e.response.status_code}"
)
return
return False
else:
raise
else:
send_message_to_fcm_device(device_to_notify, message)
# notification succeeded (otherwise raised exception before)
return True
def construct_fcm_message(
message_type: MessageType,

View file

@ -173,6 +173,15 @@ class SlackMessage(models.Model):
_slack_team_identity=self.slack_team_identity,
channel_id=channel_id,
)
# create success record
UserNotificationPolicyLogRecord.objects.create(
author=user,
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS,
notification_policy=notification_policy,
alert_group=alert_group,
notification_step=notification_policy.step,
notification_channel=notification_policy.notify_by,
)
# Check if escalated user is in channel. Otherwise send notification and request to invite him.
try:

View file

@ -0,0 +1,38 @@
from unittest.mock import patch
import pytest
from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord
@pytest.mark.django_db
def test_send_slack_notification(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_alert,
make_user_notification_policy,
make_slack_channel,
make_slack_message,
):
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
# set up notification policy and alert group
notification_policy = make_user_notification_policy(
user,
UserNotificationPolicy.Step.NOTIFY,
notify_by=UserNotificationPolicy.NotificationChannel.SLACK,
)
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
make_alert(alert_group=alert_group, raw_request_data={})
slack_channel = make_slack_channel(slack_team_identity)
slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id)
with patch("apps.slack.client.SlackClient.conversations_members") as mock_members:
mock_members.return_value = {"members": [slack_user_identity.slack_id]}
slack_message.send_slack_notification(user, alert_group, notification_policy)
log_record = notification_policy.personal_log_records.last()
assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS

View file

@ -35,6 +35,7 @@ class TelegramToUserConnector(models.Model):
alert_group=alert_group,
user=user,
notification_policy=notification_policy,
reason="No linked telegram account",
error_code=UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_IS_NOT_LINKED_TO_SLACK_ACC,
)
@ -51,19 +52,36 @@ class TelegramToUserConnector(models.Model):
self.send_full_alert_group(alert_group=alert_group, notification_policy=notification_policy)
@staticmethod
def create_telegram_notification_error(
alert_group: AlertGroup, user: User, notification_policy: UserNotificationPolicy, error_code: int
def create_telegram_notification_success(
alert_group: AlertGroup, user: User, notification_policy: UserNotificationPolicy
) -> None:
log_record = UserNotificationPolicyLogRecord(
UserNotificationPolicyLogRecord.objects.create(
author=user,
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS,
notification_policy=notification_policy,
alert_group=alert_group,
notification_step=notification_policy.step if notification_policy else None,
notification_channel=notification_policy.notify_by if notification_policy else None,
)
@staticmethod
def create_telegram_notification_error(
alert_group: AlertGroup,
user: User,
notification_policy: UserNotificationPolicy,
error_code: int,
reason: str | None,
) -> None:
UserNotificationPolicyLogRecord.objects.create(
author=user,
type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED,
notification_policy=notification_policy,
alert_group=alert_group,
notification_error_code=error_code,
reason=reason,
notification_step=notification_policy.step if notification_policy else None,
notification_channel=notification_policy.notify_by if notification_policy else None,
)
log_record.save()
# send the actual alert group and log to user's DM
def send_full_alert_group(self, alert_group: AlertGroup, notification_policy: UserNotificationPolicy) -> None:
@ -75,6 +93,7 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
reason="Invalid token",
)
return
@ -106,6 +125,7 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_BOT_IS_DELETED,
reason="Bot was blocked by the user",
)
elif e.message == "Invalid token":
TelegramToUserConnector.create_telegram_notification_error(
@ -113,6 +133,7 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
reason="Invalid token",
)
elif e.message == "Forbidden: user is deactivated":
TelegramToUserConnector.create_telegram_notification_error(
@ -120,9 +141,14 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED,
reason="Telegram user was disabled",
)
else:
raise e
else:
TelegramToUserConnector.create_telegram_notification_success(
alert_group, self.user, notification_policy
)
else:
self._nudge_about_alert_group_message(telegram_client, old_alert_group_message)
@ -136,6 +162,7 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
reason="Invalid token",
)
return
@ -159,6 +186,7 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_BOT_IS_DELETED,
reason="Bot was blocked by the user",
)
elif e.message == "Invalid token":
TelegramToUserConnector.create_telegram_notification_error(
@ -166,6 +194,7 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
reason="Invalid token",
)
elif e.message == "Forbidden: user is deactivated":
TelegramToUserConnector.create_telegram_notification_error(
@ -173,9 +202,12 @@ class TelegramToUserConnector(models.Model):
self.user,
notification_policy,
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED,
reason="Telegram user was disabled",
)
else:
raise e
else:
TelegramToUserConnector.create_telegram_notification_success(alert_group, self.user, notification_policy)
@staticmethod
@ignore_reply_to_message_deleted

View file

@ -18,7 +18,7 @@ class TelegramToUserConnectorFactory(factory.DjangoModelFactory):
class TelegramChannelFactory(factory.DjangoModelFactory):
channel_chat_id = factory.LazyAttribute(lambda v: str(UniqueFaker("pyint").generate()))
channel_chat_id = factory.LazyAttribute(lambda v: "-" + str(UniqueFaker("pyint").generate()))
channel_name = factory.Faker("word")
discussion_group_chat_id = factory.LazyAttribute(lambda v: str(UniqueFaker("pyint").generate()))
discussion_group_name = factory.Faker("word")

View file

@ -50,16 +50,22 @@ def test_personal_connector_replied_message_not_found(
@pytest.mark.parametrize(
"side_effect,notification_error_code",
"side_effect,notification_error_code,reason",
[
(
error.Unauthorized("Forbidden: bot was blocked by the user"),
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_BOT_IS_DELETED,
"Bot was blocked by the user",
),
(
error.Unauthorized("Invalid token"),
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
"Invalid token",
),
(error.Unauthorized("Invalid token"), UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR),
(
error.Unauthorized("Forbidden: user is deactivated"),
UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED,
"Telegram user was disabled",
),
],
)
@ -67,6 +73,7 @@ def test_personal_connector_replied_message_not_found(
def test_personal_connector_send_link_to_channel_message_handle_exceptions(
side_effect,
notification_error_code,
reason,
make_organization_and_user,
make_telegram_user_connector,
make_user_notification_policy,
@ -96,3 +103,86 @@ def test_personal_connector_send_link_to_channel_message_handle_exceptions(
log_records = user.personal_log_records.filter(alert_group=alert_group)
assert log_records.count() == 1
assert log_records.first().notification_error_code == notification_error_code
assert log_records.first().reason == reason
@patch.object(TelegramClient, "send_message")
@pytest.mark.django_db
def test_personal_connector_send_link_to_channel_message(
mock_send_message,
make_organization_and_user,
make_telegram_user_connector,
make_user_notification_policy,
make_alert_receive_channel,
make_alert_group,
make_alert,
make_telegram_channel,
make_telegram_message,
):
# set up a user with Telegram account connected
organization, user = make_organization_and_user()
make_telegram_user_connector(user)
notification_policy = make_user_notification_policy(
user,
UserNotificationPolicy.Step.NOTIFY,
notify_by=UserNotificationPolicy.NotificationChannel.TELEGRAM,
important=False,
)
# create an alert group with an existing Telegram message in channel
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
make_alert(alert_group=alert_group, raw_request_data=alert_receive_channel.config.example_payload)
telegram_channel = make_telegram_channel(organization, is_default_channel=True)
make_telegram_message(
alert_group=alert_group,
message_type=TelegramMessage.ALERT_GROUP_MESSAGE,
chat_id=str(telegram_channel.channel_chat_id),
)
user.telegram_connection.send_link_to_channel_message(alert_group, notification_policy)
mock_send_message.assert_called_once_with(
chat_id=user.telegram_connection.telegram_chat_id,
message_type=TelegramMessage.LINK_TO_CHANNEL_MESSAGE,
alert_group=alert_group,
)
log_record = notification_policy.personal_log_records.last()
assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS
@patch.object(TelegramClient, "send_message")
@pytest.mark.django_db
def test_personal_connector_send_full_alert_group(
mock_send_message,
make_organization_and_user,
make_telegram_user_connector,
make_user_notification_policy,
make_alert_receive_channel,
make_alert_group,
make_alert,
):
# set up a user with Telegram account connected
organization, user = make_organization_and_user()
make_telegram_user_connector(user)
notification_policy = make_user_notification_policy(
user,
UserNotificationPolicy.Step.NOTIFY,
notify_by=UserNotificationPolicy.NotificationChannel.TELEGRAM,
important=False,
)
# create an alert group with an existing Telegram message in channel
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
make_alert(alert_group=alert_group, raw_request_data=alert_receive_channel.config.example_payload)
user.telegram_connection.send_full_alert_group(alert_group, notification_policy)
mock_send_message.assert_called_once_with(
chat_id=user.telegram_connection.telegram_chat_id,
message_type=TelegramMessage.PERSONAL_MESSAGE,
alert_group=alert_group,
)
log_record = notification_policy.personal_log_records.last()
assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS

View file

@ -147,6 +147,7 @@ IS_RBAC_ENABLED = os.getenv("ONCALL_TESTING_RBAC_ENABLED", "True") == "True"
def mock_slack_api_call(monkeypatch):
def mock_api_call(*args, **kwargs):
return {
"ts": timezone.now().isoformat(),
"status": 200,
"usergroups": [],
"channel": {"id": "TEST_CHANNEL_ID"},