diff --git a/cypress/e2e/alerts.cy.ts b/cypress/e2e/alerts.cy.ts index 6f0821fa030..82bd6bc10f4 100644 --- a/cypress/e2e/alerts.cy.ts +++ b/cypress/e2e/alerts.cy.ts @@ -1,5 +1,5 @@ import { decideResponse } from '../fixtures/api/decide' -import { createInsight } from '../productAnalytics' +import { createInsight, createInsightWithBreakdown } from '../productAnalytics' describe('Alerts', () => { beforeEach(() => { @@ -115,4 +115,25 @@ describe('Alerts', () => { cy.reload() cy.contains('Alert name').should('not.exist') }) + + it('Should allow creating alerts on trends with breakdowns', () => { + createInsightWithBreakdown('insight with breakdown') + setInsightDisplayTypeAndSave('Bar chart') + + createAlert('Alert name', '10', '20', 'increases by') + cy.reload() + + // Check the alert has the same values as when it was created + cy.contains('Alerts').click() + cy.get('[data-attr=alert-list-item]').contains('Alert name').click() + cy.contains('any breakdown value').should('exist') + cy.get('[data-attr=alertForm-name]').should('have.value', 'Alert name') + cy.get('[data-attr=alertForm-lower-threshold').should('have.value', '10') + cy.get('[data-attr=alertForm-upper-threshold').should('have.value', '20') + cy.contains('Delete alert').click() + cy.wait(2000) + + cy.reload() + cy.contains('Alert name').should('not.exist') + }) }) diff --git a/cypress/productAnalytics/index.ts b/cypress/productAnalytics/index.ts index 0fc99720141..5bfdeae7813 100644 --- a/cypress/productAnalytics/index.ts +++ b/cypress/productAnalytics/index.ts @@ -29,6 +29,11 @@ export const insight = { } }) }, + applyBreakdown: (): void => { + cy.contains('Add breakdown').click() + cy.contains('Browser').click() + cy.wait(1000) + }, editName: (insightName: string): void => { if (insightName) { cy.get('[data-attr="top-bar-name"] button').click() @@ -207,6 +212,17 @@ export function createInsight(insightName: string): Cypress.Chainable { }) } +export function createInsightWithBreakdown(insightName: string): Cypress.Chainable { + savedInsights.createNewInsightOfType('TRENDS') + insight.applyBreakdown() + insight.editName(insightName) + insight.save() + // return insight id from the url + return cy.url().then((url) => { + return url.split('/').at(-1) + }) +} + export function duplicateDashboardFromMenu(duplicateTiles = false): void { cy.contains('.LemonButton', 'Duplicate').click() if (duplicateTiles) { diff --git a/frontend/src/lib/components/Alerts/AlertsButton.tsx b/frontend/src/lib/components/Alerts/AlertsButton.tsx index db228d81dbe..a04b05678a7 100644 --- a/frontend/src/lib/components/Alerts/AlertsButton.tsx +++ b/frontend/src/lib/components/Alerts/AlertsButton.tsx @@ -35,7 +35,7 @@ export function AlertsButton({ insight, insightLogicProps, text, ...props }: Ale onClick={() => push(urls.insightAlerts(insight.short_id!))} disabledReason={ !areAlertsSupportedForInsight(insight.query) - ? 'Alerts are only available for trends without breakdowns. Change the insight representation to add alerts.' + ? 'Alerts are only available for trends. Change the insight representation to add alerts.' : undefined } {...props} diff --git a/frontend/src/lib/components/Alerts/insightAlertsLogic.ts b/frontend/src/lib/components/Alerts/insightAlertsLogic.ts index f95c941eb38..8892be4d745 100644 --- a/frontend/src/lib/components/Alerts/insightAlertsLogic.ts +++ b/frontend/src/lib/components/Alerts/insightAlertsLogic.ts @@ -4,7 +4,7 @@ import api from 'lib/api' import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' import { AlertConditionType, GoalLine, InsightThresholdType } from '~/queries/schema' -import { getBreakdown, isInsightVizNode, isTrendsQuery } from '~/queries/utils' +import { isInsightVizNode, isTrendsQuery } from '~/queries/utils' import { InsightLogicProps } from '~/types' import type { insightAlertsLogicType } from './insightAlertsLogicType' @@ -16,13 +16,7 @@ export interface InsightAlertsLogicProps { } export const areAlertsSupportedForInsight = (query?: Record | null): boolean => { - return ( - !!query && - isInsightVizNode(query) && - isTrendsQuery(query.source) && - query.source.trendsFilter !== null && - !getBreakdown(query.source) - ) + return !!query && isInsightVizNode(query) && isTrendsQuery(query.source) && query.source.trendsFilter !== null } export const insightAlertsLogic = kea([ diff --git a/frontend/src/lib/components/Alerts/views/EditAlertModal.tsx b/frontend/src/lib/components/Alerts/views/EditAlertModal.tsx index b3c63ea6973..404bc5e213a 100644 --- a/frontend/src/lib/components/Alerts/views/EditAlertModal.tsx +++ b/frontend/src/lib/components/Alerts/views/EditAlertModal.tsx @@ -1,4 +1,11 @@ -import { LemonCheckbox, LemonInput, LemonSegmentedButton, LemonSelect, SpinnerOverlay } from '@posthog/lemon-ui' +import { + LemonBanner, + LemonCheckbox, + LemonInput, + LemonSegmentedButton, + LemonSelect, + SpinnerOverlay, +} from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { Form, Group } from 'kea-forms' import { AlertStateIndicator } from 'lib/components/Alerts/views/ManageAlertsModal' @@ -85,7 +92,7 @@ export function EditAlertModal({ const { setAlertFormValue } = useActions(formLogic) const trendsLogic = trendsDataLogic({ dashboardItemId: insightShortId }) - const { alertSeries, isNonTimeSeriesDisplay } = useValues(trendsLogic) + const { alertSeries, isNonTimeSeriesDisplay, isBreakdownValid } = useValues(trendsLogic) const creatingNewAlert = alertForm.id === undefined @@ -138,6 +145,12 @@ export function EditAlertModal({

Definition

+ {isBreakdownValid && ( + + For trends with breakdown, the alert will fire if any of the breakdown + values breaches the threshold. + + )}
When
@@ -146,9 +159,16 @@ export function EditAlertModal({ fullWidth data-attr="alertForm-series-index" options={alertSeries?.map(({ event }, index) => ({ - label: `${alphabet[index]} - ${event}`, + label: isBreakdownValid + ? 'any breakdown value' + : `${alphabet[index]} - ${event}`, value: index, }))} + disabledReason={ + isBreakdownValid && + `For trends with breakdown, the alert will fire if any of the breakdown + values breaches the threshold.` + } /> diff --git a/frontend/src/lib/components/Alerts/views/ManageAlertsModal.tsx b/frontend/src/lib/components/Alerts/views/ManageAlertsModal.tsx index 6d93d94f840..6c8b2ad4ae2 100644 --- a/frontend/src/lib/components/Alerts/views/ManageAlertsModal.tsx +++ b/frontend/src/lib/components/Alerts/views/ManageAlertsModal.tsx @@ -1,4 +1,5 @@ import { IconCheck, IconX } from '@posthog/icons' +import { Link } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { router } from 'kea-router' import { LemonButton } from 'lib/lemon-ui/LemonButton' @@ -85,8 +86,12 @@ export function ManageAlertsModal(props: ManageAlertsModalProps): JSX.Element {
With alerts, PostHog will monitor your insight and notify you when certain conditions are met. We do not evaluate alerts in real-time, but rather on a schedule (hourly, daily...). Please note that - alerts are in alpha and may not be fully reliable. + alerts are in alpha and may not be fully reliable.
+ + View all your alerts here +
+ {alerts.length ? (
diff --git a/frontend/src/scenes/trends/trendsDataLogic.ts b/frontend/src/scenes/trends/trendsDataLogic.ts index ceef893e621..0b2bea5c105 100644 --- a/frontend/src/scenes/trends/trendsDataLogic.ts +++ b/frontend/src/scenes/trends/trendsDataLogic.ts @@ -9,7 +9,16 @@ import { BREAKDOWN_OTHER_STRING_LABEL, } from 'scenes/insights/utils' -import { EventsNode, InsightQueryNode, LifecycleQuery, MathType, TrendsFilter, TrendsQuery } from '~/queries/schema' +import { + BreakdownFilter, + EventsNode, + InsightQueryNode, + LifecycleQuery, + MathType, + TrendsFilter, + TrendsQuery, +} from '~/queries/schema' +import { isValidBreakdown } from '~/queries/utils' import { ChartDisplayType, CountPerActorMathType, @@ -124,6 +133,11 @@ export const trendsDataLogic = kea([ }, ], + isBreakdownValid: [ + (s) => [s.breakdownFilter], + (breakdownFilter: BreakdownFilter | null) => isValidBreakdown(breakdownFilter), + ], + indexedResults: [ (s) => [s.results, s.display, s.lifecycleFilter], (results, display, lifecycleFilter): IndexedTrendResult[] => { diff --git a/posthog/tasks/alerts/test/test_alert_checks.py b/posthog/tasks/alerts/test/test_alert_checks.py index 1ac728f00f5..617113b79e4 100644 --- a/posthog/tasks/alerts/test/test_alert_checks.py +++ b/posthog/tasks/alerts/test/test_alert_checks.py @@ -99,7 +99,10 @@ 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 for previous day (1) is more than upper threshold (0.0)" in anomalies_descriptions[0] + assert ( + "The insight value ($pageview) for previous day (1) is more than upper threshold (0.0)" + in anomalies_descriptions[0] + ) def test_alert_is_not_triggered_for_events_beyond_interval( self, mock_send_notifications_for_breaches: MagicMock, mock_send_errors: MagicMock @@ -127,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 for previous day (0) is less than lower threshold (1.0)" in anomalies + assert "The insight value ($pageview) for previous day (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 @@ -315,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 for previous day (0) is less than lower threshold (1.0)" in anomalies + assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies @patch("posthog.tasks.alerts.utils.EmailMessage") def test_send_emails( diff --git a/posthog/tasks/alerts/test/test_trends_absolute_alerts.py b/posthog/tasks/alerts/test/test_trends_absolute_alerts.py index 3dddf37a001..22ff95bb681 100644 --- a/posthog/tasks/alerts/test/test_trends_absolute_alerts.py +++ b/posthog/tasks/alerts/test/test_trends_absolute_alerts.py @@ -16,12 +16,11 @@ from posthog.schema import ( TrendsFilter, IntervalType, InsightDateRange, - EventPropertyFilter, - PropertyOperator, BaseMathType, AlertState, AlertCalculationInterval, BreakdownFilter, + Breakdown, ) from posthog.models import AlertConfiguration @@ -67,13 +66,6 @@ class TestTimeSeriesTrendsAbsoluteAlerts(APIBaseTest, ClickhouseDestroyTablesMix EventsNode( event="signed_up", math=BaseMathType.TOTAL, - properties=[ - EventPropertyFilter( - key="$browser", - operator=PropertyOperator.EXACT, - value=["Chrome"], - ) - ], ), EventsNode( event="$pageview", @@ -119,7 +111,7 @@ class TestTimeSeriesTrendsAbsoluteAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_once_with( - ANY, ["The insight value for previous week (0) is less than lower threshold (1.0)"] + ANY, ["The insight value (signed_up) for previous week (0) is less than lower threshold (1.0)"] ) def test_trend_high_threshold_breached(self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock) -> None: @@ -153,7 +145,7 @@ class TestTimeSeriesTrendsAbsoluteAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_once_with( - ANY, ["The insight value for previous week (2) is more than upper threshold (1.0)"] + ANY, ["The insight value (signed_up) for previous week (2) is more than upper threshold (1.0)"] ) def test_trend_no_threshold_breached(self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock) -> None: @@ -179,3 +171,127 @@ class TestTimeSeriesTrendsAbsoluteAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.calculated_value == 1 assert alert_check.state == AlertState.NOT_FIRING assert alert_check.error is None + + def test_trend_breakdown_high_threshold_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_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(days=1)): + _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 week (2.0) is more than upper threshold (1.0)"] + ) + + def test_trend_breakdown_low_threshold_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight(BreakdownFilter(breakdowns=[Breakdown(property="$browser")])) + alert = self.create_alert(insight, series_index=0, lower=2) + + with freeze_time(FROZEN_TIME - dateutil.relativedelta.relativedelta(days=1)): + _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 == 1 + 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 - Firefox) for previous week (1.0) is less than lower threshold (2.0)"] + ) + + def test_trend_breakdown_no_threshold_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight(BreakdownFilter(breakdowns=[Breakdown(property="$browser")])) + alert = self.create_alert(insight, series_index=0, lower=1) + + with freeze_time(FROZEN_TIME - dateutil.relativedelta.relativedelta(days=1)): + _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.NOT_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 is None + assert alert_check.state == AlertState.NOT_FIRING + assert alert_check.error is None + + mock_send_breaches.assert_not_called() diff --git a/posthog/tasks/alerts/test/test_trends_relative_alerts.py b/posthog/tasks/alerts/test/test_trends_relative_alerts.py index 67d78ae4335..de530d3817f 100644 --- a/posthog/tasks/alerts/test/test_trends_relative_alerts.py +++ b/posthog/tasks/alerts/test/test_trends_relative_alerts.py @@ -1,5 +1,5 @@ from typing import Optional, Any -from unittest.mock import ANY, MagicMock, patch +from unittest.mock import ANY, call, MagicMock, patch import dateutil @@ -18,14 +18,13 @@ from posthog.schema import ( TrendsFilter, IntervalType, InsightDateRange, - EventPropertyFilter, - PropertyOperator, BaseMathType, AlertState, AlertCalculationInterval, AlertConditionType, InsightThresholdType, BreakdownFilter, + Breakdown, ) from posthog.models import AlertConfiguration @@ -80,13 +79,6 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix EventsNode( event="signed_up", math=BaseMathType.TOTAL, - properties=[ - EventPropertyFilter( - key="$browser", - operator=PropertyOperator.EXACT, - value=["Chrome"], - ) - ], ), EventsNode( event="$pageview", @@ -185,7 +177,7 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_once_with( - ANY, ["The insight value for previous week (2) increased more than upper threshold (1.0)"] + ANY, ["The insight value (signed_up) for previous week (2) increased more than upper threshold (1.0)"] ) def test_relative_increase_upper_threshold_breached( @@ -346,7 +338,7 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_once_with( - ANY, ["The insight value for previous week (-1) increased less than lower threshold (2.0)"] + ANY, ["The insight value (signed_up) for previous week (-1) increased less than lower threshold (2.0)"] ) # check percentage alert @@ -363,7 +355,8 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_with( - ANY, ["The insight value for previous week (-50.00%) increased less than lower threshold (50.00%)"] + ANY, + ["The insight value (signed_up) for previous week (-50.00%) increased less than lower threshold (50.00%)"], ) def test_relative_increase_lower_threshold_breached_2( @@ -524,7 +517,7 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_once_with( - ANY, ["The insight value for previous week (2) decreased more than upper threshold (1.0)"] + ANY, ["The insight value (signed_up) for previous week (2) decreased more than upper threshold (1.0)"] ) check_alert(percentage_alert["id"]) @@ -540,7 +533,8 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_with( - ANY, ["The insight value for previous week (66.67%) decreased more than upper threshold (20.00%)"] + ANY, + ["The insight value (signed_up) for previous week (66.67%) decreased more than upper threshold (20.00%)"], ) def test_relative_decrease_lower_threshold_breached( @@ -613,7 +607,7 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_once_with( - ANY, ["The insight value for previous week (1) decreased less than lower threshold (2.0)"] + ANY, ["The insight value (signed_up) for previous week (1) decreased less than lower threshold (2.0)"] ) check_alert(percentage_alert["id"]) @@ -629,7 +623,8 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.error is None mock_send_breaches.assert_called_with( - ANY, ["The insight value for previous week (50.00%) decreased less than lower threshold (80.00%)"] + ANY, + ["The insight value (signed_up) for previous week (50.00%) decreased less than lower threshold (80.00%)"], ) def test_relative_increase_no_threshold_breached( @@ -801,3 +796,660 @@ class TestTimeSeriesTrendsRelativeAlerts(APIBaseTest, ClickhouseDestroyTablesMix assert alert_check.calculated_value == (2 / 3) assert alert_check.state == AlertState.NOT_FIRING assert alert_check.error is None + + def test_breakdown_relative_increase_upper_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight( + interval=IntervalType.WEEK, breakdown=BreakdownFilter(breakdowns=[Breakdown(property="$browser")]) + ) + + # alert if sign ups increase by more than 1 + absolute_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_INCREASE, + threshold_type=InsightThresholdType.ABSOLUTE, + upper=1, + ) + + # alert if sign ups increase by more than 20% + percentage_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_INCREASE, + threshold_type=InsightThresholdType.PERCENTAGE, + upper=0.2, + ) + + # FROZEN_TIME is on Tue, insight has weekly interval + # we aggregate our weekly insight numbers to display for Sun (19th May, 26th May, 2nd June) + + # set previous to previous interval (last to last week) to have 1 event + last_to_last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=2) + + with freeze_time(last_to_last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="1", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # set previous interval to have 2 event + # add events for last week (last Tue) + last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=1) + with freeze_time(last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="2", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="3", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="4", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # alert should fire as we had *increase* in events of (2 or 200%) week over week + check_alert(absolute_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=absolute_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=absolute_alert["id"]).latest("created_at") + + assert alert_check.calculated_value == 2 + assert alert_check.state == AlertState.FIRING + assert alert_check.error is None + + check_alert(percentage_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=percentage_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=percentage_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_has_calls( + [ + call( + ANY, + [ + "The insight value (signed_up - Chrome) for previous week (2.0) increased more than upper threshold (1.0)" + ], + ), + call( + ANY, + [ + "The insight value (signed_up - Chrome) for previous week (200.00%) increased more than upper threshold (20.00%)" + ], + ), + ] + ) + + def test_breakdown_relative_increase_lower_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight( + interval=IntervalType.WEEK, breakdown=BreakdownFilter(breakdowns=[Breakdown(property="$browser")]) + ) + + # alert if sign ups don't increase by more than 1 + absolute_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_INCREASE, + threshold_type=InsightThresholdType.ABSOLUTE, + lower=1, + ) + + # alert if sign ups don't increase by more than 20% + percentage_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_INCREASE, + threshold_type=InsightThresholdType.PERCENTAGE, + lower=0.2, + ) + + # FROZEN_TIME is on Tue, insight has weekly interval + # we aggregate our weekly insight numbers to display for Sun (19th May, 26th May, 2nd June) + + # set previous to previous interval (last to last week) to have 1 event + last_to_last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=2) + + with freeze_time(last_to_last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="1", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # set previous interval to have 2 event + # add events for last week (last Tue) + last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=1) + with freeze_time(last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="2", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="3", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="4", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # alert should fire as we had *increase* in events of (2 or 200%) week over week + check_alert(absolute_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=absolute_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=absolute_alert["id"]).latest("created_at") + + assert alert_check.calculated_value == 0 + assert alert_check.state == AlertState.FIRING + assert alert_check.error is None + + check_alert(percentage_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=percentage_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=percentage_alert["id"]).latest("created_at") + + assert alert_check.calculated_value == 0 + assert alert_check.state == AlertState.FIRING + assert alert_check.error is None + + mock_send_breaches.assert_has_calls( + [ + call( + ANY, + [ + "The insight value (signed_up - Firefox) for previous week (0.0) increased less than lower threshold (1.0)" + ], + ), + call( + ANY, + [ + "The insight value (signed_up - Firefox) for previous week (0.00%) increased less than lower threshold (20.00%)" + ], + ), + ] + ) + + def test_breakdown_relative_decrease_lower_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight( + interval=IntervalType.WEEK, breakdown=BreakdownFilter(breakdowns=[Breakdown(property="$browser")]) + ) + + # alert if sign ups decrease by less than 1 + absolute_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_DECREASE, + threshold_type=InsightThresholdType.ABSOLUTE, + lower=1, + ) + + # alert if sign ups decrease by less than 20% + percentage_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_DECREASE, + threshold_type=InsightThresholdType.PERCENTAGE, + lower=0.2, + ) + + # FROZEN_TIME is on Tue, insight has weekly interval + # we aggregate our weekly insight numbers to display for Sun (19th May, 26th May, 2nd June) + + # set previous to previous interval (last to last week) to have 1 event + last_to_last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=2) + + with freeze_time(last_to_last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="1", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # set previous interval to have 2 event + # add events for last week (last Tue) + last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=1) + with freeze_time(last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="2", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="3", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="4", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # alert should fire as we had *increase* in events of (2 or 200%) week over week + check_alert(absolute_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=absolute_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=absolute_alert["id"]).latest("created_at") + + assert alert_check.calculated_value == -2 + assert alert_check.state == AlertState.FIRING + assert alert_check.error is None + + check_alert(percentage_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=percentage_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=percentage_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_has_calls( + [ + call( + ANY, + [ + "The insight value (signed_up - Chrome) for previous week (-2.0) decreased less than lower threshold (1.0)" + ], + ), + call( + ANY, + [ + "The insight value (signed_up - Chrome) for previous week (-200.00%) decreased less than lower threshold (20.00%)" + ], + ), + ] + ) + + def test_breakdown_relative_decrease_upper_breached( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight( + interval=IntervalType.WEEK, breakdown=BreakdownFilter(breakdowns=[Breakdown(property="$browser")]) + ) + + # alert if sign ups decrease by more than 1 + absolute_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_DECREASE, + threshold_type=InsightThresholdType.ABSOLUTE, + upper=1, + ) + + # alert if sign ups decrease by more than 20% + percentage_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_DECREASE, + threshold_type=InsightThresholdType.PERCENTAGE, + upper=0.2, + ) + + # FROZEN_TIME is on Tue, insight has weekly interval + # we aggregate our weekly insight numbers to display for Sun (19th May, 26th May, 2nd June) + + # set previous to previous interval (last to last week) to have 1 event + last_to_last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=2) + + with freeze_time(last_to_last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + 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"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="3", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # set previous interval to have 2 event + # add events for last week (last Tue) + last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=1) + with freeze_time(last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + + _create_event( + team=self.team, + event="signed_up", + distinct_id="4", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # alert should fire as we had *increase* in events of (2 or 200%) week over week + check_alert(absolute_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=absolute_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=absolute_alert["id"]).latest("created_at") + + assert alert_check.calculated_value == 2 + assert alert_check.state == AlertState.FIRING + assert alert_check.error is None + + check_alert(percentage_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=percentage_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=percentage_alert["id"]).latest("created_at") + + assert alert_check.calculated_value == (2 / 3) + assert alert_check.state == AlertState.FIRING + assert alert_check.error is None + + mock_send_breaches.assert_has_calls( + [ + call( + ANY, + [ + "The insight value (signed_up - Chrome) for previous week (2.0) decreased more than upper threshold (1.0)" + ], + ), + call( + ANY, + [ + "The insight value (signed_up - Chrome) for previous week (66.67%) decreased more than upper threshold (20.00%)" + ], + ), + ] + ) + + def test_breakdown_relative_decrease_no_breaches( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight( + interval=IntervalType.WEEK, breakdown=BreakdownFilter(breakdowns=[Breakdown(property="$browser")]) + ) + + # alert if sign ups decrease by more than 1 + absolute_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_DECREASE, + threshold_type=InsightThresholdType.ABSOLUTE, + upper=1, + ) + + # alert if sign ups decrease by more than 20% + percentage_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_DECREASE, + threshold_type=InsightThresholdType.PERCENTAGE, + upper=0.2, + ) + + # FROZEN_TIME is on Tue, insight has weekly interval + # we aggregate our weekly insight numbers to display for Sun (19th May, 26th May, 2nd June) + + # set previous to previous interval (last to last week) to have 1 event + last_to_last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=2) + + with freeze_time(last_to_last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="1", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # set previous interval to have 2 event + # add events for last week (last Tue) + last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=1) + with freeze_time(last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + + _create_event( + team=self.team, + event="signed_up", + distinct_id="4", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # alert should fire as we had *increase* in events of (2 or 200%) week over week + check_alert(absolute_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=absolute_alert["id"]) + assert updated_alert.state == AlertState.NOT_FIRING + assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1) + + alert_check = AlertCheck.objects.filter(alert_configuration=absolute_alert["id"]).latest("created_at") + + assert alert_check.calculated_value is None + assert alert_check.state == AlertState.NOT_FIRING + assert alert_check.error is None + + check_alert(percentage_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=percentage_alert["id"]) + assert updated_alert.state == AlertState.NOT_FIRING + assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1) + + alert_check = AlertCheck.objects.filter(alert_configuration=percentage_alert["id"]).latest("created_at") + + assert alert_check.calculated_value is None + assert alert_check.state == AlertState.NOT_FIRING + assert alert_check.error is None + + mock_send_breaches.assert_not_called() + + def test_breakdown_relative_increase_no_breaches( + self, mock_send_breaches: MagicMock, mock_send_errors: MagicMock + ) -> None: + insight = self.create_time_series_trend_insight( + interval=IntervalType.WEEK, breakdown=BreakdownFilter(breakdowns=[Breakdown(property="$browser")]) + ) + + # alert if sign ups decrease by more than 1 + absolute_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_INCREASE, + threshold_type=InsightThresholdType.ABSOLUTE, + upper=1, + ) + + # alert if sign ups decrease by more than 20% + percentage_alert = self.create_alert( + insight, + series_index=0, + condition_type=AlertConditionType.RELATIVE_INCREASE, + threshold_type=InsightThresholdType.PERCENTAGE, + upper=0.2, + ) + + # FROZEN_TIME is on Tue, insight has weekly interval + # we aggregate our weekly insight numbers to display for Sun (19th May, 26th May, 2nd June) + + # set previous to previous interval (last to last week) to have 1 event + last_to_last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=2) + + with freeze_time(last_to_last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="signed_up", + distinct_id="1", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # set previous interval to have 2 event + # add events for last week (last Tue) + last_tue = FROZEN_TIME - dateutil.relativedelta.relativedelta(weeks=1) + with freeze_time(last_tue): + _create_event( + team=self.team, + event="signed_up", + distinct_id="11", + properties={"$browser": "Firefox"}, + ) + + _create_event( + team=self.team, + event="signed_up", + distinct_id="4", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() + + # alert should fire as we had *increase* in events of (2 or 200%) week over week + check_alert(absolute_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=absolute_alert["id"]) + assert updated_alert.state == AlertState.NOT_FIRING + assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1) + + alert_check = AlertCheck.objects.filter(alert_configuration=absolute_alert["id"]).latest("created_at") + + assert alert_check.calculated_value is None + assert alert_check.state == AlertState.NOT_FIRING + assert alert_check.error is None + + check_alert(percentage_alert["id"]) + + updated_alert = AlertConfiguration.objects.get(pk=percentage_alert["id"]) + assert updated_alert.state == AlertState.NOT_FIRING + assert updated_alert.next_check_at == FROZEN_TIME + dateutil.relativedelta.relativedelta(days=1) + + alert_check = AlertCheck.objects.filter(alert_configuration=percentage_alert["id"]).latest("created_at") + + assert alert_check.calculated_value is None + assert alert_check.state == AlertState.NOT_FIRING + assert alert_check.error is None + + mock_send_breaches.assert_not_called() diff --git a/posthog/tasks/alerts/trends.py b/posthog/tasks/alerts/trends.py index 30471425955..7a87c4d54a4 100644 --- a/posthog/tasks/alerts/trends.py +++ b/posthog/tasks/alerts/trends.py @@ -54,6 +54,10 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend if not threshold: return AlertEvaluationResult(value=0, breaches=[]) + has_breakdown = query.breakdownFilter and ( + (query.breakdownFilter.breakdown and query.breakdownFilter.breakdown_type) or query.breakdownFilter.breakdowns + ) + match condition.type: case AlertConditionType.ABSOLUTE_VALUE: if threshold.type != InsightThresholdType.ABSOLUTE: @@ -79,13 +83,42 @@ 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}") - prev_interval_value = _pick_interval_value_from_trend_result(config, query, calculation_result, -1) - breaches = _validate_bounds( - threshold.bounds, prev_interval_value, threshold.type, condition.type, query.interval - ) + if has_breakdown: + # for breakdowns, we need to check all values in calculation_result.result + breakdown_results = calculation_result.result - return AlertEvaluationResult(value=prev_interval_value, breaches=breaches) + for breakdown_result in breakdown_results: + 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, + breakdown_result["label"], + ) + if breaches: + # found one breakdown value that breached the threshold + return AlertEvaluationResult(value=prev_interval_value, breaches=breaches) + + # None of the breakdown values breached the threshold + return AlertEvaluationResult(value=None, breaches=[]) + else: + # for non breakdowns, we pick the series (config.series_index) from calculation_result.result + selected_series_result = _pick_series_result(config, calculation_result) + + prev_interval_value = _pick_interval_value_from_trend_result(query, selected_series_result, -1) + breaches = _validate_bounds( + threshold.bounds, + prev_interval_value, + threshold.type, + condition.type, + query.interval, + selected_series_result["label"], + ) + + return AlertEvaluationResult(value=prev_interval_value, breaches=breaches) case AlertConditionType.RELATIVE_INCREASE: if _is_non_time_series_trend(query): raise ValueError(f"Relative alerts not supported for non time series trends") @@ -104,21 +137,55 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend filters_override=filters_overrides, ) - prev_interval_value = _pick_interval_value_from_trend_result(config, query, calculation_result, -1) - prev_prev_interval_value = _pick_interval_value_from_trend_result(config, query, calculation_result, -2) + results_to_evaluate = [] - if threshold.type == InsightThresholdType.ABSOLUTE: - increase = prev_interval_value - prev_prev_interval_value - breaches = _validate_bounds(threshold.bounds, increase, threshold.type, condition.type, query.interval) - elif threshold.type == InsightThresholdType.PERCENTAGE: - increase = (prev_interval_value - prev_prev_interval_value) / prev_prev_interval_value - breaches = _validate_bounds(threshold.bounds, increase, threshold.type, condition.type, query.interval) + if has_breakdown: + # for breakdowns, we need to check all values in calculation_result.result + breakdown_results = calculation_result.result + results_to_evaluate.extend(breakdown_results) else: - raise ValueError( - f"Neither relative nor absolute threshold configured for alert condition RELATIVE_INCREASE" - ) + # for non breakdowns, we pick the series (config.series_index) from calculation_result.result + selected_series_result = _pick_series_result(config, calculation_result) + results_to_evaluate.append(selected_series_result) - return AlertEvaluationResult(value=increase, breaches=breaches) + # if we don't have breakdown, we'll have to evaluate just one result + # and increase will be the evaluated value of that result + increase = None + + for result in results_to_evaluate: + prev_interval_value = _pick_interval_value_from_trend_result(query, result, -1) + prev_prev_interval_value = _pick_interval_value_from_trend_result(query, result, -2) + + if threshold.type == InsightThresholdType.ABSOLUTE: + increase = prev_interval_value - prev_prev_interval_value + breaches = _validate_bounds( + threshold.bounds, + increase, + threshold.type, + condition.type, + query.interval, + result["label"], + ) + elif threshold.type == InsightThresholdType.PERCENTAGE: + increase = (prev_interval_value - prev_prev_interval_value) / prev_prev_interval_value + breaches = _validate_bounds( + threshold.bounds, + increase, + threshold.type, + condition.type, + query.interval, + result["label"], + ) + else: + raise ValueError( + f"Neither relative nor absolute threshold configured for alert condition RELATIVE_INCREASE" + ) + + if breaches: + # found a breach for one of the results so alert + return AlertEvaluationResult(value=increase, breaches=breaches) + + return AlertEvaluationResult(value=(increase if not has_breakdown else None), breaches=breaches) case AlertConditionType.RELATIVE_DECREASE: if _is_non_time_series_trend(query): @@ -138,21 +205,58 @@ def check_trends_alert(alert: AlertConfiguration, insight: Insight, query: Trend filters_override=filters_overrides, ) - prev_interval_value = _pick_interval_value_from_trend_result(config, query, calculation_result, -1) - prev_prev_interval_value = _pick_interval_value_from_trend_result(config, query, calculation_result, -2) + results_to_evaluate = [] - if threshold.type == InsightThresholdType.ABSOLUTE: - decrease = prev_prev_interval_value - prev_interval_value - breaches = _validate_bounds(threshold.bounds, decrease, threshold.type, condition.type, query.interval) - elif threshold.type == InsightThresholdType.PERCENTAGE: - decrease = (prev_prev_interval_value - prev_interval_value) / prev_prev_interval_value - breaches = _validate_bounds(threshold.bounds, decrease, threshold.type, condition.type, query.interval) + if has_breakdown: + # for breakdowns, we need to check all values in calculation_result.result + breakdown_results = calculation_result.result + results_to_evaluate.extend(breakdown_results) else: - raise ValueError( - f"Neither relative nor absolute threshold configured for alert condition RELATIVE_INCREASE" - ) + # for non breakdowns, we pick the series (config.series_index) from calculation_result.result + selected_series_result = _pick_series_result(config, calculation_result) + results_to_evaluate.append(selected_series_result) - return AlertEvaluationResult(value=decrease, breaches=breaches) + # for non breakdowns, we pick the series (config.series_index) from calculation_result.result + selected_series_result = _pick_series_result(config, calculation_result) + + # if we don't have breakdown, we'll have to evaluate just one result + # and increase will be the evaluated value of that result + decrease = None + + for result in results_to_evaluate: + prev_interval_value = _pick_interval_value_from_trend_result(query, result, -1) + prev_prev_interval_value = _pick_interval_value_from_trend_result(query, result, -2) + + if threshold.type == InsightThresholdType.ABSOLUTE: + decrease = prev_prev_interval_value - prev_interval_value + breaches = _validate_bounds( + threshold.bounds, + decrease, + threshold.type, + condition.type, + query.interval, + result["label"], + ) + elif threshold.type == InsightThresholdType.PERCENTAGE: + decrease = (prev_prev_interval_value - prev_interval_value) / prev_prev_interval_value + breaches = _validate_bounds( + threshold.bounds, + decrease, + threshold.type, + condition.type, + query.interval, + result["label"], + ) + else: + raise ValueError( + f"Neither relative nor absolute threshold configured for alert condition RELATIVE_INCREASE" + ) + + if breaches: + # found a breach for one of the results so alert + return AlertEvaluationResult(value=decrease, breaches=breaches) + + return AlertEvaluationResult(value=(decrease if not has_breakdown else None), breaches=breaches) case _: raise NotImplementedError(f"Unsupported alert condition type: {condition.type}") @@ -182,17 +286,19 @@ def _date_range_override_for_intervals(query: TrendsQuery, last_x_intervals: int return {"date_from": date_from} -def _pick_interval_value_from_trend_result( - config: TrendsAlertConfig, query: TrendsQuery, results: InsightResult, interval_to_pick: int = 0 -) -> float: +def _pick_series_result(config: TrendsAlertConfig, results: InsightResult) -> TrendResult: + series_index = config.series_index + result = cast(list[TrendResult], results.result)[series_index] + + return result + + +def _pick_interval_value_from_trend_result(query: TrendsQuery, result: TrendResult, interval_to_pick: int = 0) -> float: """ interval_to_pick to controls whether to pick value for current (0), last (-1), one before last (-2)... """ assert interval_to_pick <= 0 - series_index = config.series_index - result = cast(list[TrendResult], results.result)[series_index] - if _is_non_time_series_trend(query): # only one value in result return result["aggregated_value"] @@ -209,6 +315,7 @@ def _validate_bounds( threshold_type: InsightThresholdType, condition_type: AlertConditionType, interval_type: IntervalType | None, + series: str, ) -> list[str]: if not bounds: return [] @@ -228,12 +335,12 @@ def _validate_bounds( if bounds.lower is not None and calculated_value < bounds.lower: lower_value = f"{bounds.lower:.2%}" if is_percentage else bounds.lower return [ - f"The insight value for previous {interval_type or 'interval'} ({formatted_value}) {condition_text} less than lower threshold ({lower_value})" + f"The insight value ({series}) for previous {interval_type or 'interval'} ({formatted_value}) {condition_text} less than lower threshold ({lower_value})" ] if bounds.upper is not None and calculated_value > bounds.upper: upper_value = f"{bounds.upper:.2%}" if is_percentage else bounds.upper return [ - f"The insight value for previous {interval_type or 'interval'} ({formatted_value}) {condition_text} more than upper threshold ({upper_value})" + f"The insight value ({series}) for previous {interval_type or 'interval'} ({formatted_value}) {condition_text} more than upper threshold ({upper_value})" ] return []