Skip to content

Commit

Permalink
ref(alerts): Remove crash rate v1 code
Browse files Browse the repository at this point in the history
  • Loading branch information
@ceorourke
ceorourke committed Sep 27, 2024
1 parent 286cce3 commit c281aac
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 184 deletions.
78 changes: 2 additions & 76 deletions src/sentry/incidents/subscription_processor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
from sentry.snuba.dataset import Dataset
from sentry.snuba.entity_subscription import (
ENTITY_TIME_COLUMNS,
BaseCrashRateMetricsEntitySubscription,
get_entity_key_from_query_builder,
get_entity_subscription_from_snuba_query,
)
Expand Down Expand Up @@ -336,78 +335,6 @@ def get_crash_rate_alert_aggregation_value(

def get_crash_rate_alert_metrics_aggregation_value(
self, subscription_update: QuerySubscriptionUpdate
) -> float | None:
"""
Handle both update formats.
Once all subscriptions have been updated to v2,
we can remove v1 and replace this function with current v2.
"""
rows = subscription_update["values"]["data"]
if BaseCrashRateMetricsEntitySubscription.is_crash_rate_format_v2(rows):
version = "v2"
result = self._get_crash_rate_alert_metrics_aggregation_value_v2(subscription_update)
else:
version = "v1"
result = self._get_crash_rate_alert_metrics_aggregation_value_v1(subscription_update)

metrics.incr(
"incidents.alert_rules.get_crash_rate_alert_metrics_aggregation_value",
tags={"format": version},
sample_rate=1.0,
)
return result

def _get_crash_rate_alert_metrics_aggregation_value_v1(
self, subscription_update: QuerySubscriptionUpdate
) -> float | None:
"""
Handles validation and extraction of Crash Rate Alerts subscription updates values over
metrics dataset.
The subscription update looks like
[
{'project_id': 8, 'tags[5]': 6, 'value': 2.0},
{'project_id': 8, 'tags[5]': 13,'value': 1.0}
]
where each entry represents a session status and the count of that specific session status.
As an example, `tags[5]` represents string `session.status`, while `tags[5]: 6` could
mean something like there are 2 sessions of status `crashed`. Likewise the other entry
represents the number of sessions started. In this method, we need to reverse match these
strings to end up with something that looks like
{"init": 2, "crashed": 4}
- `init` represents sessions or users sessions that were started, hence to get the crash
free percentage, we would need to divide number of crashed sessions by that number,
and subtract that value from 1. This is also used when CRASH_RATE_ALERT_MINIMUM_THRESHOLD is
set in the sense that if the minimum threshold is greater than the session count,
then the update is dropped. If the minimum threshold is not set then the total sessions
count is just ignored
- `crashed` represents the total sessions or user counts that crashed.
"""
(
total_session_count,
crash_count,
) = BaseCrashRateMetricsEntitySubscription.translate_sessions_tag_keys_and_values(
data=subscription_update["values"]["data"],
org_id=self.subscription.project.organization.id,
)

if total_session_count == 0:
self.reset_trigger_counts()
metrics.incr("incidents.alert_rules.ignore_update_no_session_data")
return None

if CRASH_RATE_ALERT_MINIMUM_THRESHOLD is not None:
min_threshold = int(CRASH_RATE_ALERT_MINIMUM_THRESHOLD)
if total_session_count < min_threshold:
self.reset_trigger_counts()
metrics.incr("incidents.alert_rules.ignore_update_count_lower_than_min_threshold")
return None

aggregation_value = round((1 - crash_count / total_session_count) * 100, 3)

return aggregation_value

def _get_crash_rate_alert_metrics_aggregation_value_v2(
self, subscription_update: QuerySubscriptionUpdate
) -> float | None:
"""
Handles validation and extraction of Crash Rate Alerts subscription updates values over
Expand All @@ -425,14 +352,13 @@ def _get_crash_rate_alert_metrics_aggregation_value_v2(
- `crashed` represents the total sessions or user counts that crashed.
"""
row = subscription_update["values"]["data"][0]
total_session_count = row["count"]
crash_count = row["crashed"]

total_session_count = row.get("count", 0)
if total_session_count == 0:
self.reset_trigger_counts()
metrics.incr("incidents.alert_rules.ignore_update_no_session_data")
return None

crash_count = row.get("crashed", 0)
if CRASH_RATE_ALERT_MINIMUM_THRESHOLD is not None:
min_threshold = int(CRASH_RATE_ALERT_MINIMUM_THRESHOLD)
if total_session_count < min_threshold:
Expand Down
46 changes: 0 additions & 46 deletions src/sentry/snuba/entity_subscription.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,54 +397,8 @@ def translate_sessions_tag_keys_and_values(
total_session_count = crash_count = 0
return total_session_count, crash_count

@staticmethod
def is_crash_rate_format_v2(data: list[dict[str, Any]]) -> bool:
"""Check if this is the new update format.
This function can be removed once all subscriptions have been updated.
"""
return bool(data) and "crashed" in data[0]

def aggregate_query_results(
self, data: list[dict[str, Any]], alias: str | None = None
) -> list[dict[str, Any]]:
"""Handle both update formats. Once all subscriptions have been updated
to v2, we can remove v1 and replace this function with current v2.
"""
if self.is_crash_rate_format_v2(data):
version = "v2"
result = self._aggregate_query_results_v2(data, alias)
else:
version = "v1"
result = self._aggregate_query_results_v1(data, alias)

metrics.incr(
"incidents.entity_subscription.aggregate_query_results",
tags={"format": version},
sample_rate=1.0,
)
return result

def _aggregate_query_results_v1(
self, data: list[dict[str, Any]], alias: str | None = None
) -> list[dict[str, Any]]:
aggregated_results: list[dict[str, Any]]
total_session_count, crash_count = self.translate_sessions_tag_keys_and_values(
org_id=self.org_id, data=data, alias=alias
)
if total_session_count == 0:
metrics.incr(
"incidents.entity_subscription.metrics.aggregate_query_results.no_session_data"
)
crash_free_rate = None
else:
crash_free_rate = round((1 - crash_count / total_session_count) * 100, 3)

col_name = alias if alias else CRASH_RATE_ALERT_AGGREGATE_ALIAS
aggregated_results = [{col_name: crash_free_rate}]
return aggregated_results

def _aggregate_query_results_v2(
self, data: list[dict[str, Any]], alias: str | None = None
) -> list[dict[str, Any]]:
aggregated_results: list[dict[str, Any]]
if not data:
Expand Down
63 changes: 1 addition & 62 deletions tests/sentry/incidents/test_subscription_processor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
from sentry.seer.anomaly_detection.utils import translate_direction
from sentry.sentry_metrics.configuration import UseCaseKey
from sentry.sentry_metrics.indexer.postgres.models import MetricsKeyIndexer
from sentry.sentry_metrics.utils import resolve_tag_key, resolve_tag_value
from sentry.sentry_metrics.utils import resolve_tag_key
from sentry.snuba.dataset import Dataset
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
from sentry.testutils.cases import BaseMetricsTestCase, SnubaTestCase, TestCase
Expand Down Expand Up @@ -3534,11 +3534,6 @@ def test_ensure_case_when_no_metrics_index_not_found_is_handled_gracefully(self)
}
)
self.assert_no_active_incident(rule)
self.entity_subscription_metrics.incr.assert_has_calls(
[
call("incidents.entity_subscription.metric_index_not_found"),
]
)
self.metrics.incr.assert_has_calls(
[
call("incidents.alert_rules.ignore_update_no_session_data"),
Expand All @@ -3548,62 +3543,6 @@ def test_ensure_case_when_no_metrics_index_not_found_is_handled_gracefully(self)
)


class MetricsCrashRateAlertProcessUpdateV1Test(MetricsCrashRateAlertProcessUpdateTest):
"""Repeat MetricsCrashRateUpdateAlertTest with old (v1) subscription updates.
This entire test class can be removed once all subscriptions have been migrated to v2
"""

def send_crash_rate_alert_update(self, rule, value, subscription, time_delta=None, count=EMPTY):
org_id = self.organization.id
self.email_action_handler.reset_mock()
if time_delta is None:
time_delta = timedelta()
processor = SubscriptionProcessor(subscription)

if time_delta is not None:
timestamp = timezone.now() + time_delta
else:
timestamp = timezone.now()
timestamp = timestamp.replace(microsecond=0)

with (
self.feature(["organizations:incidents", "organizations:performance-view"]),
self.capture_on_commit_callbacks(execute=True),
):
if value is None:
numerator, denominator = 0, 0
else:
if count is EMPTY:
numerator, denominator = value.as_integer_ratio()
else:
denominator = count
numerator = int(value * denominator)
session_status = resolve_tag_key(UseCaseKey.RELEASE_HEALTH, org_id, "session.status")
tag_value_init = resolve_tag_value(UseCaseKey.RELEASE_HEALTH, org_id, "init")
tag_value_crashed = resolve_tag_value(UseCaseKey.RELEASE_HEALTH, org_id, "crashed")
processor.process_update(
{
"entity": "entity",
"subscription_id": (
subscription.subscription_id if subscription else uuid4().hex
),
"values": {
"data": [
{"project_id": 8, session_status: tag_value_init, "value": denominator},
{
"project_id": 8,
session_status: tag_value_crashed,
"value": numerator,
},
]
},
"timestamp": timestamp,
}
)
return processor


class TestBuildAlertRuleStatKeys(unittest.TestCase):
def test(self):
stat_keys = build_alert_rule_stat_keys(AlertRule(id=1), QuerySubscription(project_id=2))
Expand Down

0 comments on commit c281aac

Please sign in to comment.