Browse Source

Fix(group-details): multiple tag value calls (#18383)

- Currently we're calling snuba once per first/last release. This can be
  done in a single snuba query which should give us a significant time
  improvement
William Mak 4 years ago
parent
commit
bdfa71543b

+ 20 - 3
src/sentry/api/endpoints/group_details.py

@@ -160,9 +160,22 @@ class GroupDetailsEndpoint(GroupEndpoint, EnvironmentMixin):
                 version=version,
                 version=version,
             )
             )
         except Release.DoesNotExist:
         except Release.DoesNotExist:
-            return {"version": version}
+            release = {"version": version}
         return serialize(release, request.user)
         return serialize(release, request.user)
 
 
+    def _get_first_last_release_info(self, request, group, versions):
+        releases = {
+            release.version: release
+            for release in Release.objects.filter(
+                projects=group.project,
+                organization_id=group.project.organization_id,
+                version__in=versions,
+            )
+        }
+        return serialize(
+            [releases.get(version, {"version": version}) for version in versions], request.user,
+        )
+
     @attach_scenarios([retrieve_aggregate_scenario])
     @attach_scenarios([retrieve_aggregate_scenario])
     def get(self, request, group):
     def get(self, request, group):
         """
         """
@@ -199,9 +212,13 @@ class GroupDetailsEndpoint(GroupEndpoint, EnvironmentMixin):
 
 
         action_list = self._get_actions(request, group)
         action_list = self._get_actions(request, group)
 
 
-        if first_release:
+        if first_release is not None and last_release is not None:
+            first_release, last_release = self._get_first_last_release_info(
+                request, group, [first_release, last_release]
+            )
+        elif first_release is not None:
             first_release = self._get_release_info(request, group, first_release)
             first_release = self._get_release_info(request, group, first_release)
-        if last_release:
+        elif last_release is not None:
             last_release = self._get_release_info(request, group, last_release)
             last_release = self._get_release_info(request, group, last_release)
 
 
         get_range = functools.partial(tsdb.get_range, environment_ids=environment_ids)
         get_range = functools.partial(tsdb.get_range, environment_ids=environment_ids)

+ 2 - 2
src/sentry/api/serializers/models/release.py

@@ -186,10 +186,10 @@ class ReleaseSerializer(Serializer):
 
 
         first_seen = {}
         first_seen = {}
         last_seen = {}
         last_seen = {}
-        tvs = tagstore.get_release_tags(
+        tag_values = tagstore.get_release_tags(
             project_ids, environment_id=None, versions=[o.version for o in item_list]
             project_ids, environment_id=None, versions=[o.version for o in item_list]
         )
         )
-        for tv in tvs:
+        for tv in tag_values:
             first_val = first_seen.get(tv.value)
             first_val = first_seen.get(tv.value)
             last_val = last_seen.get(tv.value)
             last_val = last_seen.get(tv.value)
             first_seen[tv.value] = min(tv.first_seen, first_val) if first_val else tv.first_seen
             first_seen[tv.value] = min(tv.first_seen, first_val) if first_val else tv.first_seen

+ 2 - 2
tests/sentry/api/endpoints/test_group_details.py

@@ -25,11 +25,11 @@ from sentry.models import (
     Release,
     Release,
     Integration,
     Integration,
 )
 )
-from sentry.testutils import APITestCase
+from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.plugins.base import plugins
 from sentry.plugins.base import plugins
 
 
 
 
-class GroupDetailsTest(APITestCase):
+class GroupDetailsTest(APITestCase, SnubaTestCase):
     def test_with_numerical_id(self):
     def test_with_numerical_id(self):
         self.login_as(user=self.user)
         self.login_as(user=self.user)
 
 

+ 61 - 0
tests/snuba/api/endpoints/test_group_details.py

@@ -1,9 +1,11 @@
 from __future__ import absolute_import, print_function
 from __future__ import absolute_import, print_function
 
 
+import six
 from sentry.utils.compat import mock
 from sentry.utils.compat import mock
 
 
 from sentry.models import Environment
 from sentry.models import Environment
 from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.testutils import APITestCase, SnubaTestCase
+from sentry.testutils.helpers.datetime import before_now, iso_format
 
 
 
 
 class GroupDetailsTest(APITestCase, SnubaTestCase):
 class GroupDetailsTest(APITestCase, SnubaTestCase):
@@ -31,3 +33,62 @@ class GroupDetailsTest(APITestCase, SnubaTestCase):
 
 
         response = self.client.get("%s?environment=invalid" % (url,), format="json")
         response = self.client.get("%s?environment=invalid" % (url,), format="json")
         assert response.status_code == 404
         assert response.status_code == 404
+
+    def test_with_first_last_release(self):
+        self.login_as(user=self.user)
+        first_release = {
+            "first_seen": before_now(minutes=3),
+            "last_seen": before_now(minutes=2, seconds=30),
+        }
+        last_release = {
+            "first_seen": before_now(minutes=1),
+            "last_seen": before_now(minutes=1, seconds=30),
+        }
+
+        for timestamp in first_release.values():
+            self.store_event(
+                data={"release": "1.0", "timestamp": iso_format(timestamp)},
+                project_id=self.project.id,
+            )
+        self.store_event(
+            data={"release": "1.1", "timestamp": iso_format(before_now(minutes=2))},
+            project_id=self.project.id,
+        )
+        for timestamp in last_release.values():
+            event = self.store_event(
+                data={"release": "1.0a", "timestamp": iso_format(timestamp)},
+                project_id=self.project.id,
+            )
+
+        group = event.group
+
+        url = u"/api/0/issues/{}/".format(group.id)
+        response = self.client.get(url, format="json")
+
+        assert response.status_code == 200, response.content
+        assert response.data["id"] == six.text_type(group.id)
+        assert response.data["firstRelease"]["version"] == "1.0"
+        assert response.data["lastRelease"]["version"] == "1.0a"
+
+    def test_first_last_only_one_tagstore(self):
+        self.login_as(user=self.user)
+
+        event = self.store_event(
+            data={"release": "1.0", "timestamp": iso_format(before_now(days=3))},
+            project_id=self.project.id,
+        )
+        self.store_event(
+            data={"release": "1.1", "timestamp": iso_format(before_now(minutes=3))},
+            project_id=self.project.id,
+        )
+
+        group = event.group
+
+        url = u"/api/0/issues/{}/".format(group.id)
+
+        with mock.patch(
+            "sentry.api.endpoints.group_details.tagstore.get_release_tags"
+        ) as get_release_tags:
+            response = self.client.get(url, format="json")
+            assert response.status_code == 200
+            assert get_release_tags.call_count == 1