mirrors/posthog
0
0
Fork 0
mirror of https://github.com/PostHog/posthog.git synced 2024-11-24 00:47:50 +01:00
Code Issues Releases Activity

fix(alerts): pass correct filters for aggregate trends (#26357)

Browse Source
This commit is contained in:
Anirudh Pillai 2024-11-22 20:55:18 +00:00 committed by GitHub
parent 140708819b
commit bcc98da2ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 129 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
posthog/tasks/alerts/test/test_alert_checks.py
View File

@ -100,7 +100,7 @@ class TestAlertChecks(APIBaseTest, ClickhouseDestroyTablesMixin):
anomalies_descriptions = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
assert len(anomalies_descriptions) == 1
assert (
"The insight value ($pageview) for previous day (1) is more than upper threshold (0.0)"
"The insight value ($pageview) for previous interval (1) is more than upper threshold (0.0)"
in anomalies_descriptions[0]
)
@ -130,7 +130,7 @@ class TestAlertChecks(APIBaseTest, ClickhouseDestroyTablesMixin):
assert mock_send_notifications_for_breaches.call_count == 1
anomalies = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies
assert "The insight value ($pageview) for previous interval (0) is less than lower threshold (1.0)" in anomalies
def test_alert_triggers_but_does_not_send_notification_during_firing(
self, mock_send_notifications_for_breaches: MagicMock, mock_send_errors: MagicMock
@ -318,7 +318,7 @@ class TestAlertChecks(APIBaseTest, ClickhouseDestroyTablesMixin):
assert mock_send_notifications_for_breaches.call_count == 1
anomalies = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies
assert "The insight value ($pageview) for previous interval (0) is less than lower threshold (1.0)" in anomalies
@patch("posthog.tasks.alerts.utils.EmailMessage")
def test_send_emails(

112
posthog/tasks/alerts/test/test_trends_absolute_alerts.py
View File

@ -88,6 +88,34 @@ class TestTimeSeriesTrendsAbsoluteAlerts(APIBaseTest, ClickhouseDestroyTablesMix
return insight
def create_aggregate_trend_insight(self, breakdown: Optional[BreakdownFilter] = None) -> dict[str, Any]:
query_dict = TrendsQuery(
series=[
EventsNode(
event="signed_up",
math=BaseMathType.TOTAL,
),
EventsNode(
event="$pageview",
name="Pageview",
math=BaseMathType.TOTAL,
),
],
breakdownFilter=breakdown,
trendsFilter=TrendsFilter(display=ChartDisplayType.ACTIONS_PIE),
interval=IntervalType.WEEK,
dateRange=InsightDateRange(date_from="-8w"),
).model_dump()
insight = self.dashboard_api.create_insight(
data={
"name": "insight",
"query": query_dict,
}
)[1]
return insight
def test_alert_lower_threshold_breached(self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock) -> None:
insight = self.create_time_series_trend_insight()
alert = self.create_alert(insight, series_index=0, lower=1)
@ -295,3 +323,87 @@ class TestTimeSeriesTrendsAbsoluteAlerts(APIBaseTest, ClickhouseDestroyTablesMix
assert alert_check.error is None
mock_send_breaches.assert_not_called()
def test_aggregate_trend_high_threshold_breached(
self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock
) -> None:
insight = self.create_aggregate_trend_insight()
alert = self.create_alert(insight, series_index=0, upper=1)
with freeze_time(FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=4)):
_create_event(
team=self.team,
event="signed_up",
distinct_id="3",
properties={"$browser": "Firefox"},
)
_create_event(
team=self.team,
event="signed_up",
distinct_id="1",
properties={"$browser": "Chrome"},
)
_create_event(
team=self.team,
event="signed_up",
distinct_id="2",
properties={"$browser": "Chrome"},
)
flush_persons_and_events()
check_alert(alert["id"])
updated_alert = AlertConfiguration.objects.get(pk=alert["id"])
assert updated_alert.state == AlertState.FIRING
assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1)
alert_check = AlertCheck.objects.filter(alert_configuration=alert["id"]).latest("created_at")
assert alert_check.calculated_value == 3
assert alert_check.state == AlertState.FIRING
assert alert_check.error is None
mock_send_breaches.assert_called_once_with(
ANY, ["The insight value (signed_up) for previous interval (3) is more than upper threshold (1.0)"]
)
def test_aggregate_trend_with_breakdown_high_threshold_breached(
self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock
) -> None:
insight = self.create_aggregate_trend_insight(BreakdownFilter(breakdowns=[Breakdown(property="$browser")]))
alert = self.create_alert(insight, series_index=0, upper=1)
with freeze_time(FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=4)):
_create_event(
team=self.team,
event="signed_up",
distinct_id="3",
properties={"$browser": "Firefox"},
)
_create_event(
team=self.team,
event="signed_up",
distinct_id="1",
properties={"$browser": "Chrome"},
)
_create_event(
team=self.team,
event="signed_up",
distinct_id="2",
properties={"$browser": "Chrome"},
)
flush_persons_and_events()
check_alert(alert["id"])
updated_alert = AlertConfiguration.objects.get(pk=alert["id"])
assert updated_alert.state == AlertState.FIRING
assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1)
alert_check = AlertCheck.objects.filter(alert_configuration=alert["id"]).latest("created_at")
assert alert_check.calculated_value == 2
assert alert_check.state == AlertState.FIRING
assert alert_check.error is None
mock_send_breaches.assert_called_once_with(
ANY, ["The insight value (signed_up - Chrome) for previous interval (2) is more than upper threshold (1.0)"]
)

23
posthog/tasks/alerts/trends.py
View File

@ -58,19 +58,21 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
(query.breakdownFilter.breakdown and query.breakdownFilter.breakdown_type) or query.breakdownFilter.breakdowns
)
is_non_time_series = _is_non_time_series_trend(query)
match condition.type:
case AlertConditionType.ABSOLUTE_VALUE:
if threshold.type != InsightThresholdType.ABSOLUTE:
raise ValueError(f"Absolute threshold not configured for alert condition ABSOLUTE_VALUE")
# want value for current interval (last hour, last day, last week, last month)
# depending on the alert calculation interval
if _is_non_time_series_trend(query):
filters_override = _date_range_override_for_intervals(query, last_x_intervals=2)
else:
if is_non_time_series:
# for non time series, it's an aggregated value for full interval
# so we need to compute full insight
filters_override = None
else:
# want values back till previous interval (last hour, last day, last week, last month)
# depending on the alert calculation interval
filters_override = _date_range_override_for_intervals(query, last_x_intervals=2)
calculation_result = calculate_for_query_based_insight(
insight,
@ -83,18 +85,21 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
if not calculation_result.result:
raise RuntimeError(f"No results found for insight with alert id = {alert.id}")
interval = query.interval if not is_non_time_series else None
if has_breakdown:
# for breakdowns, we need to check all values in calculation_result.result
breakdown_results = calculation_result.result
for breakdown_result in breakdown_results:
# pick previous interval value
prev_interval_value = _pick_interval_value_from_trend_result(query, breakdown_result, -1)
breaches = _validate_bounds(
threshold.bounds,
prev_interval_value,
threshold.type,
condition.type,
query.interval,
interval,
breakdown_result["label"],
)
@ -114,13 +119,13 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
prev_interval_value,
threshold.type,
condition.type,
query.interval,
interval,
selected_series_result["label"],
)
return AlertEvaluationResult(value=prev_interval_value, breaches=breaches)
case AlertConditionType.RELATIVE_INCREASE:
if _is_non_time_series_trend(query):
if is_non_time_series:
raise ValueError(f"Relative alerts not supported for non time series trends")
# to measure relative increase, we can't alert until current interval has completed
@ -188,7 +193,7 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend
return AlertEvaluationResult(value=(increase if not has_breakdown else None), breaches=breaches)
case AlertConditionType.RELATIVE_DECREASE:
if _is_non_time_series_trend(query):
if is_non_time_series:
raise ValueError(f"Relative alerts not supported for non time series trends")
# to measure relative decrease, we can't alert until current interval has completed