diff --git a/elementary/monitor/alerts/alert.py b/elementary/monitor/alerts/alert.py index 8e1a3b351..eb1af224f 100644 --- a/elementary/monitor/alerts/alert.py +++ b/elementary/monitor/alerts/alert.py @@ -99,8 +99,12 @@ def data(self) -> Dict: raise NotImplementedError @property - def concise_name(self): - return "Alert" + def concise_name(self) -> str: + raise NotImplementedError + + @property + def asset_type(self) -> str: + raise NotImplementedError @property def summary(self) -> str: diff --git a/elementary/monitor/alerts/alert_messages/builder.py b/elementary/monitor/alerts/alert_messages/builder.py index 3e5e71e43..486cf9692 100644 --- a/elementary/monitor/alerts/alert_messages/builder.py +++ b/elementary/monitor/alerts/alert_messages/builder.py @@ -1,5 +1,5 @@ from datetime import timedelta -from typing import Any, Dict, List, Literal, Optional, Sequence, Tuple, Union +from typing import Any, Dict, List, Optional, Sequence, Tuple, Union from elementary.messages.block_builders import ( BoldTextBlock, @@ -33,7 +33,7 @@ TextStyle, ) from elementary.messages.message_body import Color, MessageBlock, MessageBody -from elementary.monitor.alerts.alert import OrchestratorInfo +from elementary.monitor.alerts.alert import AlertModel, OrchestratorInfo from elementary.monitor.alerts.alert_messages.alert_fields import AlertField from elementary.monitor.alerts.alerts_groups.alerts_group import AlertsGroup from elementary.monitor.alerts.alerts_groups.base_alerts_group import BaseAlertsGroup @@ -51,12 +51,7 @@ ) from elementary.utils.pydantic_shim import BaseModel -AlertType = Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - BaseAlertsGroup, -] +AlertType = Union[AlertModel, BaseAlertsGroup] class MessageBuilderConfig(BaseModel): @@ -104,7 +99,7 @@ def _get_alert_title( def _get_run_alert_subtitle_block( self, - type: Literal["test", "snapshot", "model", "source"], + type: str, name: str, status: Optional[str] = None, detected_at_str: Optional[str] = None, @@ -172,7 +167,7 @@ def _get_run_alert_subtitle_block( def _get_run_alert_subtitle_links( self, - alert: Union[TestAlertModel, SourceFreshnessAlertModel, ModelAlertModel], + alert: AlertModel, ) -> List[ReportLinkData]: report_link = alert.get_report_link() if report_link: @@ -181,25 +176,14 @@ def _get_run_alert_subtitle_links( def _get_run_alert_subtitle_blocks( self, - alert: Union[TestAlertModel, SourceFreshnessAlertModel, ModelAlertModel], + alert: AlertModel, ) -> List[MessageBlock]: - asset_type: Literal["test", "snapshot", "model", "source"] - asset_name: str - if isinstance(alert, TestAlertModel): - asset_type = "test" - asset_name = alert.concise_name - elif isinstance(alert, SourceFreshnessAlertModel): - asset_type = "source" - asset_name = f"{alert.source_name}.{alert.identifier}" - elif isinstance(alert, ModelAlertModel): - asset_type = "snapshot" if alert.materialization == "snapshot" else "model" - asset_name = alert.alias links = self._get_run_alert_subtitle_links(alert) orchestrator_info = alert.orchestrator_info return [ self._get_run_alert_subtitle_block( - type=asset_type, - name=asset_name, + type=alert.asset_type, + name=alert.concise_name, status=alert.status, detected_at_str=alert.detected_at_str, suppression_interval=alert.suppression_interval, @@ -472,11 +456,7 @@ def _get_source_freshness_alert_config_blocks( def _get_alert_list_line( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - ], + alert: AlertModel, ) -> LineBlock: inlines: List[InlineBlock] = [ TextBlock(text=alert.summary, style=TextStyle.BOLD), @@ -507,13 +487,7 @@ def _get_alert_list_blocks( self, title: str, bullet_icon: Icon, - alerts: Sequence[ - Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - ] - ], + alerts: Sequence[AlertModel], ) -> List[MessageBlock]: blocks: List[MessageBlock] = [] if not alerts: @@ -526,10 +500,10 @@ def _get_alert_list_blocks( def _get_sub_alert_groups_blocks( self, - test_errors: List[Union[TestAlertModel, SourceFreshnessAlertModel]], - test_warnings: List[Union[TestAlertModel, SourceFreshnessAlertModel]], - test_failures: List[Union[TestAlertModel, SourceFreshnessAlertModel]], - model_errors: List[ModelAlertModel], + test_errors: Sequence[AlertModel], + test_warnings: Sequence[AlertModel], + test_failures: Sequence[AlertModel], + model_errors: Sequence[AlertModel], ) -> List[MessageBlock]: blocks: List[MessageBlock] = [] model_errors_alert_list_blocks = self._get_alert_list_blocks( diff --git a/elementary/monitor/alerts/alerts_groups/alerts_group.py b/elementary/monitor/alerts/alerts_groups/alerts_group.py index b2c3ec34f..3b5ae6067 100644 --- a/elementary/monitor/alerts/alerts_groups/alerts_group.py +++ b/elementary/monitor/alerts/alerts_groups/alerts_group.py @@ -1,20 +1,19 @@ -from typing import List, Optional, Union +from typing import List, Optional, Sequence +from elementary.monitor.alerts.alert import AlertModel from elementary.monitor.alerts.alerts_groups.base_alerts_group import BaseAlertsGroup from elementary.monitor.alerts.model_alert import ModelAlertModel -from elementary.monitor.alerts.source_freshness_alert import SourceFreshnessAlertModel -from elementary.monitor.alerts.test_alert import TestAlertModel class AlertsGroup(BaseAlertsGroup): - test_errors: List[Union[TestAlertModel, SourceFreshnessAlertModel]] - test_warnings: List[Union[TestAlertModel, SourceFreshnessAlertModel]] - test_failures: List[Union[TestAlertModel, SourceFreshnessAlertModel]] + test_errors: List[AlertModel] + test_warnings: List[AlertModel] + test_failures: List[AlertModel] model_errors: List[ModelAlertModel] def __init__( self, - alerts: List[Union[TestAlertModel, ModelAlertModel, SourceFreshnessAlertModel]], + alerts: Sequence[AlertModel], env: Optional[str] = None, ) -> None: super().__init__(alerts, env=env) diff --git a/elementary/monitor/alerts/alerts_groups/base_alerts_group.py b/elementary/monitor/alerts/alerts_groups/base_alerts_group.py index 1cc34d74a..513ccfd00 100644 --- a/elementary/monitor/alerts/alerts_groups/base_alerts_group.py +++ b/elementary/monitor/alerts/alerts_groups/base_alerts_group.py @@ -1,18 +1,14 @@ from abc import ABC, abstractmethod from datetime import datetime -from typing import Dict, List, Optional, Sequence, Union +from typing import Dict, List, Optional, Sequence -from elementary.monitor.alerts.model_alert import ModelAlertModel -from elementary.monitor.alerts.source_freshness_alert import SourceFreshnessAlertModel -from elementary.monitor.alerts.test_alert import TestAlertModel +from elementary.monitor.alerts.alert import AlertModel class BaseAlertsGroup(ABC): def __init__( self, - alerts: Sequence[ - Union[TestAlertModel, ModelAlertModel, SourceFreshnessAlertModel] - ], + alerts: Sequence[AlertModel], env: Optional[str] = None, ) -> None: self.alerts = alerts diff --git a/elementary/monitor/alerts/model_alert.py b/elementary/monitor/alerts/model_alert.py index e28514581..70dbfc9a3 100644 --- a/elementary/monitor/alerts/model_alert.py +++ b/elementary/monitor/alerts/model_alert.py @@ -100,13 +100,13 @@ def data(self) -> Dict: full_refresh=self.full_refresh, ) + @property + def asset_type(self) -> str: + return "snapshot" if self.materialization == "snapshot" else "model" + @property def concise_name(self) -> str: - if self.materialization == "snapshot": - dbt_type = "snapshot" - else: - dbt_type = "model" - return f"dbt {dbt_type} alert - {self.alias}" + return self.alias @property def summary(self) -> str: diff --git a/elementary/monitor/alerts/source_freshness_alert.py b/elementary/monitor/alerts/source_freshness_alert.py index d1561bb4f..692e79b59 100644 --- a/elementary/monitor/alerts/source_freshness_alert.py +++ b/elementary/monitor/alerts/source_freshness_alert.py @@ -155,9 +155,13 @@ def data(self) -> Dict: freshness_description=self.freshness_description, ) + @property + def asset_type(self) -> str: + return "source" + @property def concise_name(self) -> str: - return f"source freshness alert - {self.source_name}.{self.identifier}" + return f"{self.source_name}.{self.identifier}" @property def error_message(self) -> str: diff --git a/elementary/monitor/alerts/test_alert.py b/elementary/monitor/alerts/test_alert.py index f2514e31d..1e5305f75 100644 --- a/elementary/monitor/alerts/test_alert.py +++ b/elementary/monitor/alerts/test_alert.py @@ -166,6 +166,10 @@ def data(self) -> Dict: env=self.env, ) + @property + def asset_type(self) -> str: + return "test" + @property def concise_name(self) -> str: if self.test_sub_type_display_name.lower() not in ( diff --git a/elementary/monitor/api/alerts/alert_filters.py b/elementary/monitor/api/alerts/alert_filters.py index 719a24145..cabaa9830 100644 --- a/elementary/monitor/api/alerts/alert_filters.py +++ b/elementary/monitor/api/alerts/alert_filters.py @@ -29,7 +29,7 @@ def _get_alert_node_name(alert: PendingAlertSchema) -> Optional[str]: alert_node_name = None alert_type = AlertTypes(alert.type) if alert_type is AlertTypes.TEST: - alert_node_name = alert.data.test_name # type: ignore[union-attr] + alert_node_name = alert.data.test_name # type: ignore[attr-defined] elif alert_type is AlertTypes.MODEL or alert_type is AlertTypes.SOURCE_FRESHNESS: alert_node_name = alert.data.model_unique_id else: diff --git a/elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py b/elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py index ed0b78fd8..0b0158f4d 100644 --- a/elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py +++ b/elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py @@ -22,6 +22,7 @@ from elementary.messages.messaging_integrations.exceptions import ( MessagingIntegrationError, ) +from elementary.monitor.alerts.alert import AlertModel from elementary.monitor.alerts.alert_messages.builder import ( AlertMessageBuilder, MessageBuilderConfig, @@ -385,13 +386,7 @@ def _send_alerts( self.execution_properties["sent_alert_count"] = self.sent_alert_count return - sent_successfully_alerts: List[ - Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - ] - ] = [] + sent_successfully_alerts: List[AlertModel] = [] with alive_bar(len(alerts), title="Sending alerts") as bar: for alert in alerts: diff --git a/elementary/monitor/data_monitoring/alerts/integrations/base_integration.py b/elementary/monitor/data_monitoring/alerts/integrations/base_integration.py index 753632887..67ee48d8a 100644 --- a/elementary/monitor/data_monitoring/alerts/integrations/base_integration.py +++ b/elementary/monitor/data_monitoring/alerts/integrations/base_integration.py @@ -1,6 +1,7 @@ from abc import ABC, abstractmethod from typing import Union +from elementary.monitor.alerts.alert import AlertModel from elementary.monitor.alerts.alerts_groups import GroupedByTableAlerts from elementary.monitor.alerts.alerts_groups.base_alerts_group import BaseAlertsGroup from elementary.monitor.alerts.model_alert import ModelAlertModel @@ -21,13 +22,7 @@ def _initial_client(self, *args, **kwargs): def _get_alert_template( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], *args, **kwargs, ): @@ -83,12 +78,7 @@ def _get_alerts_group_template(self, alert: BaseAlertsGroup, *args, **kwargs): @abstractmethod def _get_fallback_template( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - ], + alert: Union[AlertModel, GroupedByTableAlerts], *args, **kwargs, ): @@ -97,13 +87,7 @@ def _get_fallback_template( @abstractmethod def send_alert( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], *args, **kwargs, ) -> bool: diff --git a/elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py b/elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py index a0e5ce30d..03770b0db 100644 --- a/elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py +++ b/elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py @@ -9,6 +9,7 @@ from elementary.clients.slack.schema import SlackBlocksType, SlackMessageSchema from elementary.clients.slack.slack_message_builder import MessageColor from elementary.config.config import Config +from elementary.monitor.alerts.alert import AlertModel from elementary.monitor.alerts.alerts_groups import AlertsGroup, GroupedByTableAlerts from elementary.monitor.alerts.alerts_groups.base_alerts_group import BaseAlertsGroup from elementary.monitor.alerts.model_alert import ModelAlertModel @@ -94,13 +95,7 @@ def _initial_client(self, *args, **kwargs) -> SlackClient: def _get_alert_template( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], *args, **kwargs, ) -> SlackMessageSchema: @@ -875,14 +870,7 @@ def _get_group_by_table_template( def _add_compact_sub_group_details_block( self, details_blocks: list, - alerts: Sequence[ - Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - ], - ], + alerts: Sequence[Union[AlertModel, GroupedByTableAlerts]], sub_title: str, bullet_icon: str, ) -> None: @@ -941,14 +929,7 @@ def _get_alerts_group_compact_template( def _add_sub_group_details_block( self, details_blocks: list, - alerts: Sequence[ - Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - ], - ], + alerts: Sequence[Union[AlertModel, GroupedByTableAlerts]], sub_title: str, bullet_icon: str, ) -> None: @@ -1036,13 +1017,7 @@ def _get_model_error_block_body(model_error_alerts: List[ModelAlertModel]) -> st def _get_fallback_template( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], *args, **kwargs, ) -> SlackMessageSchema: @@ -1079,13 +1054,7 @@ def _get_user_id(email: str) -> str: def _fix_owners_and_subscribers( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], ): if isinstance(alert, BaseAlertsGroup): for inner_alert in alert.alerts: @@ -1099,13 +1068,7 @@ def _fix_owners_and_subscribers( def send_alert( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], *args, **kwargs, ) -> bool: @@ -1164,13 +1127,7 @@ def send_test_message(self, channel_name: str, *args, **kwargs) -> bool: def _get_integration_params( self, - alert: Union[ - TestAlertModel, - ModelAlertModel, - SourceFreshnessAlertModel, - GroupedByTableAlerts, - BaseAlertsGroup, - ], + alert: Union[AlertModel, GroupedByTableAlerts, BaseAlertsGroup], *args, **kwargs, ) -> Dict[str, Any]: diff --git a/elementary/monitor/fetchers/alerts/schema/alert_data.py b/elementary/monitor/fetchers/alerts/schema/alert_data.py index 5add0df8b..8d8e62689 100644 --- a/elementary/monitor/fetchers/alerts/schema/alert_data.py +++ b/elementary/monitor/fetchers/alerts/schema/alert_data.py @@ -34,6 +34,7 @@ class BaseAlertDataSchema(BaseModel): owners: Optional[List[str]] = None model_meta: Optional[Dict] = None status: str + resource_type: Optional[ResourceType] = None # Orchestrator fields job_id: Optional[str] = None job_name: Optional[str] = None diff --git a/elementary/monitor/fetchers/alerts/schema/pending_alerts.py b/elementary/monitor/fetchers/alerts/schema/pending_alerts.py index 89edfc59c..f35a1aeea 100644 --- a/elementary/monitor/fetchers/alerts/schema/pending_alerts.py +++ b/elementary/monitor/fetchers/alerts/schema/pending_alerts.py @@ -1,8 +1,9 @@ from datetime import datetime, timezone from enum import Enum -from typing import Optional, Union +from typing import Optional from elementary.monitor.fetchers.alerts.schema.alert_data import ( + BaseAlertDataSchema, ModelAlertDataSchema, SourceFreshnessAlertDataSchema, TestAlertDataSchema, @@ -40,9 +41,7 @@ class PendingAlertSchema(BaseModel): created_at: datetime updated_at: datetime status: AlertStatus - data: Union[ - TestAlertDataSchema, ModelAlertDataSchema, SourceFreshnessAlertDataSchema - ] + data: BaseAlertDataSchema sent_at: Optional[datetime] = None class Config: diff --git a/tests/unit/monitor/alerts/__init__.py b/tests/unit/monitor/alerts/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/monitor/alerts/test_alert_models.py b/tests/unit/monitor/alerts/test_alert_models.py new file mode 100644 index 000000000..13c5c5673 --- /dev/null +++ b/tests/unit/monitor/alerts/test_alert_models.py @@ -0,0 +1,97 @@ +import pytest + +from elementary.monitor.alerts.alert import AlertModel +from elementary.monitor.alerts.model_alert import ModelAlertModel +from elementary.monitor.alerts.source_freshness_alert import SourceFreshnessAlertModel +from elementary.monitor.alerts.test_alert import TestAlertModel + + +def _make_test_alert( + test_sub_type: str = "generic", test_short_name: str = "my_test" +) -> TestAlertModel: + return TestAlertModel( + id="id", + test_unique_id="tuid", + elementary_unique_id="euid", + test_name="test_name", + severity="error", + test_type="dbt_test", + test_sub_type=test_sub_type, + test_short_name=test_short_name, + alert_class_id="acid", + ) + + +def _make_model_alert( + alias: str = "my_model", materialization: str = "table" +) -> ModelAlertModel: + return ModelAlertModel( + id="id", + alias=alias, + path="models/m.sql", + original_path="models/m.sql", + full_refresh=False, + alert_class_id="acid", + materialization=materialization, + ) + + +def _make_source_freshness_alert( + source_name: str = "my_source", identifier: str = "my_table" +) -> SourceFreshnessAlertModel: + return SourceFreshnessAlertModel( + id="id", + source_name=source_name, + identifier=identifier, + original_status="fail", + path="models/src.yml", + error=None, + alert_class_id="acid", + source_freshness_execution_id="sfeid", + ) + + +class TestAlertModelContract: + def test_base_asset_type_raises(self) -> None: + alert = AlertModel(id="id", alert_class_id="acid") + with pytest.raises(NotImplementedError): + _ = alert.asset_type + + def test_base_concise_name_raises(self) -> None: + alert = AlertModel(id="id", alert_class_id="acid") + with pytest.raises(NotImplementedError): + _ = alert.concise_name + + +class TestTestAlertModel: + def test_asset_type_is_test(self) -> None: + assert _make_test_alert().asset_type == "test" + + def test_concise_name_generic(self) -> None: + alert = _make_test_alert(test_sub_type="generic", test_short_name="my_test") + assert alert.concise_name == "my_test" + + def test_concise_name_non_generic_includes_sub_type(self) -> None: + alert = _make_test_alert(test_sub_type="row_count", test_short_name="my_test") + assert alert.concise_name == "my_test - Row Count" + + +class TestModelAlertModel: + def test_asset_type_model_when_not_snapshot(self) -> None: + assert _make_model_alert(materialization="table").asset_type == "model" + + def test_asset_type_snapshot_when_snapshot(self) -> None: + assert _make_model_alert(materialization="snapshot").asset_type == "snapshot" + + def test_concise_name_is_alias(self) -> None: + alert = _make_model_alert(alias="my_model") + assert alert.concise_name == "my_model" + + +class TestSourceFreshnessAlertModel: + def test_asset_type_is_source(self) -> None: + assert _make_source_freshness_alert().asset_type == "source" + + def test_concise_name_is_source_dot_identifier(self) -> None: + alert = _make_source_freshness_alert(source_name="raw", identifier="orders") + assert alert.concise_name == "raw.orders"