Merge pull request #1382 from elementary-data/ele-1987-alert-title-an…

…d-color

Alerts: Replace icon with color and make header informative

elementary/clients/slack/slack_message_builder.py

@@ -1,11 +1,17 @@
-from typing import List, Optional, Union
+from enum import Enum
+from typing import List, Union
 
 from slack_sdk.models.blocks import SectionBlock
 
 from elementary.clients.slack.schema import SlackBlocksType, SlackMessageSchema
 from elementary.utils.json_utils import unpack_and_flatten_str_to_list
 
 
+class MessageColor(Enum):
+    RED = "#ff0000"
+    YELLOW = "#ffcc00"
+
+
 class SlackMessageBuilder:
     _LONGEST_MARKDOWN_SUFFIX_LEN = 3
     _CONTINUATION_SYMBOL = "..."
@@ -17,6 +23,14 @@ class SlackMessageBuilder:
     def __init__(self) -> None:
         self.slack_message = self._initial_slack_message()
 
+    @property
+    def blocks(self) -> list:
+        return self.slack_message.get("blocks", [])
+
+    @property
+    def attachments(self) -> list:
+        return self.slack_message.get("attachments", [])
+
     @classmethod
     def _initial_slack_message(cls) -> dict:
         return {"blocks": [], "attachments": [{"blocks": []}]}
@@ -159,15 +173,6 @@ def create_compacted_sections_blocks(section_msgs: list) -> List[dict]:
         attachments.append(attachment)
         return attachments
 
-    @staticmethod
-    def get_slack_status_icon(status: Optional[str]) -> str:
-        icon = ":small_red_triangle:"
-        if status == "warn":
-            icon = ":warning:"
-        elif status == "error":
-            icon = ":x:"
-        return icon
-
     def get_slack_message(self, *args, **kwargs) -> SlackMessageSchema:
         return SlackMessageSchema(**self.slack_message)
 
@@ -181,3 +186,9 @@ def prettify_and_dedup_list(str_list: Union[List[str], str]) -> str:
         if isinstance(str_list, str):
             str_list = unpack_and_flatten_str_to_list(str_list)
         return ", ".join(sorted(set(str_list)))
+
+    def add_message_color(self, color: MessageColor):
+        for block in self.blocks:
+            block["color"] = color.value
+        for attachment in self.attachments:
+            attachment["color"] = color.value

elementary/monitor/alerts/alert.py

@@ -3,6 +3,9 @@
 
 from dateutil import tz
 
+from elementary.monitor.data_monitoring.alerts.integrations.utils.report_link import (
+    ReportLinkData,
+)
 from elementary.utils.log import get_logger
 from elementary.utils.time import DATETIME_FORMAT
 
@@ -70,3 +73,10 @@ def data(self) -> Dict:
     @property
     def concise_name(self):
         return "Alert"
+
+    @property
+    def summary(self) -> str:
+        raise NotImplementedError
+
+    def get_report_link(self) -> Optional[ReportLinkData]:
+        raise NotImplementedError

elementary/monitor/alerts/model_alert.py

@@ -95,7 +95,7 @@ def concise_name(self) -> str:
 
     @property
     def summary(self):
-        return f"dbt failed to build {self.materialization} {self.alias}"
+        return f'dbt failed to build {self.materialization} "{self.alias}"'
 
     def get_report_link(self) -> Optional[ReportLinkData]:
         return get_model_runs_link(self.report_url, self.model_unique_id)

elementary/monitor/alerts/source_freshness_alert.py

@@ -153,8 +153,8 @@ def error_message(self) -> str:
     @property
     def summary(self) -> str:
         if self.original_status == "runtime error":
-            return f"Failed to calculate the source freshness of `{self.source_name}`"
-        return f"Freshness exceeded the acceptable times on source `{self.source_name}`"
+            return f'Failed to calculate the source freshness of "{self.source_name}"'
+        return f'Freshness exceeded the acceptable times on source "{self.source_name}"'
 
     def get_report_link(self) -> Optional[ReportLinkData]:
         return get_test_runs_link(self.report_url, self.source_freshness_execution_id)

elementary/monitor/alerts/test_alert.py

@@ -157,9 +157,14 @@ def concise_name(self) -> str:
 
     @property
     def summary(self) -> str:
+        if self.test_type == "schema_change":
+            return (
+                f"{self.test_sub_type_display_name} on "
+                f"{self.table_full_name + '.' + self.column_name if self.column_name else self.table_full_name}"
+            )
         return (
-            f"*{self.concise_name}* test failed on "
-            f"`{self.table_full_name + '.' + self.column_name if self.column_name else self.table_full_name}`"
+            f'"{self.concise_name}" test failed on '
+            f"{self.table_full_name + '.' + self.column_name if self.column_name else self.table_full_name}"
         )
 
     def get_report_link(self) -> Optional[ReportLinkData]:

elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py

@@ -7,6 +7,7 @@
 
 from elementary.clients.slack.client import SlackClient, SlackWebClient
 from elementary.clients.slack.schema import SlackMessageSchema
+from elementary.clients.slack.slack_message_builder import MessageColor
 from elementary.config.config import Config
 from elementary.monitor.alerts.group_of_alerts import GroupedByTableAlerts
 from elementary.monitor.alerts.model_alert import ModelAlertModel
@@ -51,6 +52,12 @@
     TEST_RESULTS_SAMPLE_FIELD,
 ]
 
+STATUS_DISPLAYS: Dict[str, Dict] = {
+    "fail": {"color": MessageColor.RED, "display_name": "Failure"},
+    "warn": {"color": MessageColor.YELLOW, "display_name": "Warning"},
+    "error": {"color": MessageColor.RED, "display_name": "Error"},
+}
+
 
 class SlackIntegration(BaseIntegration):
     def __init__(
@@ -96,9 +103,12 @@ def _get_alert_template(
     def _get_dbt_test_template(
         self, alert: TestAlertModel, *args, **kwargs
     ) -> SlackMessageSchema:
-        icon = self.message_builder.get_slack_status_icon(alert.status)
-
-        title = [self.message_builder.create_header_block(f"{icon} dbt test alert")]
+        self.message_builder.add_message_color(self._get_color(alert.status))
+        title = [
+            self.message_builder.create_header_block(
+                f"{self._get_display_name(alert.status)}: {alert.summary}"
+            )
+        ]
         if alert.suppression_interval:
             title.extend(
                 [
@@ -250,19 +260,18 @@ def _get_dbt_test_template(
     def _get_elementary_test_template(
         self, alert: TestAlertModel, *args, **kwargs
     ) -> SlackMessageSchema:
-        icon = self.message_builder.get_slack_status_icon(alert.status)
-
-        anomalous_value = None
-        if alert.test_type == "schema_change":
-            alert_title = "Schema change detected"
-        elif alert.test_type == "anomaly_detection":
-            alert_title = "Data anomaly detected"
-            anomalous_value = alert.other or None
-        else:
-            raise ValueError("Invalid test type.", alert.test_type)
+        self.message_builder.add_message_color(self._get_color(alert.status))
+
+        anomalous_value = (
+            alert.other if alert.test_type == "anomaly_detection" else None
+        )
 
         title = [
-            self.message_builder.create_header_block(f"{icon} {alert_title}"),
+            self.message_builder.create_header_block(
+                f"{alert.summary}"
+                if alert.test_type == "schema_change"
+                else f"{self._get_display_name(alert.status)}: {alert.summary}"
+            ),
         ]
         if alert.suppression_interval:
             title.extend(
@@ -402,9 +411,13 @@ def _get_model_template(
         tags = self.message_builder.prettify_and_dedup_list(alert.tags)
         owners = self.message_builder.prettify_and_dedup_list(alert.owners)
         subscribers = self.message_builder.prettify_and_dedup_list(alert.subscribers)
-        icon = self.message_builder.get_slack_status_icon(alert.status)
+        self.message_builder.add_message_color(self._get_color(alert.status))
 
-        title = [self.message_builder.create_header_block(f"{icon} dbt model alert")]
+        title = [
+            self.message_builder.create_header_block(
+                f"{self._get_display_name(alert.status)}: {alert.summary}"
+            )
+        ]
         if alert.suppression_interval:
             title.extend(
                 [
@@ -498,9 +511,13 @@ def _get_snapshot_template(
         tags = self.message_builder.prettify_and_dedup_list(alert.tags)
         owners = self.message_builder.prettify_and_dedup_list(alert.owners)
         subscribers = self.message_builder.prettify_and_dedup_list(alert.subscribers)
-        icon = self.message_builder.get_slack_status_icon(alert.status)
+        self.message_builder.add_message_color(self._get_color(alert.status))
 
-        title = [self.message_builder.create_header_block(f"{icon} dbt snapshot alert")]
+        title = [
+            self.message_builder.create_header_block(
+                f"{self._get_display_name(alert.status)}: {alert.summary}"
+            )
+        ]
         if alert.suppression_interval:
             title.extend(
                 [
@@ -580,13 +597,13 @@ def _get_source_freshness_template(
         subscribers = self.message_builder.prettify_and_dedup_list(
             alert.subscribers or []
         )
-        icon = self.message_builder.get_slack_status_icon(alert.status)
-
+        self.message_builder.add_message_color(self._get_color(alert.status))
         title = [
             self.message_builder.create_header_block(
-                f"{icon} dbt source freshness alert"
+                f"{self._get_display_name(alert.status)}: {alert.summary}"
             )
         ]
+
         if alert.suppression_interval:
             title.extend(
                 [
@@ -717,15 +734,14 @@ def _get_group_by_table_template(
         self, alert: GroupedByTableAlerts, *args, **kwargs
     ):
         alerts = alert.alerts
-        model = alert.model
 
-        title_blocks = []  # title, [banner], number of passed or failed,
-        title_blocks.append(
+        self.message_builder.add_message_color(self._get_color(alert.status))
+
+        title_blocks = [
             self.message_builder.create_header_block(
-                f":small_red_triangle: Table issues detected - {model}"
+                f"{self._get_display_name(alert.status)}: {alert.summary}"
             )
-        )
-
+        ]
         # summary of number of failed, errors, etc.
         fields_summary: List[str] = []
         # summary of number of failed, errors, etc.
@@ -985,3 +1001,15 @@ def _get_integration_params(
                 )
             )
         return integration_params
+
+    @staticmethod
+    def _get_display_name(alert_status: Optional[str]) -> str:
+        if alert_status is None:
+            return "Unknown"
+        return STATUS_DISPLAYS.get(alert_status, {}).get("display_name", alert_status)
+
+    @staticmethod
+    def _get_color(alert_status: Optional[str]) -> MessageColor:
+        if alert_status is None:
+            return MessageColor.RED
+        return STATUS_DISPLAYS.get(alert_status, {}).get("color", MessageColor.RED)

tests/unit/clients/slack/test_slack_message_builder.py

@@ -215,15 +215,6 @@ def test_create_compacted_sections_blocks():
     )
 
 
-def test_get_slack_status_icon():
-    assert SlackMessageBuilder.get_slack_status_icon("warn") == ":warning:"
-    assert SlackMessageBuilder.get_slack_status_icon("error") == ":x:"
-    assert (
-        SlackMessageBuilder.get_slack_status_icon("anything else")
-        == ":small_red_triangle:"
-    )
-
-
 def test_get_slack_message():
     slack_message_builder = SlackMessageBuilder()
     slack_message = slack_message_builder.get_slack_message()