Browse Source

chore(starfish): Remove unused percent_change functions (#53776)

- This removes the unused percent_change functions until we decide to
revisit them
- This is so we don't need to convert them to the metrics_layer today in
order to enable the layer overall
William Mak 1 year ago
parent
commit
b25c431c33

+ 1 - 6
src/sentry/search/events/constants.py

@@ -236,16 +236,11 @@ FUNCTION_ALIASES = {
     "tps": "eps",
 }
 
-METRICS_FUNCTION_ALIASES = {
-    "tps_percent_change": "eps_percent_change",
-    "tpm_percent_change": "epm_percent_change",
-}
+METRICS_FUNCTION_ALIASES: Dict[str, str] = {}
 
 SPAN_FUNCTION_ALIASES = {
     "sps": "eps",
     "spm": "epm",
-    "sps_percent_change": "eps_percent_change",
-    "spm_percent_change": "epm_percent_change",
 }
 
 # Mapping of public aliases back to the metrics identifier

+ 0 - 103
src/sentry/search/events/datasets/metrics.py

@@ -686,37 +686,6 @@ class MetricsDatasetConfig(DatasetConfig):
                     ),
                     default_result_type="duration",
                 ),
-                fields.MetricsFunction(
-                    "percentile_percent_change",
-                    required_args=[
-                        fields.MetricArg(
-                            "column",
-                            allowed_columns=["transaction.duration"],
-                            allow_custom_measurements=False,
-                        ),
-                        fields.NumberRange("percentile", 0, 1),
-                    ],
-                    calculated_args=[resolve_metric_id],
-                    snql_distribution=self._resolve_percentile_percent_change,
-                    default_result_type="percent_change",
-                ),
-                fields.MetricsFunction(
-                    "http_error_count_percent_change",
-                    snql_distribution=self._resolve_http_error_count_percent_change,
-                    default_result_type="percent_change",
-                ),
-                fields.MetricsFunction(
-                    "epm_percent_change",
-                    snql_distribution=self._resolve_epm_percent_change,
-                    optional_args=[fields.IntervalDefault("interval", 1, None)],
-                    default_result_type="percent_change",
-                ),
-                fields.MetricsFunction(
-                    "eps_percent_change",
-                    snql_distribution=self._resolve_eps_percent_change,
-                    optional_args=[fields.IntervalDefault("interval", 1, None)],
-                    default_result_type="percent_change",
-                ),
             ]
         }
 
@@ -1258,75 +1227,3 @@ class MetricsDatasetConfig(DatasetConfig):
             ],
             alias,
         )
-
-    def _resolve_http_error_count_percent_change(
-        self,
-        _: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = self._resolve_http_error_count({}, None, self.builder.first_half_condition())
-        second_half = self._resolve_http_error_count({}, None, self.builder.second_half_condition())
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_percentile_percent_change(
-        self,
-        args: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = function_aliases.resolve_metrics_percentile(
-            args=args,
-            alias=None,
-            fixed_percentile=args["percentile"],
-            extra_conditions=[self.builder.first_half_condition()],
-        )
-        second_half = function_aliases.resolve_metrics_percentile(
-            args=args,
-            alias=None,
-            fixed_percentile=args["percentile"],
-            extra_conditions=[self.builder.second_half_condition()],
-        )
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_epm_percent_change(
-        self,
-        args: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = self._resolve_epm(args, None, self.builder.first_half_condition())
-        second_half = self._resolve_epm(args, None, self.builder.second_half_condition())
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_eps_percent_change(
-        self,
-        args: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = self._resolve_eps(args, None, self.builder.first_half_condition())
-        second_half = self._resolve_eps(args, None, self.builder.second_half_condition())
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_percent_change_function(self, first_half, second_half, alias):
-        return Function(
-            "if",
-            [
-                Function(
-                    "greater",
-                    [
-                        first_half,
-                        0,
-                    ],
-                ),
-                Function(
-                    "divide",
-                    [
-                        Function(
-                            "minus",
-                            [second_half, first_half],
-                        ),
-                        first_half,
-                    ],
-                ),
-                None,
-            ],
-            alias,
-        )

+ 0 - 87
src/sentry/search/events/datasets/spans_metrics.py

@@ -321,37 +321,6 @@ class SpansMetricsDatasetConfig(DatasetConfig):
                     ),
                     default_result_type="duration",
                 ),
-                fields.MetricsFunction(
-                    "percentile_percent_change",
-                    required_args=[
-                        fields.MetricArg(
-                            "column",
-                            allowed_columns=constants.SPAN_METRIC_DURATION_COLUMNS,
-                            allow_custom_measurements=False,
-                        ),
-                        fields.NumberRange("percentile", 0, 1),
-                    ],
-                    calculated_args=[resolve_metric_id],
-                    snql_percentile=self._resolve_percentile_percent_change,
-                    default_result_type="percent_change",
-                ),
-                fields.MetricsFunction(
-                    "http_error_count_percent_change",
-                    snql_distribution=self._resolve_http_error_count_percent_change,
-                    default_result_type="percent_change",
-                ),
-                fields.MetricsFunction(
-                    "epm_percent_change",
-                    snql_distribution=self._resolve_epm_percent_change,
-                    optional_args=[fields.IntervalDefault("interval", 1, None)],
-                    default_result_type="percent_change",
-                ),
-                fields.MetricsFunction(
-                    "eps_percent_change",
-                    snql_distribution=self._resolve_eps_percent_change,
-                    optional_args=[fields.IntervalDefault("interval", 1, None)],
-                    default_result_type="percent_change",
-                ),
             ]
         }
 
@@ -526,62 +495,6 @@ class SpansMetricsDatasetConfig(DatasetConfig):
             alias,
         )
 
-    def _resolve_http_error_count_percent_change(
-        self,
-        _: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = self._resolve_http_error_count({}, None, self.builder.first_half_condition())
-        second_half = self._resolve_http_error_count({}, None, self.builder.second_half_condition())
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_percentile_percent_change(
-        self,
-        args: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = function_aliases.resolve_metrics_percentile(
-            args=args,
-            alias=None,
-            fixed_percentile=args["percentile"],
-            extra_conditions=[self.builder.first_half_condition()],
-        )
-        second_half = function_aliases.resolve_metrics_percentile(
-            args=args,
-            alias=None,
-            fixed_percentile=args["percentile"],
-            extra_conditions=[self.builder.second_half_condition()],
-        )
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_epm_percent_change(
-        self,
-        args: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = self._resolve_epm(args, None, self.builder.first_half_condition())
-        second_half = self._resolve_epm(args, None, self.builder.second_half_condition())
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_eps_percent_change(
-        self,
-        args: Mapping[str, Union[str, Column, SelectType, int, float]],
-        alias: Optional[str] = None,
-    ) -> SelectType:
-        first_half = self._resolve_eps(args, None, self.builder.first_half_condition())
-        second_half = self._resolve_eps(args, None, self.builder.second_half_condition())
-        return self._resolve_percent_change_function(first_half, second_half, alias)
-
-    def _resolve_percent_change_function(self, first_half, second_half, alias):
-        return self.builder.resolve_division(
-            Function(
-                "minus",
-                [second_half, first_half],
-            ),
-            first_half,
-            alias,
-        )
-
     @property
     def orderby_converter(self) -> Mapping[str, OrderBy]:
         return {}

+ 0 - 141
tests/snuba/api/endpoints/test_organization_events_span_metrics.py

@@ -365,131 +365,6 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
         assert meta["fields"]["http_error_count()"] == "integer"
         assert meta["fields"]["http_error_rate()"] == "percentage"
 
-    def test_percentile_percent_change(self):
-        self.store_span_metric(
-            5,
-            internal_metric=constants.SELF_TIME_LIGHT,
-            timestamp=self.six_min_ago,
-        )
-        self.store_span_metric(
-            10,
-            internal_metric=constants.SELF_TIME_LIGHT,
-            timestamp=self.min_ago,
-        )
-        response = self.do_request(
-            {
-                "field": ["percentile_percent_change(span.self_time, 0.95)"],
-                "query": "",
-                "orderby": ["-percentile_percent_change(span.self_time, 0.95)"],
-                "project": self.project.id,
-                "dataset": "spansMetrics",
-                "statsPeriod": "10m",
-            }
-        )
-        assert response.status_code == 200, response.content
-        data = response.data["data"]
-        meta = response.data["meta"]
-        assert len(data) == 1
-        assert data[0]["percentile_percent_change(span.self_time, 0.95)"] == 1
-        assert meta["dataset"] == "spansMetrics"
-        assert meta["fields"]["percentile_percent_change(span.self_time, 0.95)"] == "percent_change"
-
-    def test_http_error_count_percent_change(self):
-        for _ in range(4):
-            self.store_span_metric(
-                1,
-                internal_metric=constants.SELF_TIME_LIGHT,
-                tags={"span.status_code": "500"},
-                timestamp=self.six_min_ago,
-            )
-        self.store_span_metric(
-            1,
-            internal_metric=constants.SELF_TIME_LIGHT,
-            tags={"span.status_code": "500"},
-            timestamp=self.min_ago,
-        )
-        response = self.do_request(
-            {
-                "field": ["http_error_count_percent_change()"],
-                "query": "",
-                "orderby": ["-http_error_count_percent_change()"],
-                "project": self.project.id,
-                "dataset": "spansMetrics",
-                "statsPeriod": "10m",
-            }
-        )
-        assert response.status_code == 200, response.content
-        data = response.data["data"]
-        meta = response.data["meta"]
-        assert len(data) == 1
-        assert data[0]["http_error_count_percent_change()"] == -0.75
-        assert meta["dataset"] == "spansMetrics"
-        assert meta["fields"]["http_error_count_percent_change()"] == "percent_change"
-
-    def test_epm_percent_change(self):
-        for _ in range(4):
-            self.store_span_metric(
-                1,
-                internal_metric=constants.SELF_TIME_LIGHT,
-                timestamp=self.six_min_ago,
-            )
-        self.store_span_metric(
-            1,
-            internal_metric=constants.SELF_TIME_LIGHT,
-            timestamp=self.min_ago,
-        )
-        response = self.do_request(
-            {
-                "field": ["epm_percent_change()", "spm_percent_change()"],
-                "query": "",
-                "orderby": ["-epm_percent_change()"],
-                "project": self.project.id,
-                "dataset": "spansMetrics",
-                "statsPeriod": "10m",
-            }
-        )
-        assert response.status_code == 200, response.content
-        data = response.data["data"]
-        meta = response.data["meta"]
-        assert len(data) == 1
-        assert data[0]["epm_percent_change()"] == pytest.approx(-0.75)
-        assert data[0]["spm_percent_change()"] == pytest.approx(-0.75)
-        assert meta["dataset"] == "spansMetrics"
-        assert meta["fields"]["epm_percent_change()"] == "percent_change"
-        assert meta["fields"]["spm_percent_change()"] == "percent_change"
-
-    def test_eps_percent_change(self):
-        for _ in range(4):
-            self.store_span_metric(
-                1,
-                internal_metric=constants.SELF_TIME_LIGHT,
-                timestamp=self.min_ago,
-            )
-        self.store_span_metric(
-            1,
-            internal_metric=constants.SELF_TIME_LIGHT,
-            timestamp=self.six_min_ago,
-        )
-        response = self.do_request(
-            {
-                "field": ["eps_percent_change()", "sps_percent_change()"],
-                "query": "",
-                "orderby": ["-eps_percent_change()"],
-                "project": self.project.id,
-                "dataset": "spansMetrics",
-                "statsPeriod": "10m",
-            }
-        )
-        assert response.status_code == 200, response.content
-        data = response.data["data"]
-        meta = response.data["meta"]
-        assert len(data) == 1
-        assert data[0]["eps_percent_change()"] == pytest.approx(3)
-        assert data[0]["sps_percent_change()"] == pytest.approx(3)
-        assert meta["dataset"] == "spansMetrics"
-        assert meta["fields"]["eps_percent_change()"] == "percent_change"
-        assert meta["fields"]["sps_percent_change()"] == "percent_change"
-
     def test_use_self_time_light(self):
         self.store_span_metric(
             100,
@@ -622,22 +497,6 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithMetricLayer(
     def test_http_error_rate_and_count(self):
         super().test_http_error_rate_and_count()
 
-    @pytest.mark.xfail(reason="Not implemented")
-    def test_percentile_percent_change(self):
-        super().test_percentile_percent_change()
-
-    @pytest.mark.xfail(reason="Not implemented")
-    def test_http_error_count_percent_change(self):
-        super().test_http_error_count_percent_change()
-
-    @pytest.mark.xfail(reason="Not implemented")
-    def test_epm_percent_change(self):
-        super().test_epm_percent_change()
-
-    @pytest.mark.xfail(reason="Not implemented")
-    def test_eps_percent_change(self):
-        super().test_eps_percent_change()
-
     @pytest.mark.xfail(reason="Cannot group by transform")
     def test_span_module(self):
         super().test_span_module()