From c87d3018b9135bc3b6b066d1dd8cfc8d2918c75a Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Mon, 26 Aug 2024 12:27:07 +0200 Subject: [PATCH 1/5] Handle TimeoutError on sending message to Slack (#4916) # What this PR does ## Which issue(s) this PR closes Related to https://github.com/grafana/oncall-private/issues/2758 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- .../apps/slack/scenarios/distribute_alerts.py | 2 +- .../test_distribute_alerts.py | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index d25fd193c5..52ded93241 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -73,7 +73,7 @@ def process_signal(self, alert: Alert) -> None: else alert.group.channel.organization.general_log_channel_id ) self._send_first_alert(alert, channel_id) - except SlackAPIError: + except (SlackAPIError, TimeoutError): AlertGroup.objects.filter(pk=alert.group.pk).update(slack_message_sent=False) raise diff --git a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py index 1eba2b9a3c..7d73febee8 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py @@ -39,6 +39,38 @@ def test_restricted_action_error( assert not alert.delivered +@pytest.mark.django_db +def test_timeout_error( + make_slack_team_identity, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, +): + SlackAlertShootingStep = ScenarioStep.get_step("distribute_alerts", "AlertShootingStep") + slack_team_identity = make_slack_team_identity() + organization = make_organization( + slack_team_identity=slack_team_identity, general_log_channel_id="DEFAULT_CHANNEL_ID" + ) + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + alert = make_alert(alert_group, raw_request_data="{}") + + step = SlackAlertShootingStep(slack_team_identity) + + with pytest.raises(TimeoutError): + with patch.object(step._slack_client, "api_call") as mock_slack_api_call: + mock_slack_api_call.side_effect = TimeoutError + step.process_signal(alert) + + alert_group.refresh_from_db() + alert.refresh_from_db() + assert alert_group.slack_message is None + assert alert_group.slack_message_sent is False + assert SlackMessage.objects.count() == 0 + assert not alert.delivered + + @patch.object(AlertShootingStep, "_post_alert_group_to_slack") @pytest.mark.django_db def test_alert_shooting_no_channel_filter( From 0764526acd555c05c0d6da95337f8ba1167fdfd8 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 26 Aug 2024 10:33:50 -0400 Subject: [PATCH 2/5] update oncall tilt setup (#4911) --- Tiltfile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Tiltfile b/Tiltfile index b0ad6724c9..bd5cc8fe60 100644 --- a/Tiltfile +++ b/Tiltfile @@ -73,13 +73,6 @@ def load_oncall_helm(): # --- GRAFANA START ---- -# Generate and load the grafana deploy yaml -configmap_create( - "grafana-oncall-app-provisioning", - namespace="default", - from_file="dev/grafana/provisioning/plugins/grafana-oncall-app-provisioning.yaml", -) - if not running_under_parent_tiltfile: # Load the custom Grafana extensions v1alpha1.extension_repo( @@ -98,6 +91,13 @@ def load_grafana(): grafana_version = os.getenv("GRAFANA_VERSION", "latest") if 'plugin' in profiles: + # Generate and load the grafana deploy yaml + configmap_create( + "grafana-oncall-app-provisioning", + namespace="default", + from_file="dev/grafana/provisioning/plugins/grafana-oncall-app-provisioning.yaml", + ) + k8s_resource( objects=["grafana-oncall-app-provisioning:configmap"], new_name="grafana-oncall-app-provisioning-configmap", From 1ac0ae6ce25edac1e5fb29fd99c70df6cf5d7b57 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 26 Aug 2024 10:34:12 -0400 Subject: [PATCH 3/5] update `CODEOWNERS` to account for a few extra golang files under `grafana-plugin` (#4917) --- .github/CODEOWNERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 11e50eaedf..2586bdfc14 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,6 +1,9 @@ * @grafana/grafana-oncall-backend /grafana-plugin @grafana/grafana-oncall-frontend /grafana-plugin/pkg @grafana/grafana-oncall-backend +/grafana-plugin/go.mod @grafana/grafana-oncall-backend +/grafana-plugin/go.sum @grafana/grafana-oncall-backend +/grafana-plugin/Magefile.go @grafana/grafana-oncall-backend /docs @grafana/docs-gops @grafana/grafana-oncall # `make docs` procedure is owned by @jdbaldry of @grafana/docs-squad. From 6fc342dc15d114a2e51c32c7286738eb4e04f8b6 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 26 Aug 2024 10:55:05 -0400 Subject: [PATCH 4/5] some minor changes for running oncall via `grafana/irm` tilt setup (#4927) Related to the changes under `./packages/grafana-oncall-app` in https://github.com/grafana/irm/pull/45 --- grafana-plugin/.bra.toml | 22 ++++++++++++++++++++++ grafana-plugin/package.json | 1 + 2 files changed, 23 insertions(+) create mode 100644 grafana-plugin/.bra.toml diff --git a/grafana-plugin/.bra.toml b/grafana-plugin/.bra.toml new file mode 100644 index 0000000000..0f1593e148 --- /dev/null +++ b/grafana-plugin/.bra.toml @@ -0,0 +1,22 @@ +# default configuration created by the `mage watch` command. +# this file can be edited and should be checked into source control. +# see https://github.com/unknwon/bra/blob/master/templates/default.bra.toml for more configuration options. +[run] +init_cmds = [ + ["mage", "-v", "build:debug"], + ["mage", "-v" , "reloadPlugin"] +] +watch_all = true +follow_symlinks = false +ignore = [".git", "node_modules", "dist"] +ignore_files = ["mage_output_file.go"] +watch_dirs = [ + "pkg", + # "src", +] +watch_exts = [".go", ".json"] +build_delay = 2000 +cmds = [ + ["mage", "-v", "build:debug"], + ["mage", "-v" , "reloadPlugin"] +] diff --git a/grafana-plugin/package.json b/grafana-plugin/package.json index 8a48337072..7fcc4f4ca9 100644 --- a/grafana-plugin/package.json +++ b/grafana-plugin/package.json @@ -11,6 +11,7 @@ "build:dev": "NODE_ENV=development webpack -c ./webpack.config.ts --env development", "labels:link": "yarn --cwd ../../gops-labels/frontend link && yarn link \"@grafana/labels\" && yarn --cwd ../../gops-labels/frontend watch", "labels:unlink": "yarn --cwd ../../gops-labels/frontend unlink", + "mage:watch": "go mod download && mage -v buildAll", "test-utc": "TZ=UTC jest --verbose --testNamePattern '^((?!@london-tz).)*$'", "test-london-tz": "TZ=Europe/London jest --verbose --testNamePattern '@london-tz'", "test": "PLUGIN_ID=grafana-oncall-app yarn test-utc && yarn test-london-tz", From 0ac7c40671f866b23151821ab9f0d8ab08666261 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Mon, 26 Aug 2024 12:03:26 -0300 Subject: [PATCH 5/5] Update org creation to use DB uniqueness constraint (#4926) Fix issue related to [logs](https://ops.grafana-ops.net/explore?schemaVersion=1&panes=%7B%2257p%22:%7B%22datasource%22:%22000000193%22,%22queries%22:%5B%7B%22refId%22:%22A%22,%22expr%22:%22%7Bnamespace%3D%5C%22amixr-prod%5C%22,%20cluster%3D%5C%22prod-us-central-0%5C%22,%20job%3D%5C%22amixr-prod%2Famixr-engine%5C%22%7D%20%7C%3D%20%5C%22user_management_organization.user_management_organization_stack_id_org_id_727b4929_uniq%5C%22%22,%22queryType%22:%22range%22,%22datasource%22:%7B%22type%22:%22loki%22,%22uid%22:%22000000193%22%7D,%22editorMode%22:%22code%22%7D%5D,%22range%22:%7B%22from%22:%22now-2d%22,%22to%22:%22now%22%7D%7D%7D&orgId=1) (check for existing org using the unique DB index) --- engine/apps/grafana_plugin/helpers/gcom.py | 22 +++++---- engine/apps/grafana_plugin/tests/test_gcom.py | 47 +++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/engine/apps/grafana_plugin/helpers/gcom.py b/engine/apps/grafana_plugin/helpers/gcom.py index e83ab9696a..d55c1b3652 100644 --- a/engine/apps/grafana_plugin/helpers/gcom.py +++ b/engine/apps/grafana_plugin/helpers/gcom.py @@ -60,18 +60,20 @@ def check_gcom_permission(token_string: str, context) -> GcomToken: ) # Get org from db or create a new one - organization, _ = Organization.objects.get_or_create( + organization, _ = Organization.objects.update_or_create( stack_id=instance_info["id"], - stack_slug=instance_info["slug"], - grafana_url=instance_info["url"], org_id=instance_info["orgId"], - org_slug=instance_info["orgSlug"], - org_title=instance_info["orgName"], - region_slug=instance_info["regionSlug"], - cluster_slug=instance_info["clusterSlug"], - gcom_token=token_string, - api_token=grafana_token, - defaults={"gcom_token_org_last_time_synced": timezone.now()}, + defaults={ + "gcom_token_org_last_time_synced": timezone.now(), + "stack_slug": instance_info["slug"], + "grafana_url": instance_info["url"], + "org_slug": instance_info["orgSlug"], + "org_title": instance_info["orgName"], + "region_slug": instance_info["regionSlug"], + "cluster_slug": instance_info["clusterSlug"], + "gcom_token": token_string, + "api_token": grafana_token, + }, ) else: organization.stack_slug = instance_info["slug"] diff --git a/engine/apps/grafana_plugin/tests/test_gcom.py b/engine/apps/grafana_plugin/tests/test_gcom.py index c70f0719e4..b13ac0c130 100644 --- a/engine/apps/grafana_plugin/tests/test_gcom.py +++ b/engine/apps/grafana_plugin/tests/test_gcom.py @@ -3,6 +3,7 @@ import pytest from apps.grafana_plugin.helpers.gcom import check_gcom_permission +from apps.user_management.models import Organization @pytest.mark.parametrize( @@ -59,3 +60,49 @@ def test_check_gcom_permission_updates_fields(make_organization, api_token, api_ assert org.api_token == api_token if api_token_updated else broken_token assert org.gcom_token == gcom_token assert org.gcom_token_org_last_time_synced != last_time_gcom_synced + + +@pytest.mark.django_db +def test_check_gcom_permission_uniqueness_update_fields(make_organization): + gcom_token = "gcom:test_token" + fixed_token = "fixed_token" + instance_info = { + "id": 324534, + "slug": "testinstance", + "url": "http://example.com", + "orgId": 5671, + "orgSlug": "testorg", + "orgName": "Test Org", + "regionSlug": "us", + "clusterSlug": "us-test", + } + context = { + "stack_id": str(instance_info["id"]), + "org_id": str(instance_info["orgId"]), + "grafana_token": fixed_token, + } + + org = make_organization(stack_id=instance_info["id"], org_id=instance_info["orgId"], api_token="broken_token") + + # organization does not exist in the first check but it is created before the second check + with patch( + "apps.grafana_plugin.helpers.gcom.Organization.objects.filter", return_value=Organization.objects.none() + ): + with patch( + "apps.grafana_plugin.helpers.GcomAPIClient.get_instance_info", + return_value=instance_info, + ) as mock_instance_info: + check_gcom_permission(gcom_token, context) + mock_instance_info.assert_called() + + org.refresh_from_db() + assert org.stack_id == instance_info["id"] + assert org.stack_slug == instance_info["slug"] + assert org.grafana_url == instance_info["url"] + assert org.org_id == instance_info["orgId"] + assert org.org_slug == instance_info["orgSlug"] + assert org.org_title == instance_info["orgName"] + assert org.region_slug == instance_info["regionSlug"] + assert org.cluster_slug == instance_info["clusterSlug"] + assert org.api_token == fixed_token + assert org.gcom_token == gcom_token