Skip to content

Commit

Permalink
Move slack log report from message thread to button (#4686)
Browse files Browse the repository at this point in the history
# What this PR does

Continuation of [Add slack button to show log report
#4641](#4641)

## Which issue(s) this PR closes

Closes #3849

## 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] 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.

---------

Co-authored-by: Joey Orlando <joseph.t.orlando@gmail.com>
Co-authored-by: Joey Orlando <joey.orlando@grafana.com>
  • Loading branch information
3 people authored Aug 19, 2024
1 parent 344cd0e commit 4872588
Show file tree
Hide file tree
Showing 8 changed files with 8 additions and 207 deletions.
2 changes: 1 addition & 1 deletion engine/apps/alerts/models/alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.
resolved_by_user: typing.Optional["User"]
root_alert_group: typing.Optional["AlertGroup"]
silenced_by_user: typing.Optional["User"]
slack_log_message: typing.Optional["SlackMessage"]
slack_messages: "RelatedManager['SlackMessage']"
users: "RelatedManager['User']"
labels: "RelatedManager['AlertGroupAssociatedLabel']"
Expand Down Expand Up @@ -396,6 +395,7 @@ def status(self) -> int:
related_name="wiped_alert_groups",
)

# TODO: drop this column in future release
slack_log_message = models.OneToOneField(
"slack.SlackMessage",
on_delete=models.SET_NULL,
Expand Down
4 changes: 0 additions & 4 deletions engine/apps/alerts/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@
AlertGroupSlackRepresentative.on_alert_group_action_triggered,
)

alert_group_update_log_report_signal.connect(
AlertGroupSlackRepresentative.on_alert_group_update_log_report,
)

alert_group_update_resolution_note_signal.connect(
AlertGroupSlackRepresentative.on_alert_group_update_resolution_note,
)
Expand Down
40 changes: 1 addition & 39 deletions engine/apps/slack/representatives/alert_group_representative.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,7 @@ def on_alert_group_action_triggered_async(log_record_id):
autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None
)
def on_alert_group_update_log_report_async(alert_group_id):
from apps.alerts.models import AlertGroup

alert_group = AlertGroup.objects.get(pk=alert_group_id)
logger.debug(f"Start on_alert_group_update_log_report for alert_group {alert_group_id}")
organization = alert_group.channel.organization
if alert_group.slack_message and organization.slack_team_identity:
logger.debug(f"Process on_alert_group_update_log_report for alert_group {alert_group_id}")
UpdateLogReportMessageStep = ScenarioStep.get_step("distribute_alerts", "UpdateLogReportMessageStep")
step = UpdateLogReportMessageStep(organization.slack_team_identity, organization)
step.process_signal(alert_group)
else:
logger.debug(f"Drop on_alert_group_update_log_report for alert_group {alert_group_id}")
logger.debug(f"Finish on_alert_group_update_log_report for alert_group {alert_group_id}")
return "Deprecated, will be removed after queue cleanup"


class AlertGroupSlackRepresentative(AlertGroupAbstractRepresentative):
Expand Down Expand Up @@ -173,32 +161,6 @@ def on_alert_group_action_triggered(cls, **kwargs):
logger.debug(f"SLACK on_alert_group_action_triggered: async {log_record_id} {force_sync}")
on_alert_group_action_triggered_async.apply_async((log_record_id,))

@classmethod
def on_alert_group_update_log_report(cls, **kwargs):
from apps.alerts.models import AlertGroup

alert_group = kwargs["alert_group"]

if isinstance(alert_group, AlertGroup):
alert_group_id = alert_group.pk
else:
alert_group_id = alert_group
try:
alert_group = AlertGroup.objects.get(pk=alert_group_id)
except AlertGroup.DoesNotExist as e:
logger.warning(f"SLACK update log report: alert group {alert_group_id} has been deleted")
raise e

logger.debug(
f"Received alert_group_update_log_report signal in SLACK representative for alert_group {alert_group_id}"
)

if alert_group.notify_in_slack_enabled is False:
logger.debug(f"Skipping alert_group {alert_group_id} since notify_in_slack is disabled")
return

on_alert_group_update_log_report_async.apply_async((alert_group_id,))

@classmethod
def on_alert_group_update_resolution_note(cls, **kwargs):
alert_group = kwargs["alert_group"]
Expand Down
87 changes: 1 addition & 86 deletions engine/apps/slack/scenarios/distribute_alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from datetime import datetime

from django.core.cache import cache
from django.utils import timezone

from apps.alerts.constants import ActionSource
from apps.alerts.incident_appearance.renderers.constants import DEFAULT_BACKUP_TITLE
Expand All @@ -14,25 +13,17 @@
from apps.slack.chatops_proxy_routing import make_private_metadata, make_value
from apps.slack.constants import CACHE_UPDATE_INCIDENT_SLACK_MESSAGE_LIFETIME
from apps.slack.errors import (
SlackAPICantUpdateMessageError,
SlackAPIChannelArchivedError,
SlackAPIChannelInactiveError,
SlackAPIChannelNotFoundError,
SlackAPIError,
SlackAPIInvalidAuthError,
SlackAPIMessageNotFoundError,
SlackAPIRatelimitError,
SlackAPIRestrictedActionError,
SlackAPITokenError,
)
from apps.slack.scenarios import scenario_step
from apps.slack.scenarios.slack_renderer import AlertGroupLogSlackRenderer
from apps.slack.slack_formatter import SlackFormatter
from apps.slack.tasks import (
post_or_update_log_report_message_task,
send_message_to_thread_if_bot_not_in_channel,
update_incident_slack_message,
)
from apps.slack.tasks import send_message_to_thread_if_bot_not_in_channel, update_incident_slack_message
from apps.slack.types import (
Block,
BlockActionType,
Expand Down Expand Up @@ -95,7 +86,6 @@ def process_signal(self, alert: Alert) -> None:
else:
# check if alert group was posted to slack before posting message to thread
if not alert.group.skip_escalation_in_slack:
self._send_log_report_message(alert.group, channel_id)
self._send_message_to_thread_if_bot_not_in_channel(alert.group, channel_id)
else:
# check if alert group was posted to slack before updating its message
Expand Down Expand Up @@ -208,11 +198,6 @@ def _send_debug_mode_notice(self, alert_group: AlertGroup, channel_id: str) -> N
blocks=blocks,
)

def _send_log_report_message(self, alert_group: AlertGroup, channel_id: str) -> None:
post_or_update_log_report_message_task.apply_async(
(alert_group.pk, self.slack_team_identity.pk),
)

def _send_message_to_thread_if_bot_not_in_channel(self, alert_group: AlertGroup, channel_id: str) -> None:
send_message_to_thread_if_bot_not_in_channel.apply_async(
(alert_group.pk, self.slack_team_identity.pk, channel_id),
Expand Down Expand Up @@ -895,76 +880,6 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None:
message.delete()


class UpdateLogReportMessageStep(scenario_step.ScenarioStep):
def process_signal(self, alert_group: AlertGroup) -> None:
if alert_group.skip_escalation_in_slack or alert_group.channel.is_rate_limited_in_slack:
return

self.update_log_message(alert_group)

def update_log_message(self, alert_group: AlertGroup) -> None:
slack_message = alert_group.slack_message
if slack_message is None:
logger.info(
f"Cannot update log message for alert_group {alert_group.pk} because SlackMessage doesn't exist"
)
return None

slack_log_message = alert_group.slack_log_message

if slack_log_message is not None:
# prevent too frequent updates
if timezone.now() <= slack_log_message.last_updated + timezone.timedelta(seconds=5):
return

attachments = AlertGroupLogSlackRenderer.render_incident_log_report_for_slack(alert_group)
logger.debug(
f"Update log message for alert_group {alert_group.pk}, slack_log_message {slack_log_message.pk}"
)
try:
self._slack_client.chat_update(
channel=slack_message.channel_id,
text="Alert Group log",
ts=slack_log_message.slack_id,
attachments=attachments,
)
except SlackAPIRatelimitError as e:
if not alert_group.channel.is_rate_limited_in_slack:
alert_group.channel.start_send_rate_limit_message_task(e.retry_after)
except SlackAPIMessageNotFoundError:
alert_group.slack_log_message = None
alert_group.save(update_fields=["slack_log_message"])
except (
SlackAPITokenError,
SlackAPIChannelNotFoundError,
SlackAPIChannelArchivedError,
SlackAPIChannelInactiveError,
SlackAPIInvalidAuthError,
SlackAPICantUpdateMessageError,
):
pass
else:
slack_log_message.last_updated = timezone.now()
slack_log_message.save(update_fields=["last_updated"])
logger.debug(
f"Finished update log message for alert_group {alert_group.pk}, "
f"slack_log_message {slack_log_message.pk}"
)
# check how much time has passed since slack message was created
# to prevent eternal loop of restarting update log message task
elif timezone.now() <= slack_message.created_at + timezone.timedelta(minutes=5):
logger.debug(
f"Update log message failed for alert_group {alert_group.pk}: "
f"log message does not exist yet. Restarting post_or_update_log_report_message_task..."
)
post_or_update_log_report_message_task.apply_async(
(alert_group.pk, self.slack_team_identity.pk, True),
countdown=3,
)
else:
logger.debug(f"Update log message failed for alert_group {alert_group.pk}: " f"log message does not exist.")


STEPS_ROUTING: ScenarioRoute.RoutingSteps = [
{
"payload_type": PayloadType.INTERACTIVE_MESSAGE,
Expand Down
14 changes: 0 additions & 14 deletions engine/apps/slack/scenarios/slack_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,3 @@ def render_alert_group_future_log_report_text(alert_group: "AlertGroup"):
for plan_line in escalation_policies_plan[time]:
result += f"*{humanize.naturaldelta(time)}:* {plan_line}\n"
return result

@staticmethod
def render_incident_log_report_for_slack(alert_group: "AlertGroup"):
attachments = []
past = AlertGroupLogSlackRenderer.render_alert_group_past_log_report_text(alert_group)
future = AlertGroupLogSlackRenderer.render_alert_group_future_log_report_text(alert_group)
text = past + future
if len(text) > 0:
attachments.append(
{
"text": text,
}
)
return attachments
23 changes: 1 addition & 22 deletions engine/apps/slack/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
SlackAPITokenError,
SlackAPIUsergroupNotFoundError,
)
from apps.slack.scenarios.scenario_step import ScenarioStep
from apps.slack.utils import (
get_cache_key_update_incident_slack_message,
get_populate_slack_channel_task_id_key,
Expand Down Expand Up @@ -289,27 +288,7 @@ def populate_slack_user_identities(organization_pk):
autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None
)
def post_or_update_log_report_message_task(alert_group_pk, slack_team_identity_pk, update=False):
logger.debug(f"Start post_or_update_log_report_message_task for alert_group {alert_group_pk}")
from apps.alerts.models import AlertGroup
from apps.slack.models import SlackTeamIdentity

UpdateLogReportMessageStep = ScenarioStep.get_step("distribute_alerts", "UpdateLogReportMessageStep")

slack_team_identity = SlackTeamIdentity.objects.get(pk=slack_team_identity_pk)
alert_group = AlertGroup.objects.get(pk=alert_group_pk)
step = UpdateLogReportMessageStep(slack_team_identity, alert_group.channel.organization)

if alert_group.skip_escalation_in_slack or alert_group.channel.is_rate_limited_in_slack:
return

if update: # flag to prevent multiple posting log message to slack
step.update_log_message(alert_group)
else:
# don't post a new message, as it is available from the button
# this is an intermediate step, so we will only update posted messages but not post new ones
# once majority of messages are updated, we can remove this step (https://github.com/grafana/oncall/pull/4686)
pass
logger.debug(f"Finish post_or_update_log_report_message_task for alert_group {alert_group_pk}")
return "Deprecated, will be removed after queue cleanup"


@shared_dedicated_queue_retry_task(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
from unittest.mock import patch

import pytest
from django.utils import timezone

from apps.alerts.models import AlertGroup
from apps.slack.errors import SlackAPICantUpdateMessageError, SlackAPIRestrictedActionError
from apps.slack.errors import SlackAPIRestrictedActionError
from apps.slack.models import SlackMessage
from apps.slack.scenarios.distribute_alerts import AlertShootingStep, UpdateLogReportMessageStep
from apps.slack.scenarios.distribute_alerts import AlertShootingStep
from apps.slack.scenarios.scenario_step import ScenarioStep
from apps.slack.tests.conftest import build_slack_response

Expand Down Expand Up @@ -65,37 +64,3 @@ def test_alert_shooting_no_channel_filter(

mock_post_alert_group_to_slack.assert_called_once()
assert mock_post_alert_group_to_slack.call_args[1]["channel_id"] == "DEFAULT_CHANNEL_ID"


@pytest.mark.django_db
def test_update_log_report_cant_update(
make_slack_team_identity,
make_organization,
make_alert_receive_channel,
make_alert_group,
make_alert,
make_slack_message,
):
slack_team_identity = make_slack_team_identity()
organization = make_organization(
slack_team_identity=slack_team_identity, general_log_channel_id="DEFAULT_CHANNEL_ID"
)
alert_receive_channel = make_alert_receive_channel(organization)

alert_group = make_alert_group(alert_receive_channel, channel_filter=None)
# alert = make_alert(alert_group, raw_request_data={})
log_message = make_slack_message(
alert_group=alert_group,
channel_id="RANDOM_CHANNEL_ID",
slack_id="RANDOM_MESSAGE_ID",
last_updated=timezone.now() - timezone.timedelta(minutes=5),
)
alert_group.slack_log_message = log_message

step = UpdateLogReportMessageStep(slack_team_identity, organization)
with patch.object(step._slack_client, "api_call") as mock_slack_api_call:
mock_slack_api_call.side_effect = SlackAPICantUpdateMessageError(
response=build_slack_response({"error": "cant_update_message"})
)
# not raising error, will not retry
step.update_log_message(alert_group)
6 changes: 2 additions & 4 deletions engine/settings/celery_task_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@
"apps.grafana_plugin.tasks.sync_v2.sync_organizations_v2": {"queue": "long"},
# SLACK
"apps.integrations.tasks.notify_about_integration_ratelimit_in_slack": {"queue": "slack"},
"apps.slack.helpers.alert_group_representative.on_alert_group_action_triggered_async": {"queue": "slack"},
"apps.slack.helpers.alert_group_representative.on_alert_group_update_log_report_async": {"queue": "slack"},
"apps.slack.helpers.alert_group_representative.on_create_alert_slack_representative_async": {"queue": "slack"},
"apps.slack.tasks.clean_slack_channel_leftovers": {"queue": "slack"},
"apps.slack.tasks.check_slack_message_exists_before_post_message_to_thread": {"queue": "slack"},
"apps.slack.tasks.clean_slack_integration_leftovers": {"queue": "slack"},
Expand All @@ -167,7 +164,6 @@
"apps.slack.tasks.populate_slack_user_identities": {"queue": "slack"},
"apps.slack.tasks.populate_slack_usergroups": {"queue": "slack"},
"apps.slack.tasks.populate_slack_usergroups_for_team": {"queue": "slack"},
"apps.slack.tasks.post_or_update_log_report_message_task": {"queue": "slack"},
"apps.slack.tasks.post_slack_rate_limit_message": {"queue": "slack"},
"apps.slack.tasks.send_message_to_thread_if_bot_not_in_channel": {"queue": "slack"},
"apps.slack.tasks.start_update_slack_user_group_for_schedules": {"queue": "slack"},
Expand All @@ -178,6 +174,8 @@
"queue": "slack"
},
"apps.slack.representatives.alert_group_representative.on_alert_group_action_triggered_async": {"queue": "slack"},
# TODO: remove post_or_update_log_report_message_task and on_alert_group_update_log_report_async in subsequent PR
"apps.slack.tasks.post_or_update_log_report_message_task": {"queue": "slack"},
"apps.slack.representatives.alert_group_representative.on_alert_group_update_log_report_async": {"queue": "slack"},
# TELEGRAM
"apps.telegram.tasks.edit_message": {"queue": "telegram"},
Expand Down

0 comments on commit 4872588

Please sign in to comment.