Skip to content

Commit

Permalink
fix(ecosystem): Track metrics for issue detail ticket creation (#82436)
Browse files Browse the repository at this point in the history
  • Loading branch information
GabeVillalobos authored Dec 20, 2024
1 parent fe35f16 commit 8d5d0b5
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 19 deletions.
26 changes: 20 additions & 6 deletions src/sentry/api/endpoints/group_integration_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,26 @@ def post(self, request: Request, group, integration_id) -> Response:
)

installation = integration.get_installation(organization_id=organization_id)
try:
data = installation.create_issue(request.data)
except IntegrationFormError as exc:
return Response(exc.field_errors, status=400)
except IntegrationError as e:
return Response({"non_field_errors": [str(e)]}, status=400)

with ProjectManagementEvent(
action_type=ProjectManagementActionType.CREATE_EXTERNAL_ISSUE_VIA_ISSUE_DETAIL,
integration=integration,
).capture() as lifecycle:
lifecycle.add_extras(
{
"provider": integration.provider,
"integration_id": integration.id,
}
)

try:
data = installation.create_issue(request.data)
except IntegrationFormError as exc:
lifecycle.record_halt(exc)
return Response(exc.field_errors, status=400)
except IntegrationError as e:
lifecycle.record_failure(e)
return Response({"non_field_errors": [str(e)]}, status=400)

external_issue_key = installation.make_external_key(data)
external_issue, created = ExternalIssue.objects.get_or_create(
Expand Down
4 changes: 1 addition & 3 deletions src/sentry/integrations/project_management/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ class ProjectManagementActionType(StrEnum):
OUTBOUND_STATUS_SYNC = "outbound_status_sync"
INBOUND_STATUS_SYNC = "inbound_status_sync"
LINK_EXTERNAL_ISSUE = "link_external_issue"

def __str__(self):
return self.value.lower()
CREATE_EXTERNAL_ISSUE_VIA_ISSUE_DETAIL = "create_external_issue_via_issue_detail"


class ProjectManagementHaltReason(StrEnum):
Expand Down
67 changes: 57 additions & 10 deletions tests/sentry/api/endpoints/test_group_integration_details.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from typing import Any
from unittest import mock

from django.db.utils import IntegrityError
Expand Down Expand Up @@ -45,6 +46,18 @@ def setUp(self):
)
self.group = self.event.group

def assert_metric_recorded(
self, mock_metric_method, expected_exc: type[Exception], exc_args: Any | None = None
):

assert mock_metric_method.call_count == 1
mock_metric_method.assert_called_with(mock.ANY)
call_arg = mock_metric_method.call_args_list[0][0][0]
assert isinstance(call_arg, expected_exc)

if exc_args:
assert call_arg.args == (exc_args,)

def test_simple_get_link(self):
self.login_as(user=self.user)
org = self.organization
Expand Down Expand Up @@ -317,21 +330,18 @@ def test_put_group_after_link_raises_exception(
response = self.client.put(path, data={"externalIssue": "APP-123"})
assert response.status_code == 400

mock_record_halt.assert_called_once_with(mock.ANY)

call_arg = mock_record_halt.call_args_list[0][0][0]
assert isinstance(call_arg, IntegrationFormError)
assert call_arg.field_errors == {"foo": "Invalid foo provided"}
self.assert_metric_recorded(
mock_record_halt, IntegrationFormError, str({"foo": "Invalid foo provided"})
)

# Test with IntegrationError
mock_after_link_issue.side_effect = raise_integration_error
response = self.client.put(path, data={"externalIssue": "APP-123"})
assert response.status_code == 400

mock_record_failure.assert_called_once_with(mock.ANY)
call_arg = mock_record_failure.call_args_list[0][0][0]
assert isinstance(call_arg, IntegrationError)
assert call_arg.args == ("The whole operation was invalid",)
self.assert_metric_recorded(
mock_record_failure, IntegrationError, "The whole operation was invalid"
)

def test_put_feature_disabled(self):
self.login_as(user=self.user)
Expand All @@ -353,7 +363,8 @@ def test_put_feature_disabled(self):
assert response.status_code == 400
assert response.data["detail"] == "Your organization does not have access to this feature."

def test_simple_post(self):
@mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_simple_post(self, mock_record_event):
self.login_as(user=self.user)
org = self.organization
group = self.create_group()
Expand Down Expand Up @@ -403,6 +414,42 @@ def test_simple_post(self):
"new": True,
}

mock_record_event.assert_called_with(EventLifecycleOutcome.SUCCESS, None)

@mock.patch.object(ExampleIntegration, "create_issue")
@mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_halt")
@mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_failure")
def test_post_raises_issue_creation_exception(
self, mock_record_failure, mock_record_halt, mock_create_issue
):
self.login_as(user=self.user)
org = self.organization
group = self.create_group()
integration = self.create_integration(
organization=org, provider="example", name="Example", external_id="example:1"
)

path = f"/api/0/issues/{group.id}/integrations/{integration.id}/"
with self.feature("organizations:integrations-issue-basic"):
mock_create_issue.side_effect = raise_integration_error
response = self.client.post(path, data={})
assert response.status_code == 400

assert mock_record_failure.call_count == 1

self.assert_metric_recorded(
mock_record_failure, IntegrationError, "The whole operation was invalid"
)

mock_create_issue.side_effect = raise_integration_form_error

response = self.client.post(path, data={})
assert response.status_code == 400

self.assert_metric_recorded(
mock_record_halt, IntegrationFormError, str({"foo": "Invalid foo provided"})
)

def test_post_feature_disabled(self):
self.login_as(user=self.user)
org = self.organization
Expand Down

0 comments on commit 8d5d0b5

Please sign in to comment.