Browse Source

feat(cmp_alerts): Return current and comparison count from stats api, instead of percent (#29245)

This changes the response from `OrganizationEventsStatsEndpoint` to include the current count and
comparisonCount values when `comparisonDelta` is passed, rather than returning the % change. To
calculate the delta change value on the frontend use `((count / comparisonCount) - 1) * 100`, making
sure to not divide by 0.
Dan Fuller 3 years ago
parent
commit
8450f6939b

+ 4 - 0
src/sentry/api/bases/organization_events.py

@@ -385,11 +385,15 @@ class OrganizationEventsV2EndpointBase(OrganizationEventsEndpointBase):
                     zerofill_results=zerofill_results,
                 )
             else:
+                extra_columns = None
+                if comparison_delta:
+                    extra_columns = ["comparisonCount"]
                 serialized_result = serializer.serialize(
                     result,
                     resolve_axis_column(query_columns[0]),
                     allow_partial_buckets=allow_partial_buckets,
                     zerofill_results=zerofill_results,
+                    extra_columns=extra_columns,
                 )
 
             return serialized_result

+ 10 - 1
src/sentry/api/serializers/snuba.py

@@ -294,7 +294,13 @@ class SnubaTSResultSerializer(BaseSnubaSerializer):
     """
 
     def serialize(
-        self, result, column="count", order=None, allow_partial_buckets=False, zerofill_results=True
+        self,
+        result,
+        column="count",
+        order=None,
+        allow_partial_buckets=False,
+        zerofill_results=True,
+        extra_columns=None,
     ):
         data = [
             (key, list(group))
@@ -309,6 +315,9 @@ class SnubaTSResultSerializer(BaseSnubaSerializer):
             row = []
             for r in v:
                 item = {"count": r.get(column, 0)}
+                if extra_columns is not None:
+                    for extra_column in extra_columns:
+                        item[extra_column] = r.get(extra_column, 0)
                 if self.lookup:
                     value = value_from_row(r, self.lookup.columns)
                     item[self.lookup.name] = (attrs.get(value),)

+ 2 - 5
src/sentry/snuba/discover.py

@@ -546,11 +546,8 @@ def timeseries_query(
         # If we have two sets of results then we're doing a comparison queries. Divide the primary
         # results by the comparison results.
         for result, cmp_result in zip(results[0], results[1]):
-            result_val, cmp_result_val = result.get(col_name, 0), cmp_result.get(col_name, 0)
-            comparison_value = 0
-            if cmp_result_val:
-                comparison_value = ((result_val / cmp_result_val) - 1) * 100
-            result[col_name] = comparison_value
+            cmp_result_val = cmp_result.get(col_name, 0)
+            result["comparisonCount"] = cmp_result_val
 
     results = results[0]
 

+ 9 - 4
tests/sentry/snuba/test_discover.py

@@ -5738,7 +5738,9 @@ class TimeseriesQueryTest(TimeseriesBase):
         )
         assert len(result.data["data"]) == 3
         # Values should all be 0, since there is no comparison period data at all.
-        assert [0, 0, 0] == [val["count"] for val in result.data["data"] if "count" in val]
+        assert [(0, 0), (3, 0), (0, 0)] == [
+            (val.get("count", 0), val.get("comparisonCount", 0)) for val in result.data["data"]
+        ]
 
         self.store_event(
             data={
@@ -5775,7 +5777,9 @@ class TimeseriesQueryTest(TimeseriesBase):
         assert len(result.data["data"]) == 3
         # In the second bucket we have 3 events in the current period and 2 in the comparison, so
         # we get a result of 50% increase
-        assert [0, 50, 0] == [val["count"] for val in result.data["data"] if "count" in val]
+        assert [(0, 0), (3, 2), (0, 0)] == [
+            (val.get("count", 0), val.get("comparisonCount", 0)) for val in result.data["data"]
+        ]
 
         result = discover.timeseries_query(
             selected_columns=["count_unique(user)"],
@@ -5791,8 +5795,9 @@ class TimeseriesQueryTest(TimeseriesBase):
         assert len(result.data["data"]) == 3
         # In the second bucket we have 1 unique user in the current period and 2 in the comparison, so
         # we get a result of -50%
-        assert [0, -50, 0] == [
-            val["count_unique_user"] for val in result.data["data"] if "count_unique_user" in val
+        assert [(0, 0), (1, 2), (0, 0)] == [
+            (val.get("count_unique_user", 0), val.get("comparisonCount", 0))
+            for val in result.data["data"]
         ]
 
     def test_count_miserable(self):

+ 2 - 2
tests/snuba/api/endpoints/test_organization_events_stats.py

@@ -900,8 +900,8 @@ class OrganizationEventsStatsEndpointTest(APITestCase, SnubaTestCase):
         )
 
         assert [attrs for time, attrs in response.data["data"]] == [
-            [{"count": -50}],
-            [{"count": 100}],
+            [{"count": 1, "comparisonCount": 2}],
+            [{"count": 2, "comparisonCount": 1}],
         ]
 
     def test_comparison_invalid(self):