Browse Source

feat(trace-view): Adding issue urls to the response (#24728)

- We need these urls to link back and forth between performance and
  issues
- Also added an organization_slug param to get_absolute_url to avoid a
  n+1 query
William Mak 4 years ago
parent
commit
72c440c2d6

+ 16 - 0
src/sentry/api/endpoints/organization_events_trace.py

@@ -9,6 +9,7 @@ from rest_framework.response import Response
 from sentry import features, eventstore
 from sentry import features, eventstore
 from sentry.api.bases import OrganizationEventsV2EndpointBase, NoProjects
 from sentry.api.bases import OrganizationEventsV2EndpointBase, NoProjects
 from sentry.snuba import discover
 from sentry.snuba import discover
+from sentry.models import Group
 from sentry_relay.consts import SPAN_STATUS_CODE_TO_NAME
 from sentry_relay.consts import SPAN_STATUS_CODE_TO_NAME
 
 
 
 
@@ -21,6 +22,7 @@ ERROR_COLUMNS = [
     "timestamp",
     "timestamp",
     "trace.span",
     "trace.span",
     "transaction",
     "transaction",
+    "issue",
 ]
 ]
 
 
 
 
@@ -61,6 +63,7 @@ class OrganizationEventsTraceEndpointBase(OrganizationEventsV2EndpointBase):
             "span": event["trace.span"],
             "span": event["trace.span"],
             "project_id": event["project.id"],
             "project_id": event["project.id"],
             "project_slug": event["project"],
             "project_slug": event["project"],
+            "url": event["url"],
         }
         }
 
 
     def construct_span_map(self, events, key):
     def construct_span_map(self, events, key):
@@ -161,6 +164,19 @@ class OrganizationEventsTraceEndpointBase(OrganizationEventsV2EndpointBase):
         else:
         else:
             current_transaction = find_event(result["data"], lambda t: t["id"] == event_id)
             current_transaction = find_event(result["data"], lambda t: t["id"] == event_id)
             errors = self.get_errors(organization, trace_id, params, current_transaction, event_id)
             errors = self.get_errors(organization, trace_id, params, current_transaction, event_id)
+            if errors:
+                groups = {
+                    group.id: group
+                    for group in Group.objects.filter(
+                        id__in=[row["issue.id"] for row in errors],
+                        project_id__in=params.get("project_id", []),
+                        project__organization=organization,
+                    )
+                }
+                for row in errors:
+                    row["url"] = groups[row["issue.id"]].get_absolute_url(
+                        organization_slug=organization.slug
+                    )
 
 
         return Response(
         return Response(
             self.serialize(result["data"], errors, root, warning_extra, event_id, detailed)
             self.serialize(result["data"], errors, root, warning_extra, event_id, detailed)

+ 5 - 2
src/sentry/models/group.py

@@ -346,12 +346,15 @@ class Group(Model):
         )
         )
         super().save(*args, **kwargs)
         super().save(*args, **kwargs)
 
 
-    def get_absolute_url(self, params=None, event_id=None):
+    def get_absolute_url(self, params=None, event_id=None, organization_slug=None):
         # Built manually in preference to django.core.urlresolvers.reverse,
         # Built manually in preference to django.core.urlresolvers.reverse,
         # because reverse has a measured performance impact.
         # because reverse has a measured performance impact.
         event_path = f"events/{event_id}/" if event_id else ""
         event_path = f"events/{event_id}/" if event_id else ""
         url = "organizations/{org}/issues/{id}/{event_path}{params}".format(
         url = "organizations/{org}/issues/{id}/{event_path}{params}".format(
-            org=urlquote(self.organization.slug),
+            # Pass organization_slug if this needs to be called multiple times to avoid n+1 queries
+            org=urlquote(
+                self.organization.slug if organization_slug is None else organization_slug
+            ),
             id=self.id,
             id=self.id,
             event_path=event_path,
             event_path=event_path,
             params="?" + urlencode(params) if params else "",
             params="?" + urlencode(params) if params else "",

+ 8 - 0
tests/snuba/api/endpoints/test_organization_events_trace.py

@@ -3,6 +3,7 @@ from datetime import timedelta
 
 
 from django.core.urlresolvers import reverse
 from django.core.urlresolvers import reverse
 
 
+from sentry.models import Group
 from sentry.utils.samples import load_data
 from sentry.utils.samples import load_data
 from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.helpers.datetime import before_now, iso_format
@@ -47,6 +48,9 @@ class OrganizationEventsTraceEndpointBase(APITestCase, SnubaTestCase):
                 data["measurements"][key]["value"] = value
                 data["measurements"][key]["value"] = value
         return self.store_event(data, project_id=project_id)
         return self.store_event(data, project_id=project_id)
 
 
+    def get_error_url(self, error):
+        return Group.objects.get(id=error.group_id).get_absolute_url()
+
     def setUp(self):
     def setUp(self):
         """
         """
         Span structure:
         Span structure:
@@ -451,6 +455,7 @@ class OrganizationEventsTraceLightEndpointTest(OrganizationEventsTraceEndpointBa
             assert event["parent_span_id"] == self.root_span_ids[0]
             assert event["parent_span_id"] == self.root_span_ids[0]
             assert len(event["errors"]) == 1
             assert len(event["errors"]) == 1
             assert event["errors"][0]["event_id"] == error.event_id
             assert event["errors"][0]["event_id"] == error.event_id
+            assert event["errors"][0]["url"] == self.get_error_url(error)
 
 
         with self.feature(self.FEATURES):
         with self.feature(self.FEATURES):
             response = self.client.get(
             response = self.client.get(
@@ -799,12 +804,14 @@ class OrganizationEventsTraceEndpointTest(OrganizationEventsTraceEndpointBase):
             "span": self.gen1_span_ids[0],
             "span": self.gen1_span_ids[0],
             "project_id": self.gen1_project.id,
             "project_id": self.gen1_project.id,
             "project_slug": self.gen1_project.slug,
             "project_slug": self.gen1_project.slug,
+            "url": self.get_error_url(error),
         } in gen1_event["errors"]
         } in gen1_event["errors"]
         assert {
         assert {
             "event_id": error1.event_id,
             "event_id": error1.event_id,
             "span": self.gen1_span_ids[0],
             "span": self.gen1_span_ids[0],
             "project_id": self.gen1_project.id,
             "project_id": self.gen1_project.id,
             "project_slug": self.gen1_project.slug,
             "project_slug": self.gen1_project.slug,
+            "url": self.get_error_url(error1),
         } in gen1_event["errors"]
         } in gen1_event["errors"]
 
 
     def test_with_default(self):
     def test_with_default(self):
@@ -838,4 +845,5 @@ class OrganizationEventsTraceEndpointTest(OrganizationEventsTraceEndpointBase):
             "span": self.root_span_ids[0],
             "span": self.root_span_ids[0],
             "project_id": self.gen1_project.id,
             "project_id": self.gen1_project.id,
             "project_slug": self.gen1_project.slug,
             "project_slug": self.gen1_project.slug,
+            "url": self.get_error_url(default_event),
         } in root_event["errors"]
         } in root_event["errors"]