Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/integrations/utils/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def record_event(
if outcome == EventLifecycleOutcome.FAILURE:
logger.warning(key, **log_params)
elif outcome == EventLifecycleOutcome.HALTED:
logger.info(key, **log_params)
logger.warning(key, **log_params)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant conditional branches with identical logic

Low Severity

The if/elif on lines 210–213 now has two branches that both call logger.warning(key, **log_params). After changing HALTED from logger.info to logger.warning, the conditional is redundant — a single logger.warning(key, **log_params) call (without the branching) would behave identically. Keeping the duplicate branches adds maintenance risk: a future change to one branch could accidentally diverge from the other.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 10c7abe. Configure here.


@staticmethod
def _report_flow_error(message) -> None:
Expand Down
16 changes: 8 additions & 8 deletions tests/sentry/integrations/utils/test_lifecycle_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def test_recording_halt(
with metric_obj.capture(assume_success=False):
pass
self._check_metrics_call_args(mock_metrics, "halted")
mock_logger.info.assert_called_once_with(
mock_logger.warning.assert_called_once_with(
"integrations.slo.halted",
extra={
"integration_domain": "messaging",
Expand All @@ -111,7 +111,7 @@ def test_recording_explicit_halt_with_exception(
lifecycle.record_halt(ExampleException(""), extra={"even": "more"})

self._check_metrics_call_args(mock_metrics, "halted")
mock_logger.info.assert_called_once_with(
mock_logger.warning.assert_called_once_with(
"integrations.slo.halted",
extra={
"extra": "value",
Expand All @@ -135,7 +135,7 @@ def test_recording_explicit_halt_with_str(
lifecycle.record_halt("Integration went boom", extra={"even": "more"})

self._check_metrics_call_args(mock_metrics, "halted")
mock_logger.info.assert_called_once_with(
mock_logger.warning.assert_called_once_with(
"integrations.slo.halted",
extra={
"outcome_reason": "Integration went boom",
Expand Down Expand Up @@ -254,7 +254,7 @@ def test_recording_halt_with_create_issue_true(

self._check_metrics_call_args(mock_metrics, "halted")
mock_sentry_sdk.capture_exception.assert_called_once()
mock_logger.info.assert_called_once_with(
mock_logger.warning.assert_called_once_with(
"integrations.slo.halted",
extra={
"extra": "value",
Expand Down Expand Up @@ -366,7 +366,7 @@ def test_sample_log_rate_halt_with_sampling(
# Metrics should always be called
self._check_metrics_call_args(mock_metrics, "halted")
# Logger should be called since 0.05 < 0.2
mock_logger.info.assert_called_once()
mock_logger.warning.assert_called_once()
mock_random.random.assert_called_once()

@mock.patch("sentry.integrations.utils.metrics.random")
Expand Down Expand Up @@ -406,7 +406,7 @@ def test_per_call_sample_log_rate_skips_when_below_threshold(
# Metrics should always be called
self._check_metrics_call_args(mock_metrics, "halted")
# Logger should NOT be called since 0.15 > 0.05 (per-call rate)
mock_logger.info.assert_not_called()
mock_logger.warning.assert_not_called()
mock_random.random.assert_called_once()

@mock.patch("sentry.integrations.utils.metrics.random")
Expand Down Expand Up @@ -466,7 +466,7 @@ def test_sample_log_rate_on_assume_success_false_exit(
# Metrics should always be called
self._check_metrics_call_args(mock_metrics, "halted")
# Logger should NOT be called since 0.25 > 0.2
mock_logger.info.assert_not_called()
mock_logger.warning.assert_not_called()
mock_random.random.assert_called_once()

@mock.patch("sentry.integrations.utils.metrics.logger")
Expand All @@ -492,4 +492,4 @@ def test_default_sample_log_rate_is_one(
pass # Will record halt

# Should log since default is 1.0
mock_logger.info.assert_called_once()
mock_logger.warning.assert_called_once()
Loading