Browse Source

feat(metrics): Add `transform_null_to_unparameterized` filter capability [TET-503] (#40702)

Riccardo Busetti 2 years ago
parent
commit
f8777ac534

+ 2 - 2
src/sentry/snuba/metrics/fields/base.py

@@ -1563,10 +1563,10 @@ DERIVED_OPS: Mapping[MetricOperationType, DerivedOp] = {
         ),
         DerivedOp(
             op="transform_null_to_unparameterized",
-            can_orderby=False,  # TODO: a better and more comprehensive
+            can_orderby=False,
             can_groupby=True,
+            can_filter=True,
             snql_func=transform_null_to_unparameterized_snql,
-            default_null_value=0,
         ),
     ]
 }

+ 4 - 1
src/sentry/snuba/metrics/query_builder.py

@@ -73,6 +73,7 @@ from sentry.snuba.metrics.utils import (
     DerivedMetricParseException,
     MetricDoesNotExistException,
     get_intervals,
+    require_rhs_condition_resolution,
 )
 from sentry.snuba.sessions_v2 import finite_or_none
 from sentry.utils.dates import parse_stats_period, to_datetime
@@ -538,7 +539,9 @@ class SnubaQueryBuilder:
                                 alias=condition.lhs.alias,
                             )[0],
                             op=condition.op,
-                            rhs=condition.rhs,
+                            rhs=resolve_tag_value(self._use_case_id, self._org_id, condition.rhs)
+                            if require_rhs_condition_resolution(condition.lhs.op)
+                            else condition.rhs,
                         )
                     )
                 except IndexError:

+ 14 - 0
src/sentry/snuba/metrics/utils.py

@@ -144,6 +144,20 @@ GENERIC_OP_TO_SNUBA_FUNCTION = {
     "generic_metrics_sets": OP_TO_SNUBA_FUNCTION["metrics_sets"],
 }
 
+# This set contains all the operations that require the "rhs" condition to be resolved
+# in a "MetricConditionField". This solution is the simplest one and doesn't require any
+# changes in the transformer, however it requires this list to be discovered and updated
+# in case new operations are added, which is not ideal but given the fact that we already
+# define operations in this file, it is not a deal-breaker.
+REQUIRES_RHS_CONDITION_RESOLUTION = {"transform_null_to_unparameterized"}
+
+
+def require_rhs_condition_resolution(op: MetricOperationType) -> bool:
+    """
+    Checks whether a given operation requires its right operand to be resolved.
+    """
+    return op in REQUIRES_RHS_CONDITION_RESOLUTION
+
 
 def generate_operation_regex():
     """

+ 68 - 0
tests/sentry/snuba/metrics/test_metrics_layer/test_metrics_enhanced_performance.py

@@ -1301,6 +1301,74 @@ class PerformanceMetricsLayerTestCase(BaseMetricsLayerTestCase, TestCase):
             key=lambda elem: elem["name"],
         )
 
+    def test_transform_null_to_unparameterized_with_filter(self):
+        for transaction, value in ((None, 0), ("/foo", 1), ("/bar", 2)):
+            self.store_performance_metric(
+                type="distribution",
+                name=TransactionMRI.DURATION.value,
+                tags={} if transaction is None else {"transaction": transaction},
+                value=value,
+            )
+
+        metrics_query = self.build_metrics_query(
+            before_now="1h",
+            granularity="1h",
+            select=[
+                MetricField(
+                    op="count", metric_mri=TransactionMRI.DURATION.value, alias="duration_count"
+                ),
+            ],
+            where=[
+                MetricConditionField(
+                    lhs=MetricField(
+                        op="transform_null_to_unparameterized",
+                        metric_mri="d:transactions/duration@millisecond",
+                        params={"tag_key": "transaction"},
+                        alias="transaction",
+                    ),
+                    op=Op.NEQ,
+                    rhs="<< unparameterized >>",
+                )
+            ],
+            limit=Limit(limit=50),
+            offset=Offset(offset=0),
+            groupby=[
+                MetricGroupByField(
+                    field=MetricField(
+                        op="transform_null_to_unparameterized",
+                        metric_mri=TransactionMRI.DURATION.value,
+                        params={"tag_key": "transaction"},
+                        alias="transformed_transaction",
+                    )
+                ),
+            ],
+            include_series=False,
+        )
+        data = get_series(
+            [self.project],
+            metrics_query=metrics_query,
+            include_meta=True,
+            use_case_id=UseCaseKey.PERFORMANCE,
+        )
+
+        assert sorted(data["groups"], key=lambda group: group["by"]["transformed_transaction"]) == [
+            {
+                "by": {"transformed_transaction": "/bar"},
+                "totals": {"duration_count": 1},
+            },
+            {
+                "by": {"transformed_transaction": "/foo"},
+                "totals": {"duration_count": 1},
+            },
+        ]
+        assert data["meta"] == sorted(
+            [
+                {"name": "duration_count", "type": "UInt64"},
+                {"name": "transformed_transaction", "type": "string"},
+            ],
+            key=lambda elem: elem["name"],
+        )
+
     def test_transform_null_to_unparameterized_with_null_and_unparameterized_transactions(self):
         for transaction, value in ((None, 0), ("<< unparameterized >>", 1)):
             self.store_performance_metric(