diff --git a/superset/migrations/versions/f2672aa8350a_add_slack_to_alerts.py b/superset/migrations/versions/f2672aa8350a_add_slack_to_alerts.py new file mode 100644 index 0000000000000..c7c8c4c2d4c73 --- /dev/null +++ b/superset/migrations/versions/f2672aa8350a_add_slack_to_alerts.py @@ -0,0 +1,42 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""add_slack_to_alerts + +Revision ID: f2672aa8350a +Revises: 2f1d15e8a6af +Create Date: 2020-08-08 18:10:51.973551 + +""" + +# revision identifiers, used by Alembic. +revision = "f2672aa8350a" +down_revision = "2f1d15e8a6af" + +import sqlalchemy as sa +from alembic import op + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("alerts", sa.Column("slack_channel", sa.Text(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("alerts", "slack_channel") + # ### end Alembic commands ### diff --git a/superset/models/alerts.py b/superset/models/alerts.py index 435124864b34d..cbea6571c588c 100644 --- a/superset/models/alerts.py +++ b/superset/models/alerts.py @@ -59,6 +59,7 @@ class Alert(Model): alert_type = Column(String(50)) owners = relationship(security_manager.user_model, secondary=alert_owner) recipients = Column(Text) + slack_channel = Column(Text) log_retention = Column(Integer, default=90) grace_period = Column(Integer, default=60 * 60 * 24) diff --git a/superset/tasks/schedules.py b/superset/tasks/schedules.py index e297da2d2a5ca..9ebdcfec3170f 100644 --- a/superset/tasks/schedules.py +++ b/superset/tasks/schedules.py @@ -29,6 +29,7 @@ Dict, Iterator, List, + NamedTuple, Optional, Tuple, TYPE_CHECKING, @@ -98,6 +99,18 @@ ) +class ScreenshotData(NamedTuple): + url: str # url to chat/dashboard for this screenshot + image: Optional[bytes] # bytes for the screenshot + + +class AlertContent(NamedTuple): + label: str # alert name + sql: str # sql statement for alert + alert_url: str # url to alert details + image_data: Optional[ScreenshotData] # data for the alert screenshot + + def _get_email_to_and_bcc( recipients: str, deliver_as_group: bool ) -> Iterator[Tuple[str, str]]: @@ -400,6 +413,24 @@ def _get_slice_data(slc: Slice, delivery_type: EmailDeliveryType) -> ReportConte return ReportContent(body, data, None, slack_message, content) +def _get_slice_screenshot(slice_id: int) -> ScreenshotData: + slice_obj = db.session.query(Slice).get(slice_id) + + chart_url = get_url_path("Superset.slice", slice_id=slice_obj.id, standalone="true") + screenshot = ChartScreenshot(chart_url, slice_obj.digest) + image_url = _get_url_path( + "Superset.slice", user_friendly=True, slice_id=slice_obj.id, + ) + + user = security_manager.find_user(current_app.config["THUMBNAIL_SELENIUM_USER"]) + image_data = screenshot.compute_and_cache( + user=user, cache=thumbnail_cache, force=True, + ) + + db.session.commit() + return ScreenshotData(image_url, image_data) + + def _get_slice_visualization( slc: Slice, delivery_type: EmailDeliveryType ) -> ReportContent: @@ -546,7 +577,7 @@ def schedule_alert_query( # pylint: disable=unused-argument report_type: ScheduleType, schedule_id: int, recipients: Optional[str] = None, - is_test_alert: Optional[bool] = False, + slack_channel: Optional[str] = None, ) -> None: model_cls = get_scheduler_model(report_type) @@ -559,8 +590,8 @@ def schedule_alert_query( # pylint: disable=unused-argument return if report_type == ScheduleType.alert: - if is_test_alert and recipients: - deliver_alert(schedule.id, recipients) + if recipients or slack_channel: + deliver_alert(schedule.id, recipients, slack_channel) return if run_alert_query( @@ -584,56 +615,87 @@ class AlertState: PASS = "pass" -def deliver_alert(alert_id: int, recipients: Optional[str] = None) -> None: +def deliver_alert( + alert_id: int, recipients: Optional[str] = None, slack_channel: Optional[str] = None +) -> None: alert = db.session.query(Alert).get(alert_id) logging.info("Triggering alert: %s", alert) - img_data = None - images = {} recipients = recipients or alert.recipients + slack_channel = slack_channel or alert.slack_channel if alert.slice: - - chart_url = get_url_path( - "Superset.slice", slice_id=alert.slice.id, standalone="true" - ) - screenshot = ChartScreenshot(chart_url, alert.slice.digest) - image_url = _get_url_path( - "Superset.slice", - user_friendly=True, - slice_id=alert.slice.id, - standalone="true", - ) - standalone_index = image_url.find("/?standalone=true") - if standalone_index != -1: - image_url = image_url[:standalone_index] - - user = security_manager.find_user(current_app.config["THUMBNAIL_SELENIUM_USER"]) - img_data = screenshot.compute_and_cache( - user=user, cache=thumbnail_cache, force=True, + alert_content = AlertContent( + alert.label, + alert.sql, + _get_url_path("AlertModelView.show", user_friendly=True, pk=alert_id), + _get_slice_screenshot(alert.slice.id), ) else: # TODO: dashboard delivery! - image_url = "https://media.giphy.com/media/dzaUX7CAG0Ihi/giphy.gif" + alert_content = AlertContent( + alert.label, + alert.sql, + _get_url_path("AlertModelView.show", user_friendly=True, pk=alert_id), + None, + ) - # generate the email + if recipients: + deliver_email_alert(alert_content, recipients) + if slack_channel: + deliver_slack_alert(alert_content, slack_channel) + + +def deliver_email_alert(alert_content: AlertContent, recipients: str) -> None: # TODO add sql query results to email - subject = f"[Superset] Triggered alert: {alert.label}" + subject = f"[Superset] Triggered alert: {alert_content.label}" deliver_as_group = False data = None - if img_data: - images = {"screenshot": img_data} + images = {} + # TODO(JasonD28): add support for emails with no screenshot + image_url = None + if alert_content.image_data: + image_url = alert_content.image_data.url + if alert_content.image_data.image: + images = {"screenshot": alert_content.image_data.image} + body = render_template( "email/alert.txt", - alert_url=_get_url_path("AlertModelView.show", user_friendly=True, pk=alert.id), - label=alert.label, - sql=alert.sql, + alert_url=alert_content.alert_url, + label=alert_content.label, + sql=alert_content.sql, image_url=image_url, ) _deliver_email(recipients, deliver_as_group, subject, body, data, images) +def deliver_slack_alert(alert_content: AlertContent, slack_channel: str) -> None: + subject = __("[Alert] %(label)s", label=alert_content.label) + + image = None + if alert_content.image_data: + slack_message = render_template( + "slack/alert.txt", + label=alert_content.label, + sql=alert_content.sql, + url=alert_content.image_data.url, + alert_url=alert_content.alert_url, + ) + image = alert_content.image_data.image + else: + slack_message = render_template( + "slack/alert_no_screenshot.txt", + label=alert_content.label, + sql=alert_content.sql, + alert_url=alert_content.alert_url, + ) + + deliver_slack_msg( + slack_channel, subject, slack_message, image, + ) + + def run_alert_query( alert_id: int, database_id: int, sql: str, label: str ) -> Optional[bool]: diff --git a/superset/tasks/slack_util.py b/superset/tasks/slack_util.py index 09d5ca04b2cb0..865aa59782455 100644 --- a/superset/tasks/slack_util.py +++ b/superset/tasks/slack_util.py @@ -16,7 +16,7 @@ # under the License. import logging from io import IOBase -from typing import cast, Union +from typing import cast, Optional, Union from retry.api import retry from slack import WebClient @@ -26,21 +26,30 @@ from superset import app # Globals -config = app.config +config = app.config # type: ignore logger = logging.getLogger("tasks.slack_util") @retry(SlackApiError, delay=10, backoff=2, tries=5) def deliver_slack_msg( - slack_channel: str, subject: str, body: str, file: Union[str, IOBase] + slack_channel: str, + subject: str, + body: str, + file: Optional[Union[str, IOBase, bytes]], ) -> None: client = WebClient(token=config["SLACK_API_TOKEN"], proxy=config["SLACK_PROXY"]) # files_upload returns SlackResponse as we run it in sync mode. - response = cast( - SlackResponse, - client.files_upload( - channels=slack_channel, file=file, initial_comment=body, title=subject - ), - ) + if file: + response = cast( + SlackResponse, + client.files_upload( + channels=slack_channel, file=file, initial_comment=body, title=subject + ), + ) + assert response["file"], str(response) # the uploaded file + else: + response = cast( + SlackResponse, client.chat_postMessage(channel=slack_channel, text=body), + ) + assert response["message"]["text"], str(response) logger.info("Sent the report to the slack %s", slack_channel) - assert response["file"], str(response) # the uploaded file diff --git a/superset/templates/slack/alert.txt b/superset/templates/slack/alert.txt new file mode 100644 index 0000000000000..2264eea77cedf --- /dev/null +++ b/superset/templates/slack/alert.txt @@ -0,0 +1,22 @@ +{# + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +#} +*Triggered Alert: {{label}} :redalert:* +SQL Statement:```{{sql}}``` +<{{alert_url}}|View Alert Details> +<{{url}}|*Explore in Superset*> diff --git a/superset/templates/slack/alert_no_screenshot.txt b/superset/templates/slack/alert_no_screenshot.txt new file mode 100644 index 0000000000000..84b74dafcb184 --- /dev/null +++ b/superset/templates/slack/alert_no_screenshot.txt @@ -0,0 +1,21 @@ +{# + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +#} +*Triggered Alert: {{label}} :redalert:* +SQL Statement:```{{sql}}``` +<{{alert_url}}|View Alert Details> diff --git a/superset/views/alerts.py b/superset/views/alerts.py index f034903cc8d15..70573cbff3428 100644 --- a/superset/views/alerts.py +++ b/superset/views/alerts.py @@ -74,6 +74,7 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors # "alert_type", "owners", "recipients", + "slack_channel", "slice", # TODO: implement dashboard screenshots with alerts # "dashboard", @@ -81,6 +82,7 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors "grace_period", "test_alert", "test_email_recipients", + "test_slack_channel", ) label_columns = { "sql": "SQL", @@ -123,6 +125,12 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors description="List of recipients to send test email to. " "If empty, an email will be sent to the original recipients.", ), + "test_slack_channel": StringField( + "Test Slack Channel", + default=None, + description="A slack channel to send a test message to. " + "If empty, an alert will be sent to the original channel.", + ), } edit_form_extra_fields = add_form_extra_fields edit_columns = add_columns @@ -133,8 +141,15 @@ def process_form(self, form: Form, is_created: bool) -> None: if form.test_email_recipients.data: email_recipients = get_email_address_str(form.test_email_recipients.data) + test_slack_channel = ( + form.test_slack_channel.data.strip() + if form.test_slack_channel.data + else None + ) + self._extra_data["test_alert"] = form.test_alert.data self._extra_data["test_email_recipients"] = email_recipients + self._extra_data["test_slack_channel"] = test_slack_channel def pre_add(self, item: "AlertModelView") -> None: item.recipients = get_email_address_str(item.recipients) @@ -145,8 +160,9 @@ def pre_add(self, item: "AlertModelView") -> None: def post_add(self, item: "AlertModelView") -> None: if self._extra_data["test_alert"]: recipients = self._extra_data["test_email_recipients"] or item.recipients + slack_channel = self._extra_data["test_slack_channel"] or item.slack_channel args = (ScheduleType.alert, item.id) - kwargs = dict(recipients=recipients, is_test_alert=True) + kwargs = dict(recipients=recipients, slack_channel=slack_channel) schedule_alert_query.apply_async(args=args, kwargs=kwargs) def post_update(self, item: "AlertModelView") -> None: diff --git a/tests/alerts_tests.py b/tests/alerts_tests.py index 5749821484e91..f8c7da9435bb2 100644 --- a/tests/alerts_tests.py +++ b/tests/alerts_tests.py @@ -24,9 +24,14 @@ from superset.models.alerts import Alert, AlertLog from superset.models.schedules import ScheduleType from superset.models.slice import Slice -from superset.tasks.schedules import run_alert_query, schedule_alert_query +from superset.tasks.schedules import ( + deliver_alert, + run_alert_query, + schedule_alert_query, +) from superset.utils import core as utils from tests.test_app import app +from tests.utils import read_fixture logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) @@ -57,6 +62,8 @@ def setup_database(): sql="SELECT 55", alert_type="email", slice_id=slice_id, + recipients="recipient1@superset.com", + slack_channel="#test_channel", database_id=database_id, ), Alert( @@ -139,7 +146,38 @@ def test_schedule_alert_query(mock_run_alert, mock_deliver_alert, setup_database report_type=ScheduleType.alert, schedule_id=active_alert.id, recipients="testing@email.com", - is_test_alert=True, ) assert mock_run_alert.call_count == 1 assert mock_deliver_alert.call_count == 1 + + +@patch("superset.tasks.slack_util.WebClient.files_upload") +@patch("superset.tasks.schedules.send_email_smtp") +@patch("superset.tasks.schedules._get_url_path") +@patch("superset.utils.screenshots.ChartScreenshot.compute_and_cache") +def test_deliver_alert_screenshot( + screenshot_mock, url_mock, email_mock, file_upload_mock, setup_database +): + dbsession = setup_database + alert = dbsession.query(Alert).filter_by(id=2).one() + + screenshot = read_fixture("sample.png") + screenshot_mock.return_value = screenshot + + # TODO: fix AlertModelView.show url call from test + url_mock.side_effect = [ + f"http://0.0.0.0:8080/alert/show/{alert.id}", + f"http://0.0.0.0:8080/superset/slice/{alert.slice_id}/", + ] + + deliver_alert(alert_id=alert.id) + assert email_mock.call_args[1]["images"]["screenshot"] == screenshot + assert file_upload_mock.call_args[1] == { + "channels": alert.slack_channel, + "file": screenshot, + "initial_comment": f"\n*Triggered Alert: {alert.label} :redalert:*\n" + f"SQL Statement:```{alert.sql}```\n\n", + "title": f"[Alert] {alert.label}", + }