Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

Commit

Permalink
Allow DAGs to silence only errors matching predicate (#654)
Browse files Browse the repository at this point in the history
* Only silence alert if it matches one of the configured error predicates

* Update check_silenced_dags dag to use new config

* Make error comparison case-insensitive
  • Loading branch information
stacimc authored Aug 24, 2022
1 parent a941769 commit 03ce84f
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 10 deletions.
4 changes: 3 additions & 1 deletion openverse_catalog/dags/common/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,9 @@ def send_alert(
known_failures = Variable.get(
"silenced_slack_alerts", default_var={}, deserialize_json=True
)
if dag_id in known_failures:
if dag_id in known_failures and any(
error.lower() in text.lower() for error in known_failures[dag_id]["errors"]
):
log.info(f"Skipping Slack alert for {dag_id}: {text}")
return

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ def get_dags_with_closed_issues(github_pat, silenced_dags):
gh = GitHubAPI(github_pat)

dags_to_reenable = []
for dag_id, issue_url in silenced_dags.items():
for dag_id, conf in silenced_dags.items():
issue_url = conf["issue"]
owner, repo, issue_number = get_issue_info(issue_url)
github_issue = gh.get_issue(repo, issue_number, owner)

Expand Down
47 changes: 43 additions & 4 deletions tests/dags/common/test_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,55 @@ def test_send_alert():
)


def test_send_alert_skips_when_silenced():
@pytest.mark.parametrize(
"silenced_errors, alert, should_send",
[
# silenced errors includes alert being raised
(
[
"Error triggering data refresh",
],
"Error triggering data refresh",
False,
),
# alert contains substring matching a silenced error
(
[
"Authorization failed",
"data refresh",
],
"Error triggering data refresh",
False,
),
# error matches are case-insensitive
(
[
"kEYeRROR",
],
"KeyError: 'image'",
False,
),
# none of the silenced errors matches alert string
(
["Authorization failed", "data refresh", "No unit codes"],
"Error due to invalid selection criteria",
True,
),
],
)
def test_send_alert_skips_when_silenced(silenced_errors, alert, should_send):
mock_silenced_dags = {
"silenced_dag_id": "https://github.com/WordPress/openverse/issues/1"
"silenced_dag_id": {
"issue": "https://github.com/WordPress/openverse/issues/1",
"errors": silenced_errors,
}
}
with mock.patch("common.slack.send_message") as send_message_mock, mock.patch(
"common.slack.Variable"
) as MockVariable:
MockVariable.get.side_effect = [mock_silenced_dags]
send_alert("Sample text", dag_id="silenced_dag_id")
send_message_mock.assert_not_called()
send_alert(alert, dag_id="silenced_dag_id")
assert send_message_mock.called == should_send


@pytest.mark.parametrize(
Expand Down
28 changes: 24 additions & 4 deletions tests/dags/maintenance/test_check_silenced_dags.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@
),
# One DAG to reenable
(
{"dag_a_id": "https://github.com/WordPress/openverse/issues/1"},
{
"dag_a_id": {
"issue": "https://github.com/WordPress/openverse/issues/1",
"errors": [
"Test exception",
],
}
},
[
("dag_a_id", "https://github.com/WordPress/openverse/issues/1"),
],
Expand All @@ -32,8 +39,18 @@
# Multiple DAGs to reenable
(
{
"dag_a_id": "https://github.com/WordPress/openverse/issues/1",
"dag_b_id": "https://github.com/WordPress/openverse/issues/2",
"dag_a_id": {
"issue": "https://github.com/WordPress/openverse/issues/1",
"errors": [
"Test exception",
],
},
"dag_b_id": {
"issue": "https://github.com/WordPress/openverse/issues/2",
"errors": [
"A different error",
],
},
},
[
("dag_a_id", "https://github.com/WordPress/openverse/issues/1"),
Expand Down Expand Up @@ -110,7 +127,10 @@ def mock_get_issue(repo, issue_number, owner):
) as MockGetIssue:
MockGetIssue.side_effect = mock_get_issue

silenced_dags = {f"dag_{issue}": issue for issue in open_issues + closed_issues}
silenced_dags = {
f"dag_{issue}": {"issue": issue, "errors": ["test"]}
for issue in open_issues + closed_issues
}

dags_to_reenable = get_dags_with_closed_issues("not_set", silenced_dags)

Expand Down

0 comments on commit 03ce84f

Please sign in to comment.