feat(sampling) - Changed times_seen increments on groups to take error sample rate into account (#93003)

As part of error extrapolation/upsampling we need to now make sure the
times_seen on groups is calculated correctly on projects where events
have a sample rate.
Now in event manager for projects in the allowlist we increment
times_seen not be 1 but by the inverse of sample_rate, both when
creating a new group and when saving a new event.

Author: yuvmen

src/sentry/event_manager.py

@@ -749,7 +749,7 @@ def _derive_client_error_sampling_rate(jobs: Sequence[Job], projects: ProjectsMa
                 )
 
                 if client_sample_rate is not None and isinstance(client_sample_rate, (int, float)):
-                    if 0 <= client_sample_rate <= 1:
+                    if 0 < client_sample_rate <= 1:
                         job["data"]["sample_rate"] = client_sample_rate
                     else:
                         logger.warning(
@@ -1471,6 +1471,13 @@ def _create_group(
     group_data["metadata"]["initial_priority"] = priority
     group_creation_kwargs["data"] = group_data
 
+    # Set initial times_seen
+    group_creation_kwargs["times_seen"] = 1
+
+    # If the project is in the allowlist, use the client sample rate to weight the times_seen
+    if project.id in options.get("issues.client_error_sampling.project_allowlist"):
+        group_creation_kwargs["times_seen"] = _get_error_weighted_times_seen(event)
+
     try:
         with transaction.atomic(router.db_for_write(Group)):
             # This is the 99.999% path. The rest of the function is all to handle a very rare and
@@ -1510,6 +1517,14 @@ def _create_group(
     return group
 
 
+def _get_error_weighted_times_seen(event: BaseEvent) -> int:
+    if event.get_event_type() in ("error", "default"):
+        error_sample_rate = event.data.get("sample_rate")
+        if error_sample_rate is not None and error_sample_rate > 0:
+            return int(1 / error_sample_rate)
+    return 1
+
+
 def _is_stuck_counter_error(err: Exception, project: Project, short_id: int) -> bool:
     """Decide if this is `UniqueViolation` error on the `Group` table's project and short id values."""
 
@@ -1821,7 +1836,11 @@ def _process_existing_aggregate(
 
     # We pass `times_seen` separately from all of the other columns so that `buffer_inr` knows to
     # increment rather than overwrite the existing value
-    buffer_incr(Group, {"times_seen": 1}, {"id": group.id}, updated_group_values)
+    times_seen = 1
+    if group.project.id in options.get("issues.client_error_sampling.project_allowlist"):
+        times_seen = _get_error_weighted_times_seen(event)
+
+    buffer_incr(Group, {"times_seen": times_seen}, {"id": group.id}, updated_group_values)
 
     return bool(is_regression)
 

tests/sentry/event_manager/test_event_manager.py

@@ -2935,6 +2935,213 @@ def test_derive_client_error_sampling_rate_invalid_range(self) -> None:
             # Check that sample_rate was not set due to invalid range
             assert "sample_rate" not in event.data
 
+    def test_times_seen_new_group_default_behavior(self) -> None:
+        """Test that new groups start with times_seen=1 when no sample rate is provided"""
+        manager = EventManager(make_event(message="test message"))
+        manager.normalize()
+
+        with self.tasks():
+            event = manager.save(self.project.id)
+
+        group = event.group
+        assert group is not None
+        assert group.times_seen == 1
+
+    def test_times_seen_existing_group_increment(self) -> None:
+        """Test that existing groups have their times_seen incremented"""
+        # Create first event to establish the group
+        manager1 = EventManager(make_event(message="test message", fingerprint=["group1"]))
+        manager1.normalize()
+
+        with self.tasks():
+            event1 = manager1.save(self.project.id)
+
+        group = event1.group
+        assert group is not None
+        initial_times_seen = group.times_seen
+        assert initial_times_seen == 1
+
+        # Create second event for the same group
+        manager2 = EventManager(make_event(message="test message 2", fingerprint=["group1"]))
+        manager2.normalize()
+
+        with self.tasks():
+            event2 = manager2.save(self.project.id)
+
+        # Should be the same group
+        assert event2.group_id == event1.group_id
+
+        # Refresh group from database to get updated times_seen
+        group.refresh_from_db()
+        assert group.times_seen == initial_times_seen + 1
+
+    def test_times_seen_weighted_with_sample_rate_option_enabled(self) -> None:
+        """Test that times_seen is weighted by 1/sample_rate when the project is in the allowlist"""
+
+        with self.options({"issues.client_error_sampling.project_allowlist": [self.project.id]}):
+            # Create event with a sample rate of 0.5 (50%)
+            event_data = make_event(
+                message="sampled event", contexts={"error_sampling": {"client_sample_rate": 0.5}}
+            )
+
+            manager = EventManager(event_data)
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+
+            group = event.group
+            assert group is not None
+            # With sample rate 0.5, times_seen should be 1/0.5 = 2
+            assert group.times_seen == 2
+
+    def test_times_seen_weighted_with_sample_rate_option_disabled(self) -> None:
+        """Test that times_seen is not weighted when the project is not in the allowlist"""
+
+        # Create event with a sample rate of 0.5 (50%) but project not in allowlist
+        event_data = make_event(
+            message="sampled event", contexts={"error_sampling": {"client_sample_rate": 0.5}}
+        )
+
+        manager = EventManager(event_data)
+        manager.normalize()
+
+        with self.tasks():
+            event = manager.save(self.project.id)
+
+        group = event.group
+        assert group is not None
+        # With the project not in allowlist, times_seen should remain 1 regardless of sample rate
+        assert group.times_seen == 1
+
+    def test_times_seen_weighted_existing_group_with_sample_rate(self) -> None:
+        """Test that existing groups are incremented by weighted amount when project is in allowlist"""
+
+        # Create first event to establish the group
+        manager1 = EventManager(make_event(message="test message", fingerprint=["group1"]))
+        manager1.normalize()
+
+        with self.tasks():
+            event1 = manager1.save(self.project.id)
+
+        group = event1.group
+        assert group is not None
+        initial_times_seen = group.times_seen
+        assert initial_times_seen == 1
+
+        with self.options({"issues.client_error_sampling.project_allowlist": [self.project.id]}):
+            # Create second event for the same group with sample rate 0.25 (25%)
+            event_data = make_event(
+                message="test message 2",
+                fingerprint=["group1"],
+                contexts={"error_sampling": {"client_sample_rate": 0.25}},
+            )
+
+            manager2 = EventManager(event_data)
+            manager2.normalize()
+
+            with self.tasks():
+                event2 = manager2.save(self.project.id)
+
+            # Should be the same group
+            assert event2.group_id == event1.group_id
+
+            # Refresh group from database to get updated times_seen
+            group.refresh_from_db()
+            # Should be incremented by 1/0.25 = 4
+            assert group.times_seen == initial_times_seen + 4
+
+    def test_times_seen_no_sample_rate_meta(self) -> None:
+        """Test that times_seen defaults to 1 when no sample rate meta exists"""
+        with self.options({"issues.client_error_sampling.project_allowlist": [self.project.id]}):
+            # Create event with no error_sampling context
+            manager = EventManager(make_event(fingerprint=["no_context"]))
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
+            # Create event with empty error_sampling context
+            manager = EventManager(
+                make_event(fingerprint=["empty_context"], contexts={"error_sampling": {}})
+            )
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
+            # Create event with null client_sample_rate
+            manager = EventManager(
+                make_event(
+                    fingerprint=["null_client_sample_rate"],
+                    contexts={"error_sampling": {"client_sample_rate": None}},
+                )
+            )
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
+    def test_times_seen_invalid_sample_rate(self) -> None:
+        """Test times_seen calculation with invalid sample rates (null, 0, negative, > 1)"""
+        with self.options({"issues.client_error_sampling.project_allowlist": [self.project.id]}):
+            # Test null sample rate
+            manager = EventManager(make_event(fingerprint=["null_sample_rate"]))
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
+            # Test sample rate of 0 (should result in times_seen = 1)
+            manager = EventManager(
+                make_event(
+                    fingerprint=["zero_sample_rate"],
+                    contexts={"error_sampling": {"client_sample_rate": 0}},
+                )
+            )
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
+            # Test negative sample rate (should result in times_seen = 1)
+            manager = EventManager(
+                make_event(
+                    fingerprint=["negative_sample_rate"],
+                    contexts={"error_sampling": {"client_sample_rate": -0.5}},
+                )
+            )
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
+            # Test sample rate > 1 (should result in times_seen = 1)
+            manager = EventManager(
+                make_event(
+                    fingerprint=["high_sample_rate"],
+                    contexts={"error_sampling": {"client_sample_rate": 1.5}},
+                )
+            )
+            manager.normalize()
+
+            with self.tasks():
+                event = manager.save(self.project.id)
+            assert event.group is not None
+            assert event.group.times_seen == 1
+
 
 class ReleaseIssueTest(TestCase):
     def setUp(self) -> None: