Skip to content

chore(spans-migration): add isolation scope to misfire and action tags in alerts comparison #103816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 21, 2025
Merged

chore(spans-migration): add isolation scope to misfire and action tags in alerts comparison #103816

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea011ea
add isolation scope to misfire and action tags
nikkikapadia Nov 21, 2025
735660a
add sentry message to capture
nikkikapadia Nov 21, 2025
07875c9
fix tests
nikkikapadia Nov 21, 2025
0985705
another cursor fix
nikkikapadia Nov 21, 2025
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
10 changes: 6 additions & 4 deletions src/sentry/discover/compare_timeseries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,12 @@ def assert_timeseries_close(aligned_timeseries, alert_rule):
"confidence": values.get("confidence"),
}

sentry_sdk.set_tag("false_positive_misfires", false_positive_misfire)
sentry_sdk.set_tag("false_negative_misfires", false_negative_misfire)
for trigger_action_type, count in trigger_action_types.items():
sentry_sdk.set_tag(f"trigger_action_type.{trigger_action_type}", count)
with sentry_sdk.isolation_scope() as scope:
scope.set_tag("false_positive_misfires", false_positive_misfire)
scope.set_tag("false_negative_misfires", false_negative_misfire)
for trigger_action_type, count in trigger_action_types.items():
scope.set_tag(f"trigger_action_type.{trigger_action_type}", count)
Copy link
Contributor

@cursor cursor bot Nov 21, 2025

Choose a reason for hiding this comment

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

Bug: Misfire tags captured in scope with no event

The isolation scope at lines 330-334 sets misfire and trigger action tags but doesn't capture any event within that scope. The tags exit the scope without being associated with any Sentry event, making them ineffective. These tags should either be moved into the isolation scope where capture_message is called at line 352, or capture_message should be called within this scope.

Fix in Cursor Fix in Web

sentry_sdk.capture_message("False Misfires", level="info", scope=scope)

if mismatches:
with sentry_sdk.isolation_scope() as scope:
Expand Down
22 changes: 14 additions & 8 deletions tests/sentry/discover/test_compare_timeseries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ def test_compare_mri_alert(self) -> None:
result = compare_timeseries_for_alert_rule(alert_rule)
assert result["skipped"] is True

@mock.patch("sentry.discover.compare_timeseries.sentry_sdk.set_tag")
def test_false_positive_misfires(self, mock_set_tag: mock.MagicMock) -> None:
@mock.patch("sentry.discover.compare_timeseries.sentry_sdk.isolation_scope")
def test_false_positive_misfires(self, mock_isolation_scope: mock.MagicMock) -> None:
mock_scope = mock.MagicMock()
mock_isolation_scope.return_value.__enter__.return_value = mock_scope

alert_rule = self.create_alert_rule(
dataset=Dataset.PerformanceMetrics,
query_type=SnubaQuery.Type.PERFORMANCE,
Expand All @@ -192,11 +195,14 @@ def test_false_positive_misfires(self, mock_set_tag: mock.MagicMock) -> None:
}

assert_timeseries_close(aligned_timeseries=aligned_timeseries, alert_rule=alert_rule)
mock_set_tag.assert_any_call("false_positive_misfires", 1)
mock_set_tag.assert_any_call("false_negative_misfires", 0)
mock_scope.set_tag.assert_any_call("false_positive_misfires", 1)
mock_scope.set_tag.assert_any_call("false_negative_misfires", 0)

@mock.patch("sentry.discover.compare_timeseries.sentry_sdk.isolation_scope")
def test_false_negative_misfires(self, mock_isolation_scope: mock.MagicMock) -> None:
mock_scope = mock.MagicMock()
mock_isolation_scope.return_value.__enter__.return_value = mock_scope

@mock.patch("sentry.discover.compare_timeseries.sentry_sdk.set_tag")
def test_false_negative_misfires(self, mock_set_tag: mock.MagicMock) -> None:
alert_rule = self.create_alert_rule(
dataset=Dataset.PerformanceMetrics,
query_type=SnubaQuery.Type.PERFORMANCE,
Expand All @@ -211,5 +217,5 @@ def test_false_negative_misfires(self, mock_set_tag: mock.MagicMock) -> None:
}

assert_timeseries_close(aligned_timeseries=aligned_timeseries, alert_rule=alert_rule)
mock_set_tag.assert_any_call("false_positive_misfires", 0)
mock_set_tag.assert_any_call("false_negative_misfires", 1)
mock_scope.set_tag.assert_any_call("false_positive_misfires", 0)
mock_scope.set_tag.assert_any_call("false_negative_misfires", 1)
Loading