feat(dyn-sampling): Add BE validation for uniform rules [TET-191]

src/sentry/api/endpoints/project_details.py

@@ -96,6 +96,36 @@ class DynamicSamplingSerializer(serializers.Serializer):
     rules = serializers.ListSerializer(child=DynamicSamplingRuleSerializer())
     next_id = serializers.IntegerField(min_value=0, required=False)
 
+    @staticmethod
+    def _is_uniform_sampling_rule(rule):
+        # A uniform sampling rule must be an 'and' with no rules. An 'or' with no rules will not
+        # match anything.
+        assert rule["condition"]["op"] == "and"
+        # Matching the uniform sampling rule check on UI because currently we only support
+        # uniform rules on traces, not on single transactions. If we change this spec in the
+        # future, we will have to update this to also support single transactions.
+        return len(rule["condition"]["inner"]) == 0 and rule["type"] == "trace"
+
+    def validate_uniform_sampling_rule(self, rules):
+        # Guards against deletion of uniform sampling rule i.e. sending a payload with no rules
+        if len(rules) == 0:
+            raise serializers.ValidationError(
+                "Payload must contain a uniform dynamic sampling rule"
+            )
+
+        uniform_rule = rules[-1]
+        # Guards against placing uniform sampling rule not in last position or adding multiple
+        # uniform sampling rules
+        for rule in rules[:-1]:
+            if self._is_uniform_sampling_rule(rule):
+                raise serializers.ValidationError("Uniform rule must be in the last position only")
+
+        # Ensures last rule in rules is always a uniform sampling rule
+        if not self._is_uniform_sampling_rule(uniform_rule):
+            raise serializers.ValidationError(
+                "Last rule is reserved for uniform rule which must have no conditions"
+            )
+
     def validate(self, data):
         """
         Additional validation using sentry-relay to make sure that
@@ -106,6 +136,7 @@ def validate(self, data):
         try:
             config_str = json.dumps(data)
             validate_sampling_configuration(config_str)
+            self.validate_uniform_sampling_rule(data.get("rules", []))
         except ValueError as err:
             reason = err.args[0] if len(err.args) > 0 else "invalid configuration"
             raise serializers.ValidationError(reason)
@@ -432,8 +463,7 @@ def put(self, request: Request, project) -> Response:
         serializer = serializer_cls(
             data=request.data, partial=True, context={"project": project, "request": request}
         )
-        if not serializer.is_valid():
-            return Response(serializer.errors, status=400)
+        serializer.is_valid()
 
         result = serializer.validated_data
 
@@ -448,6 +478,9 @@ def put(self, request: Request, project) -> Response:
                 status=403,
             )
 
+        if not serializer.is_valid():
+            return Response(serializer.errors, status=400)
+
         if not has_project_write:
             # options isn't part of the serializer, but should not be editable by members
             for key in chain(ProjectAdminSerializer().fields.keys(), ["options"]):
@@ -797,6 +830,7 @@ def _fix_rule_ids(self, project, raw_dynamic_sampling):
 
         if raw_dynamic_sampling is not None:
             rules = raw_dynamic_sampling.get("rules", [])
+
             for rule in rules:
                 rid = rule.get("id", 0)
                 original_rule = original_rules_dict.get(rid)

tests/sentry/api/endpoints/test_project_details.py

@@ -34,35 +34,61 @@
 from sentry.utils import json
 
 
-def _dyn_sampling_data():
-    return {
-        "rules": [
-            {
-                "sampleRate": 0.7,
-                "type": "trace",
-                "active": True,
-                "condition": {
-                    "op": "and",
-                    "inner": [
-                        {"op": "eq", "name": "field1", "value": ["val"]},
-                        {"op": "glob", "name": "field1", "value": ["val"]},
-                    ],
-                },
-                "id": 0,
+def _dyn_sampling_data(multiple_uniform_rules=False, uniform_rule_last_position=True):
+    rules = [
+        {
+            "sampleRate": 0.7,
+            "type": "trace",
+            "active": True,
+            "condition": {
+                "op": "and",
+                "inner": [
+                    {"op": "eq", "name": "field1", "value": ["val"]},
+                    {"op": "glob", "name": "field1", "value": ["val"]},
+                ],
+            },
+            "id": 0,
+        },
+        {
+            "sampleRate": 0.8,
+            "type": "trace",
+            "active": True,
+            "condition": {
+                "op": "and",
+                "inner": [
+                    {"op": "eq", "name": "field1", "value": ["val"]},
+                ],
             },
+            "id": 0,
+        },
+    ]
+    if uniform_rule_last_position:
+        rules.append(
             {
                 "sampleRate": 0.8,
                 "type": "trace",
                 "active": True,
                 "condition": {
                     "op": "and",
-                    "inner": [
-                        {"op": "eq", "name": "field1", "value": ["val"]},
-                    ],
+                    "inner": [],
                 },
                 "id": 0,
             },
-        ]
+        )
+    if multiple_uniform_rules:
+        new_rule_1 = {
+            "sampleRate": 0.22,
+            "type": "trace",
+            "condition": {
+                "op": "and",
+                "inner": [],
+            },
+            "id": 0,
+        }
+        rules.insert(0, new_rule_1)
+
+    return {
+        "rules": rules,
     }
 
 
@@ -678,7 +704,9 @@ def test_dynamic_sampling_rule_id_handling(self):
                     "type": "trace",
                     "condition": {
                         "op": "and",
-                        "inner": [],
+                        "inner": [
+                            {"op": "eq", "name": "field1", "value": ["val"]},
+                        ],
                     },
                     "id": 0,
                 },
@@ -687,7 +715,9 @@ def test_dynamic_sampling_rule_id_handling(self):
                     "type": "trace",
                     "condition": {
                         "op": "and",
-                        "inner": [],
+                        "inner": [
+                            {"op": "eq", "name": "field1", "value": ["val"]},
+                        ],
                     },
                     "id": 0,
                 },
@@ -727,7 +757,9 @@ def test_dynamic_sampling_rule_id_handling(self):
             "type": "trace",
             "condition": {
                 "op": "and",
-                "inner": [],
+                "inner": [
+                    {"op": "eq", "name": "field1", "value": ["val"]},
+                ],
             },
             "id": 0,
         }
@@ -770,6 +802,52 @@ def test_dynamic_sampling_rules_have_active_flag(self):
         saved_config = response.data["dynamicSampling"]
         assert all([rule["active"] for rule in saved_config["rules"]])
 
+    def test_dynamic_sampling_rules_should_contain_single_uniform_rule(self):
+        """
+        Tests that ensures you can only have one uniform rule
+        """
+        with Feature({"organizations:server-side-sampling": True}):
+            response = self.get_response(
+                self.org_slug,
+                self.proj_slug,
+                dynamicSampling=_dyn_sampling_data(multiple_uniform_rules=True),
+            )
+            assert response.status_code == 400
+            assert (
+                response.json()["dynamicSampling"]["non_field_errors"][0] == "Uniform rule "
+                "must be in the last position only"
+            )
+
+    def test_dynamic_sampling_rules_single_uniform_rule_in_last_position(self):
+        """
+        Tests that ensures you can only have one uniform rule, and it is at the last position
+        """
+        with Feature({"organizations:server-side-sampling": True}):
+            response = self.get_response(
+                self.org_slug,
+                self.proj_slug,
+                dynamicSampling=_dyn_sampling_data(uniform_rule_last_position=False),
+            )
+            assert response.status_code == 400
+            assert (
+                response.json()["dynamicSampling"]["non_field_errors"][0]
+                == "Last rule is reserved for uniform rule which must have no conditions"
+            )
+
+    def test_dynamic_sampling_rules_should_contain_uniform_rule(self):
+        """
+        Tests that ensures that payload has a uniform rule i.e. guards against deletion of rule
+        """
+        with Feature({"organizations:server-side-sampling": True}):
+            response = self.get_response(
+                self.org_slug, self.proj_slug, dynamicSampling={"rules": []}
+            )
+            assert response.status_code == 400
+            assert (
+                response.json()["dynamicSampling"]["non_field_errors"][0]
+                == "Payload must contain a uniform dynamic sampling rule"
+            )
+
     def test_cap_secondary_grouping_expiry(self):
         now = time()
 
@@ -1209,7 +1287,17 @@ def test_rule_config_serializer():
                         {"op": "glob", "name": "field1", "value": ["val"]},
                     ],
                 },
-            }
+            },
+            {
+                "sampleRate": 0.7,
+                "type": "trace",
+                "active": False,
+                "id": 2,
+                "condition": {
+                    "op": "and",
+                    "inner": [],
+                },
+            },
         ],
         "next_id": 22,
     }