diff --git a/src/sentry/incidents/typings/metric_detector.py b/src/sentry/incidents/typings/metric_detector.py index f04187c379cdaf..f556ffb073195d 100644 --- a/src/sentry/incidents/typings/metric_detector.py +++ b/src/sentry/incidents/typings/metric_detector.py @@ -14,7 +14,7 @@ from sentry.incidents.models.incident import Incident, IncidentStatus from sentry.issues.issue_occurrence import IssueOccurrence from sentry.models.group import Group, GroupStatus -from sentry.snuba.models import SnubaQuery +from sentry.snuba.models import QuerySubscription, SnubaQuery from sentry.types.group import PriorityLevel from sentry.workflow_engine.models import Action, Detector @@ -60,6 +60,7 @@ class NotificationContext: NotificationContext is a dataclass that represents the context required send a notification. """ + id: int integration_id: int | None = None target_identifier: str | None = None target_display: str | None = None @@ -69,6 +70,7 @@ class NotificationContext: @classmethod def from_alert_rule_trigger_action(cls, action: AlertRuleTriggerAction) -> NotificationContext: return cls( + id=action.id, integration_id=action.integration_id, target_identifier=action.target_identifier, target_display=action.target_display, @@ -79,6 +81,7 @@ def from_alert_rule_trigger_action(cls, action: AlertRuleTriggerAction) -> Notif def from_action_model(cls, action: Action) -> NotificationContext: if action.type == Action.Type.SENTRY_APP: return cls( + id=action.id, integration_id=action.integration_id, target_display=action.config.get("target_display"), sentry_app_config=action.data.get("settings"), @@ -87,6 +90,7 @@ def from_action_model(cls, action: Action) -> NotificationContext: ) elif action.type == Action.Type.OPSGENIE or action.type == Action.Type.PAGERDUTY: return cls( + id=action.id, integration_id=action.integration_id, target_identifier=action.config.get("target_identifier"), target_display=action.config.get("target_display"), @@ -95,6 +99,7 @@ def from_action_model(cls, action: Action) -> NotificationContext: # TODO(iamrajjoshi): Add support for email here return cls( + id=action.id, integration_id=action.integration_id, target_identifier=action.config.get("target_identifier"), target_display=action.config.get("target_display"), @@ -103,9 +108,11 @@ def from_action_model(cls, action: Action) -> NotificationContext: @dataclass class MetricIssueContext: - open_period_identifier: int + id: int + open_period_identifier: int # Used for link building snuba_query: SnubaQuery new_status: IncidentStatus + subscription: QuerySubscription | None metric_value: float | None @classmethod @@ -128,6 +135,10 @@ def _get_snuba_query(cls, occurrence: IssueOccurrence) -> SnubaQuery: raise ValueError("Snuba query does not exist") from e return query + @classmethod + def _get_subscription(cls, occurrence: IssueOccurrence) -> QuerySubscription | None: + return occurrence.evidence_data.get("subscription_id") + @classmethod def _get_metric_value(cls, occurrence: IssueOccurrence) -> float: if (metric_value := occurrence.evidence_data.get("metric_value")) is None: @@ -144,8 +155,11 @@ def from_group_event(cls, group_event: GroupEvent) -> MetricIssueContext: # TODO(iamrajjoshi): Replace with something once we know how we want to build the link # If we store open periods in the database, we can use the id from that # Otherwise, we can use the issue id + id=group.id, + # TODO(iamrajjoshi): This should probably be the id of the latest open period open_period_identifier=group.id, snuba_query=cls._get_snuba_query(occurrence), + subscription=cls._get_subscription(occurrence), new_status=cls._get_new_status(group, occurrence), metric_value=cls._get_metric_value(occurrence), ) @@ -158,8 +172,10 @@ def from_legacy_models( metric_value: float | None = None, ) -> MetricIssueContext: return cls( + id=incident.id, open_period_identifier=incident.identifier, snuba_query=incident.alert_rule.snuba_query, + subscription=incident.subscription, new_status=new_status, metric_value=metric_value, ) diff --git a/src/sentry/integrations/slack/message_builder/incidents.py b/src/sentry/integrations/slack/message_builder/incidents.py index 44a1b405ef86e9..4f2f0a7f4db3ba 100644 --- a/src/sentry/integrations/slack/message_builder/incidents.py +++ b/src/sentry/integrations/slack/message_builder/incidents.py @@ -1,7 +1,6 @@ from datetime import datetime -from sentry.incidents.models.incident import IncidentStatus -from sentry.incidents.typings.metric_detector import AlertContext +from sentry.incidents.typings.metric_detector import AlertContext, MetricIssueContext from sentry.integrations.metric_alerts import incident_attachment_info from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder from sentry.integrations.slack.message_builder.types import ( @@ -11,7 +10,6 @@ ) from sentry.integrations.slack.utils.escape import escape_slack_text from sentry.models.organization import Organization -from sentry.snuba.models import SnubaQuery def get_started_at(timestamp: datetime | None) -> str: @@ -26,12 +24,9 @@ class SlackIncidentsMessageBuilder(BlockSlackMessageBuilder): def __init__( self, alert_context: AlertContext, - open_period_identifier: int, + metric_issue_context: MetricIssueContext, organization: Organization, - snuba_query: SnubaQuery, - new_status: IncidentStatus, date_started: datetime, - metric_value: float | None = None, chart_url: str | None = None, notification_uuid: str | None = None, ) -> None: @@ -45,11 +40,11 @@ def __init__( """ super().__init__() self.alert_context = alert_context - self.open_period_identifier = open_period_identifier + self.open_period_identifier = metric_issue_context.open_period_identifier self.organization = organization - self.snuba_query = snuba_query - self.metric_value = metric_value - self.new_status = new_status + self.snuba_query = metric_issue_context.snuba_query + self.metric_value = metric_issue_context.metric_value + self.new_status = metric_issue_context.new_status self.date_started = date_started self.chart_url = chart_url self.notification_uuid = notification_uuid diff --git a/src/sentry/integrations/slack/utils/notifications.py b/src/sentry/integrations/slack/utils/notifications.py index 0e80aa6692fa03..e50b32017e247c 100644 --- a/src/sentry/integrations/slack/utils/notifications.py +++ b/src/sentry/integrations/slack/utils/notifications.py @@ -22,7 +22,12 @@ ) from sentry.incidents.models.alert_rule import AlertRuleTriggerAction from sentry.incidents.models.incident import Incident, IncidentStatus -from sentry.incidents.typings.metric_detector import AlertContext, OpenPeriodContext +from sentry.incidents.typings.metric_detector import ( + AlertContext, + MetricIssueContext, + NotificationContext, + OpenPeriodContext, +) from sentry.integrations.messaging.metrics import ( MessagingInteractionEvent, MessagingInteractionType, @@ -31,10 +36,11 @@ from sentry.integrations.models.integration import Integration from sentry.integrations.repository import get_default_metric_alert_repository from sentry.integrations.repository.metric_alert import ( + MetricAlertNotificationMessage, MetricAlertNotificationMessageRepository, NewMetricAlertNotificationMessage, ) -from sentry.integrations.services.integration import integration_service +from sentry.integrations.services.integration import RpcIntegration, integration_service from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder from sentry.integrations.slack.message_builder.types import SlackBlock from sentry.integrations.slack.metrics import ( @@ -45,75 +51,25 @@ from sentry.integrations.slack.sdk_client import SlackSdkClient from sentry.integrations.slack.spec import SlackMessagingSpec from sentry.models.options.organization_option import OrganizationOption +from sentry.models.organization import Organization from sentry.utils import metrics _logger = logging.getLogger(__name__) -def send_incident_alert_notification( - action: AlertRuleTriggerAction, - incident: Incident, - metric_value: float | int | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, -) -> bool: - # Make sure organization integration is still active: - result = integration_service.organization_context( - organization_id=incident.organization_id, integration_id=action.integration_id - ) - integration = result.integration - org_integration = result.organization_integration - if org_integration is None or integration is None or integration.status != ObjectStatus.ACTIVE: - # Integration removed, but rule is still active. - return False - - organization = incident.organization - chart_url = None - if features.has("organizations:metric-alert-chartcuterie", incident.organization): - try: - alert_rule_serialized_response: AlertRuleSerializerResponse = serialize( - incident.alert_rule, None, AlertRuleSerializer() - ) - incident_serialized_response: DetailedIncidentSerializerResponse = serialize( - incident, None, DetailedIncidentSerializer() - ) - chart_url = build_metric_alert_chart( - organization=organization, - alert_rule_serialized_response=alert_rule_serialized_response, - snuba_query=incident.alert_rule.snuba_query, - alert_context=AlertContext.from_alert_rule_incident(incident.alert_rule), - open_period_context=OpenPeriodContext.from_incident(incident), - selected_incident_serialized=incident_serialized_response, - subscription=incident.subscription, - ) - except Exception as e: - sentry_sdk.capture_exception(e) - - channel = action.target_identifier - if metric_value is None: - metric_value = get_metric_count_from_incident(incident) - - attachment: SlackBlock = SlackIncidentsMessageBuilder( - alert_context=AlertContext.from_alert_rule_incident(incident.alert_rule), - open_period_identifier=incident.identifier, - organization=organization, - snuba_query=incident.alert_rule.snuba_query, - new_status=new_status, - date_started=incident.date_started, - metric_value=metric_value, - chart_url=chart_url, - notification_uuid=notification_uuid, - ).build() - text = str(attachment["text"]) - blocks = {"blocks": attachment["blocks"], "color": attachment["color"]} - attachments = orjson.dumps([blocks]).decode() +def _fetch_parent_notification_message_for_incident( + organization: Organization, + alert_context: AlertContext, + notification_context: NotificationContext, + metric_issue_context: MetricIssueContext, + repository: MetricAlertNotificationMessageRepository, +) -> MetricAlertNotificationMessage | None: + parent_notification_message = None with MessagingInteractionEvent( interaction_type=MessagingInteractionType.GET_PARENT_NOTIFICATION, spec=SlackMessagingSpec(), ).capture() as lifecycle: - repository: MetricAlertNotificationMessageRepository = get_default_metric_alert_repository() - parent_notification_message = None # Only grab the parent notification message for thread use if the feature is on # Otherwise, leave it empty, and it will not create a thread if OrganizationOption.objects.get_value( @@ -123,37 +79,68 @@ def send_incident_alert_notification( ): try: parent_notification_message = repository.get_parent_notification_message( - alert_rule_id=incident.alert_rule_id, - incident_id=incident.id, - trigger_action_id=action.id, + alert_rule_id=alert_context.action_identifier_id, + incident_id=metric_issue_context.id, + trigger_action_id=notification_context.id, ) except Exception as e: lifecycle.record_halt(e) # if there's an error trying to grab a parent notification, don't let that error block this flow pass - new_notification_message_object = NewMetricAlertNotificationMessage( - incident_id=incident.id, - trigger_action_id=action.id, - ) + return parent_notification_message - reply_broadcast = False - thread_ts = None - # If a parent notification exists for this rule and action, then we can reply in a thread - if parent_notification_message is not None: - # Make sure we track that this reply will be in relation to the parent row - new_notification_message_object.parent_notification_message_id = ( - parent_notification_message.id - ) - # To reply to a thread, use the specific key in the payload as referenced by the docs - # https://api.slack.com/methods/chat.postMessage#arg_thread_ts - thread_ts = parent_notification_message.message_identifier - # If the incident is critical status, even if it's in a thread, send to main channel - if incident.status == IncidentStatus.CRITICAL.value: - reply_broadcast = True +def _build_notification_payload( + organization: Organization, + alert_context: AlertContext, + metric_issue_context: MetricIssueContext, + open_period_context: OpenPeriodContext, + alert_rule_serialized_response: AlertRuleSerializerResponse, + incident_serialized_response: DetailedIncidentSerializerResponse, + notification_uuid: str | None, +) -> tuple[str, str]: + chart_url = None + if features.has("organizations:metric-alert-chartcuterie", organization): + try: + chart_url = build_metric_alert_chart( + organization=organization, + alert_rule_serialized_response=alert_rule_serialized_response, + snuba_query=metric_issue_context.snuba_query, + alert_context=alert_context, + open_period_context=open_period_context, + selected_incident_serialized=incident_serialized_response, + subscription=metric_issue_context.subscription, + ) + except Exception as e: + sentry_sdk.capture_exception(e) + + attachment: SlackBlock = SlackIncidentsMessageBuilder( + alert_context=alert_context, + metric_issue_context=metric_issue_context, + organization=organization, + date_started=open_period_context.date_started, + chart_url=chart_url, + notification_uuid=notification_uuid, + ).build() + text = str(attachment["text"]) + blocks = {"blocks": attachment["blocks"], "color": attachment["color"]} + attachments = orjson.dumps([blocks]).decode() + + return attachments, text - success = False + +def _send_notification( + integration: RpcIntegration, + metric_issue_context: MetricIssueContext, + attachments: str, + text: str, + channel: str, + thread_ts: str | None, + reply_broadcast: bool, + notification_message_object: NewMetricAlertNotificationMessage, + repository: MetricAlertNotificationMessageRepository, +) -> bool: with MessagingInteractionEvent( interaction_type=MessagingInteractionType.SEND_INCIDENT_ALERT_NOTIFICATION, spec=SlackMessagingSpec(), @@ -171,8 +158,8 @@ def send_incident_alert_notification( ) except SlackApiError as e: # Record the error code and details from the exception - new_notification_message_object.error_code = e.response.status_code - new_notification_message_object.error_details = { + notification_message_object.error_code = e.response.status_code + notification_message_object.error_details = { "msg": str(e), "data": e.response.data, "url": e.response.api_url, @@ -180,14 +167,11 @@ def send_incident_alert_notification( log_params: dict[str, str | int] = { "error": str(e), - "incident_id": incident.id, - "incident_status": str(new_status), - "attachments": attachments, + "incident_id": metric_issue_context.id, + "incident_status": str(metric_issue_context.new_status), } if channel: log_params["channel_id"] = channel - if action.target_display: - log_params["channel_name"] = action.target_display lifecycle.add_extras(log_params) # If the error is a channel not found or archived, we can halt the flow @@ -195,19 +179,120 @@ def send_incident_alert_notification( record_lifecycle_termination_level(lifecycle, e) else: - success = True ts = response.get("ts") - new_notification_message_object.message_identifier = str(ts) if ts is not None else None + notification_message_object.message_identifier = str(ts) if ts is not None else None - # Save the notification message we just sent with the response id or error we received + _save_notification_message(notification_message_object, repository) + return True + + +def _save_notification_message( + notification_message_object: NewMetricAlertNotificationMessage, + repository: MetricAlertNotificationMessageRepository, +) -> None: try: - repository.create_notification_message(data=new_notification_message_object) + repository = get_default_metric_alert_repository() + repository.create_notification_message(data=notification_message_object) except Exception: - # If we had an unexpected error with saving a record to our datastore, - # do not let the error bubble up, nor block this flow from finishing pass + +def send_incident_alert_notification( + action: AlertRuleTriggerAction, + incident: Incident, + metric_value: float | int | None, + new_status: IncidentStatus, + notification_uuid: str | None = None, +) -> bool: + # Make sure organization integration is still active: + result = integration_service.organization_context( + organization_id=incident.organization_id, integration_id=action.integration_id + ) + integration = result.integration + org_integration = result.organization_integration + if org_integration is None or integration is None or integration.status != ObjectStatus.ACTIVE: + # Integration removed, but rule is still active. + return False + + if metric_value is None: + metric_value = get_metric_count_from_incident(incident) + + alert_context = AlertContext.from_alert_rule_incident(incident.alert_rule) + notification_context = NotificationContext.from_alert_rule_trigger_action(action) + incident_context = MetricIssueContext.from_legacy_models( + incident=incident, + new_status=new_status, + metric_value=metric_value, + ) + open_period_context = OpenPeriodContext.from_incident(incident) + + organization = incident.organization + + channel = notification_context.target_identifier + if channel is None: + sentry_sdk.capture_message("Channel is None", level="error") + return False + + alert_rule_serialized_response: AlertRuleSerializerResponse = serialize( + incident.alert_rule, None, AlertRuleSerializer() + ) + incident_serialized_response: DetailedIncidentSerializerResponse = serialize( + incident, None, DetailedIncidentSerializer() + ) + attachments, text = _build_notification_payload( + organization=organization, + alert_context=alert_context, + metric_issue_context=incident_context, + open_period_context=open_period_context, + alert_rule_serialized_response=alert_rule_serialized_response, + incident_serialized_response=incident_serialized_response, + notification_uuid=notification_uuid, + ) + + repository = get_default_metric_alert_repository() + + parent_notification_message = _fetch_parent_notification_message_for_incident( + organization=organization, + alert_context=alert_context, + notification_context=notification_context, + metric_issue_context=incident_context, + repository=repository, + ) + + new_notification_message_object = NewMetricAlertNotificationMessage( + incident_id=incident_context.id, + trigger_action_id=notification_context.id, + ) + + reply_broadcast = False + thread_ts = None + # If a parent notification exists for this rule and action, then we can reply in a thread + if parent_notification_message is not None: + # Make sure we track that this reply will be in relation to the parent row + new_notification_message_object.parent_notification_message_id = ( + parent_notification_message.id + ) + # To reply to a thread, use the specific key in the payload as referenced by the docs + # https://api.slack.com/methods/chat.postMessage#arg_thread_ts + thread_ts = parent_notification_message.message_identifier + + # If the incident is critical status, even if it's in a thread, send to main channel + if incident_context.new_status.value == IncidentStatus.CRITICAL.value: + reply_broadcast = True + + success = _send_notification( + integration=integration, + metric_issue_context=incident_context, + attachments=attachments, + text=text, + channel=channel, + thread_ts=thread_ts, + reply_broadcast=reply_broadcast, + notification_message_object=new_notification_message_object, + repository=repository, + ) + return success diff --git a/tests/sentry/incidents/action_handlers/test_slack.py b/tests/sentry/incidents/action_handlers/test_slack.py index aeaf5acbabd3ae..5156d54326ac34 100644 --- a/tests/sentry/incidents/action_handlers/test_slack.py +++ b/tests/sentry/incidents/action_handlers/test_slack.py @@ -10,7 +10,7 @@ from sentry.incidents.logic import update_incident_status from sentry.incidents.models.alert_rule import AlertRuleTriggerAction from sentry.incidents.models.incident import IncidentStatus, IncidentStatusMethod -from sentry.incidents.typings.metric_detector import AlertContext +from sentry.incidents.typings.metric_detector import AlertContext, MetricIssueContext from sentry.integrations.messaging.spec import MessagingActionHandler from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder from sentry.integrations.slack.spec import SlackMessagingSpec @@ -98,12 +98,13 @@ def run_test(self, incident, method, **kwargs): def _assert_blocks(self, mock_post, incident, metric_value, chart_url): slack_body = SlackIncidentsMessageBuilder( alert_context=AlertContext.from_alert_rule_incident(incident.alert_rule), - open_period_identifier=incident.identifier, + metric_issue_context=MetricIssueContext.from_legacy_models( + incident, + IncidentStatus(incident.status), + metric_value, + ), organization=incident.organization, - snuba_query=incident.alert_rule.snuba_query, - new_status=IncidentStatus(incident.status), date_started=incident.date_started, - metric_value=metric_value, chart_url=chart_url, ).build() assert isinstance(slack_body, dict) diff --git a/tests/sentry/integrations/slack/test_message_builder.py b/tests/sentry/integrations/slack/test_message_builder.py index 158ecea9bb19bd..c35d4fafc5655c 100644 --- a/tests/sentry/integrations/slack/test_message_builder.py +++ b/tests/sentry/integrations/slack/test_message_builder.py @@ -17,7 +17,7 @@ AlertRuleSensitivity, ) from sentry.incidents.models.incident import IncidentStatus -from sentry.incidents.typings.metric_detector import AlertContext +from sentry.incidents.typings.metric_detector import AlertContext, MetricIssueContext from sentry.integrations.messaging.message_builder import ( build_attachment_text, build_attachment_title, @@ -807,11 +807,10 @@ def test_no_description_in_notification(self): ) assert SlackIncidentsMessageBuilder( alert_context=AlertContext.from_alert_rule_incident(alert_rule), - open_period_identifier=incident.identifier, + metric_issue_context=MetricIssueContext.from_legacy_models( + incident, IncidentStatus.CRITICAL, 0 + ), organization=self.organization, - snuba_query=alert_rule.snuba_query, - new_status=IncidentStatus.CRITICAL, - metric_value=0, date_started=incident.date_started, ).build() == { "blocks": [ @@ -879,12 +878,11 @@ def test_simple(self): ) assert SlackIncidentsMessageBuilder( alert_context=AlertContext.from_alert_rule_incident(alert_rule), - open_period_identifier=incident.identifier, + metric_issue_context=MetricIssueContext.from_legacy_models( + incident, IncidentStatus.CLOSED, 0 + ), organization=self.organization, - snuba_query=alert_rule.snuba_query, date_started=incident.date_started, - new_status=IncidentStatus.CLOSED, - metric_value=0, ).build() == { "blocks": [ { @@ -924,11 +922,10 @@ def test_metric_value(self): # This should fail because it pulls status from `action` instead of `incident` assert SlackIncidentsMessageBuilder( alert_context=AlertContext.from_alert_rule_incident(alert_rule), - open_period_identifier=incident.identifier, + metric_issue_context=MetricIssueContext.from_legacy_models( + incident, IncidentStatus.CRITICAL, metric_value + ), organization=self.organization, - snuba_query=alert_rule.snuba_query, - new_status=IncidentStatus.CRITICAL, - metric_value=metric_value, date_started=incident.date_started, ).build() == { "blocks": [ @@ -965,11 +962,10 @@ def test_chart(self): ) assert SlackIncidentsMessageBuilder( alert_context=AlertContext.from_alert_rule_incident(alert_rule), - open_period_identifier=incident.identifier, + metric_issue_context=MetricIssueContext.from_legacy_models( + incident, IncidentStatus.CLOSED, 0 + ), organization=self.organization, - snuba_query=alert_rule.snuba_query, - new_status=IncidentStatus.CLOSED, - metric_value=0, date_started=incident.date_started, chart_url="chart-url", ).build() == { @@ -1021,11 +1017,10 @@ def test_metric_alert_with_anomaly_detection(self, mock_seer_request): ) assert SlackIncidentsMessageBuilder( alert_context=AlertContext.from_alert_rule_incident(alert_rule), - open_period_identifier=incident.identifier, + metric_issue_context=MetricIssueContext.from_legacy_models( + incident, IncidentStatus.CRITICAL, 0 + ), organization=self.organization, - snuba_query=alert_rule.snuba_query, - new_status=IncidentStatus.CRITICAL, - metric_value=0, date_started=incident.date_started, ).build() == { "blocks": [