diff --git a/ee/tasks/test/subscriptions/test_subscriptions.py b/ee/tasks/test/subscriptions/test_subscriptions.py index 717ab7b42d592..5f6db8011b231 100644 --- a/ee/tasks/test/subscriptions/test_subscriptions.py +++ b/ee/tasks/test/subscriptions/test_subscriptions.py @@ -15,7 +15,6 @@ from posthog.models.exported_asset import ExportedAsset from posthog.models.insight import Insight from posthog.models.instance_setting import set_instance_setting -from posthog.models.subscription import Subscription from posthog.test.base import APIBaseTest @@ -24,7 +23,6 @@ @patch("ee.tasks.subscriptions.generate_assets") @freeze_time("2022-02-02T08:55:00.000Z") class TestSubscriptionsTasks(APIBaseTest): - subscriptions: list[Subscription] = None # type: ignore dashboard: Dashboard insight: Insight tiles: list[DashboardTile] = None # type: ignore diff --git a/frontend/src/lib/components/Alerts/views/EditAlert.tsx b/frontend/src/lib/components/Alerts/views/EditAlert.tsx index b48a0b1dd1f59..46d0dfd627cc1 100644 --- a/frontend/src/lib/components/Alerts/views/EditAlert.tsx +++ b/frontend/src/lib/components/Alerts/views/EditAlert.tsx @@ -60,10 +60,18 @@ export function EditAlert(props: EditAlertProps): JSX.Element { - + - + diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 877f00d23f8b2..c1eb20423960e 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -1,6 +1,18 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AbsoluteThreshold": { + "additionalProperties": false, + "properties": { + "lower": { + "type": ["number", "null"] + }, + "upper": { + "type": ["number", "null"] + } + }, + "type": "object" + }, "ActionsNode": { "additionalProperties": false, "properties": { @@ -183,6 +195,16 @@ "enum": ["numeric", "duration", "duration_ms", "percentage", "percentage_scaled"], "type": "string" }, + "AnomalyCondition": { + "additionalProperties": false, + "properties": { + "absoluteThreshold": { + "$ref": "#/definitions/AbsoluteThreshold" + } + }, + "required": ["absoluteThreshold"], + "type": "object" + }, "AnyDataNode": { "anyOf": [ { diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index e38436e1bf086..b63935a28f88a 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -1680,3 +1680,12 @@ export interface DashboardFilter { date_to?: string | null properties?: AnyPropertyFilter[] | null } + +export interface AbsoluteThreshold { + lower?: number | null + upper?: number | null +} + +export interface AnomalyCondition { + absoluteThreshold: AbsoluteThreshold +} diff --git a/frontend/src/scenes/insights/InsightPageHeader.tsx b/frontend/src/scenes/insights/InsightPageHeader.tsx index c5bd094f6c86f..f0953c7bf77cf 100644 --- a/frontend/src/scenes/insights/InsightPageHeader.tsx +++ b/frontend/src/scenes/insights/InsightPageHeader.tsx @@ -35,7 +35,7 @@ import { ExporterFormat, InsightLogicProps, ItemMode, NotebookNodeType } from '~ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: InsightLogicProps }): JSX.Element { // insightSceneLogic - const { insightMode, subscriptionId } = useValues(insightSceneLogic) + const { insightMode, itemId } = useValues(insightSceneLogic) const { setInsightMode } = useActions(insightSceneLogic) // insightLogic @@ -77,7 +77,7 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In isOpen={insightMode === ItemMode.Subscriptions} closeModal={() => push(urls.insightView(insight.short_id))} insightShortId={insight.short_id} - subscriptionId={subscriptionId} + subscriptionId={itemId} /> diff --git a/frontend/src/scenes/insights/insightSceneLogic.tsx b/frontend/src/scenes/insights/insightSceneLogic.tsx index 2e1220067564f..72d3d294c6feb 100644 --- a/frontend/src/scenes/insights/insightSceneLogic.tsx +++ b/frontend/src/scenes/insights/insightSceneLogic.tsx @@ -45,10 +45,10 @@ export const insightSceneLogic = kea([ actions({ setInsightId: (insightId: InsightShortId) => ({ insightId }), setInsightMode: (insightMode: ItemMode, source: InsightEventSource | null) => ({ insightMode, source }), - setSceneState: (insightId: InsightShortId, insightMode: ItemMode, subscriptionId: string | undefined) => ({ + setSceneState: (insightId: InsightShortId, insightMode: ItemMode, itemId: string | undefined) => ({ insightId, insightMode, - subscriptionId, + itemId, }), setInsightLogicRef: (logic: BuiltLogic | null, unmount: null | (() => void)) => ({ logic, @@ -73,15 +73,11 @@ export const insightSceneLogic = kea([ setSceneState: (_, { insightMode }) => insightMode, }, ], - subscriptionId: [ + itemId: [ null as null | number | 'new', { - setSceneState: (_, { subscriptionId }) => - subscriptionId !== undefined - ? subscriptionId === 'new' - ? 'new' - : parseInt(subscriptionId, 10) - : null, + setSceneState: (_, { itemId }) => + itemId !== undefined ? (itemId === 'new' ? 'new' : parseInt(itemId, 10)) : null, }, ], insightLogicRef: [ @@ -202,8 +198,8 @@ export const insightSceneLogic = kea([ setSceneState: sharedListeners.reloadInsightLogic, })), urlToAction(({ actions, values }) => ({ - '/insights/:shortId(/:mode)(/:subscriptionId)': ( - { shortId, mode, subscriptionId }, // url params + '/insights/:shortId(/:mode)(/:itemId)': ( + { shortId, mode, itemId }, // url params { dashboard, ...searchParams }, // search params { insight: insightType, q }, // hash params { method, initial }, // "location changed" event payload @@ -237,12 +233,8 @@ export const insightSceneLogic = kea([ return } - if ( - insightId !== values.insightId || - insightMode !== values.insightMode || - subscriptionId !== values.subscriptionId - ) { - actions.setSceneState(insightId, insightMode, subscriptionId) + if (insightId !== values.insightId || insightMode !== values.insightMode || itemId !== values.itemId) { + actions.setSceneState(insightId, insightMode, itemId) } let query: Node | null = null diff --git a/frontend/src/scenes/scenes.ts b/frontend/src/scenes/scenes.ts index ef2c0a9ae6b39..22e3b9d67b770 100644 --- a/frontend/src/scenes/scenes.ts +++ b/frontend/src/scenes/scenes.ts @@ -457,8 +457,8 @@ export const routes: Record = { [urls.insightEdit(':shortId' as InsightShortId)]: Scene.Insight, [urls.insightView(':shortId' as InsightShortId)]: Scene.Insight, [urls.insightSubcriptions(':shortId' as InsightShortId)]: Scene.Insight, - [urls.insightSubcription(':shortId' as InsightShortId, ':subscriptionId')]: Scene.Insight, - [urls.alert(':shortId' as InsightShortId, ':subscriptionId')]: Scene.Insight, + [urls.insightSubcription(':shortId' as InsightShortId, ':itemId')]: Scene.Insight, + [urls.alert(':shortId' as InsightShortId, ':itemId')]: Scene.Insight, [urls.alerts(':shortId' as InsightShortId)]: Scene.Insight, [urls.insightSharing(':shortId' as InsightShortId)]: Scene.Insight, [urls.savedInsights()]: Scene.SavedInsights, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 10bbea2b7cb36..3cb3931558659 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -33,6 +33,7 @@ import { Scene } from 'scenes/sceneTypes' import { QueryContext } from '~/queries/types' import type { + AnomalyCondition, DashboardFilter, DatabaseSchemaField, HogQLQuery, @@ -4381,13 +4382,6 @@ export type HogFunctionInvocationGlobals = { > } -export interface AnomalyCondition { - absoluteThreshold: { - lower?: number - upper?: number - } -} - export interface AlertType { id: number name: string diff --git a/posthog/caching/calculate_results.py b/posthog/caching/calculate_results.py index 56f7d45f33b6d..8044ac0c73d12 100644 --- a/posthog/caching/calculate_results.py +++ b/posthog/caching/calculate_results.py @@ -123,7 +123,7 @@ def get_cache_type(cacheable: Optional[FilterType] | Optional[dict]) -> CacheTyp def calculate_for_query_based_insight( - insight: Insight, *, dashboard: Optional[Dashboard] = None, execution_mode: ExecutionMode, user: User + insight: Insight, *, dashboard: Optional[Dashboard] = None, execution_mode: ExecutionMode, user: Optional[User] ) -> "InsightResult": from posthog.caching.fetch_from_cache import InsightResult, NothingInCacheResult from posthog.caching.insight_cache import update_cached_state diff --git a/posthog/models/alert.py b/posthog/models/alert.py index b68720e5f4ec5..8aa62cf977b7a 100644 --- a/posthog/models/alert.py +++ b/posthog/models/alert.py @@ -13,7 +13,7 @@ def are_alerts_supported_for_insight(insight: Insight) -> bool: class Alert(models.Model): team: models.ForeignKey = models.ForeignKey("Team", on_delete=models.CASCADE) - insight = models.ForeignKey("posthog.Insight", on_delete=models.CASCADE) + insight: models.ForeignKey = models.ForeignKey("posthog.Insight", on_delete=models.CASCADE) name: models.CharField = models.CharField(max_length=100) target_value: models.TextField = models.TextField() diff --git a/posthog/schema.py b/posthog/schema.py index 85ef2f7638017..727d0e5525f30 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -12,6 +12,14 @@ class SchemaRoot(RootModel[Any]): root: Any +class AbsoluteThreshold(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + lower: Optional[float] = None + upper: Optional[float] = None + + class MathGroupTypeIndex(float, Enum): NUMBER_0 = 0 NUMBER_1 = 1 @@ -28,6 +36,13 @@ class AggregationAxisFormat(StrEnum): PERCENTAGE_SCALED = "percentage_scaled" +class AnomalyCondition(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + absoluteThreshold: AbsoluteThreshold + + class Kind(StrEnum): METHOD = "Method" FUNCTION = "Function" diff --git a/posthog/tasks/alerts/__init__.py b/posthog/tasks/alerts/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/posthog/tasks/alerts/checks.py b/posthog/tasks/alerts/checks.py new file mode 100644 index 0000000000000..e5c34f578a048 --- /dev/null +++ b/posthog/tasks/alerts/checks.py @@ -0,0 +1,107 @@ +from celery import shared_task +from celery.canvas import group, chain +from django.utils import timezone +import math +import structlog + +from posthog.api.services.query import ExecutionMode +from posthog.caching.calculate_results import calculate_for_query_based_insight +from posthog.email import EmailMessage +from posthog.hogql_queries.legacy_compatibility.flagged_conversion_manager import ( + conversion_to_query_based, +) +from posthog.models import Alert +from posthog.schema import AnomalyCondition + +logger = structlog.get_logger(__name__) + + +def check_all_alerts() -> None: + alert_ids = list(Alert.objects.all().values_list("id", flat=True)) + + group_count = 10 + # All groups but the last one will have a group_size size. + # The last group will have at most group_size size. + group_size = int(math.ceil(len(alert_ids) / group_count)) + + groups = [] + for i in range(0, len(alert_ids), group_size): + alert_id_group = alert_ids[i : i + group_size] + chained_calls = chain([check_alert_task.si(alert_id) for alert_id in alert_id_group]) + groups.append(chained_calls) + + group(groups).apply_async() + + +def check_alert(alert_id: int) -> None: + alert = Alert.objects.get(pk=alert_id) + insight = alert.insight + + with conversion_to_query_based(insight): + calculation_result = calculate_for_query_based_insight( + insight, + execution_mode=ExecutionMode.RECENT_CACHE_CALCULATE_BLOCKING_IF_STALE, + user=None, + ) + + if not calculation_result.result: + raise RuntimeError(f"No results for alert {alert.id}") + + anomaly_condition = AnomalyCondition.model_validate(alert.anomaly_condition) + thresholds = anomaly_condition.absoluteThreshold + + result = calculation_result.result[0] + aggregated_value = result["aggregated_value"] + anomalies_descriptions = [] + + if thresholds.lower is not None and aggregated_value < thresholds.lower: + anomalies_descriptions += [ + f"The trend value ({aggregated_value}) is below the lower threshold ({thresholds.lower})" + ] + if thresholds.upper is not None and aggregated_value > thresholds.upper: + anomalies_descriptions += [ + f"The trend value ({aggregated_value}) is above the upper threshold ({thresholds.upper})" + ] + + if not anomalies_descriptions: + logger.info("No threshold met", alert_id=alert.id) + return + + send_notifications(alert, anomalies_descriptions) + + +@shared_task(ignore_result=True) +def check_all_alerts_task() -> None: + check_all_alerts() + + +@shared_task(ignore_result=True) +def check_alert_task(alert_id: int) -> None: + check_alert(alert_id) + + +def send_notifications(alert: Alert, anomalies_descriptions: list[str]) -> None: + subject = f"PostHog alert {alert.name} has anomalies" + campaign_key = f"alert-anomaly-notification-{alert.id}-{timezone.now().timestamp()}" + insight_url = f"/project/{alert.team.pk}/insights/{alert.insight.short_id}" + alert_url = f"{insight_url}/alerts/{alert.id}" + message = EmailMessage( + campaign_key=campaign_key, + subject=subject, + template_name="alert_anomaly", + template_context={ + "anomalies_descriptions": anomalies_descriptions, + "insight_url": insight_url, + "insight_name": alert.insight.name, + "alert_url": alert_url, + "alert_name": alert.name, + }, + ) + targets = list(filter(len, alert.target_value.split(","))) + if not targets: + raise RuntimeError(f"no targets configured for the alert {alert.id}") + for target in targets: + message.add_recipient(email=target) + + logger.info(f"Send notifications about {len(anomalies_descriptions)} anomalies", alert_id=alert.id) + message.send() diff --git a/posthog/tasks/alerts/test/__init__.py b/posthog/tasks/alerts/test/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/posthog/tasks/alerts/test/test_checks.py b/posthog/tasks/alerts/test/test_checks.py new file mode 100644 index 0000000000000..fb5f93b3cb166 --- /dev/null +++ b/posthog/tasks/alerts/test/test_checks.py @@ -0,0 +1,159 @@ +import pytest +from typing import Optional +from unittest.mock import MagicMock, patch + +from freezegun import freeze_time + +from posthog.models.instance_setting import set_instance_setting +from posthog.tasks.alerts.checks import send_notifications, check_alert +from posthog.test.base import APIBaseTest, _create_event, flush_persons_and_events, ClickhouseDestroyTablesMixin +from posthog.api.test.dashboards import DashboardAPI +from posthog.schema import ChartDisplayType, EventsNode, TrendsQuery, TrendsFilter +from posthog.tasks.test.utils_email_tests import mock_email_messages +from posthog.models import Alert + + +@freeze_time("2024-06-02T08:55:00.000Z") +@patch("posthog.tasks.alerts.checks.send_notifications") +class TestCheckAlertsTasks(APIBaseTest, ClickhouseDestroyTablesMixin): + def setUp(self) -> None: + super().setUp() + set_instance_setting("EMAIL_HOST", "fake_host") + set_instance_setting("EMAIL_ENABLED", True) + self.dashboard_api = DashboardAPI(self.client, self.team, self.assertEqual) + query_dict = TrendsQuery( + series=[ + EventsNode( + event="$pageview", + ), + ], + trendsFilter=TrendsFilter(display=ChartDisplayType.BOLD_NUMBER), + ).model_dump() + self.insight = self.dashboard_api.create_insight( + data={ + "name": "insight", + "query": query_dict, + } + )[1] + + self.alert = self.client.post( + f"/api/projects/{self.team.id}/alerts", + data={ + "name": "alert name", + "insight": self.insight["id"], + "target_value": "a@b.c,d@e.f", + "anomaly_condition": {"absoluteThreshold": {}}, + }, + ).json() + + def set_thresholds(self, lower: Optional[int] = None, upper: Optional[int] = None) -> None: + self.client.patch( + f"/api/projects/{self.team.id}/alerts/{self.alert['id']}", + data={"anomaly_condition": {"absoluteThreshold": {"lower": lower, "upper": upper}}}, + ) + + def get_anomalies_descriptions(self, mock_send_notifications: MagicMock, call_index: int) -> list[str]: + return mock_send_notifications.call_args_list[call_index].args[1] + + def test_alert_is_triggered_for_values_above_higher_threshold(self, mock_send_notifications: MagicMock) -> None: + self.set_thresholds(upper=0) + + with freeze_time("2024-06-02T07:55:00.000Z"): + _create_event( + team=self.team, + event="$pageview", + distinct_id="1", + ) + flush_persons_and_events() + + check_alert(self.alert["id"]) + + assert mock_send_notifications.call_count == 1 + alert = mock_send_notifications.call_args_list[0].args[0] + assert alert.id == self.alert["id"] + + anomalies_descriptions = self.get_anomalies_descriptions(mock_send_notifications, call_index=0) + assert len(anomalies_descriptions) == 1 + assert "The trend value (1) is above the upper threshold (0.0)" in anomalies_descriptions[0] + + def test_alert_is_not_triggered_for_events_beyond_interval(self, mock_send_notifications: MagicMock) -> None: + self.set_thresholds(upper=0) + + with freeze_time("2024-05-02T07:55:00.000Z"): + _create_event( + team=self.team, + event="$pageview", + distinct_id="1", + ) + flush_persons_and_events() + + check_alert(self.alert["id"]) + + assert mock_send_notifications.call_count == 0 + + def test_alert_is_triggered_for_value_below_lower_threshold(self, mock_send_notifications: MagicMock) -> None: + self.set_thresholds(lower=1) + + check_alert(self.alert["id"]) + + assert mock_send_notifications.call_count == 1 + anomalies = self.get_anomalies_descriptions(mock_send_notifications, call_index=0) + assert "The trend value (0) is below the lower threshold (1.0)" in anomalies + + def test_alert_is_not_triggered_for_normal_values(self, mock_send_notifications: MagicMock) -> None: + self.set_thresholds(lower=0, upper=1) + + check_alert(self.alert["id"]) + + assert mock_send_notifications.call_count == 0 + + def test_error_while_calculating_no_alert(self, mock_send_notifications: MagicMock) -> None: + query_dict = TrendsQuery( + series=[ + EventsNode( + event="$pageview", + ), + ], + # This query is not represented as a bold number, so calculating the insight aggregated value + # causes an error + ).model_dump() + insight = self.dashboard_api.create_insight( + data={ + "name": "insight", + "query": query_dict, + } + )[1] + + self.client.patch(f"/api/projects/{self.team.id}/alerts/{self.alert['id']}", data={"insight": insight["id"]}) + + with pytest.raises(KeyError): + check_alert(self.alert["id"]) + assert mock_send_notifications.call_count == 0 + + def test_alert_with_insight_with_filter(self, mock_send_notifications: MagicMock) -> None: + insight = self.dashboard_api.create_insight( + data={"name": "insight", "filters": {"events": [{"id": "$pageview"}], "display": "BoldNumber"}} + )[1] + + self.client.patch(f"/api/projects/{self.team.id}/alerts/{self.alert['id']}", data={"insight": insight["id"]}) + self.set_thresholds(lower=1) + + check_alert(self.alert["id"]) + + assert mock_send_notifications.call_count == 1 + anomalies = self.get_anomalies_descriptions(mock_send_notifications, call_index=0) + assert "The trend value (0) is below the lower threshold (1.0)" in anomalies + + @patch("posthog.tasks.alerts.checks.EmailMessage") + def test_send_emails(self, MockEmailMessage: MagicMock, mock_send_notifications: MagicMock) -> None: + mocked_email_messages = mock_email_messages(MockEmailMessage) + alert = Alert.objects.get(pk=self.alert["id"]) + send_notifications(alert, ["first anomaly description", "second anomaly description"]) + + assert len(mocked_email_messages) == 1 + email = mocked_email_messages[0] + assert len(email.to) == 2 + assert email.to[0]["recipient"] == "a@b.c" + assert email.to[1]["recipient"] == "d@e.f" + assert "first anomaly description" in email.html_body + assert "second anomaly description" in email.html_body diff --git a/posthog/tasks/scheduled.py b/posthog/tasks/scheduled.py index d94dadf0c6998..c0ff2a468391b 100644 --- a/posthog/tasks/scheduled.py +++ b/posthog/tasks/scheduled.py @@ -8,6 +8,7 @@ from posthog.caching.warming import schedule_warming_for_teams_task from posthog.celery import app +from posthog.tasks.alerts.checks import check_all_alerts_task from posthog.tasks.integrations import refresh_integrations from posthog.tasks.tasks import ( calculate_cohort, @@ -259,6 +260,12 @@ def setup_periodic_tasks(sender: Celery, **kwargs: Any) -> None: name="update survey iteration based on date", ) + sender.add_periodic_task( + crontab(hour="*", minute="20"), + check_all_alerts_task.s(), + name="detect alerts' anomalies and notify about them", + ) + if settings.EE_AVAILABLE: # every interval seconds, we calculate N replay embeddings # the goal is to process _enough_ every 24 hours that diff --git a/posthog/templates/email/alert_anomaly.html b/posthog/templates/email/alert_anomaly.html new file mode 100644 index 0000000000000..49636488288dc --- /dev/null +++ b/posthog/templates/email/alert_anomaly.html @@ -0,0 +1,10 @@ +{% extends "email/base.html" %} {% load posthog_assets %} {% block section %} +

+ The {{ alert_name }} alert detected following anomalies for {{ insight_name }}: +

    + {% for anomaly_description in anomalies_descriptions %} +
  • {{ anomaly_description }}
    • + {% endfor %} +
+

+{% endblock %}{% load posthog_filters %}