From c3acf8227f299406e7d02f0954f0398215ccc873 Mon Sep 17 00:00:00 2001 From: Austin Dickey Date: Tue, 19 Dec 2023 09:02:40 -0600 Subject: [PATCH] benchalerts: Add slack pipeline steps (#1555) * Add slack pipeline steps * lint * pass comment details through too --- benchalerts/benchalerts/integrations/slack.py | 66 +++++++++ benchalerts/benchalerts/message_formatting.py | 15 ++ .../benchalerts/pipeline_steps/__init__.py | 3 + .../benchalerts/pipeline_steps/slack.py | 129 ++++++++++++++++++ benchalerts/tests/unit_tests/conftest.py | 10 ++ .../POST_slack_chat_postMessage_123.json | 53 +++++++ .../POST_slack_chat_postMessage_fail.json | 13 ++ benchalerts/tests/unit_tests/mocks.py | 41 +++++- .../test_integrations/test_slack.py | 21 +++ .../test_pipeline_steps/test_slack.py | 111 +++++++++++++++ 10 files changed, 461 insertions(+), 1 deletion(-) create mode 100644 benchalerts/benchalerts/integrations/slack.py create mode 100644 benchalerts/benchalerts/pipeline_steps/slack.py create mode 100644 benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_123.json create mode 100644 benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_fail.json create mode 100644 benchalerts/tests/unit_tests/test_integrations/test_slack.py create mode 100644 benchalerts/tests/unit_tests/test_pipeline_steps/test_slack.py diff --git a/benchalerts/benchalerts/integrations/slack.py b/benchalerts/benchalerts/integrations/slack.py new file mode 100644 index 000000000..4cd5ba702 --- /dev/null +++ b/benchalerts/benchalerts/integrations/slack.py @@ -0,0 +1,66 @@ +import os + +from benchclients.http import RetryingHTTPClient +from benchclients.logging import fatal_and_log + + +class SlackClient(RetryingHTTPClient): + """A client to interact with Slack. + + This uses the token-based authentication method, not the Incoming Webhooks method. + + Notes + ----- + Environment variables + ~~~~~~~~~~~~~~~~~~~~~ + ``SLACK_TOKEN`` + A Slack token; see https://api.slack.com/authentication/token-types. Tokens look + like ``xoxb-...`` if they're bot tokens. + """ + + default_retry_for_seconds = 60 + timeout_long_running_requests = (3.5, 10) + + def __init__(self) -> None: + token = os.getenv("SLACK_TOKEN", "") + if not token: + fatal_and_log("Environment variable SLACK_TOKEN not found.") + + super().__init__() + self.session.headers.update({"Authorization": f"Bearer {token}"}) + + @property + def _base_url(self) -> str: + return "https://slack.com/api" + + def _login_or_raise(self) -> None: + pass + + def post_message(self, channel_id: str, message: str) -> dict: + """Post a message to a Slack channel. + + Parameters + ---------- + channel_id + The ID of the channel to post to. + message + The message text. + + Returns + ------- + dict + The response body from the Slack HTTP API as a dict. + """ + resp_dict = self._make_request( + "POST", + self._abs_url_from_path("/chat.postMessage"), + 200, + json={"channel": channel_id, "text": message}, + ).json() + + if not resp_dict.get("ok"): + fatal_and_log( + f"Failed to send message to Slack. Deserialized response body: {resp_dict}", + ) + + return resp_dict diff --git a/benchalerts/benchalerts/message_formatting.py b/benchalerts/benchalerts/message_formatting.py index 273f9cc20..2088ae048 100644 --- a/benchalerts/benchalerts/message_formatting.py +++ b/benchalerts/benchalerts/message_formatting.py @@ -305,3 +305,18 @@ def github_pr_comment( ) return comment + + def slack_message( + self, + full_comparison: FullComparisonInfo, + check_details: dict, + comment_details: Optional[dict], + ) -> str: + """Generate a Slack message that links to a GitHub Check.""" + status = self.github_check_status(full_comparison) + link = check_details["html_url"] + message = f"Check run posted with status `{status.value}`: <{link}|check link>" + if comment_details: + message += f", <{comment_details['html_url']}|comment link>" + + return message diff --git a/benchalerts/benchalerts/pipeline_steps/__init__.py b/benchalerts/benchalerts/pipeline_steps/__init__.py index 82d94a1be..2ff1b7c75 100644 --- a/benchalerts/benchalerts/pipeline_steps/__init__.py +++ b/benchalerts/benchalerts/pipeline_steps/__init__.py @@ -8,6 +8,7 @@ GitHubCheckStep, GitHubPRCommentAboutCheckStep, ) +from .slack import SlackErrorHandler, SlackMessageAboutBadCheckStep __all__ = [ "BaselineRunCandidates", @@ -16,4 +17,6 @@ "GitHubCheckErrorHandler", "GitHubCheckStep", "GitHubPRCommentAboutCheckStep", + "SlackErrorHandler", + "SlackMessageAboutBadCheckStep", ] diff --git a/benchalerts/benchalerts/pipeline_steps/slack.py b/benchalerts/benchalerts/pipeline_steps/slack.py new file mode 100644 index 000000000..954bfcf2d --- /dev/null +++ b/benchalerts/benchalerts/pipeline_steps/slack.py @@ -0,0 +1,129 @@ +"""Pipeline steps to talk to Slack.""" + +from typing import Any, Dict, Optional + +from benchclients.logging import log + +from ..alert_pipeline import AlertPipelineErrorHandler, AlertPipelineStep +from ..integrations.github import CheckStatus +from ..integrations.slack import SlackClient +from ..message_formatting import Alerter + + +class SlackMessageAboutBadCheckStep(AlertPipelineStep): + """An ``AlertPipeline`` step to post to Slack about a failing GitHub Check that was + created by a previously-run ``GitHubCheckStep``. This is useful if you're running + benchmarks on a merge-commit, and no one is necessarily monitoring the Checks on the + default branch. + + Parameters + ---------- + channel_id + The ID of the Slack channel to post to. + slack_client + A SlackClient instance. If not provided, will default to ``SlackClient()``. + check_step_name + The name of the ``GitHubCheckStep`` that ran earlier in the pipeline. Defaults + to "GitHubCheckStep" (which was the default if no name was given to that step). + pr_comment_step_name + [Optional] The name of the ``GitHubPRCommentStep`` that ran earlier in the + pipeline. If provided, will include a link to the comment in the Slack message. + step_name + The name for this step. If not given, will default to this class's name. + alerter + Advanced usage; should not be necessary in most cases. An optional Alerter + instance to use to format the message. If not provided, will default to + ``Alerter()``. + + Returns + ------- + dict + The response body from the Slack HTTP API as a dict, or None if no message was + posted (e.g. if the check was successful). + + Notes + ----- + Environment variables + ~~~~~~~~~~~~~~~~~~~~~ + ``SLACK_TOKEN`` + A Slack token; see https://api.slack.com/authentication/token-types. Tokens look + like ``xoxb-...`` if they're bot tokens. Only required if ``slack_client`` is + not provided. + """ + + def __init__( + self, + channel_id: str, + slack_client: Optional[SlackClient] = None, + check_step_name: str = "GitHubCheckStep", + pr_comment_step_name: Optional[str] = None, + step_name: Optional[str] = None, + alerter: Optional[Alerter] = None, + ) -> None: + super().__init__(step_name=step_name) + self.channel_id = channel_id + self.slack_client = slack_client or SlackClient() + self.check_step_name = check_step_name + self.pr_comment_step_name = pr_comment_step_name + self.alerter = alerter or Alerter() + + def run_step(self, previous_outputs: Dict[str, Any]) -> Optional[dict]: + check_details, full_comparison = previous_outputs[self.check_step_name] + if self.pr_comment_step_name: + comment_details = previous_outputs[self.pr_comment_step_name] + else: + comment_details = None + + if self.alerter.github_check_status(full_comparison) == CheckStatus.SUCCESS: + log.info("GitHub Check was successful; not posting to Slack.") + return None + + res = self.slack_client.post_message( + message=self.alerter.slack_message( + full_comparison=full_comparison, + check_details=check_details, + comment_details=comment_details, + ), + channel_id=self.channel_id, + ) + return res + + +class SlackErrorHandler(AlertPipelineErrorHandler): + """Handle errors in a pipeline by posting a Slack message. + + Parameters + ---------- + channel_id + The ID of the Slack channel to post to. + slack_client + A SlackClient instance. If not provided, will default to ``SlackClient()``. + build_url + An optional build URL to include in the message. + + Notes + ----- + Environment variables + ~~~~~~~~~~~~~~~~~~~~~ + ``SLACK_TOKEN`` + A Slack token; see https://api.slack.com/authentication/token-types. Tokens look + like ``xoxb-...`` if they're bot tokens. Only required if ``slack_client`` is + not provided. + """ + + def __init__( + self, + channel_id: str, + slack_client: Optional[SlackClient] = None, + build_url: Optional[str] = None, + ) -> None: + self.channel_id = channel_id + self.slack_client = slack_client or SlackClient() + self.build_url = build_url + + def handle_error(self, exc: BaseException, traceback: str) -> None: + res = self.slack_client.post_message( + channel_id=self.channel_id, + message=f"Error in benchalerts pipeline. {self.build_url=}", + ) + log.debug(res) diff --git a/benchalerts/tests/unit_tests/conftest.py b/benchalerts/tests/unit_tests/conftest.py index 922308017..1c07c3523 100644 --- a/benchalerts/tests/unit_tests/conftest.py +++ b/benchalerts/tests/unit_tests/conftest.py @@ -59,6 +59,16 @@ def github_auth(request: SubRequest, monkeypatch: pytest.MonkeyPatch) -> str: return auth_type +@pytest.fixture +def slack_env(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("SLACK_TOKEN", "xoxb-123") + + +@pytest.fixture +def slack_env_missing(monkeypatch: pytest.MonkeyPatch): + monkeypatch.delenv("SLACK_TOKEN", raising=False) + + @pytest.fixture def conbench_env(monkeypatch: pytest.MonkeyPatch): monkeypatch.setenv("CONBENCH_URL", "https://conbench.biz") diff --git a/benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_123.json b/benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_123.json new file mode 100644 index 000000000..a0955cd5a --- /dev/null +++ b/benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_123.json @@ -0,0 +1,53 @@ +{ + "data": { + "channel": "123", + "message": { + "app_id": "123", + "blocks": [ + { + "block_id": "yxoJ", + "elements": [ + { + "elements": [ + { + "text": "hello", + "type": "text" + } + ], + "type": "rich_text_section" + } + ], + "type": "rich_text" + } + ], + "bot_id": "123", + "bot_profile": { + "app_id": "123", + "deleted": false, + "icons": { + "image_36": "https://avatars.slack-edge.com/", + "image_48": "https://avatars.slack-edge.com/", + "image_72": "https://avatars.slack-edge.com/" + }, + "id": "123", + "name": "abc", + "team_id": "123", + "updated": 1657225466 + }, + "team": "123", + "text": "hello", + "ts": "1702579355.753289", + "type": "message", + "user": "123" + }, + "ok": true, + "response_metadata": { + "warnings": [ + "missing_charset" + ] + }, + "ts": "1702579355.753289", + "warning": "missing_charset" + }, + "status_code": 200 +} diff --git a/benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_fail.json b/benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_fail.json new file mode 100644 index 000000000..8e389a9b1 --- /dev/null +++ b/benchalerts/tests/unit_tests/mocked_responses/POST_slack_chat_postMessage_fail.json @@ -0,0 +1,13 @@ +{ + "data": { + "error": "channel_not_found", + "ok": false, + "response_metadata": { + "warnings": [ + "missing_charset" + ] + }, + "warning": "missing_charset" + }, + "status_code": 200 +} diff --git a/benchalerts/tests/unit_tests/mocks.py b/benchalerts/tests/unit_tests/mocks.py index 529e48f3b..ca96c2b39 100644 --- a/benchalerts/tests/unit_tests/mocks.py +++ b/benchalerts/tests/unit_tests/mocks.py @@ -14,13 +14,14 @@ import json import pathlib -from typing import List, Tuple +from typing import List, Optional, Tuple import pytest import requests from benchclients.conbench import ConbenchClient from requests.adapters import HTTPAdapter +from benchalerts.integrations.slack import SlackClient from benchclients import log response_dir = pathlib.Path(__file__).parent / "mocked_responses" @@ -67,6 +68,7 @@ def clean_base_url(url: str) -> str: "https://api.github.com/repos/some/repo": "github", "https://api.github.com/app": "github_app", "https://conbench.biz/api": "conbench", + "https://slack.com/api": "slack", } for base_url, base_name in bases.items(): if url.startswith(base_url): @@ -96,6 +98,13 @@ def send(self, *args, **kwargs): method = req.method clean_url = self.clean_base_url(req.url) + + # switch Slack response based on channel ID too + if clean_url == "slack_chat_postMessage": + body = json.loads(req.body) + clean_url += "_" + body["channel"] + log.info("Slack comment: " + body["text"]) + response_path = response_dir / f"{method}_{clean_url}.json" if not response_path.exists(): @@ -122,6 +131,16 @@ def _login_or_raise(self) -> None: pass +class MockSlackClient(SlackClient): + """A SlackClient that uses MockAdapter to intercept all requests and return + mocked responses, constructed from JSON files. + """ + + def __init__(self): + super().__init__() + self.session.mount("https://", MockAdapter()) + + def check_posted_markdown( caplog: pytest.LogCaptureFixture, expected_markdowns: List[Tuple[str, str]] ): @@ -192,3 +211,23 @@ def check_posted_comment( assert ( expected_comment.strip() == actual_comment.strip() ), f"see tests/unit_tests/expected_md/{expected_comment_filename}.md" + + +def check_posted_slack_message( + caplog: pytest.LogCaptureFixture, expected_message: Optional[str] +): + """After we run a test, search through the logs for what message we + mock-posted to Slack, and assert it is what we expected. + """ + actual_messages = [ + log_record.message[15:] + for log_record in caplog.records + if log_record.levelname == "INFO" + and log_record.filename == "mocks.py" + and log_record.message.startswith("Slack comment: ") + ] + if expected_message: + assert len(actual_messages) == 1 + assert expected_message.strip() == actual_messages[0].strip() + else: + assert len(actual_messages) == 0 diff --git a/benchalerts/tests/unit_tests/test_integrations/test_slack.py b/benchalerts/tests/unit_tests/test_integrations/test_slack.py new file mode 100644 index 000000000..eef0ad7e4 --- /dev/null +++ b/benchalerts/tests/unit_tests/test_integrations/test_slack.py @@ -0,0 +1,21 @@ +import pytest + +from ..mocks import MockSlackClient + + +class TestSlackClient: + @property + def slack(self): + return MockSlackClient() + + def test_post_message(self, slack_env): + resp = self.slack.post_message(channel_id="123", message="hello") + assert resp["ok"] + + def test_post_message_fails(self, slack_env): + with pytest.raises(ValueError, match="Failed to send message"): + self.slack.post_message(channel_id="fail", message="hello") + + def test_missing_token(self, slack_env_missing): + with pytest.raises(ValueError, match="SLACK_TOKEN"): + self.slack diff --git a/benchalerts/tests/unit_tests/test_pipeline_steps/test_slack.py b/benchalerts/tests/unit_tests/test_pipeline_steps/test_slack.py new file mode 100644 index 000000000..1a7b5cb6e --- /dev/null +++ b/benchalerts/tests/unit_tests/test_pipeline_steps/test_slack.py @@ -0,0 +1,111 @@ +from traceback import format_exc +from typing import Optional + +import pytest + +from benchalerts import Alerter +from benchalerts.conbench_dataclasses import FullComparisonInfo +from benchalerts.pipeline_steps.slack import ( + SlackErrorHandler, + SlackMessageAboutBadCheckStep, +) + +from ..mocks import ( + MockResponse, + MockSlackClient, + check_posted_slack_message, + response_dir, +) + + +class MockAlerter(Alerter): + def slack_message(self, **kwargs) -> str: + base_message = super().slack_message(**kwargs) + return base_message + "\n\nThis message was generated from pytest." + + +CHECK_LINK = "" +COMMENT_LINK = ( + ", " +) +MESSAGE_TEMPLATE = """ +Check run posted with status `{status}`: {check_link}{comment_link} + +This message was generated from pytest. +""" + + +@pytest.mark.parametrize( + ["mock_comparison_info", "expected_status"], + [ + ("errors_baselines", "action_required"), + ("errors_nobaselines", "action_required"), + ("noerrors_nobaselines", "skipped"), + ("regressions", "failure"), + ("noregressions", None), + ("nocommit", None), + ("noruns", "action_required"), + ("noresults", "action_required"), + ], + indirect=["mock_comparison_info"], +) +@pytest.mark.parametrize("pr_comment_step_name", [None, "pr_comment_step"]) +def test_SlackMessageAboutBadCheckStep( + mock_comparison_info: FullComparisonInfo, + expected_status: Optional[str], + pr_comment_step_name: Optional[str], + caplog: pytest.LogCaptureFixture, + slack_env, +): + """Test that SlackMessageAboutBadCheckStep posts the right message to Slack.""" + mock_check_response = MockResponse.from_file( + response_dir / "POST_github_check-runs.json" + ).json() + mock_comment_response = MockResponse.from_file( + response_dir / "POST_github_issues_1_comments.json" + ).json() + + step = SlackMessageAboutBadCheckStep( + channel_id="123", + slack_client=MockSlackClient(), + check_step_name="check_step", + pr_comment_step_name=pr_comment_step_name, + alerter=MockAlerter(), + ) + res = step.run_step( + { + "check_step": (mock_check_response, mock_comparison_info), + "pr_comment_step": mock_comment_response, + } + ) + if expected_status: + assert res + if pr_comment_step_name: + expected_message = MESSAGE_TEMPLATE.format( + status=expected_status, check_link=CHECK_LINK, comment_link=COMMENT_LINK + ) + else: + expected_message = MESSAGE_TEMPLATE.format( + status=expected_status, check_link=CHECK_LINK, comment_link="" + ) + else: + assert not res + expected_message = None + check_posted_slack_message(caplog, expected_message) + + +def test_SlackErrorHandler(caplog: pytest.LogCaptureFixture, slack_env): + """Test that SlackErrorHandler posts the right message to Slack.""" + try: + 1 / 0 + except Exception as e: + exc = e + traceback = format_exc() + + handler = SlackErrorHandler( + channel_id="123", slack_client=MockSlackClient(), build_url="https://google.com" + ) + handler.handle_error(exc=exc, traceback=traceback) + check_posted_slack_message( + caplog, "Error in benchalerts pipeline. self.build_url='https://google.com'" + )