🔧 chore(slos): Update Halt to Success for Bot Commands (#81271)

user errors such as: trying to link identity when identity is already
linked, unlinking when there was no identity to begin with was currently
recorded as a halt, leading to "bad events" in our SLO. Updating these
states to success.

Author: iamrajjoshi

Diff for: src/sentry/integrations/discord/webhooks/command.py

@@ -80,7 +80,7 @@ def help_handler(self, input: CommandInput) -> IntegrationResponse[str]:
     def link_user_handler(self, _: CommandInput) -> IntegrationResponse[str]:
         if self.request.has_identity():
             return IntegrationResponse(
-                interaction_result=EventLifecycleOutcome.HALTED,
+                interaction_result=EventLifecycleOutcome.SUCCESS,
                 response=ALREADY_LINKED_MESSAGE.format(email=self.request.get_identity_str()),
                 outcome_reason=str(MessageCommandHaltReason.ALREADY_LINKED),
                 context_data={
@@ -120,7 +120,7 @@ def link_user_handler(self, _: CommandInput) -> IntegrationResponse[str]:
     def unlink_user_handler(self, input: CommandInput) -> IntegrationResponse[str]:
         if not self.request.has_identity():
             return IntegrationResponse(
-                interaction_result=EventLifecycleOutcome.HALTED,
+                interaction_result=EventLifecycleOutcome.SUCCESS,
                 response=NOT_LINKED_MESSAGE,
                 outcome_reason=str(MessageCommandHaltReason.NOT_LINKED),
             )

Diff for: src/sentry/integrations/msteams/webhook.py

@@ -669,7 +669,7 @@ def link_user_handler(self, input: CommandInput) -> IntegrationResponse[Adaptive
 
         if has_linked_identity:
             return IntegrationResponse(
-                interaction_result=EventLifecycleOutcome.HALTED,
+                interaction_result=EventLifecycleOutcome.SUCCESS,
                 response=build_already_linked_identity_command_card(),
                 outcome_reason=str(MessageCommandHaltReason.ALREADY_LINKED),
                 context_data={

Diff for: src/sentry/integrations/slack/webhooks/base.py

@@ -166,7 +166,7 @@ def link_user_handler(self, input: CommandInput) -> IntegrationResponse[Response
         response = self.endpoint.link_user(self.request)
         if ALREADY_LINKED_MESSAGE.format(username=self.request.identity_str) in str(response.data):
             return IntegrationResponse(
-                interaction_result=EventLifecycleOutcome.HALTED,
+                interaction_result=EventLifecycleOutcome.SUCCESS,
                 response=response,
                 outcome_reason=str(MessageCommandHaltReason.ALREADY_LINKED),
                 context_data={
@@ -182,7 +182,7 @@ def unlink_user_handler(self, input: CommandInput) -> IntegrationResponse[Respon
         response = self.endpoint.unlink_user(self.request)
         if NOT_LINKED_MESSAGE in str(response.data):
             return IntegrationResponse(
-                interaction_result=EventLifecycleOutcome.HALTED,
+                interaction_result=EventLifecycleOutcome.SUCCESS,
                 response=response,
                 outcome_reason=str(MessageCommandHaltReason.NOT_LINKED),
                 context_data={
@@ -200,7 +200,7 @@ def link_team_handler(self, input: CommandInput) -> IntegrationResponse[Response
         for message, reason in self.TEAM_HALT_MAPPINGS.items():
             if message in str(response.data):
                 return IntegrationResponse(
-                    interaction_result=EventLifecycleOutcome.HALTED,
+                    interaction_result=EventLifecycleOutcome.SUCCESS,
                     response=response,
                     outcome_reason=str(reason),
                 )
@@ -215,7 +215,7 @@ def unlink_team_handler(self, input: CommandInput) -> IntegrationResponse[Respon
         for message, reason in self.TEAM_HALT_MAPPINGS.items():
             if message in str(response.data):
                 return IntegrationResponse(
-                    interaction_result=EventLifecycleOutcome.HALTED,
+                    interaction_result=EventLifecycleOutcome.SUCCESS,
                     response=response,
                     outcome_reason=str(reason),
                 )

Diff for: tests/sentry/integrations/discord/webhooks/test_command.py

@@ -4,16 +4,10 @@
 from sentry.integrations.discord.requests.base import DiscordRequestTypes
 from sentry.integrations.discord.webhooks.command import HELP_MESSAGE, NOT_LINKED_MESSAGE
 from sentry.integrations.discord.webhooks.types import DiscordResponseTypes
-from sentry.integrations.messaging.metrics import (
-    MessageCommandFailureReason,
-    MessageCommandHaltReason,
-)
+from sentry.integrations.messaging.metrics import MessageCommandFailureReason
 from sentry.integrations.types import EventLifecycleOutcome
 from sentry.testutils.cases import APITestCase
-from tests.sentry.integrations.utils.test_assert_metrics import (
-    assert_failure_metric,
-    assert_halt_metric,
-)
+from tests.sentry.integrations.utils.test_assert_metrics import assert_failure_metric
 
 WEBHOOK_URL = "/extensions/discord/interactions/"
 
@@ -211,10 +205,9 @@ def test_link_already_linked(self, mock_record):
             assert data["data"]["flags"] == EPHEMERAL_FLAG
             assert response.status_code == 200
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.ALREADY_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     def test_unlink_no_identity(self, mock_record):
@@ -239,10 +232,9 @@ def test_unlink_no_identity(self, mock_record):
             assert data["data"]["flags"] == EPHEMERAL_FLAG
             assert response.status_code == 200
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.NOT_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     def test_unlink(self, mock_record):

Diff for: tests/sentry/integrations/msteams/test_webhook.py

@@ -8,7 +8,6 @@
 from django.test import override_settings
 from django.urls import reverse
 
-from sentry.integrations.messaging.metrics import MessageCommandHaltReason
 from sentry.integrations.models.integration import Integration
 from sentry.integrations.msteams.utils import ACTION_TYPE
 from sentry.integrations.types import EventLifecycleOutcome
@@ -17,7 +16,6 @@
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.users.models.identity import Identity
 from sentry.utils import jwt
-from tests.sentry.integrations.utils.test_assert_metrics import assert_halt_metric
 
 from .test_helpers import (
     DECODED_TOKEN,
@@ -546,10 +544,9 @@ def test_link_command_already_linked(self, mock_time, mock_decode, mock_record):
         )
         assert "Bearer my_token" in responses.calls[3].request.headers["Authorization"]
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.ALREADY_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @responses.activate
     @mock.patch("sentry.utils.jwt.decode")

Diff for: tests/sentry/integrations/slack/webhooks/commands/test_link_team.py

@@ -4,7 +4,6 @@
 import responses
 from rest_framework import status
 
-from sentry.integrations.messaging.metrics import MessageCommandHaltReason
 from sentry.integrations.slack.webhooks.command import (
     CHANNEL_ALREADY_LINKED_MESSAGE,
     INSUFFICIENT_ROLE_MESSAGE,
@@ -18,7 +17,6 @@
 from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.silo import assume_test_silo_mode
 from tests.sentry.integrations.slack.webhooks.commands import SlackCommandsTest
-from tests.sentry.integrations.utils.test_assert_metrics import assert_halt_metric
 
 OTHER_SLACK_ID = "UXXXXXXX2"
 
@@ -67,10 +65,9 @@ def test_link_another_team_to_channel(self, mock_record):
         assert CHANNEL_ALREADY_LINKED_MESSAGE in get_response_text(data)
 
         assert len(mock_record.mock_calls) == 2
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.CHANNEL_ALREADY_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @with_feature("organizations:slack-multiple-team-single-channel-linking")
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -117,10 +114,9 @@ def test_link_team_from_dm(self, mock_record):
         data = orjson.loads(response.content)
         assert LINK_FROM_CHANNEL_MESSAGE in get_response_text(data)
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.LINK_FROM_CHANNEL.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @responses.activate
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -134,10 +130,9 @@ def test_link_team_identity_does_not_exist(self, mock_record):
         data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID)
         assert LINK_USER_FIRST_MESSAGE in get_response_text(data)
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.LINK_USER_FIRST.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @responses.activate
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -156,10 +151,9 @@ def test_link_team_insufficient_role(self, mock_record):
         data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID)
         assert INSUFFICIENT_ROLE_MESSAGE in get_response_text(data)
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.INSUFFICIENT_ROLE.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @responses.activate
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -231,10 +225,9 @@ def test_unlink_no_team(self, mock_record):
         )
         assert TEAM_NOT_LINKED_MESSAGE in get_response_text(data)
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.TEAM_NOT_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @responses.activate
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")

Diff for: tests/sentry/integrations/slack/webhooks/commands/test_link_user.py

@@ -1,6 +1,5 @@
 from unittest.mock import patch
 
-from sentry.integrations.messaging.metrics import MessageCommandHaltReason
 from sentry.integrations.models.organization_integration import OrganizationIntegration
 from sentry.integrations.slack.views.link_identity import SUCCESS_LINKED_MESSAGE, build_linking_url
 from sentry.integrations.slack.views.unlink_identity import (
@@ -13,7 +12,6 @@
 from sentry.testutils.silo import control_silo_test
 from sentry.users.models.identity import Identity
 from tests.sentry.integrations.slack.webhooks.commands import SlackCommandsTest
-from tests.sentry.integrations.utils.test_assert_metrics import assert_halt_metric
 
 
 @control_silo_test
@@ -51,10 +49,9 @@ def test_link_command_already_linked(self, mock_record):
         data = self.send_slack_message("link")
         assert "You are already linked as" in get_response_text(data)
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.ALREADY_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS
 
 
 @control_silo_test
@@ -134,7 +131,6 @@ def test_unlink_command_already_unlinked(self, mock_record):
         data = self.send_slack_message("unlink")
         assert NOT_LINKED_MESSAGE in get_response_text(data)
 
-        start, halt = mock_record.mock_calls
+        start, success = mock_record.mock_calls
         assert start.args[0] == EventLifecycleOutcome.STARTED
-        assert halt.args[0] == EventLifecycleOutcome.HALTED
-        assert_halt_metric(mock_record, MessageCommandHaltReason.NOT_LINKED.value)
+        assert success.args[0] == EventLifecycleOutcome.SUCCESS