refactor and add test for default sampling mode

getsentry · Nov 5, 2024 · 677469f · 677469f
1 parent 62fdb14
commit 677469f
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 21 deletions.
diff --git a/src/sentry/api/endpoints/organization_details.py b/src/sentry/api/endpoints/organization_details.py
@@ -430,7 +430,7 @@ def validate(self, attrs):
             request_sampling_mode == DynamicSamplingMode.PROJECT.value
             or (
                 sampling_mode == DynamicSamplingMode.PROJECT.value
-                or request_sampling_mode != DynamicSamplingMode.ORGANIZATION.value
+                and request_sampling_mode != DynamicSamplingMode.ORGANIZATION.value
             )
         ) and request_target_sample_rate is not None:
             raise serializers.ValidationError(
@@ -967,23 +967,20 @@ def put(self, request: Request, organization: Organization) -> Response:
             with transaction.atomic(router.db_for_write(Organization)):
                 organization, changed_data = serializer.save()
 
-            if (
-                organization.get_option("sentry:sampling_mode")
-                == DynamicSamplingMode.ORGANIZATION.value
-            ) and "targetSampleRate" in changed_data:
-                with transaction.atomic(router.db_for_write(ProjectOption)):
-                    organization.update_option(
-                        "sentry:target_sample_rate", request.data["targetSampleRate"]
-                    )
+            sampling_mode = organization.get_option("sentry:sampling_mode", SAMPLING_MODE_DEFAULT)
+            is_org_mode = sampling_mode == DynamicSamplingMode.ORGANIZATION.value
+            is_project_mode = sampling_mode == DynamicSamplingMode.PROJECT.value
+
+            if is_org_mode and "targetSampleRate" in changed_data:
+                organization.update_option(
+                    "sentry:target_sample_rate", serializer.validated_data["targetSampleRate"]
+                )
 
             if "samplingMode" in changed_data and request.access.has_scope("org:write"):
-                if (
-                    organization.get_option("sentry:sampling_mode")
-                    == DynamicSamplingMode.PROJECT.value
-                ):
-                    with transaction.atomic(router.db_for_write(ProjectOption)):
+                with transaction.atomic(router.db_for_write(ProjectOption)):
+                    if is_project_mode:
                         for project in organization.project_set.all():
-                            current_rate, is_fallback = get_boost_low_volume_projects_sample_rate(
+                            current_rate, _ = get_boost_low_volume_projects_sample_rate(
                                 org_id=organization.id,
                                 project_id=project.id,
                                 error_sample_rate_fallback=None,
@@ -993,14 +990,11 @@ def put(self, request: Request, organization: Organization) -> Response:
 
                         organization.delete_option("sentry:target_sample_rate")
 
-                if (
-                    organization.get_option("sentry:sampling_mode")
-                    == DynamicSamplingMode.ORGANIZATION.value
-                ):
-                    with transaction.atomic(router.db_for_write(ProjectOption)):
+                    elif is_org_mode:
                         if "targetSampleRate" in changed_data:
                             organization.update_option(
-                                "sentry:target_sample_rate", request.data["targetSampleRate"]
+                                "sentry:target_sample_rate",
+                                serializer.validated_data["targetSampleRate"],
                             )
 
                         for project in organization.project_set.all():

diff --git a/tests/sentry/api/endpoints/test_organization_details.py b/tests/sentry/api/endpoints/test_organization_details.py
@@ -411,6 +411,22 @@ def test_cannot_set_target_sample_rate_in_project_mode(self):
             ]
         }
 
+    @django_db_all
+    def test_sampling_mode_default_when_not_set(self):
+        """
+        Test that when sentry:sampling_mode is not set, it uses SAMPLING_MODE_DEFAULT
+        when validating targetSampleRate
+        """
+        # Ensure no sampling mode is set
+        self.organization.delete_option("sentry:sampling_mode")
+
+        with self.feature("organizations:dynamic-sampling-custom"):
+            # Since SAMPLING_MODE_DEFAULT is ORGANIZATION, this should succeed
+            response = self.get_response(self.organization.slug, method="put", targetSampleRate=0.5)
+
+        assert response.status_code == 200
+        assert self.organization.get_option("sentry:target_sample_rate") == 0.5
+
     @django_db_all
     def test_sampling_mode_org_to_project(self):
         """