diff --git a/src/sentry/event_manager.py b/src/sentry/event_manager.py index 43e5de23fccf61..f163f180e5cac6 100644 --- a/src/sentry/event_manager.py +++ b/src/sentry/event_manager.py @@ -1624,9 +1624,10 @@ def _save_aggregate( group = _create_group(project, event, **kwargs) - if features.has( - "projects:first-event-severity-calculation", event.project - ) and not group.data.get("metadata", {}).get("severity"): + if ( + features.has("projects:first-event-severity-calculation", event.project) + and group.data.get("metadata", {}).get("severity") is None + ): logger.error( "Group created without severity score", extra={ @@ -1843,7 +1844,7 @@ def _create_group(project: Project, event: Event, **kwargs: Any) -> Group: group_data = kwargs.pop("data", {}) if features.has("projects:first-event-severity-calculation", event.project): severity = _get_severity_score(event) - if severity: + if severity is not None: # Severity can be 0 group_data.setdefault("metadata", {}) group_data["metadata"]["severity"] = severity @@ -2510,9 +2511,10 @@ def _save_grouphash_and_group( group = _create_group(project, event, **group_kwargs) group_hash.update(group=group) - if features.has( - "projects:first-event-severity-calculation", event.project - ) and not group.data.get("metadata", {}).get("severity"): + if ( + features.has("projects:first-event-severity-calculation", event.project) + and group.data.get("metadata", {}).get("severity") is None + ): logger.error( "Group created without severity score", extra={ diff --git a/tests/sentry/event_manager/test_event_manager.py b/tests/sentry/event_manager/test_event_manager.py index 3009b14d9a3821..d03c9298711dc6 100644 --- a/tests/sentry/event_manager/test_event_manager.py +++ b/tests/sentry/event_manager/test_event_manager.py @@ -2639,7 +2639,37 @@ def test_severity_score_flag_off(self, mock_get_severity_score: MagicMock): event = manager.save(self.project.id) mock_get_severity_score.assert_not_called() - assert event.group and event.group.get_event_metadata().get("severity") is None + assert event.group and "severity" not in event.group.get_event_metadata() + + @patch("sentry.event_manager._get_severity_score", return_value=None) + def test_no_severity_score_assigned_when_value_is_None( + self, mock_get_severity_score: MagicMock + ): + with self.feature({"projects:first-event-severity-calculation": True}): + manager = EventManager( + make_event( + exception={"values": [{"type": "NopeError", "value": "Nopey McNopeface"}]} + ) + ) + event = manager.save(self.project.id) + + mock_get_severity_score.assert_called() + assert event.group and "severity" not in event.group.get_event_metadata() + + @patch("sentry.event_manager._get_severity_score", return_value=0) + def test_severity_score_still_assigned_when_value_is_zero( + self, mock_get_severity_score: MagicMock + ): + with self.feature({"projects:first-event-severity-calculation": True}): + manager = EventManager( + make_event( + exception={"values": [{"type": "NopeError", "value": "Nopey McNopeface"}]} + ) + ) + event = manager.save(self.project.id) + + mock_get_severity_score.assert_called() + assert event.group and event.group.get_event_metadata().get("severity") == 0 class AutoAssociateCommitTest(TestCase, EventManagerTestMixin): @@ -3717,6 +3747,29 @@ def test_logs_error_on_no_severity_score_when_enabled( }, ) + @patch("sentry.event_manager.logger.error") + @patch("sentry.event_manager._get_severity_score", return_value=0) + def test_no_error_logged_on_zero_severity_score_when_enabled( + self, + mock_get_severity_score: MagicMock, + mock_logger_error: MagicMock, + ): + with self.feature({"projects:first-event-severity-calculation": True}): + event = _get_event_instance( + make_event( + exception={"values": [{"type": "NopeError", "value": "Nopey McNopeface"}]} + ), + self.project.id, + ) + + group, _ = _save_grouphash_and_group(self.project, event, "dogs are great") + + logger_messages = [call.args[0] for call in mock_logger_error.mock_calls] + + mock_get_severity_score.assert_called() + assert group.data["metadata"]["severity"] == 0 + assert "Group created without severity score" not in logger_messages + @patch("sentry.event_manager.logger.error") @patch("sentry.event_manager._get_severity_score", return_value=None) def test_no_error_logged_on_no_severity_score_when_disabled( @@ -3768,6 +3821,30 @@ def test_logs_error_on_no_severity_score_when_enabled( }, ) + @patch("sentry.event_manager._save_aggregate", wraps=_save_aggregate) + @patch("sentry.event_manager.logger.error") + @patch("sentry.event_manager._get_severity_score", return_value=0) + def test_no_error_logged_on_zero_severity_score_when_enabled( + self, + mock_get_severity_score: MagicMock, + mock_logger_error: MagicMock, + mock_save_aggregate: MagicMock, + ): + with self.feature({"projects:first-event-severity-calculation": True}): + manager = EventManager( + make_event( + exception={"values": [{"type": "NopeError", "value": "Nopey McNopeface"}]} + ) + ) + event = manager.save(self.project.id) + + logger_messages = [call.args[0] for call in mock_logger_error.mock_calls] + + mock_save_aggregate.assert_called() + mock_get_severity_score.assert_called() + assert event.group and event.group.data["metadata"]["severity"] == 0 + assert "Group created without severity score" not in logger_messages + @patch("sentry.event_manager._save_aggregate", wraps=_save_aggregate) @patch("sentry.event_manager.logger.error") @patch("sentry.event_manager._get_severity_score", return_value=None) @@ -3789,5 +3866,4 @@ def test_no_error_logged_on_no_severity_score_when_disabled( mock_save_aggregate.assert_called() mock_get_severity_score.assert_not_called() - assert "Group created without severity score" not in logger_messages