Prepare OnCall for Unified Slack App #4232

Konstantinov-Innokentii · 2024-04-16T04:33:27Z

This PR does a bunch of changes to prepare OnCall for Unified Slack App:

Install Slack via Chatops-Proxy. This change contains two parts: getting a Slack install link from chatops-proxy (code) and receiving a "slack installed" event from chatops-proxy (code). Also it means that OnCall doesn't need to register slack_links anymore when slack is connected/disconnected. These changes are behind UNIFIED_SLACK_APP_ENABLED flag and should be no-op if flag is not enabled.
Get rid of Multiregionatily restrictions - instrument all slack interactions with a ProxyMeta - json data telling chatops-proxy where to route the interaction. Note, that it doesn't apply for "Add to resolution notes" message action - it will be handled differently in following PR.
Move all chatops-proxy related stuff from common/oncall-gateway to apps/chatops-proxy

Minor changes:

Remove usage of CHATOPS_V3 flag. Chatops v3 is already released (It's a refactoring from previous quarter)

It's a modal on the alert group message, not mesasge action (in 3-dot menu).

engine/apps/slack/oauth.py

Move slack installation & uninstallation code to separate functions to reuse betwen social-auth and chatops-proxy

…-install-multiple-responses' into handle_chatops_proxy_broadcast

matiasb

Users will need to disconnect/reconnect Slack to switch to the unified app? Or is there any migration/transition plan?
There are a few data representation changes in the slack scenarios (e.g. schedules), could that break interaction with previously posted messages?
Also, it would be nice to add some tests for the chatops_proxy additions? (auth, handlers)

matiasb · 2024-05-22T18:44:22Z

engine/apps/alerts/incident_appearance/renderers/slack_renderer.py

@@ -436,9 +437,9 @@ def _alert_group_action_value(self, **kwargs):

        data = {
            "organization_id": self.alert_group.channel.organization_id,
-            "alert_group_pk": self.alert_group.pk,
            # eventually replace using alert_group_pk with alert_group_public_pk in slack payload


I guess this comment can be removed now?

Yep, guess so

matiasb · 2024-05-22T18:48:34Z

engine/apps/chatops_proxy/client.py

@@ -32,6 +33,15 @@ class Tenant:
    msteams_links: List[MSTeamsLink] = field(default_factory=list)


+@dataclass
+class OauthInstallation:


nit, but maybe this should be OAuthInstallation? (OAuth is shoft for "Open Authorization")

matiasb · 2024-05-22T18:51:07Z

engine/apps/chatops_proxy/client.py

    ) -> tuple[Tenant, requests.models.Response]:
        url = f"{self.api_base_url}/tenants/register"
        d = {
            "tenant": {
                "service_tenant_id": service_tenant_id,
                "cluster_slug": cluster_slug,
                "service_type": service_type,
+                "stack_id": stack_id,


Is this value backfilled/updated somehow for already registered tenants?

It will be. Probably I'll add a new endpoint on ChatopsProxy, allowing to PUT tenants and set their stack ids.
Currently it will be just ignored by ChatopsProxy, since new API is not released yet.

matiasb · 2024-05-22T18:53:43Z

engine/apps/chatops_proxy/events/root_handler.py

+                    f"msg=\"ChatopsEventsHandler: Found matching handler {h.__name__}\" event_type={event_data.get('event_type')}"
+                )
+                h.handle(event_data.get("data", {}))
+                break


Maybe we could log if we got an event but there is no match?

matiasb · 2024-05-22T18:55:42Z

engine/apps/chatops_proxy/tasks.py

+@shared_dedicated_queue_retry_task(
+    autoretry_for=(Exception,),
+    retry_backoff=True,
+    max_retries=100,


Does it make sense to have 100 retries, or 10 should be the same for this purpose?

The idea behind 100 retries is to make sure we didn't miss any OnCall org creation. Initially it was endless, but me and @vadimkerr decided to make it some finite number.

Konstantinov-Innokentii · 2024-05-23T04:15:03Z

Users will need to disconnect/reconnect Slack to switch to the unified app? Or is there any migration/transition plan? There are a few data representation changes in the Slack scenarios (e.g., schedules); could that break interaction with previously posted messages? Also, it would be nice to add some tests for the chatops_proxy additions. (auth, handlers)

To fully switch to unified app users must reconnect the app to submit new permissions. It will be same app, under different name and I'm planning to introduce "soft-reinstall" to not to disconnect anything. Readon behind that is that Incident has some permissions we don't. However, OnCall parts should still work. There is no migration plan yet, I'll start to work on that once work on Unified App itself will be done.
Data representation changes should be fully compatible with old messages. @vadimkerr just pinging to confirm that.
I'll add some tests. Didn't got the point about auth tests? What auth are u referring to?

matiasb

Thanks for the updates! 👍
LGTM

matiasb · 2024-05-23T17:51:56Z

engine/apps/chatops_proxy/tests/test_events.py

+        "payload": {
+            # It's not actual payload we are getting from slack, just a placeholder
+            "slack_id"
+            "some_slack_id"


Is this sample data ok ^?

Thanks for noticing. missed a coma. Anyway, I went ahead and used a more accurate payload example from slack docs instead of placeholder

Konstantinov-Innokentii · 2024-05-24T08:21:47Z

Thanks for the updates! 👍 LGTM

Thanks! Added more tests covering our slack installation.

…rafana/oncall into handle_chatops_proxy_broadcast

Konstantinov-Innokentii added 5 commits April 9, 2024 21:57

Add chatops-proxy routing data to AlertGroup Buttons

6ee54a4

Make "Responders" button work

3a699c4

Make "Attach to" work

9581552

Make "Resolution notes" btn work

8bd9ef3

It's a modal on the alert group message, not mesasge action (in 3-dot menu).

Start work on the slack installation from the chatops-proxy

2dda654

Konstantinov-Innokentii temporarily deployed to github-pages April 16, 2024 04:33 — with GitHub Actions Inactive

github-advanced-security bot found potential problems Apr 16, 2024

View reviewed changes

engine/apps/slack/oauth.py Fixed Show fixed Hide fixed

Support slack installation via chatops-proxy.

b187082

Move slack installation & uninstallation code to separate functions to reuse betwen social-auth and chatops-proxy

Konstantinov-Innokentii temporarily deployed to github-pages April 24, 2024 08:23 — with GitHub Actions Inactive

Fix get_installation_link_from_chatops_proxy util

c97a994

Konstantinov-Innokentii temporarily deployed to github-pages April 24, 2024 09:33 — with GitHub Actions Inactive

Add SELF_HOSTED_STACK_ID env var

97d140a

Konstantinov-Innokentii temporarily deployed to github-pages April 26, 2024 04:28 — with GitHub Actions Inactive

draft event receiver

7103c5c

Konstantinov-Innokentii had a problem deploying to github-pages April 26, 2024 12:51 — with GitHub Actions Failure

Merge branch 'slack_add_tenant_id' into handle_chatops_proxy_broadcast

0131c47

Konstantinov-Innokentii mentioned this pull request Apr 29, 2024

Instrument slack elements with routing data needed for chatops-proxy #4196

Closed

Konstantinov-Innokentii had a problem deploying to github-pages April 29, 2024 06:55 — with GitHub Actions Failure

Reorganize chatops-event handler

4cf952c

Konstantinov-Innokentii had a problem deploying to github-pages April 29, 2024 08:14 — with GitHub Actions Failure

Fix types

5cf0bb7

Konstantinov-Innokentii had a problem deploying to github-pages April 29, 2024 08:35 — with GitHub Actions Failure

Fixes

0215e8c

Konstantinov-Innokentii had a problem deploying to github-pages April 29, 2024 09:57 — with GitHub Actions Failure

Move common.oncall_gateway to apps.chatops-proxy

70f0e53

Konstantinov-Innokentii had a problem deploying to github-pages April 30, 2024 07:02 — with GitHub Actions Failure

Introduce UNIFIED_SLACK_APP_ENABLED env var

62204f2

Konstantinov-Innokentii had a problem deploying to github-pages April 30, 2024 07:37 — with GitHub Actions Failure

Fix registering oncall tenant

ed0f6de

Konstantinov-Innokentii had a problem deploying to github-pages April 30, 2024 09:26 — with GitHub Actions Failure

Merge remote-tracking branch 'refs/remotes/origin/rares/support-slack…

252b268

…-install-multiple-responses' into handle_chatops_proxy_broadcast

Konstantinov-Innokentii requested a review from a team as a code owner May 21, 2024 07:06

Konstantinov-Innokentii had a problem deploying to github-pages May 21, 2024 07:06 — with GitHub Actions Failure

Bring back unlink_slack if UNIFIED SLACK APP isn't enabled

1fa0469

Konstantinov-Innokentii mentioned this pull request May 21, 2024

Support slack install multiple response status codes #4350

Closed

Konstantinov-Innokentii had a problem deploying to github-pages May 21, 2024 07:14 — with GitHub Actions Failure

iskhakov removed the needs triage label May 21, 2024

matiasb reviewed May 22, 2024

View reviewed changes

Konstantinov-Innokentii added 8 commits May 23, 2024 12:19

Rename OauthInstallation to OAuthInstallation

52e8771

Add logging if there is no handler for chatops-event

ae2eeca

Remove outdated comment

8ddd8a1

Add tests for ChatopsEventsView

b2ddd02

Fix test_event_handler

a650357

Add tests for SlackInstallationHandler

e717174

Add tests for install_slack_integration

9d8b129

Add tests for uninstall_slack_integration

a5eb5fd

matiasb approved these changes May 23, 2024

View reviewed changes

Konstantinov-Innokentii and others added 3 commits May 24, 2024 14:19

Simplify test_root_event_handler

3afcb4f

Correct test data

705e117

Merge branch 'dev' into handle_chatops_proxy_broadcast

bede2ad

teodosii temporarily deployed to github-pages May 24, 2024 07:55 — with GitHub Actions Inactive

Add tests for start slack installation via social auth

89a3c0c

Konstantinov-Innokentii had a problem deploying to github-pages May 24, 2024 08:21 — with GitHub Actions Error

teodosii added 2 commits May 24, 2024 11:27

lint

f3a951d

Merge branch 'handle_chatops_proxy_broadcast' of https://github.com/g…

e3dd665

…rafana/oncall into handle_chatops_proxy_broadcast

teodosii temporarily deployed to github-pages May 24, 2024 08:28 — with GitHub Actions Inactive

Merge branch 'dev' into handle_chatops_proxy_broadcast

82f3695

Konstantinov-Innokentii temporarily deployed to github-pages May 24, 2024 10:51 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare OnCall for Unified Slack App #4232

Prepare OnCall for Unified Slack App #4232

Konstantinov-Innokentii commented Apr 16, 2024 •

edited

matiasb left a comment

matiasb May 22, 2024

Konstantinov-Innokentii May 23, 2024

matiasb May 22, 2024

matiasb May 22, 2024

Konstantinov-Innokentii May 23, 2024 •

edited

matiasb May 22, 2024

matiasb May 22, 2024

Konstantinov-Innokentii May 23, 2024

Konstantinov-Innokentii commented May 23, 2024 •

edited

matiasb left a comment

matiasb May 23, 2024

Konstantinov-Innokentii May 24, 2024

Konstantinov-Innokentii commented May 24, 2024

Prepare OnCall for Unified Slack App #4232

Are you sure you want to change the base?

Prepare OnCall for Unified Slack App #4232

Conversation

Konstantinov-Innokentii commented Apr 16, 2024 • edited

matiasb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Konstantinov-Innokentii May 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Konstantinov-Innokentii commented May 23, 2024 • edited

matiasb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Konstantinov-Innokentii commented May 24, 2024

Konstantinov-Innokentii commented Apr 16, 2024 •

edited

Konstantinov-Innokentii May 23, 2024 •

edited

Konstantinov-Innokentii commented May 23, 2024 •

edited