Browse Source

fix(discover): Division by zero in equations should not error (#30507)

When graphing an equation that contains a division, the denominator can be 0 in
some places. This causes a json encoding error with the infs. Convert them to
`NULL` and filter them out.
Tony Xiao 3 years ago
parent
commit
b06f207c37

+ 3 - 2
src/sentry/search/events/fields.py

@@ -2923,8 +2923,9 @@ class QueryFields(QueryBase):
         """Convert this tree of Operations to the equivalent snql functions"""
         lhs = self._resolve_equation_operand(equation.lhs)
         rhs = self._resolve_equation_operand(equation.rhs)
-        result = Function(equation.operator, [lhs, rhs], alias)
-        return result
+        if equation.operator == "divide":
+            rhs = Function("nullIf", [rhs, 0])
+        return Function(equation.operator, [lhs, rhs], alias)
 
     def _resolve_equation_operand(self, operand: OperandType) -> Union[SelectType, float]:
         if isinstance(operand, Operation):

+ 3 - 1
tests/snuba/api/endpoints/test_organization_events_spans_performance.py

@@ -727,7 +727,9 @@ class OrganizationEventsSpansPerformanceEndpointBase(APITestCase, SnubaTestCase)
                     "divide",
                     [
                         Function("count", [], "count"),
-                        Function("uniq", [Column("event_id")], "count_unique_id"),
+                        Function(
+                            "nullIf", [Function("uniq", [Column("event_id")], "count_unique_id"), 0]
+                        ),
                     ],
                     "equation[0]",
                 ),

+ 19 - 0
tests/snuba/api/endpoints/test_organization_events_stats.py

@@ -878,6 +878,25 @@ class OrganizationEventsStatsEndpointTestWithSnql(OrganizationEventsStatsEndpoin
         # Should've reset to the default for 24h
         assert mock_query.mock_calls[1].args[0][0].granularity.granularity == 300
 
+    def test_equations_divide_by_zero(self):
+        response = self.do_request(
+            data={
+                "start": iso_format(self.day_ago),
+                "end": iso_format(self.day_ago + timedelta(hours=2)),
+                "interval": "1h",
+                # force a 0 in the denominator by doing 1 - 1
+                # since a 0 literal is illegal as the denominator
+                "yAxis": ["equation|count() / (1-1)"],
+            },
+        )
+
+        assert response.status_code == 200, response.content
+        assert len(response.data["data"]) == 2
+        assert [attrs for time, attrs in response.data["data"]] == [
+            [{"count": None}],
+            [{"count": None}],
+        ]
+
 
 class OrganizationEventsStatsTopNEvents(APITestCase, SnubaTestCase):
     def setUp(self):