From 7f6077e07fac1d9f27047c95826b58e84d7896b0 Mon Sep 17 00:00:00 2001 From: Roman Pertl Date: Sat, 25 Jun 2022 11:19:40 +0200 Subject: [PATCH 01/49] Fix Typo (HearBeat vs HeartBeat) --- engine/apps/base/models/live_setting.py | 2 +- .../metadata/heartbeat/_heartbeat_text_creator.py | 6 +++--- engine/apps/integrations/metadata/heartbeat/alertmanager.py | 4 ++-- engine/apps/integrations/metadata/heartbeat/elastalert.py | 4 ++-- .../integrations/metadata/heartbeat/formatted_webhook.py | 4 ++-- engine/apps/integrations/metadata/heartbeat/grafana.py | 4 ++-- engine/apps/integrations/metadata/heartbeat/prtg.py | 4 ++-- engine/apps/integrations/metadata/heartbeat/webhook.py | 4 ++-- engine/apps/integrations/metadata/heartbeat/zabbix.py | 4 ++-- 9 files changed, 18 insertions(+), 18 deletions(-) diff --git a/engine/apps/base/models/live_setting.py b/engine/apps/base/models/live_setting.py index 54a5299d..28d9cd68 100644 --- a/engine/apps/base/models/live_setting.py +++ b/engine/apps/base/models/live_setting.py @@ -118,7 +118,7 @@ class LiveSetting(models.Model): " source code." ), "GRAFANA_CLOUD_ONCALL_TOKEN": "Secret token for Grafana Cloud OnCall instance.", - "GRAFANA_CLOUD_ONCALL_HEARTBEAT_ENABLED": "Enable hearbeat integration with Grafana Cloud OnCall.", + "GRAFANA_CLOUD_ONCALL_HEARTBEAT_ENABLED": "Enable heartbeat integration with Grafana Cloud OnCall.", "GRAFANA_CLOUD_NOTIFICATIONS_ENABLED": "Enable SMS/call notifications via Grafana Cloud OnCall", } diff --git a/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py b/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py index 8e0e030c..84bd1235 100644 --- a/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py +++ b/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py @@ -8,12 +8,12 @@ from django.conf import settings class IntegrationHeartBeatText: heartbeat_expired_title: str = "heartbeat_expired" heartbeat_expired_message: str = "heartbeat_expired" - heartbeat_restored_title: str = "hearbeat_restored" + heartbeat_restored_title: str = "heartbeat_restored" heartbeat_restored_message: str = "heartbeat_restored" heartbeat_instruction_template: str = None -class HearBeatTextCreator: +class HeartBeatTextCreator: def __init__(self, integration_verbal): self.integration_verbal = integration_verbal.capitalize() @@ -52,7 +52,7 @@ class HearBeatTextCreator: return f"heartbeat_instructions/{self.integration_verbal.lower()}.html" -class HearBeatTextCreatorForTitleGrouping(HearBeatTextCreator): +class HeartBeatTextCreatorForTitleGrouping(HeartBeatTextCreator): """ Some integrations (Grafana, AlertManager) have default grouping template based on title """ diff --git a/engine/apps/integrations/metadata/heartbeat/alertmanager.py b/engine/apps/integrations/metadata/heartbeat/alertmanager.py index c0979787..e4935152 100644 --- a/engine/apps/integrations/metadata/heartbeat/alertmanager.py +++ b/engine/apps/integrations/metadata/heartbeat/alertmanager.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreatorForTitleGrouping +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreatorForTitleGrouping integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreatorForTitleGrouping(integration_verbal) +creator = HeartBeatTextCreatorForTitleGrouping(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template diff --git a/engine/apps/integrations/metadata/heartbeat/elastalert.py b/engine/apps/integrations/metadata/heartbeat/elastalert.py index 7e76f8fb..ab9d9415 100644 --- a/engine/apps/integrations/metadata/heartbeat/elastalert.py +++ b/engine/apps/integrations/metadata/heartbeat/elastalert.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreator +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreator integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreator(integration_verbal) +creator = HeartBeatTextCreator(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template diff --git a/engine/apps/integrations/metadata/heartbeat/formatted_webhook.py b/engine/apps/integrations/metadata/heartbeat/formatted_webhook.py index dc0beea6..adb4ec77 100644 --- a/engine/apps/integrations/metadata/heartbeat/formatted_webhook.py +++ b/engine/apps/integrations/metadata/heartbeat/formatted_webhook.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreator +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreator integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreator(integration_verbal) +creator = HeartBeatTextCreator(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template diff --git a/engine/apps/integrations/metadata/heartbeat/grafana.py b/engine/apps/integrations/metadata/heartbeat/grafana.py index 05abb838..8237e4b3 100644 --- a/engine/apps/integrations/metadata/heartbeat/grafana.py +++ b/engine/apps/integrations/metadata/heartbeat/grafana.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreatorForTitleGrouping +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreatorForTitleGrouping integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreatorForTitleGrouping(integration_verbal) +creator = HeartBeatTextCreatorForTitleGrouping(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template diff --git a/engine/apps/integrations/metadata/heartbeat/prtg.py b/engine/apps/integrations/metadata/heartbeat/prtg.py index 21c8d80c..e77a9308 100644 --- a/engine/apps/integrations/metadata/heartbeat/prtg.py +++ b/engine/apps/integrations/metadata/heartbeat/prtg.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreator +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreator integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreator(integration_verbal) +creator = HeartBeatTextCreator(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template diff --git a/engine/apps/integrations/metadata/heartbeat/webhook.py b/engine/apps/integrations/metadata/heartbeat/webhook.py index 510b30fa..03023e2e 100644 --- a/engine/apps/integrations/metadata/heartbeat/webhook.py +++ b/engine/apps/integrations/metadata/heartbeat/webhook.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreator +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreator integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreator(integration_verbal) +creator = HeartBeatTextCreator(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template diff --git a/engine/apps/integrations/metadata/heartbeat/zabbix.py b/engine/apps/integrations/metadata/heartbeat/zabbix.py index add62baf..921ec9e6 100644 --- a/engine/apps/integrations/metadata/heartbeat/zabbix.py +++ b/engine/apps/integrations/metadata/heartbeat/zabbix.py @@ -1,9 +1,9 @@ from pathlib import PurePath -from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HearBeatTextCreator +from apps.integrations.metadata.heartbeat._heartbeat_text_creator import HeartBeatTextCreator integration_verbal = PurePath(__file__).stem -creator = HearBeatTextCreator(integration_verbal) +creator = HeartBeatTextCreator(integration_verbal) heartbeat_text = creator.get_heartbeat_texts() heartbeat_instruction_template = heartbeat_text.heartbeat_instruction_template From 8d9daf064460350be5fe4ca739976d1cd0c09092 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 6 Jul 2022 15:42:00 -0600 Subject: [PATCH 02/49] Tweak urljoin usage so integrations give back the correct URL when using a BASE_URL with a path prefix --- engine/apps/alerts/models/alert_receive_channel.py | 2 +- engine/apps/oss_installation/cloud_heartbeat.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index a2a9dad7..5435fddc 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -497,7 +497,7 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): return None return urljoin( settings.BASE_URL, - f"/integrations/v1/{self.config.slug}/{self.token}/", + f"integrations/v1/{self.config.slug}/{self.token}/", ) @property diff --git a/engine/apps/oss_installation/cloud_heartbeat.py b/engine/apps/oss_installation/cloud_heartbeat.py index 8d445e83..b75d5299 100644 --- a/engine/apps/oss_installation/cloud_heartbeat.py +++ b/engine/apps/oss_installation/cloud_heartbeat.py @@ -23,7 +23,7 @@ def setup_heartbeat_integration(name=None): # don't specify a team in the data, so heartbeat integration will be created in the General. name = name or f"OnCall Cloud Heartbeat {settings.BASE_URL}" data = {"type": "formatted_webhook", "name": name} - url = urljoin(settings.GRAFANA_CLOUD_ONCALL_API_URL, "/api/v1/integrations/") + url = urljoin(settings.GRAFANA_CLOUD_ONCALL_API_URL, "api/v1/integrations/") try: headers = {"Authorization": api_token} r = requests.post(url=url, data=data, headers=headers, timeout=5) From 7b99019c71c7bd999850491ad84437f7fe11986e Mon Sep 17 00:00:00 2001 From: Yulia Shanyrova Date: Tue, 12 Jul 2022 11:57:36 +0200 Subject: [PATCH 03/49] bug fixes: integrations layout, schedules, cloud users update --- .../CreateAlertReceiveChannelContainer.module.css | 1 + .../CloudPhoneSettings/CloudPhoneSettings.tsx | 2 +- .../src/pages/schedules/Schedules.module.css | 2 +- grafana-plugin/src/pages/schedules/Schedules.tsx | 15 ++++++++------- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/grafana-plugin/src/containers/CreateAlertReceiveChannelContainer/CreateAlertReceiveChannelContainer.module.css b/grafana-plugin/src/containers/CreateAlertReceiveChannelContainer/CreateAlertReceiveChannelContainer.module.css index fdf76499..e43787f6 100644 --- a/grafana-plugin/src/containers/CreateAlertReceiveChannelContainer/CreateAlertReceiveChannelContainer.module.css +++ b/grafana-plugin/src/containers/CreateAlertReceiveChannelContainer/CreateAlertReceiveChannelContainer.module.css @@ -10,6 +10,7 @@ overflow: auto; scroll-snap-type: y mandatory; padding: 0 10px 10px 0; + min-width: 840px; } .cards_centered { diff --git a/grafana-plugin/src/containers/UserSettings/parts/tabs/CloudPhoneSettings/CloudPhoneSettings.tsx b/grafana-plugin/src/containers/UserSettings/parts/tabs/CloudPhoneSettings/CloudPhoneSettings.tsx index 74438027..4d03a306 100644 --- a/grafana-plugin/src/containers/UserSettings/parts/tabs/CloudPhoneSettings/CloudPhoneSettings.tsx +++ b/grafana-plugin/src/containers/UserSettings/parts/tabs/CloudPhoneSettings/CloudPhoneSettings.tsx @@ -148,7 +148,7 @@ const CloudPhoneSettings = observer((props: CloudPhoneSettingsProps) => { Updating... ) : ( - )} diff --git a/grafana-plugin/src/pages/schedules/Schedules.module.css b/grafana-plugin/src/pages/schedules/Schedules.module.css index 12747b4a..cc0cc165 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.module.css +++ b/grafana-plugin/src/pages/schedules/Schedules.module.css @@ -47,7 +47,6 @@ .priority-icon { width: 32px; - height: 32px; border-radius: 50%; background: var(--secondary-background); line-height: 32px; @@ -65,6 +64,7 @@ border-radius: 50px; color: #ff5286; font-weight: 400; + align-items: baseline; } .gap-between-shifts-icon { diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx index 52501020..ff557400 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.tsx +++ b/grafana-plugin/src/pages/schedules/Schedules.tsx @@ -477,11 +477,11 @@ const Event = ({ event }: EventProps) => { const dates = getDatesString(event.start, event.end, event.all_day); return ( - + <> {!event.is_gap ? ( - <> +
- {`L${event.priority_level || '0'}`} + {`L${event.priority_level || '0'}`}
@@ -493,7 +493,7 @@ const Event = ({ event }: EventProps) => { )) ) : ( - + Empty shift (event without associated user or user with Viewer access) @@ -504,13 +504,14 @@ const Event = ({ event }: EventProps) => { {dates}
- + ) : (
- Gap! Nobody On-Call... + + Gap! Nobody On-Call...
)} - + ); }; From 83ef5b54779122ee3c33119d9e3e447d141f0485 Mon Sep 17 00:00:00 2001 From: Yulia Shanyrova Date: Tue, 12 Jul 2022 17:13:57 +0200 Subject: [PATCH 04/49] Escalation chains usage icon is added, clear filters button is changed --- .../EscalationsFilters.module.css | 7 +++- .../EscalationsFilters/EscalationsFilters.tsx | 6 ++-- .../EscalationChainCard.module.css | 11 ++++++ .../EscalationChainCard.tsx | 25 +++++++++++--- .../escalation-chains/EscalationChains.tsx | 34 +++---------------- 5 files changed, 44 insertions(+), 39 deletions(-) diff --git a/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.module.css b/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.module.css index 10aaedc7..0ed200a7 100644 --- a/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.module.css +++ b/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.module.css @@ -1,8 +1,13 @@ .root { display: flex; - justify-content: space-between; + align-items: center; } .search { max-width: 400px; } + +.icon-button { + color: var(--secondary-text-color); + margin-left: 8px; +} diff --git a/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.tsx b/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.tsx index ade4e00c..c08d2c3d 100644 --- a/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.tsx +++ b/grafana-plugin/src/components/EscalationsFilters/EscalationsFilters.tsx @@ -1,6 +1,6 @@ import React, { ChangeEvent, FC, useCallback } from 'react'; -import { Icon, Input, Button } from '@grafana/ui'; +import { Icon, Input, Button, IconButton } from '@grafana/ui'; import cn from 'classnames/bind'; import styles from './EscalationsFilters.module.css'; @@ -45,9 +45,7 @@ const EscalationsFilters: FC = (props) => { value={value.searchTerm} onChange={onSearchTermChangeCallback} /> - + ); }; diff --git a/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.module.css b/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.module.css index 63d08ecc..5fb2e72c 100644 --- a/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.module.css +++ b/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.module.css @@ -1,3 +1,14 @@ .root { display: block; } + +.connected-integrations { + padding: 2px 4px; + background: rgba(27, 133, 94, 0.15); + border: 1px solid var(--success-text-color); + border-radius: 2px; +} + +.icon { + color: var(--success-text-color); +} diff --git a/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.tsx b/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.tsx index 9cb59826..f483b8d6 100644 --- a/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.tsx +++ b/grafana-plugin/src/containers/EscalationChainCard/EscalationChainCard.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { HorizontalGroup, VerticalGroup } from '@grafana/ui'; +import { HorizontalGroup, Icon, VerticalGroup, Tooltip } from '@grafana/ui'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; @@ -29,9 +29,26 @@ const EscalationChainCard = observer((props: AlertReceiveChannelCardProps) => {
- - {escalationChain.name} - + + + {escalationChain.name} + + {(escalationChain.number_of_integrations > 0 || escalationChain.number_of_routes > 0) && ( + +
+ + + + {escalationChain.number_of_integrations} + + +
+ + )} + {/* 0 || escalationChain.number_of_routes > 0) { - warningAboutModifyingEscalationChain = ( - <> - Modifying this escalation chain will affect{' '} - {escalationChain.number_of_integrations > 0 && ( - - {escalationChain.number_of_integrations} integration - {escalationChain.number_of_integrations === 1 ? '' : 's'} - - )} - {escalationChain.number_of_routes > 0 && escalationChain.number_of_integrations > 0 && ' and '} - {escalationChain.number_of_routes > 0 && ( - - {escalationChain.number_of_routes} route{escalationChain.number_of_routes === 1 ? '' : 's'} - - )} - . Escalation chains linked to multiple integrations cannot be removed. - - ); - } - return ( <> @@ -288,7 +266,7 @@ class EscalationChainsPage extends React.Component 1} + disabled={escalationChain.number_of_integrations > 0} tooltip="Remove" tooltipPlacement="top" onClick={this.handleDeleteEscalationChain} @@ -296,7 +274,7 @@ class EscalationChainsPage extends React.Component - {escalationChain.number_of_integrations > 1 && ( + {escalationChain.number_of_integrations > 0 && ( @@ -304,17 +282,13 @@ class EscalationChainsPage extends React.Component
- {warningAboutModifyingEscalationChain && ( - // @ts-ignore - - )} {escalationChainDetails ? ( {escalationChainDetails.length ? ( From 75172d5e2ccc535069712b2d2f2a86dddfd25aa1 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 12 Jul 2022 11:00:02 -0600 Subject: [PATCH 05/49] Return list of usernames in a shift that do not match users in OnCall --- engine/apps/api/views/schedule.py | 1 + engine/apps/schedules/ical_utils.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 4839ecd8..4edb8a73 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -212,6 +212,7 @@ class ScheduleView( } for user in shift["users"] ], + "missing_users": shift["missing_users"], "priority_level": shift["priority"] if shift["priority"] != 0 else None, "source": shift["source"], "calendar_type": shift["calendar_type"], diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 2705c4db..a22ddee4 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -137,6 +137,7 @@ def list_of_oncall_shifts_from_ical( "start": g.start if g.start else datetime_start, "end": g.end if g.end else datetime_end, "users": [], + "missing_users": [], "priority": None, "source": None, "calendar_type": None, @@ -157,6 +158,7 @@ def get_shifts_dict(calendar, calendar_type, schedule, datetime_start, datetime_ priority = parse_priority_from_string(event.get(ICAL_SUMMARY, "[L0]")) pk, source = parse_event_uid(event.get(ICAL_UID)) users = get_users_from_ical_event(event, schedule.organization) + missing_users = get_missing_users_from_ical_event(event, schedule.organization) # Define on-call shift out of ical event that has the actual user if len(users) > 0 or with_empty_shifts: if type(event[ICAL_DATETIME_START].dt) == datetime.date: @@ -168,6 +170,7 @@ def get_shifts_dict(calendar, calendar_type, schedule, datetime_start, datetime_ "start": start, "end": end, "users": users, + "missing_users": missing_users, "priority": priority, "source": source, "calendar_type": calendar_type, @@ -183,6 +186,7 @@ def get_shifts_dict(calendar, calendar_type, schedule, datetime_start, datetime_ "start": start, "end": end, "users": users, + "missing_users": missing_users, "priority": priority, "source": source, "calendar_type": calendar_type, @@ -379,6 +383,14 @@ def get_usernames_from_ical_event(event): return usernames_found, priority +def get_missing_users_from_ical_event(event, organization): + all_usernames, _ = get_usernames_from_ical_event(event) + users = list(get_users_from_ical_event(event, organization)) + found_usernames = [u.username for u in users] + found_emails = [u.email for u in users] + return [u for u in all_usernames if u != "" and u not in found_usernames and u not in found_emails] + + def get_users_from_ical_event(event, organization): usernames_from_ical, _ = get_usernames_from_ical_event(event) users = [] From 9ce53b8215ef8ec26fca5eebcf0a6cb33ff44daf Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 12 Jul 2022 11:47:37 -0600 Subject: [PATCH 06/49] Fix tests --- engine/apps/api/tests/test_schedules.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index c788d521..cadcb5f4 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -424,6 +424,7 @@ def test_events_calendar( "start": on_call_shift.start, "end": on_call_shift.start + on_call_shift.duration, "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", "calendar_type": OnCallSchedule.PRIMARY, @@ -487,6 +488,7 @@ def test_filter_events_calendar( "start": mon_start, "end": mon_start + on_call_shift.duration, "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", "calendar_type": OnCallSchedule.PRIMARY, @@ -501,6 +503,7 @@ def test_filter_events_calendar( "start": fri_start, "end": fri_start + on_call_shift.duration, "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", "calendar_type": OnCallSchedule.PRIMARY, @@ -567,6 +570,7 @@ def test_filter_events_range_calendar( "start": fri_start, "end": fri_start + on_call_shift.duration, "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", "calendar_type": OnCallSchedule.PRIMARY, From 74a32c444ae360ad9e3c332f47f1d5cb6bdb0b52 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 12 Jul 2022 15:42:20 -0600 Subject: [PATCH 07/49] Use utility function to create URLs --- .../alerts/models/alert_receive_channel.py | 6 ++-- engine/apps/api/views/schedule.py | 8 ++--- engine/apps/api/views/user.py | 7 ++-- .../apps/slack/scenarios/resolution_note.py | 7 ++-- .../test_resolution_note.py | 5 ++- engine/apps/twilioapp/twilio_client.py | 6 ++-- engine/apps/twilioapp/utils.py | 5 +-- engine/common/api_helpers/utils.py | 10 ++++++ engine/common/tests/test_create_engine_url.py | 35 +++++++++++++++++++ 9 files changed, 64 insertions(+), 25 deletions(-) create mode 100644 engine/common/tests/test_create_engine_url.py diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 5435fddc..e48d69a8 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -32,6 +32,7 @@ from apps.slack.constants import SLACK_RATE_LIMIT_DELAY, SLACK_RATE_LIMIT_TIMEOU from apps.slack.tasks import post_slack_rate_limit_message from apps.slack.utils import post_message_to_channel from apps.user_management.organization_log_creator import OrganizationLogType, create_organization_log +from common.api_helpers.utils import create_engine_url from common.exceptions import TeamCanNotBeChangedError, UnableToSendDemoAlert from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length @@ -495,10 +496,7 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): AlertReceiveChannel.INTEGRATION_MAINTENANCE, ]: return None - return urljoin( - settings.BASE_URL, - f"integrations/v1/{self.config.slug}/{self.token}/", - ) + return create_engine_url(f"integrations/v1/{self.config.slug}/{self.token}/") @property def inbound_email(self): diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index d27676d4..cf4040d1 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -1,8 +1,6 @@ import datetime -from urllib.parse import urljoin import pytz -from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.db.models import OuterRef, Subquery from django.db.utils import IntegrityError @@ -38,6 +36,7 @@ from common.api_helpers.mixins import ( ShortSerializerMixin, UpdateSerializerMixin, ) +from common.api_helpers.utils import create_engine_url class ScheduleView( @@ -287,10 +286,9 @@ class ScheduleView( except IntegrityError: raise Conflict("Schedule export token for user already exists") - export_url = urljoin( - settings.BASE_URL, + export_url = create_engine_url( reverse("api-public:schedules-export", kwargs={"pk": schedule.public_primary_key}) - + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}", + + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}" ) data = {"token": token, "created_at": instance.created_at, "export_url": export_url} diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index e7d20a32..ffdac928 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -1,5 +1,4 @@ import logging -from urllib.parse import urljoin from django.apps import apps from django.conf import settings @@ -44,6 +43,7 @@ from apps.user_management.organization_log_creator import OrganizationLogType, c from common.api_helpers.exceptions import Conflict from common.api_helpers.mixins import FilterSerializerMixin, PublicPrimaryKeyMixin from common.api_helpers.paginators import HundredPageSizePaginator +from common.api_helpers.utils import create_engine_url from common.constants.role import Role logger = logging.getLogger(__name__) @@ -406,10 +406,9 @@ class UserView( except IntegrityError: raise Conflict("Schedule export token for user already exists") - export_url = urljoin( - settings.BASE_URL, + export_url = create_engine_url( reverse("api-public:users-schedule-export", kwargs={"pk": user.public_primary_key}) - + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}", + + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}" ) data = {"token": token, "created_at": instance.created_at, "export_url": export_url} diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index a82ccb69..739a28ad 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -1,14 +1,13 @@ import json import logging -from urllib.parse import urljoin from django.apps import apps -from django.conf import settings from django.db.models import Q from django.utils import timezone from apps.slack.scenarios import scenario_step from apps.slack.slack_client.exceptions import SlackAPIException +from common.api_helpers.utils import create_engine_url from .step_mixins import CheckAlertIsUnarchivedMixin @@ -448,7 +447,7 @@ class ResolutionNoteModalStep(CheckAlertIsUnarchivedMixin, scenario_step.Scenari if not blocks: # there aren't any resolution notes yet, display a hint instead - link_to_instruction = urljoin(settings.BASE_URL, "static/images/postmortem.gif") + link_to_instruction = create_engine_url("static/images/postmortem.gif") blocks = [ { "type": "divider", @@ -474,7 +473,7 @@ class ResolutionNoteModalStep(CheckAlertIsUnarchivedMixin, scenario_step.Scenari return blocks def get_invite_bot_tip_blocks(self, channel): - link_to_instruction = urljoin(settings.BASE_URL, "static/images/postmortem.gif") + link_to_instruction = create_engine_url("static/images/postmortem.gif") blocks = [ { "type": "divider", diff --git a/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py b/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py index 080f5d40..f950effb 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py @@ -1,10 +1,9 @@ import json -from urllib.parse import urljoin import pytest -from django.conf import settings from apps.slack.scenarios.scenario_step import ScenarioStep +from common.api_helpers.utils import create_engine_url @pytest.mark.django_db @@ -22,7 +21,7 @@ def test_get_resolution_notes_blocks_default_if_empty( blocks = step.get_resolution_notes_blocks(alert_group, "", False) - link_to_instruction = urljoin(settings.BASE_URL, "static/images/postmortem.gif") + link_to_instruction = create_engine_url("static/images/postmortem.gif") expected_blocks = [ { "type": "divider", diff --git a/engine/apps/twilioapp/twilio_client.py b/engine/apps/twilioapp/twilio_client.py index 9c2d6cc3..30804549 100644 --- a/engine/apps/twilioapp/twilio_client.py +++ b/engine/apps/twilioapp/twilio_client.py @@ -2,7 +2,6 @@ import logging import urllib.parse from django.apps import apps -from django.conf import settings from django.urls import reverse from twilio.base.exceptions import TwilioRestException from twilio.rest import Client @@ -10,6 +9,7 @@ from twilio.rest import Client from apps.base.utils import live_settings from apps.twilioapp.constants import TEST_CALL_TEXT, TwilioLogRecordStatus, TwilioLogRecordType from apps.twilioapp.utils import get_calling_code, get_gather_message, get_gather_url, parse_phone_number +from common.api_helpers.utils import create_engine_url logger = logging.getLogger(__name__) @@ -24,7 +24,7 @@ class TwilioClient: return live_settings.TWILIO_NUMBER def send_message(self, body, to): - status_callback = settings.BASE_URL + reverse("twilioapp:sms_status_events") + status_callback = create_engine_url(reverse("twilioapp:sms_status_events")) return self.twilio_api_client.messages.create( body=body, to=to, from_=self.twilio_number, status_callback=status_callback ) @@ -135,7 +135,7 @@ class TwilioClient: ) url = "http://twimlets.com/echo?Twiml=" + twiml_query - status_callback = settings.BASE_URL + reverse("twilioapp:call_status_events") + status_callback = create_engine_url(reverse("twilioapp:call_status_events")) status_callback_events = ["initiated", "ringing", "answered", "completed"] diff --git a/engine/apps/twilioapp/utils.py b/engine/apps/twilioapp/utils.py index 10986dda..7b14b9bd 100644 --- a/engine/apps/twilioapp/utils.py +++ b/engine/apps/twilioapp/utils.py @@ -3,11 +3,12 @@ import re from string import digits from django.apps import apps -from django.conf import settings from django.urls import reverse from phonenumbers import COUNTRY_CODE_TO_REGION_CODE from twilio.twiml.voice_response import Gather, VoiceResponse +from common.api_helpers.utils import create_engine_url + logger = logging.getLogger(__name__) @@ -19,7 +20,7 @@ def get_calling_code(iso): def get_gather_url(): - gather_url = settings.BASE_URL + reverse("twilioapp:gather") + gather_url = create_engine_url(reverse("twilioapp:gather")) return gather_url diff --git a/engine/common/api_helpers/utils.py b/engine/common/api_helpers/utils.py index 1f8ecc54..06b17f0f 100644 --- a/engine/common/api_helpers/utils.py +++ b/engine/common/api_helpers/utils.py @@ -1,3 +1,5 @@ +from urllib.parse import urljoin + import requests from django.conf import settings from icalendar import Calendar @@ -50,3 +52,11 @@ def validate_ical_url(url): raise serializers.ValidationError("Ical parse failed") return url return None + + +def create_engine_url(path): + base = settings.BASE_URL + if not base.endswith("/"): + base += "/" + trimmed_path = path.lstrip("/") + return urljoin(base, trimmed_path) diff --git a/engine/common/tests/test_create_engine_url.py b/engine/common/tests/test_create_engine_url.py new file mode 100644 index 00000000..7097709f --- /dev/null +++ b/engine/common/tests/test_create_engine_url.py @@ -0,0 +1,35 @@ +from django.test.utils import override_settings + +from common.api_helpers.utils import create_engine_url + + +@override_settings(BASE_URL="http://localhost:8000") +def test_create_engine_url_no_slash(): + assert create_engine_url("destination") == "http://localhost:8000/destination" + assert create_engine_url("/destination") == "http://localhost:8000/destination" + assert create_engine_url("destination/") == "http://localhost:8000/destination/" + assert create_engine_url("/destination/") == "http://localhost:8000/destination/" + + +@override_settings(BASE_URL="http://localhost:8000/") +def test_create_engine_url_slash(): + assert create_engine_url("destination") == "http://localhost:8000/destination" + assert create_engine_url("/destination") == "http://localhost:8000/destination" + assert create_engine_url("destination/") == "http://localhost:8000/destination/" + assert create_engine_url("/destination/") == "http://localhost:8000/destination/" + + +@override_settings(BASE_URL="http://localhost:8000/test123") +def test_create_engine_url_prefix_no_slash(): + assert create_engine_url("destination") == "http://localhost:8000/test123/destination" + assert create_engine_url("/destination") == "http://localhost:8000/test123/destination" + assert create_engine_url("destination/") == "http://localhost:8000/test123/destination/" + assert create_engine_url("/destination/") == "http://localhost:8000/test123/destination/" + + +@override_settings(BASE_URL="http://localhost:8000/test123/") +def test_create_engine_url_prefix_slash(): + assert create_engine_url("destination") == "http://localhost:8000/test123/destination" + assert create_engine_url("/destination") == "http://localhost:8000/test123/destination" + assert create_engine_url("destination/") == "http://localhost:8000/test123/destination/" + assert create_engine_url("/destination/") == "http://localhost:8000/test123/destination/" From ded39d8df3b4b40c113097453046996d3ad48551 Mon Sep 17 00:00:00 2001 From: Nelson Johnstone Date: Wed, 13 Jul 2022 14:27:24 +1000 Subject: [PATCH 08/49] support regex_replace() --- docs/sources/integrations/create-custom-templates.md | 1 + engine/common/jinja_templater/filters.py | 7 +++++++ engine/common/jinja_templater/jinja_template_env.py | 3 ++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/sources/integrations/create-custom-templates.md b/docs/sources/integrations/create-custom-templates.md index 440bfe28..d99cce17 100644 --- a/docs/sources/integrations/create-custom-templates.md +++ b/docs/sources/integrations/create-custom-templates.md @@ -159,3 +159,4 @@ Built-in functions: - `tojson_pretty` - JSON prettified - `iso8601_to_time` - converts time from iso8601 (`2015-02-17T18:30:20.000Z`) to datetime - `datetimeformat` - converts time from datetime to the given format (`%H:%M / %d-%m-%Y` by default) +- `regex_replace` - performs a regex find and replace diff --git a/engine/common/jinja_templater/filters.py b/engine/common/jinja_templater/filters.py index 52c59f07..0ba87024 100644 --- a/engine/common/jinja_templater/filters.py +++ b/engine/common/jinja_templater/filters.py @@ -1,4 +1,5 @@ import json +import re from django.utils.dateparse import parse_datetime @@ -22,3 +23,9 @@ def to_pretty_json(value): return json.dumps(value, sort_keys=True, indent=4, separators=(",", ": "), ensure_ascii=False) except (ValueError, AttributeError, TypeError): return None + +def regex_replace(value, find, replace): + try: + return re.sub(find, replace, value) + except (ValueError, AttributeError, TypeError): + return None diff --git a/engine/common/jinja_templater/jinja_template_env.py b/engine/common/jinja_templater/jinja_template_env.py index f151eac6..7ac86ddc 100644 --- a/engine/common/jinja_templater/jinja_template_env.py +++ b/engine/common/jinja_templater/jinja_template_env.py @@ -2,7 +2,7 @@ from django.utils import timezone from jinja2 import BaseLoader from jinja2.sandbox import SandboxedEnvironment -from .filters import datetimeformat, iso8601_to_time, to_pretty_json +from .filters import datetimeformat, iso8601_to_time, to_pretty_json, regex_replace jinja_template_env = SandboxedEnvironment(loader=BaseLoader()) @@ -10,3 +10,4 @@ jinja_template_env.filters["datetimeformat"] = datetimeformat jinja_template_env.filters["iso8601_to_time"] = iso8601_to_time jinja_template_env.filters["tojson_pretty"] = to_pretty_json jinja_template_env.globals["time"] = timezone.now +jinja_template_env.filters["regex_replace"] = regex_replace From cede94635929e977550bcf34ddbdfd878ba049e9 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Wed, 13 Jul 2022 14:54:53 +0400 Subject: [PATCH 09/49] Reshape webhook payload 1. Remove alert.title, alert.message, alert.image_url from webhook payload, they are deprecated. 2. Pass alert_group_id to webhook payload. --- engine/apps/alerts/models/custom_button.py | 5 +---- .../OutgoingWebhookForm/OutgoingWebhookForm.config.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/engine/apps/alerts/models/custom_button.py b/engine/apps/alerts/models/custom_button.py index 9007c34f..0ecf1eae 100644 --- a/engine/apps/alerts/models/custom_button.py +++ b/engine/apps/alerts/models/custom_button.py @@ -118,11 +118,8 @@ class CustomButton(models.Model): elif self.data: rendered_data = Template(self.data).render( { - "alert_title": self._escape_string(alert.title), - "alert_message": self._escape_string(alert.message), - "alert_url": alert.link_to_upstream_details, "alert_payload": self._escape_alert_payload(alert.raw_request_data), - "alert_payload_json": json.dumps(alert.raw_request_data), + "alert_group_id": alert.group.public_primary_key, } ) post_kwargs["json"] = json.loads(rendered_data) diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.ts b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.ts index c80813f8..76fecf4f 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.ts +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.ts @@ -36,7 +36,7 @@ export const form: { name: string; fields: FormItem[] } = { name: 'data', getDisabled: (form_data) => Boolean(form_data.forward_whole_payload), type: FormItemType.TextArea, - description: 'Available variables: {{ alert_title }}, {{ alert_message }}, {{ alert_url }}, {{ alert_payload }}', + description: 'Available variables: {{ alert_payload }}, {{ alert_group_id }}', extra: { rows: 9, }, From aeed7b69529b4ef595440c47420d005ad16c211b Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Wed, 13 Jul 2022 16:06:09 +0400 Subject: [PATCH 10/49] Fix docs --- docs/sources/integrations/configure-outgoing-webhooks.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/sources/integrations/configure-outgoing-webhooks.md b/docs/sources/integrations/configure-outgoing-webhooks.md index c35b0b24..bff8c74e 100644 --- a/docs/sources/integrations/configure-outgoing-webhooks.md +++ b/docs/sources/integrations/configure-outgoing-webhooks.md @@ -32,10 +32,8 @@ To automatically send alert data to a destination URL via outgoing webhook: The format you use to call the variables must match the structure of how the fields are nested in the alert payload. The **Data** field can use the following four variables to auto-populate the webhook payload with information about the first alert in the alert group: -- `{{ alert_title }}` -- `{{ alert_message }}` -- `{{ alert_url }}` - `{{ alert_payload }}` +- `{{ alert_group_id }}`
`alert_payload` is always the first level of any variable you want to call. From 46d2e61879c0ba0ac942c98246e8d1a84862a7d7 Mon Sep 17 00:00:00 2001 From: Yulia Shanyrova Date: Wed, 13 Jul 2022 17:00:55 +0200 Subject: [PATCH 11/49] custom styles for plugin were placed in the file --- grafana-plugin/src/GrafanaPluginRootPage.tsx | 13 ++++--- .../src/img/grafanaGlobalStyles.css | 39 +++++++++++++++++++ grafana-plugin/src/index.css | 32 --------------- 3 files changed, 46 insertions(+), 38 deletions(-) create mode 100644 grafana-plugin/src/img/grafanaGlobalStyles.css diff --git a/grafana-plugin/src/GrafanaPluginRootPage.tsx b/grafana-plugin/src/GrafanaPluginRootPage.tsx index b6f0cc65..e258d30e 100644 --- a/grafana-plugin/src/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/GrafanaPluginRootPage.tsx @@ -97,14 +97,15 @@ export const Root = observer((props: AppRootProps) => { }, []); useEffect(() => { - const style = document.createElement('style'); - document.head.appendChild(style); - const index = style.sheet.insertRule('.page-body {max-width: unset !important}'); - const index2 = style.sheet.insertRule('.page-container {max-width: unset !important}'); + let link = document.createElement('link'); + link.type = 'text/css'; + link.rel = 'stylesheet'; + link.href = '/public/plugins/grafana-oncall-app/img/grafanaGlobalStyles.css'; + + document.head.appendChild(link); return () => { - style.sheet.removeRule(index); - style.sheet.removeRule(index2); + document.head.removeChild(link); }; }, []); diff --git a/grafana-plugin/src/img/grafanaGlobalStyles.css b/grafana-plugin/src/img/grafanaGlobalStyles.css new file mode 100644 index 00000000..87ff62d1 --- /dev/null +++ b/grafana-plugin/src/img/grafanaGlobalStyles.css @@ -0,0 +1,39 @@ +.page-body { + max-width: unset !important; +} + +.page-container { + max-width: unset !important; +} + +/* This is for Grafana 8, remove later */ +@media (max-width: 1540px) { + .page-header__tabs > ul > li > a > div { + display: none; + } +} + +@media (max-width: 1540px) { + .page-header__tabs > div > div > a > div { + display: none; + } +} + +@media (max-width: 1300px) { + .sidemenu { + position: fixed !important; + height: 100%; + } + + .grafana-app { + position: relative !important; + } + + .main-view { + padding-left: 50px; + } + + .page-header__tabs li a { + white-space: nowrap; + } +} diff --git a/grafana-plugin/src/index.css b/grafana-plugin/src/index.css index bdfece87..0a948ad3 100644 --- a/grafana-plugin/src/index.css +++ b/grafana-plugin/src/index.css @@ -29,35 +29,3 @@ .highlighted-row { background: var(--highlighted-row-bg); } - -/* This is for Grafana 8, remove later */ -@media (max-width: 1540px) { - .page-header__tabs > ul > li > a > div { - display: none; - } -} - -@media (max-width: 1540px) { - .page-header__tabs > div > div > a > div { - display: none; - } -} - -@media (max-width: 1300px) { - .sidemenu { - position: fixed !important; - height: 100%; - } - - .grafana-app { - position: relative !important; - } - - .main-view { - padding-left: 50px; - } - - .page-header__tabs li a { - white-space: nowrap; - } -} From 3f0f991f408c79c533403acdff437c3871a365ca Mon Sep 17 00:00:00 2001 From: Nelson Johnstone Date: Thu, 14 Jul 2022 08:20:28 +1000 Subject: [PATCH 12/49] linting and test --- engine/common/jinja_templater/filters.py | 1 + engine/common/jinja_templater/jinja_template_env.py | 2 +- engine/common/tests/test_regex_replace.py | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 engine/common/tests/test_regex_replace.py diff --git a/engine/common/jinja_templater/filters.py b/engine/common/jinja_templater/filters.py index 0ba87024..07557fca 100644 --- a/engine/common/jinja_templater/filters.py +++ b/engine/common/jinja_templater/filters.py @@ -24,6 +24,7 @@ def to_pretty_json(value): except (ValueError, AttributeError, TypeError): return None + def regex_replace(value, find, replace): try: return re.sub(find, replace, value) diff --git a/engine/common/jinja_templater/jinja_template_env.py b/engine/common/jinja_templater/jinja_template_env.py index 7ac86ddc..41e915aa 100644 --- a/engine/common/jinja_templater/jinja_template_env.py +++ b/engine/common/jinja_templater/jinja_template_env.py @@ -2,7 +2,7 @@ from django.utils import timezone from jinja2 import BaseLoader from jinja2.sandbox import SandboxedEnvironment -from .filters import datetimeformat, iso8601_to_time, to_pretty_json, regex_replace +from .filters import datetimeformat, iso8601_to_time, regex_replace, to_pretty_json jinja_template_env = SandboxedEnvironment(loader=BaseLoader()) diff --git a/engine/common/tests/test_regex_replace.py b/engine/common/tests/test_regex_replace.py new file mode 100644 index 00000000..a41c9a28 --- /dev/null +++ b/engine/common/tests/test_regex_replace.py @@ -0,0 +1,7 @@ +from common.jinja_templater.filters import regex_replace + + +def test_regex_replace_drop_field(): + original = "[ var='D0' metric='my_metric' labels={} value=140 ]" + expected = "[ metric='my_metric' labels={} value=140 ]" + assert regex_replace(original, "var=\'[a-zA-Z0-9]+\'", "") == expected From adeb87af3571d78ebc9a1540197ec91736908035 Mon Sep 17 00:00:00 2001 From: Nelson Johnstone Date: Thu, 14 Jul 2022 20:06:24 +1000 Subject: [PATCH 13/49] linting and fixed test --- engine/common/tests/test_regex_replace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/common/tests/test_regex_replace.py b/engine/common/tests/test_regex_replace.py index a41c9a28..7477c1e5 100644 --- a/engine/common/tests/test_regex_replace.py +++ b/engine/common/tests/test_regex_replace.py @@ -4,4 +4,4 @@ from common.jinja_templater.filters import regex_replace def test_regex_replace_drop_field(): original = "[ var='D0' metric='my_metric' labels={} value=140 ]" expected = "[ metric='my_metric' labels={} value=140 ]" - assert regex_replace(original, "var=\'[a-zA-Z0-9]+\'", "") == expected + assert regex_replace(original, "var='[a-zA-Z0-9]+' ", "") == expected From ba68ac3dcc0bae0d6d1b9b5b43c8299080086185 Mon Sep 17 00:00:00 2001 From: Yulia Shanyrova Date: Thu, 14 Jul 2022 12:09:40 +0200 Subject: [PATCH 14/49] Added message about missing users at schedules empty shifts --- grafana-plugin/src/models/schedule/schedule.types.ts | 1 + grafana-plugin/src/pages/schedules/Schedules.tsx | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/grafana-plugin/src/models/schedule/schedule.types.ts b/grafana-plugin/src/models/schedule/schedule.types.ts index 567be12b..dc3cc3f7 100644 --- a/grafana-plugin/src/models/schedule/schedule.types.ts +++ b/grafana-plugin/src/models/schedule/schedule.types.ts @@ -36,6 +36,7 @@ export interface ScheduleEvent { users: User[]; is_empty: boolean; is_gap: boolean; + missing_users: string[]; } export interface CreateScheduleExportTokenResponse { diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx index 52501020..de68ac65 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.tsx +++ b/grafana-plugin/src/pages/schedules/Schedules.tsx @@ -495,7 +495,15 @@ const Event = ({ event }: EventProps) => { ) : ( - Empty shift (event without associated user or user with Viewer access) + Empty shift + {event.missing_users[0] && ( + + (check if {event.missing_users[0].includes(',') ? 'some of these users -' : 'user -'}{' '} + "{event.missing_users[0]}"{' '} + {event.missing_users[0].includes(',') ? 'are' : 'is'} existing in OnCall or{' '} + {event.missing_users[0].includes(',') ? 'have' : 'has'} Viewer role) + + )} )} {event.source && — source: {event.source}} From e7b89575f848841bc739940d4846f6107c8c0ea6 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Thu, 14 Jul 2022 13:13:18 +0300 Subject: [PATCH 15/49] Update README.md --- helm/oncall/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/helm/oncall/README.md b/helm/oncall/README.md index 589bdca1..5a3613a0 100644 --- a/helm/oncall/README.md +++ b/helm/oncall/README.md @@ -13,6 +13,7 @@ Architecture diagram can be found [here](https://raw.githubusercontent.com/grafa ### Cluster requirements * ensure you can run x86-64/amd64 workloads. arm64 architecture is currently not supported +* kubernetes version 1.25+ is not supported, if cert-manager is enabled ## Install ### Prepare the repo From 16bbfbbe73fbc503d3aa196174b17c36b71eb93d Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Thu, 14 Jul 2022 15:19:25 +0100 Subject: [PATCH 16/49] Alert list view & caching rework (#216) * remove cache usage in AlertGroupView * remove CustomSearchFilter * remove caching for alerts * remove readonly db setup * render templates on alert creation * serialize only necessary fields on alert groups list * optimize AlertGroupListSerializer * return on-demand templating for alerts * return on-demand templating for alert groups * use CursorPaginator * remove templating on alert create * pass alert to AlertGroupWebRenderer * alert_count -> alerts_count * make sql joins after pagination * add migration * bring alert.save() back * fix tests * fix tests * fix tests * add perpage query param * add cursor pagination to incidents page * remove cached_render_for_web usage * post merge fix * keep cursor * lint * remove get_alert_groups_and_days_for_previous_same_period * fix pagination on navigate * refine search_fields on AlertGroupView Co-authored-by: Maxim Co-authored-by: Maxim --- .../renderers/base_renderer.py | 7 +- .../renderers/web_renderer.py | 8 +- .../migrations/0004_auto_20220711_1106.py | 21 ++ engine/apps/alerts/models/alert.py | 5 +- engine/apps/alerts/models/alert_group.py | 86 +------- .../alerts/models/alert_group_log_record.py | 6 +- .../alerts/models/alert_receive_channel.py | 16 +- engine/apps/alerts/tasks/__init__.py | 2 +- .../alerts/tasks/cache_alert_group_for_web.py | 43 +--- .../invalidate_web_cache_for_alert_group.py | 25 +-- engine/apps/api/serializers/alert_group.py | 123 ++++------- .../apps/api/serializers/resolution_note.py | 11 +- engine/apps/api/tasks.py | 55 ----- engine/apps/api/tests/test_alert_group.py | 52 ++--- engine/apps/api/views/alert_group.py | 193 +++++++----------- engine/apps/api/views/route_regex_debugger.py | 5 +- .../user_notification_policy_log_record.py | 1 - .../apps/public_api/tests/test_incidents.py | 2 +- .../slack/scenarios/alertgroup_appearance.py | 12 -- .../apps/slack/scenarios/resolution_note.py | 6 - engine/apps/user_management/models/user.py | 3 - engine/common/api_helpers/paginators.py | 9 +- .../use_random_readonly_db_manager_mixin.py | 21 -- engine/settings/dev.py | 4 - engine/settings/prod_without_db.py | 7 +- .../CursorPagination.module.css | 3 + .../CursorPagination/CursorPagination.tsx | 79 +++++++ .../IncidentsFilters/IncidentsFilters.tsx | 10 +- .../src/models/alertgroup/alertgroup.ts | 77 +++---- .../src/models/alertgroup/alertgroup.types.ts | 11 +- .../src/pages/incident/Incident.tsx | 27 ++- .../src/pages/incidents/Incidents.module.css | 5 + .../src/pages/incidents/Incidents.tsx | 156 ++++++++------ 33 files changed, 438 insertions(+), 653 deletions(-) create mode 100644 engine/apps/alerts/migrations/0004_auto_20220711_1106.py delete mode 100644 engine/apps/api/tasks.py delete mode 100644 engine/common/mixins/use_random_readonly_db_manager_mixin.py create mode 100644 grafana-plugin/src/components/CursorPagination/CursorPagination.module.css create mode 100644 grafana-plugin/src/components/CursorPagination/CursorPagination.tsx diff --git a/engine/apps/alerts/incident_appearance/renderers/base_renderer.py b/engine/apps/alerts/incident_appearance/renderers/base_renderer.py index 234c8038..f18fd6a3 100644 --- a/engine/apps/alerts/incident_appearance/renderers/base_renderer.py +++ b/engine/apps/alerts/incident_appearance/renderers/base_renderer.py @@ -18,9 +18,12 @@ class AlertBaseRenderer(ABC): class AlertGroupBaseRenderer(ABC): - def __init__(self, alert_group): + def __init__(self, alert_group, alert=None): + if alert is None: + alert = alert_group.alerts.first() + self.alert_group = alert_group - self.alert_renderer = self.alert_renderer_class(self.alert_group.alerts.first()) + self.alert_renderer = self.alert_renderer_class(alert) @property @abstractmethod diff --git a/engine/apps/alerts/incident_appearance/renderers/web_renderer.py b/engine/apps/alerts/incident_appearance/renderers/web_renderer.py index e68d453c..681f94f5 100644 --- a/engine/apps/alerts/incident_appearance/renderers/web_renderer.py +++ b/engine/apps/alerts/incident_appearance/renderers/web_renderer.py @@ -20,11 +20,11 @@ class AlertWebRenderer(AlertBaseRenderer): class AlertGroupWebRenderer(AlertGroupBaseRenderer): - def __init__(self, alert_group): - super().__init__(alert_group) + def __init__(self, alert_group, alert=None): + if alert is None: + alert = alert_group.alerts.last() - # use the last alert to render content - self.alert_renderer = self.alert_renderer_class(self.alert_group.alerts.last()) + super().__init__(alert_group, alert) @property def alert_renderer_class(self): diff --git a/engine/apps/alerts/migrations/0004_auto_20220711_1106.py b/engine/apps/alerts/migrations/0004_auto_20220711_1106.py new file mode 100644 index 00000000..28f234b8 --- /dev/null +++ b/engine/apps/alerts/migrations/0004_auto_20220711_1106.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.13 on 2022-07-11 11:06 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('alerts', '0003_grafanaalertingcontactpoint_datasource_uid'), + ] + + operations = [ + migrations.RemoveField( + model_name='alertgroup', + name='active_cache_for_web_calculation_id', + ), + migrations.RemoveField( + model_name='alertgroup', + name='cached_render_for_web', + ), + ] diff --git a/engine/apps/alerts/models/alert.py b/engine/apps/alerts/models/alert.py index 3e08e7b9..e5bd504d 100644 --- a/engine/apps/alerts/models/alert.py +++ b/engine/apps/alerts/models/alert.py @@ -5,7 +5,7 @@ from uuid import uuid4 from django.apps import apps from django.conf import settings from django.core.validators import MinLengthValidator -from django.db import models, transaction +from django.db import models from django.db.models import JSONField from django.db.models.signals import post_save @@ -261,9 +261,6 @@ def listen_for_alert_model_save(sender, instance, created, *args, **kwargs): else: distribute_alert.apply_async((instance.pk,), countdown=TASK_DELAY_SECONDS) - logger.info(f"Recalculate AG cache. Reason: save alert model {instance.pk}") - transaction.on_commit(instance.group.schedule_cache_for_web) - # Connect signal to base Alert class post_save.connect(listen_for_alert_model_save, Alert) diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 16b2d19b..0ed6b8fc 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -8,12 +8,9 @@ import pytz from celery import uuid as celery_uuid from django.apps import apps from django.conf import settings -from django.core.cache import cache from django.core.validators import MinLengthValidator -from django.db import IntegrityError, models, transaction +from django.db import IntegrityError, models from django.db.models import JSONField, Q, QuerySet -from django.db.models.signals import post_save -from django.dispatch import receiver from django.utils import timezone from django.utils.functional import cached_property @@ -22,16 +19,9 @@ from apps.alerts.incident_appearance.renderers.constants import DEFAULT_BACKUP_T from apps.alerts.incident_appearance.renderers.slack_renderer import AlertGroupSlackRenderer from apps.alerts.incident_log_builder import IncidentLogBuilder from apps.alerts.signals import alert_group_action_triggered_signal -from apps.alerts.tasks import ( - acknowledge_reminder_task, - call_ack_url, - schedule_cache_for_alert_group, - send_alert_group_signal, - unsilence_task, -) +from apps.alerts.tasks import acknowledge_reminder_task, call_ack_url, send_alert_group_signal, unsilence_task from apps.slack.slack_formatter import SlackFormatter from apps.user_management.models import User -from common.mixins.use_random_readonly_db_manager_mixin import UseRandomReadonlyDbManagerMixin from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length from common.utils import clean_markup, str_or_backup @@ -108,10 +98,6 @@ class UnarchivedAlertGroupQuerySet(models.QuerySet): return super().filter(*args, **kwargs, is_archived=False) -class AlertGroupManager(UseRandomReadonlyDbManagerMixin, models.Manager): - pass - - class AlertGroupSlackRenderingMixin: """ Ideally this mixin should not exist. Instead of this instance of AlertGroupSlackRenderer should be created and used @@ -134,8 +120,8 @@ class AlertGroupSlackRenderingMixin: class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.Model): - all_objects = AlertGroupManager.from_queryset(AlertGroupQuerySet)() - unarchived_objects = AlertGroupManager.from_queryset(UnarchivedAlertGroupQuerySet)() + all_objects = AlertGroupQuerySet.as_manager() + unarchived_objects = UnarchivedAlertGroupQuerySet.as_manager() ( NEW, @@ -242,8 +228,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. active_escalation_id = models.CharField(max_length=100, null=True, default=None) # ID generated by celery active_resolve_calculation_id = models.CharField(max_length=100, null=True, default=None) # ID generated by celery - # ID generated by celery - active_cache_for_web_calculation_id = models.CharField(max_length=100, null=True, default=None) SILENCE_DELAY_OPTIONS = ( (1800, "30 minutes"), @@ -315,8 +299,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. related_name="dependent_alert_groups", ) - cached_render_for_web = JSONField(default=dict) - last_unique_unacknowledge_process_id = models.CharField(max_length=100, null=True, default=None) is_archived = models.BooleanField(default=False) @@ -404,18 +386,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. def is_alert_a_resolve_signal(self, alert): raise NotImplementedError - def cache_for_web(self, organization): - from apps.api.serializers.alert_group import AlertGroupSerializer - - # Re-take object to switch connection from readonly db to master. - _self = AlertGroup.all_objects.get(pk=self.pk) - _self.cached_render_for_web = AlertGroupSerializer(self, context={"organization": organization}).data - self.cached_render_for_web = _self.cached_render_for_web - _self.save(update_fields=["cached_render_for_web"]) - - def schedule_cache_for_web(self): - schedule_cache_for_alert_group.apply_async((self.pk,)) - @property def permalink(self): if self.slack_message is not None: @@ -425,10 +395,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. def web_link(self): return urljoin(self.channel.organization.web_link, f"?page=incident&id={self.public_primary_key}") - @property - def alerts_count(self): - return self.alerts.count() - @property def happened_while_maintenance(self): return self.root_alert_group is not None and self.root_alert_group.maintenance_uuid is not None @@ -449,10 +415,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. self.unresolve() self.log_records.create(type=AlertGroupLogRecord.TYPE_UN_RESOLVED, author=user, reason="Acknowledge button") - # clear resolve report cache - cache_key = "render_after_resolve_report_json_{}".format(self.pk) - cache.delete(cache_key) - self.acknowledge(acknowledged_by_user=user, acknowledged_by=AlertGroup.USER) self.stop_escalation() if self.is_root_alert_group: @@ -673,9 +635,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. self.unresolve() log_record = self.log_records.create(type=AlertGroupLogRecord.TYPE_UN_RESOLVED, author=user) - # clear resolve report cache - self.drop_cached_after_resolve_report_json() - if self.is_root_alert_group: self.start_escalation_if_needed() @@ -848,10 +807,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. self.unresolve() self.log_records.create(type=AlertGroupLogRecord.TYPE_UN_RESOLVED, author=user, reason="Silence button") - # clear resolve report cache - cache_key = "render_after_resolve_report_json_{}".format(self.pk) - cache.delete(cache_key) - if self.acknowledged: self.unacknowledge() self.log_records.create(type=AlertGroupLogRecord.TYPE_UN_ACK, author=user, reason="Silence button") @@ -1060,8 +1015,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. author=user, reason="Bulk action acknowledge", ) - # clear resolve report cache - alert_group.drop_cached_after_resolve_report_json() for alert_group in alert_groups_to_unsilence_before_acknowledge_list: alert_group.log_records.create( @@ -1194,8 +1147,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. reason="Bulk action restart", ) - alert_group.drop_cached_after_resolve_report_json() - if alert_group.is_root_alert_group: alert_group.start_escalation_if_needed() @@ -1293,7 +1244,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. author=user, reason="Bulk action silence", ) - alert_group.drop_cached_after_resolve_report_json() for alert_group in alert_groups_to_unsilence_before_silence_list: alert_group.log_records.create( @@ -1483,7 +1433,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. else: return "Acknowledged" - def non_cached_after_resolve_report_json(self): + def render_after_resolve_report_json(self): AlertGroupLogRecord = apps.get_model("alerts", "AlertGroupLogRecord") UserNotificationPolicyLogRecord = apps.get_model("base", "UserNotificationPolicyLogRecord") ResolutionNote = apps.get_model("alerts", "ResolutionNote") @@ -1501,21 +1451,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. result_log_report.append(log_record.render_log_line_json()) return result_log_report - def render_after_resolve_report_json(self): - cache_key = "render_after_resolve_report_json_{}".format(self.pk) - - # cache.get_or_set in some cases returns None, so use get and set cache methods separately - log_report = cache.get(cache_key) - if log_report is None: - log_report = self.non_cached_after_resolve_report_json() - cache.set(cache_key, log_report) - return log_report - - def drop_cached_after_resolve_report_json(self): - cache_key = "render_after_resolve_report_json_{}".format(self.pk) - if cache_key in cache: - cache.delete(cache_key) - @property def has_resolution_notes(self): return self.resolution_notes.exists() @@ -1595,14 +1530,3 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. ) return stop_escalation_log - - -@receiver(post_save, sender=AlertGroup) -def listen_for_alert_group_model_save(sender, instance, created, *args, **kwargs): - if ( - kwargs is not None - and "update_fields" in kwargs - and kwargs["update_fields"] is dict - and "cached_render_for_web" not in kwargs["update_fields"] - ): - transaction.on_commit(instance.schedule_cache_for_alert_group) diff --git a/engine/apps/alerts/models/alert_group_log_record.py b/engine/apps/alerts/models/alert_group_log_record.py index 7e5e30c9..c2bacc7d 100644 --- a/engine/apps/alerts/models/alert_group_log_record.py +++ b/engine/apps/alerts/models/alert_group_log_record.py @@ -3,7 +3,7 @@ import logging import humanize from django.apps import apps -from django.db import models, transaction +from django.db import models from django.db.models import JSONField from django.db.models.signals import post_save from django.dispatch import receiver @@ -546,7 +546,6 @@ class AlertGroupLogRecord(models.Model): @receiver(post_save, sender=AlertGroupLogRecord) def listen_for_alertgrouplogrecord(sender, instance, created, *args, **kwargs): - instance.alert_group.drop_cached_after_resolve_report_json() if instance.type != AlertGroupLogRecord.TYPE_DELETED: if not instance.alert_group.is_maintenance_incident: alert_group_pk = instance.alert_group.pk @@ -555,6 +554,3 @@ def listen_for_alertgrouplogrecord(sender, instance, created, *args, **kwargs): f"alert group event: {instance.get_type_display()}" ) send_update_log_report_signal.apply_async(kwargs={"alert_group_pk": alert_group_pk}, countdown=8) - - logger.info(f"Recalculate AG cache. Reason: save alert_group_log_record model {instance.pk}") - transaction.on_commit(instance.alert_group.schedule_cache_for_web) diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 308686c0..643f737e 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -19,11 +19,7 @@ from jinja2 import Template from apps.alerts.grafana_alerting_sync_manager.grafana_alerting_sync import GrafanaAlertingSyncManager from apps.alerts.integration_options_mixin import IntegrationOptionsMixin from apps.alerts.models.maintainable_object import MaintainableObject -from apps.alerts.tasks import ( - disable_maintenance, - invalidate_web_cache_for_alert_group, - sync_grafana_alerting_contact_points, -) +from apps.alerts.tasks import disable_maintenance, sync_grafana_alerting_contact_points from apps.base.messaging import get_messaging_backend_from_id from apps.base.utils import live_settings from apps.integrations.metadata import heartbeat @@ -693,16 +689,6 @@ def listen_for_alertreceivechannel_model_save(sender, instance, created, *args, create_organization_log( instance.organization, None, OrganizationLogType.TYPE_HEARTBEAT_CREATED, description ) - else: - logger.info(f"Drop AG cache. Reason: save alert_receive_channel {instance.pk}") - if kwargs is not None: - if "update_fields" in kwargs: - if kwargs["update_fields"] is not None: - # Hack to not to invalidate web cache on AlertReceiveChannel.start_send_rate_limit_message_task - if "rate_limit_message_task_id" in kwargs["update_fields"]: - return - - invalidate_web_cache_for_alert_group.apply_async(kwargs={"channel_pk": instance.pk}) if instance.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING: if created: diff --git a/engine/apps/alerts/tasks/__init__.py b/engine/apps/alerts/tasks/__init__.py index 3ff8501e..79b8b0ed 100644 --- a/engine/apps/alerts/tasks/__init__.py +++ b/engine/apps/alerts/tasks/__init__.py @@ -9,7 +9,7 @@ from .custom_button_result import custom_button_result # noqa: F401 from .delete_alert_group import delete_alert_group # noqa: F401 from .distribute_alert import distribute_alert # noqa: F401 from .escalate_alert_group import escalate_alert_group # noqa: F401 -from .invalidate_web_cache_for_alert_group import invalidate_web_cache_for_alert_group # noqa: F401 +from .invalidate_web_cache_for_alert_group import invalidate_web_cache_for_alert_group # noqa: F401, todo: remove from .invite_user_to_join_incident import invite_user_to_join_incident # noqa: F401 from .maintenance import disable_maintenance # noqa: F401 from .notify_all import notify_all_task # noqa: F401 diff --git a/engine/apps/alerts/tasks/cache_alert_group_for_web.py b/engine/apps/alerts/tasks/cache_alert_group_for_web.py index 677e0a19..5f0c52d5 100644 --- a/engine/apps/alerts/tasks/cache_alert_group_for_web.py +++ b/engine/apps/alerts/tasks/cache_alert_group_for_web.py @@ -1,54 +1,19 @@ -from celery.utils.log import get_task_logger -from django.apps import apps from django.conf import settings -from django.core.cache import cache from common.custom_celery_tasks import shared_dedicated_queue_retry_task -logger = get_task_logger(__name__) - - -def get_cache_key_caching_alert_group_for_web(alert_group_pk): - CACHE_KEY_PREFIX = "cache_alert_group_for_web" - return f"{CACHE_KEY_PREFIX}_{alert_group_pk}" - @shared_dedicated_queue_retry_task( autoretry_for=(Exception,), retry_backoff=True, max_retries=0 if settings.DEBUG else None ) def schedule_cache_for_alert_group(alert_group_pk): - CACHE_FOR_ALERT_GROUP_LIFETIME = 60 - START_CACHE_DELAY = 5 # we introduce delay to avoid recaching after each alert. - - task = cache_alert_group_for_web.apply_async(args=[alert_group_pk], countdown=START_CACHE_DELAY) - cache_key = get_cache_key_caching_alert_group_for_web(alert_group_pk) - cache.set(cache_key, task.id, timeout=CACHE_FOR_ALERT_GROUP_LIFETIME) + # todo: remove + pass @shared_dedicated_queue_retry_task( autoretry_for=(Exception,), retry_backoff=True, max_retries=0 if settings.DEBUG else None ) def cache_alert_group_for_web(alert_group_pk): - """ - Async task to re-cache alert_group for web. - """ - cache_key = get_cache_key_caching_alert_group_for_web(alert_group_pk) - cached_task_id = cache.get(cache_key) - current_task_id = cache_alert_group_for_web.request.id - - if cached_task_id is None: - return ( - f"cache_alert_group_for_web skipped, because of current task_id ({current_task_id})" - f" for alert_group {alert_group_pk} doesn't exist in cache, which means this task is not" - f" relevant: cache was dropped by engine restart ot CACHE_FOR_ALERT_GROUP_LIFETIME" - ) - if not current_task_id == cached_task_id or cached_task_id is None: - return ( - f"cache_alert_group_for_web skipped, because of current task_id ({current_task_id})" - f" doesn't equal to cached task_id ({cached_task_id}) for alert_group {alert_group_pk}," - ) - else: - AlertGroup = apps.get_model("alerts", "AlertGroup") - alert_group = AlertGroup.all_objects.using_readonly_db.get(pk=alert_group_pk) - alert_group.cache_for_web(alert_group.channel.organization) - logger.info(f"cache_alert_group_for_web: cache refreshed for alert_group {alert_group_pk}") + # todo: remove + pass diff --git a/engine/apps/alerts/tasks/invalidate_web_cache_for_alert_group.py b/engine/apps/alerts/tasks/invalidate_web_cache_for_alert_group.py index d9c7c4f9..9c8786d9 100644 --- a/engine/apps/alerts/tasks/invalidate_web_cache_for_alert_group.py +++ b/engine/apps/alerts/tasks/invalidate_web_cache_for_alert_group.py @@ -1,32 +1,11 @@ -from django.apps import apps from django.conf import settings from common.custom_celery_tasks import shared_dedicated_queue_retry_task -from .task_logger import task_logger - @shared_dedicated_queue_retry_task( autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None ) def invalidate_web_cache_for_alert_group(org_pk=None, channel_pk=None, alert_group_pk=None, alert_group_pks=None): - AlertGroup = apps.get_model("alerts", "AlertGroup") - DynamicSetting = apps.get_model("base", "DynamicSetting") - - if channel_pk: - task_logger.debug(f"invalidate_web_cache_for_alert_group: Reason - alert_receive_channel {channel_pk}") - q = AlertGroup.all_objects.filter(channel__pk=channel_pk) - elif org_pk: - task_logger.debug(f"invalidate_web_cache_for_alert_group: Reason - organization {org_pk}") - q = AlertGroup.all_objects.filter(channel__organization__pk=org_pk) - elif alert_group_pk: - task_logger.debug(f"invalidate_web_cache_for_alert_group: Reason - alert_group {alert_group_pk}") - q = AlertGroup.all_objects.filter(pk=alert_group_pk) - elif alert_group_pks: - task_logger.debug(f"invalidate_web_cache_for_alert_group: Reason - alert_groups {alert_group_pks}") - q = AlertGroup.all_objects.filter(pk__in=alert_group_pks) - - skip_task = DynamicSetting.objects.get_or_create(name="skip_invalidate_web_cache_for_alert_group")[0] - if skip_task.boolean_value: - return "Task has been skipped because of skip_invalidate_web_cache_for_alert_group DynamicSetting" - q.update(cached_render_for_web={}) + # todo: remove + pass diff --git a/engine/apps/api/serializers/alert_group.py b/engine/apps/api/serializers/alert_group.py index 6ffc90d0..824716fb 100644 --- a/engine/apps/api/serializers/alert_group.py +++ b/engine/apps/api/serializers/alert_group.py @@ -1,7 +1,5 @@ import logging -from datetime import datetime -import humanize from rest_framework import serializers from apps.alerts.incident_appearance.renderers.web_renderer import AlertGroupWebRenderer @@ -28,50 +26,30 @@ class ShortAlertGroupSerializer(serializers.ModelSerializer): return AlertGroupWebRenderer(obj).render() -class AlertGroupSerializer(EagerLoadingMixin, serializers.ModelSerializer): - """ - Attention: It's heavily cached. Make sure to invalidate alertgroup's web cache if you update the format! - """ - +class AlertGroupListSerializer(EagerLoadingMixin, serializers.ModelSerializer): pk = serializers.CharField(read_only=True, source="public_primary_key") alert_receive_channel = FastAlertReceiveChannelSerializer(source="channel") - alerts = serializers.SerializerMethodField("get_limited_alerts") - resolved_by_verbose = serializers.CharField(source="get_resolved_by_display") + status = serializers.ReadOnlyField() resolved_by_user = FastUserSerializer(required=False) acknowledged_by_user = FastUserSerializer(required=False) silenced_by_user = FastUserSerializer(required=False) related_users = serializers.SerializerMethodField() - - last_alert_at = serializers.SerializerMethodField() - - started_at_verbose = serializers.SerializerMethodField() - acknowledged_at_verbose = serializers.SerializerMethodField() - resolved_at_verbose = serializers.SerializerMethodField() - silenced_at_verbose = serializers.SerializerMethodField() - dependent_alert_groups = ShortAlertGroupSerializer(many=True) root_alert_group = ShortAlertGroupSerializer() - alerts_count = serializers.ReadOnlyField() - - status = serializers.ReadOnlyField() + alerts_count = serializers.IntegerField(read_only=True) render_for_web = serializers.SerializerMethodField() PREFETCH_RELATED = [ - "alerts", "dependent_alert_groups", - "log_records", "log_records__author", - "log_records__escalation_policy", - "log_records__invitation__invitee", ] SELECT_RELATED = [ - "slack_message", "channel__organization", - "slack_message___slack_team_identity", - "acknowledged_by_user", + "root_alert_group", "resolved_by_user", + "acknowledged_by_user", "silenced_by_user", ] @@ -85,7 +63,6 @@ class AlertGroupSerializer(EagerLoadingMixin, serializers.ModelSerializer): "alert_receive_channel", "resolved", "resolved_by", - "resolved_by_verbose", "resolved_by_user", "resolved_at", "acknowledged_at", @@ -96,44 +73,18 @@ class AlertGroupSerializer(EagerLoadingMixin, serializers.ModelSerializer): "silenced", "silenced_by_user", "silenced_at", - "silenced_at_verbose", "silenced_until", "started_at", - "last_alert_at", "silenced_until", - "permalink", - "alerts", "related_users", - "started_at_verbose", - "acknowledged_at_verbose", - "resolved_at_verbose", "render_for_web", - "render_after_resolve_report_json", "dependent_alert_groups", "root_alert_group", "status", ] - def get_last_alert_at(self, obj): - last_alert = obj.alerts.last() - # TODO: This is a Hotfix for 0.0.27 - if last_alert is None: - logger.warning(f"obj {obj} doesn't have last_alert!") - return "" - return str(last_alert.created_at) - - def get_limited_alerts(self, obj): - """ - Overriding default alerts because there are alert_groups with thousands of them. - It's just too slow, we need to cut here. - """ - alerts = obj.alerts.all()[:100] - - if len(alerts) > 90: - for alert in alerts: - alert.title = str(alert.title) + " Only last 100 alerts are shown. Use Amixr API to fetch all of them." - - return AlertSerializer(alerts, many=True).data + def get_render_for_web(self, obj): + return AlertGroupWebRenderer(obj, obj.last_alert).render() def get_related_users(self, obj): users_ids = set() @@ -159,37 +110,39 @@ class AlertGroupSerializer(EagerLoadingMixin, serializers.ModelSerializer): users_ids.add(log_record.author.public_primary_key) return users - def get_started_at_verbose(self, obj): - started_at_verbose = None - if obj.started_at is not None: - started_at_verbose = humanize.naturaltime( - datetime.now().replace(tzinfo=None) - obj.started_at.replace(tzinfo=None) - ) - return started_at_verbose - def get_acknowledged_at_verbose(self, obj): - acknowledged_at_verbose = None - if obj.acknowledged_at is not None: - acknowledged_at_verbose = humanize.naturaltime( - datetime.now().replace(tzinfo=None) - obj.acknowledged_at.replace(tzinfo=None) - ) # TODO: Deal with timezones - return acknowledged_at_verbose +class AlertGroupSerializer(AlertGroupListSerializer): + alerts = serializers.SerializerMethodField("get_limited_alerts") + last_alert_at = serializers.SerializerMethodField() - def get_resolved_at_verbose(self, obj): - resolved_at_verbose = None - if obj.resolved_at is not None: - resolved_at_verbose = humanize.naturaltime( - datetime.now().replace(tzinfo=None) - obj.resolved_at.replace(tzinfo=None) - ) # TODO: Deal with timezones - return resolved_at_verbose - - def get_silenced_at_verbose(self, obj): - silenced_at_verbose = None - if obj.silenced_at is not None: - silenced_at_verbose = humanize.naturaltime( - datetime.now().replace(tzinfo=None) - obj.silenced_at.replace(tzinfo=None) - ) # TODO: Deal with timezones - return silenced_at_verbose + class Meta(AlertGroupListSerializer.Meta): + fields = AlertGroupListSerializer.Meta.fields + [ + "alerts", + "render_after_resolve_report_json", + "permalink", + "last_alert_at", + ] def get_render_for_web(self, obj): return AlertGroupWebRenderer(obj).render() + + def get_last_alert_at(self, obj): + last_alert = obj.alerts.last() + + if not last_alert: + return obj.started_at + + return last_alert.created_at + + def get_limited_alerts(self, obj): + """ + Overriding default alerts because there are alert_groups with thousands of them. + It's just too slow, we need to cut here. + """ + alerts = obj.alerts.all()[:100] + + if len(alerts) > 90: + for alert in alerts: + alert.title = str(alert.title) + " Only last 100 alerts are shown. Use OnCall API to fetch all of them." + + return AlertSerializer(alerts, many=True).data diff --git a/engine/apps/api/serializers/resolution_note.py b/engine/apps/api/serializers/resolution_note.py index 330259e3..00178685 100644 --- a/engine/apps/api/serializers/resolution_note.py +++ b/engine/apps/api/serializers/resolution_note.py @@ -1,7 +1,6 @@ from rest_framework import serializers from apps.alerts.models import AlertGroup, ResolutionNote -from apps.alerts.tasks import invalidate_web_cache_for_alert_group from apps.api.serializers.user import FastUserSerializer from common.api_helpers.custom_fields import OrganizationFilteredPrimaryKeyRelatedField from common.api_helpers.exceptions import BadRequest @@ -39,9 +38,6 @@ class ResolutionNoteSerializer(EagerLoadingMixin, serializers.ModelSerializer): validated_data["author"] = self.context["request"].user validated_data["source"] = ResolutionNote.Source.WEB created_instance = super().create(validated_data) - # Invalidate alert group cache because resolution notes shown in alert group's timeline - created_instance.alert_group.drop_cached_after_resolve_report_json() - invalidate_web_cache_for_alert_group(alert_group_pk=created_instance.alert_group.pk) return created_instance def to_representation(self, instance): @@ -57,8 +53,5 @@ class ResolutionNoteUpdateSerializer(ResolutionNoteSerializer): def update(self, instance, validated_data): if instance.source != ResolutionNote.Source.WEB: raise BadRequest(detail="Cannot update message with this source type") - updated_instance = super().update(instance, validated_data) - # Invalidate alert group cache because resolution notes shown in alert group's timeline - updated_instance.alert_group.drop_cached_after_resolve_report_json() - invalidate_web_cache_for_alert_group(alert_group_pk=updated_instance.alert_group.pk) - return updated_instance + + return super().update(instance, validated_data) diff --git a/engine/apps/api/tasks.py b/engine/apps/api/tasks.py deleted file mode 100644 index 4240178a..00000000 --- a/engine/apps/api/tasks.py +++ /dev/null @@ -1,55 +0,0 @@ -from celery.utils.log import get_task_logger -from django.apps import apps -from django.conf import settings -from django.core.cache import cache - -from common.custom_celery_tasks import shared_dedicated_queue_retry_task - -logger = get_task_logger(__name__) - - -def get_cache_key_caching_alert_group_for_web(alert_group_pk): - CACHE_KEY_PREFIX = "cache_alert_group_for_web" - return f"{CACHE_KEY_PREFIX}_{alert_group_pk}" - - -# TODO: remove this tasks after all of them will be processed in prod -@shared_dedicated_queue_retry_task( - autoretry_for=(Exception,), retry_backoff=True, max_retries=0 if settings.DEBUG else None -) -def schedule_cache_for_alert_group(alert_group_pk): - CACHE_FOR_ALERT_GROUP_LIFETIME = 60 - START_CACHE_DELAY = 5 # we introduce delay to avoid recaching after each alert. - - task = cache_alert_group_for_web.apply_async(args=[alert_group_pk], countdown=START_CACHE_DELAY) - cache_key = get_cache_key_caching_alert_group_for_web(alert_group_pk) - cache.set(cache_key, task.id, timeout=CACHE_FOR_ALERT_GROUP_LIFETIME) - - -@shared_dedicated_queue_retry_task( - autoretry_for=(Exception,), retry_backoff=True, max_retries=0 if settings.DEBUG else None -) -def cache_alert_group_for_web(alert_group_pk): - """ - Async task to re-cache alert_group for web. - """ - cache_key = get_cache_key_caching_alert_group_for_web(alert_group_pk) - cached_task_id = cache.get(cache_key) - current_task_id = cache_alert_group_for_web.request.id - - if cached_task_id is None: - return ( - f"cache_alert_group_for_web skipped, because of current task_id ({current_task_id})" - f" for alert_group {alert_group_pk} doesn't exist in cache, which means this task is not" - f" relevant: cache was dropped by engine restart ot CACHE_FOR_ALERT_GROUP_LIFETIME" - ) - if not current_task_id == cached_task_id or cached_task_id is None: - return ( - f"cache_alert_group_for_web skipped, because of current task_id ({current_task_id})" - f" doesn't equal to cached task_id ({cached_task_id}) for alert_group {alert_group_pk}," - ) - else: - AlertGroup = apps.get_model("alerts", "AlertGroup") - alert_group = AlertGroup.all_objects.using_readonly_db.get(pk=alert_group_pk) - alert_group.cache_for_web(alert_group.channel.organization) - logger.info(f"cache_alert_group_for_web: cache refreshed for alert_group {alert_group_pk}") diff --git a/engine/apps/api/tests/test_alert_group.py b/engine/apps/api/tests/test_alert_group.py index 983a22bf..6d4a0b9e 100644 --- a/engine/apps/api/tests/test_alert_group.py +++ b/engine/apps/api/tests/test_alert_group.py @@ -63,7 +63,7 @@ def test_get_filter_started_at(alert_group_internal_api_setup, make_user_auth_he ) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 4 + assert len(response.data["results"]) == 4 @pytest.mark.django_db @@ -78,7 +78,7 @@ def test_get_filter_resolved_at_alertgroup_empty_result(alert_group_internal_api **make_user_auth_headers(user, token), ) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 0 + assert len(response.data["results"]) == 0 @pytest.mark.django_db @@ -105,7 +105,7 @@ def test_get_filter_resolved_at(alert_group_internal_api_setup, make_user_auth_h **make_user_auth_headers(user, token), ) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 1 + assert len(response.data["results"]) == 1 @pytest.mark.django_db @@ -117,7 +117,7 @@ def test_status_new(alert_group_internal_api_setup, make_user_auth_headers): url = reverse("api-internal:alertgroup-list") response = client.get(url + "?status=0", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 1 + assert len(response.data["results"]) == 1 assert response.data["results"][0]["pk"] == new_alert_group.public_primary_key @@ -130,7 +130,7 @@ def test_status_ack(alert_group_internal_api_setup, make_user_auth_headers): url = reverse("api-internal:alertgroup-list") response = client.get(url + "?status=1", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 1 + assert len(response.data["results"]) == 1 assert response.data["results"][0]["pk"] == ack_alert_group.public_primary_key @@ -143,7 +143,7 @@ def test_status_resolved(alert_group_internal_api_setup, make_user_auth_headers) url = reverse("api-internal:alertgroup-list") response = client.get(url + "?status=2", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 1 + assert len(response.data["results"]) == 1 assert response.data["results"][0]["pk"] == resolved_alert_group.public_primary_key @@ -156,7 +156,7 @@ def test_status_silenced(alert_group_internal_api_setup, make_user_auth_headers) url = reverse("api-internal:alertgroup-list") response = client.get(url + "?status=3", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 1 + assert len(response.data["results"]) == 1 assert response.data["results"][0]["pk"] == silenced_alert_group.public_primary_key @@ -171,7 +171,7 @@ def test_all_statuses(alert_group_internal_api_setup, make_user_auth_headers): url + "?status=0&status=1&&status=2&status=3", format="json", **make_user_auth_headers(user, token) ) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 4 + assert len(response.data["results"]) == 4 @pytest.mark.django_db @@ -213,7 +213,7 @@ def test_get_filter_resolved_by( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 1 + assert len(first_response.data["results"]) == 1 second_response = client.get( url + f"?resolved_by={second_user.public_primary_key}", @@ -221,7 +221,7 @@ def test_get_filter_resolved_by( **make_user_auth_headers(first_user, token), ) assert second_response.status_code == status.HTTP_200_OK - assert second_response.data["count"] == 0 + assert len(second_response.data["results"]) == 0 @pytest.mark.django_db @@ -269,7 +269,7 @@ def test_get_filter_resolved_by_multiple_values( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 2 + assert len(first_response.data["results"]) == 2 @pytest.mark.django_db @@ -309,7 +309,7 @@ def test_get_filter_acknowledged_by( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 1 + assert len(first_response.data["results"]) == 1 second_response = client.get( url + f"?acknowledged_by={second_user.public_primary_key}", @@ -317,7 +317,7 @@ def test_get_filter_acknowledged_by( **make_user_auth_headers(first_user, token), ) assert second_response.status_code == status.HTTP_200_OK - assert second_response.data["count"] == 0 + assert len(second_response.data["results"]) == 0 @pytest.mark.django_db @@ -363,7 +363,7 @@ def test_get_filter_acknowledged_by_multiple_values( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 2 + assert len(first_response.data["results"]) == 2 @pytest.mark.django_db @@ -402,7 +402,7 @@ def test_get_filter_silenced_by( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 1 + assert len(first_response.data["results"]) == 1 second_response = client.get( url + f"?silenced_by={second_user.public_primary_key}", @@ -410,7 +410,7 @@ def test_get_filter_silenced_by( **make_user_auth_headers(first_user, token), ) assert second_response.status_code == status.HTTP_200_OK - assert second_response.data["count"] == 0 + assert len(second_response.data["results"]) == 0 @pytest.mark.django_db @@ -455,7 +455,7 @@ def test_get_filter_silenced_by_multiple_values( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 2 + assert len(first_response.data["results"]) == 2 @pytest.mark.django_db @@ -494,7 +494,7 @@ def test_get_filter_invitees_are( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 1 + assert len(first_response.data["results"]) == 1 second_response = client.get( url + f"?invitees_are={second_user.public_primary_key}", @@ -502,7 +502,7 @@ def test_get_filter_invitees_are( **make_user_auth_headers(first_user, token), ) assert second_response.status_code == status.HTTP_200_OK - assert second_response.data["count"] == 0 + assert len(second_response.data["results"]) == 0 @pytest.mark.django_db @@ -548,7 +548,7 @@ def test_get_filter_invitees_are_multiple_values( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 2 + assert len(first_response.data["results"]) == 2 @pytest.mark.django_db @@ -593,7 +593,7 @@ def test_get_filter_invitees_are_ag_with_multiple_logs( **make_user_auth_headers(first_user, token), ) assert first_response.status_code == status.HTTP_200_OK - assert first_response.data["count"] == 1 + assert len(first_response.data["results"]) == 1 @pytest.mark.django_db @@ -611,11 +611,11 @@ def test_get_filter_with_resolution_note( # there are no alert groups with resolution_notes response = client.get(url + "?with_resolution_note=true", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 0 + assert len(response.data["results"]) == 0 response = client.get(url + "?with_resolution_note=false", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 4 + assert len(response.data["results"]) == 4 # add resolution_notes to two of four alert groups make_resolution_note(res_alert_group) @@ -623,11 +623,11 @@ def test_get_filter_with_resolution_note( response = client.get(url + "?with_resolution_note=true", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 2 + assert len(response.data["results"]) == 2 response = client.get(url + "?with_resolution_note=false", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 2 + assert len(response.data["results"]) == 2 @pytest.mark.django_db @@ -653,7 +653,7 @@ def test_get_filter_with_resolution_note_after_delete_resolution_note( response = client.get(url + "?with_resolution_note=true", format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 1 + assert len(response.data["results"]) == 1 @pytest.mark.django_db diff --git a/engine/apps/api/views/alert_group.py b/engine/apps/api/views/alert_group.py index 5ea7e93b..822fe9f5 100644 --- a/engine/apps/api/views/alert_group.py +++ b/engine/apps/api/views/alert_group.py @@ -1,10 +1,6 @@ -from datetime import datetime, timedelta +from datetime import timedelta -from django import forms -from django.db import models -from django.db.models import CharField, Q -from django.db.models.constants import LOOKUP_SEP -from django.db.models.functions import Cast +from django.db.models import Count, Max, Q from django.utils import timezone from django_filters import rest_framework as filters from django_filters.widgets import RangeWidget @@ -15,16 +11,15 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from apps.alerts.constants import ActionSource -from apps.alerts.models import AlertGroup, AlertReceiveChannel -from apps.alerts.tasks import invalidate_web_cache_for_alert_group +from apps.alerts.models import Alert, AlertGroup, AlertReceiveChannel from apps.api.permissions import MODIFY_ACTIONS, READ_ACTIONS, ActionPermission, AnyRole, IsAdminOrEditor -from apps.api.serializers.alert_group import AlertGroupSerializer +from apps.api.serializers.alert_group import AlertGroupListSerializer, AlertGroupSerializer from apps.auth_token.auth import MobileAppAuthTokenAuthentication, PluginAuthentication from apps.user_management.models import User from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import DateRangeFilterMixin, ModelFieldFilterMixin from common.api_helpers.mixins import PreviewTemplateMixin, PublicPrimaryKeyMixin -from common.api_helpers.paginators import FiftyPageSizePaginator +from common.api_helpers.paginators import TwentyFiveCursorPaginator def get_integration_queryset(request): @@ -148,34 +143,6 @@ class AlertGroupFilter(DateRangeFilterMixin, ModelFieldFilterMixin, filters.Filt return queryset -class CustomSearchFilter(SearchFilter): - def must_call_distinct(self, queryset, search_fields): - """ - Return True if 'distinct()' should be used to query the given lookups. - """ - for search_field in search_fields: - opts = queryset.model._meta - if search_field[0] in self.lookup_prefixes: - search_field = search_field[1:] - - # From https://github.com/encode/django-rest-framework/pull/6240/files#diff-01f357e474dd8fd702e4951b9227bffcR88 - # Annotated fields do not need to be distinct - if isinstance(queryset, models.QuerySet) and search_field in queryset.query.annotations: - continue - - parts = search_field.split(LOOKUP_SEP) - for part in parts: - field = opts.get_field(part) - if hasattr(field, "get_path_info"): - # This field is a relation, update opts to follow the relation - path_info = field.get_path_info() - opts = path_info[-1].to_opts - if any(path.m2m for path in path_info): - # This field is a m2m relation so we know we need to call distinct - return True - return False - - class AlertGroupView( PreviewTemplateMixin, PublicPrimaryKeyMixin, @@ -216,90 +183,85 @@ class AlertGroupView( serializer_class = AlertGroupSerializer - pagination_class = FiftyPageSizePaginator + pagination_class = TwentyFiveCursorPaginator - filter_backends = [CustomSearchFilter, filters.DjangoFilterBackend] - search_fields = ["cached_render_for_web_str"] + filter_backends = [SearchFilter, filters.DjangoFilterBackend] + # todo: add ability to search by templated title + search_fields = ["public_primary_key", "inside_organization_number"] filterset_class = AlertGroupFilter - def list(self, request, *args, **kwargs): - """ - It's compute-heavy so we rely on cache here. - Attention: Make sure to invalidate cache if you update the format! - """ - queryset = self.filter_queryset(self.get_queryset(eager=False, readonly=True)) + def get_serializer_class(self): + if self.action == "list": + return AlertGroupListSerializer - page = self.paginate_queryset(queryset) - skip_slow_rendering = request.query_params.get("skip_slow_rendering") == "true" - data = [] + return super().get_serializer_class() - for alert_group in page: - if alert_group.cached_render_for_web == {}: - # We cannot give empty data to web. So caching synchronously here. - if skip_slow_rendering: - # We just return dummy data. - # Cache is not launched because after skip_slow_rendering request should come usual one - # which will start caching - data.append({"pk": alert_group.pk, "short": True}) - else: - # Synchronously cache and return. It could be slow. - alert_group.cache_for_web(alert_group.channel.organization) - data.append(alert_group.cached_render_for_web) - else: - data.append(alert_group.cached_render_for_web) - if not skip_slow_rendering: - # Cache is not launched because after skip_slow_rendering request should come usual one - # which will start caching - alert_group.schedule_cache_for_web() + def get_queryset(self): + # make a separate query to fetch all the integrations for current organization and team (it's faster) + alert_receive_channel_pks = AlertReceiveChannel.objects_with_deleted.filter( + organization=self.request.auth.organization, team=self.request.user.current_team + ).values_list("pk", flat=True) + alert_receive_channel_pks = list(alert_receive_channel_pks) - return self.get_paginated_response(data) + # no select_related or prefetch_related is used at this point, it will be done on paginate_queryset. + queryset = AlertGroup.unarchived_objects.filter(channel_id__in=alert_receive_channel_pks) - def get_queryset(self, eager=True, readonly=False, order=True): - if readonly: - queryset = AlertGroup.unarchived_objects.using_readonly_db - else: - queryset = AlertGroup.unarchived_objects - - queryset = queryset.filter( - channel__organization=self.request.auth.organization, - channel__team=self.request.user.current_team, - ) - - if order: - queryset = queryset.order_by("-started_at") - - queryset = queryset.annotate(cached_render_for_web_str=Cast("cached_render_for_web", output_field=CharField())) - - if eager: - queryset = self.serializer_class.setup_eager_loading(queryset) return queryset - def get_alert_groups_and_days_for_previous_same_period(self): - prev_alert_groups = AlertGroup.unarchived_objects.none() - delta_days = None + def paginate_queryset(self, queryset): + """ + All SQL joins (select_related and prefetch_related) will be performed AFTER pagination, so it only joins tables + for 25 alert groups, not the whole table. + """ + alert_groups = super().paginate_queryset(queryset) + alert_groups = self.enrich(alert_groups) + return alert_groups - started_at = self.request.query_params.get("started_at", None) - if started_at is not None: - started_at_gte, started_at_lte = AlertGroupFilter.parse_custom_datetime_range(started_at) - delta_days = None - if started_at_lte is not None: - started_at_lte = forms.DateTimeField().to_python(started_at_lte) - else: - started_at_lte = datetime.now() + def get_object(self): + obj = super().get_object() + obj = self.enrich([obj])[0] + return obj - if started_at_gte is not None: - started_at_gte = forms.DateTimeField().to_python(value=started_at_gte) - delta = started_at_lte.replace(tzinfo=None) - started_at_gte.replace(tzinfo=None) - prev_alert_groups = self.get_queryset().filter( - started_at__range=[started_at_gte - delta, started_at_gte] - ) - delta_days = delta.days - return prev_alert_groups, delta_days + def enrich(self, alert_groups): + """ + This method performs select_related and prefetch_related (using setup_eager_loading) as well as in-memory joins + to add additional info like alert_count and last_alert for every alert group efficiently. + We need the last_alert because it's used by AlertGroupWebRenderer. + """ + + # enrich alert groups with select_related and prefetch_related + alert_group_pks = [alert_group.pk for alert_group in alert_groups] + queryset = AlertGroup.all_objects.filter(pk__in=alert_group_pks).order_by("-pk") + queryset = self.get_serializer_class().setup_eager_loading(queryset) + alert_groups = list(queryset) + + # get info on alerts count and last alert ID for every alert group + alerts_info = ( + Alert.objects.values("group_id") + .filter(group_id__in=alert_group_pks) + .annotate(alerts_count=Count("group_id"), last_alert_id=Max("id")) + ) + alerts_info_map = {info["group_id"]: info for info in alerts_info} + + # fetch last alerts for every alert group + last_alert_ids = [info["last_alert_id"] for info in alerts_info_map.values()] + last_alerts = Alert.objects.filter(pk__in=last_alert_ids) + for alert in last_alerts: + # link group back to alert + alert.group = [alert_group for alert_group in alert_groups if alert_group.pk == alert.group_id][0] + alerts_info_map[alert.group_id].update({"last_alert": alert}) + + # add additional "alerts_count" and "last_alert" fields to every alert group + for alert_group in alert_groups: + alert_group.last_alert = alerts_info_map[alert_group.pk]["last_alert"] + alert_group.alerts_count = alerts_info_map[alert_group.pk]["alerts_count"] + + return alert_groups @action(detail=False) def stats(self, *args, **kwargs): - alert_groups = self.filter_queryset(self.get_queryset(eager=False)) + alert_groups = self.filter_queryset(self.get_queryset()) # Only count field is used, other fields left just in case for the backward compatibility return Response( { @@ -324,7 +286,6 @@ class AlertGroupView( if alert_group.root_alert_group is not None: raise BadRequest(detail="Can't acknowledge an attached alert group") alert_group.acknowledge_by_user(self.request.user, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @@ -344,7 +305,6 @@ class AlertGroupView( raise BadRequest(detail="Can't unacknowledge a resolved alert group") alert_group.un_acknowledge_by_user(self.request.user, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @@ -365,7 +325,6 @@ class AlertGroupView( status=status.HTTP_400_BAD_REQUEST, ) alert_group.resolve_by_user(self.request.user, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @action(methods=["post"], detail=True) @@ -381,7 +340,6 @@ class AlertGroupView( raise BadRequest(detail="The alert group is not resolved") alert_group.un_resolve_by_user(self.request.user, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @action(methods=["post"], detail=True) @@ -404,8 +362,6 @@ class AlertGroupView( return Response(status=status.HTTP_400_BAD_REQUEST) alert_group.attach_by_user(self.request.user, root_alert_group, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) - invalidate_web_cache_for_alert_group(alert_group_pk=root_alert_group.pk) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @action(methods=["post"], detail=True) @@ -415,10 +371,8 @@ class AlertGroupView( raise BadRequest(detail="Can't unattach maintenance alert group") if alert_group.is_root_alert_group: raise BadRequest(detail="Can't unattach an alert group because it is not attached") - root_alert_group_pk = alert_group.root_alert_group_id + alert_group.un_attach_by_user(self.request.user, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) - invalidate_web_cache_for_alert_group(alert_group_pk=root_alert_group_pk) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @action(methods=["post"], detail=True) @@ -433,7 +387,6 @@ class AlertGroupView( raise BadRequest(detail="Can't silence an attached alert group") alert_group.silence_by_user(request.user, silence_delay=delay, action_source=ActionSource.WEB) - invalidate_web_cache_for_alert_group(alert_group_pk=alert_group.pk) return Response(AlertGroupSerializer(alert_group, context={"request": request}).data) @action(methods=["get"], detail=False) @@ -548,9 +501,9 @@ class AlertGroupView( raise BadRequest(detail="Please specify a delay for silence") kwargs["silence_delay"] = delay - alert_groups = self.get_queryset(eager=False).filter(public_primary_key__in=alert_group_public_pks) - alert_group_pks = list(alert_groups.values_list("id", flat=True)) - invalidate_web_cache_for_alert_group(alert_group_pks=alert_group_pks) + alert_groups = AlertGroup.unarchived_objects.filter( + channel__organization=self.request.auth.organization, public_primary_key__in=alert_group_public_pks + ) kwargs["user"] = self.request.user kwargs["alert_groups"] = alert_groups diff --git a/engine/apps/api/views/route_regex_debugger.py b/engine/apps/api/views/route_regex_debugger.py index 527684ac..ffa9cc71 100644 --- a/engine/apps/api/views/route_regex_debugger.py +++ b/engine/apps/api/views/route_regex_debugger.py @@ -43,10 +43,7 @@ class RouteRegexDebuggerView(APIView): if len(incidents_matching_regex) < MAX_INCIDENTS_TO_SHOW: first_alert = ag.alerts.all()[0] if re.search(regex, json.dumps(first_alert.raw_request_data)): - if ag.cached_render_for_web: - title = ag.cached_render_for_web["render_for_web"]["title"] - else: - title = AlertWebRenderer(first_alert).render()["title"] + title = AlertWebRenderer(first_alert).render()["title"] incidents_matching_regex.append( { "title": title, diff --git a/engine/apps/base/models/user_notification_policy_log_record.py b/engine/apps/base/models/user_notification_policy_log_record.py index d8afed2d..ed261b2b 100644 --- a/engine/apps/base/models/user_notification_policy_log_record.py +++ b/engine/apps/base/models/user_notification_policy_log_record.py @@ -315,7 +315,6 @@ class UserNotificationPolicyLogRecord(models.Model): @receiver(post_save, sender=UserNotificationPolicyLogRecord) def listen_for_usernotificationpolicylogrecord_model_save(sender, instance, created, *args, **kwargs): - instance.alert_group.drop_cached_after_resolve_report_json() alert_group_pk = instance.alert_group.pk if instance.type != UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FINISHED: logger.debug( diff --git a/engine/apps/public_api/tests/test_incidents.py b/engine/apps/public_api/tests/test_incidents.py index d43a1fb8..ea1198a0 100644 --- a/engine/apps/public_api/tests/test_incidents.py +++ b/engine/apps/public_api/tests/test_incidents.py @@ -32,7 +32,7 @@ def construct_expected_response_from_incidents(incidents): "id": incident.public_primary_key, "integration_id": incident.channel.public_primary_key, "route_id": incident.channel_filter.public_primary_key, - "alerts_count": incident.alerts_count, + "alerts_count": incident.alerts.count(), "state": incident.state, "created_at": created_at, "resolved_at": resolved_at, diff --git a/engine/apps/slack/scenarios/alertgroup_appearance.py b/engine/apps/slack/scenarios/alertgroup_appearance.py index 1ccba05f..588b70d0 100644 --- a/engine/apps/slack/scenarios/alertgroup_appearance.py +++ b/engine/apps/slack/scenarios/alertgroup_appearance.py @@ -247,10 +247,6 @@ class UpdateAppearanceStep(scenario_step.ScenarioStep): if new_value is None and old_value is not None: setattr(alert_receive_channel, attr_name, None) alert_receive_channel.save() - # Drop caches for current alert group - if notification_channel == "web": - setattr(alert_group, f"cached_render_for_web_{templatizable_attr}", None) - alert_group.save() elif new_value is not None: default_values = getattr( AlertReceiveChannel, @@ -265,18 +261,10 @@ class UpdateAppearanceStep(scenario_step.ScenarioStep): jinja_template_env.from_string(new_value) setattr(alert_receive_channel, attr_name, new_value) alert_receive_channel.save() - # Drop caches for current alert group - if notification_channel == "web": - setattr(alert_group, f"cached_render_for_web_{templatizable_attr}", None) - alert_group.save() elif default_value is not None and new_value.strip() == default_value.strip(): new_value = None setattr(alert_receive_channel, attr_name, new_value) alert_receive_channel.save() - # Drop caches for current alert group - if notification_channel == "web": - setattr(alert_group, f"cached_render_for_web_{templatizable_attr}", None) - alert_group.save() except TemplateSyntaxError: return Response( {"response_action": "errors", "errors": {attr_name: "Template has incorrect format"}}, diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index 364704b7..f6c78305 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -674,7 +674,6 @@ class AddRemoveThreadMessageStep(UpdateResolutionNoteStep, scenario_step.Scenari add_to_resolution_note = True if value["msg_value"].startswith("add") else False slack_thread_message = None resolution_note = None - drop_ag_cache = False alert_group = AlertGroup.all_objects.get(pk=alert_group_pk) @@ -695,7 +694,6 @@ class AddRemoveThreadMessageStep(UpdateResolutionNoteStep, scenario_step.Scenari else: resolution_note.recreate() self.add_resolution_note_reaction(slack_thread_message) - drop_ag_cache = True elif not add_to_resolution_note: # Check if resolution_note can be removed if ( @@ -720,13 +718,9 @@ class AddRemoveThreadMessageStep(UpdateResolutionNoteStep, scenario_step.Scenari slack_thread_message.added_to_resolution_note = False slack_thread_message.save(update_fields=["added_to_resolution_note"]) self.remove_resolution_note_reaction(slack_thread_message) - drop_ag_cache = True self.update_alert_group_resolution_note_button( alert_group, ) - if drop_ag_cache: - alert_group.drop_cached_after_resolve_report_json() - alert_group.schedule_cache_for_web() resolution_note_data = json.loads(payload["actions"][0]["value"]) resolution_note_data["resolution_note_window_action"] = "edit_update" ResolutionNoteModalStep(slack_team_identity, self.organization, self.user).process_scenario( diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index ca769243..84794e2e 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -8,7 +8,6 @@ from django.db.models.signals import post_save from django.dispatch import receiver from emoji import demojize -from apps.alerts.tasks import invalidate_web_cache_for_alert_group from apps.schedules.tasks import drop_cached_ical_for_custom_events_for_organization from common.constants.role import Role from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length @@ -264,5 +263,3 @@ def listen_for_user_model_save(sender, instance, created, *args, **kwargs): drop_cached_ical_for_custom_events_for_organization.apply_async( (instance.organization_id,), ) - logger.info(f"Drop AG cache. Reason: save user {instance.pk}") - invalidate_web_cache_for_alert_group.apply_async(kwargs={"org_pk": instance.organization_id}) diff --git a/engine/common/api_helpers/paginators.py b/engine/common/api_helpers/paginators.py index 023f2294..01ce2cc6 100644 --- a/engine/common/api_helpers/paginators.py +++ b/engine/common/api_helpers/paginators.py @@ -1,4 +1,4 @@ -from rest_framework.pagination import PageNumberPagination +from rest_framework.pagination import CursorPagination, PageNumberPagination class HundredPageSizePaginator(PageNumberPagination): @@ -11,3 +11,10 @@ class FiftyPageSizePaginator(PageNumberPagination): class TwentyFivePageSizePaginator(PageNumberPagination): page_size = 25 + + +class TwentyFiveCursorPaginator(CursorPagination): + page_size = 25 + max_page_size = 100 + page_size_query_param = "perpage" + ordering = "-pk" diff --git a/engine/common/mixins/use_random_readonly_db_manager_mixin.py b/engine/common/mixins/use_random_readonly_db_manager_mixin.py deleted file mode 100644 index 46559aa4..00000000 --- a/engine/common/mixins/use_random_readonly_db_manager_mixin.py +++ /dev/null @@ -1,21 +0,0 @@ -import random - -from django.conf import settings - - -class UseRandomReadonlyDbManagerMixin: - """ - Use this Mixin in ModelManagers, when you want to use the random readonly replica - """ - - @property - def using_readonly_db(self): - """Select one of the readonly databases this QuerySet should execute against.""" - if hasattr(settings, "READONLY_DATABASES") and len(settings.READONLY_DATABASES) > 0: - using_db = random.choice(list(settings.READONLY_DATABASES.keys())) - return self.using(using_db) - else: - # Use "default" database - # Django uses the database with the alias of default when no other database has been selected. - # https://docs.djangoproject.com/en/3.2/topics/db/multi-db/#defining-your-databases - return self.using("default") diff --git a/engine/settings/dev.py b/engine/settings/dev.py index d63b6f74..b5e0e2f5 100644 --- a/engine/settings/dev.py +++ b/engine/settings/dev.py @@ -32,10 +32,6 @@ DATABASES = { TESTING = "pytest" in sys.modules or "unittest" in sys.modules -READONLY_DATABASES = {} - -# Dictionaries concatenation, introduced in python3.9 -DATABASES = DATABASES | READONLY_DATABASES CACHES = { "default": { diff --git a/engine/settings/prod_without_db.py b/engine/settings/prod_without_db.py index 60b4cc28..5b8a83b4 100644 --- a/engine/settings/prod_without_db.py +++ b/engine/settings/prod_without_db.py @@ -84,12 +84,11 @@ CELERY_TASK_ROUTES = { "apps.alerts.tasks.create_contact_points_for_datasource.create_contact_points_for_datasource": {"queue": "default"}, "apps.alerts.tasks.sync_grafana_alerting_contact_points.sync_grafana_alerting_contact_points": {"queue": "default"}, "apps.alerts.tasks.delete_alert_group.delete_alert_group": {"queue": "default"}, - "apps.alerts.tasks.invalidate_web_cache_for_alert_group.invalidate_web_cache_for_alert_group": {"queue": "default"}, + "apps.alerts.tasks.invalidate_web_cache_for_alert_group.invalidate_web_cache_for_alert_group": { + "queue": "default" + }, # todo: remove "apps.alerts.tasks.send_alert_group_signal.send_alert_group_signal": {"queue": "default"}, "apps.alerts.tasks.wipe.wipe": {"queue": "default"}, - # TODO: remove cache_alert_group_for_web and schedule_cache_for_alert_group once existing task will be processed - "apps.api.tasks.cache_alert_group_for_web": {"queue": "default"}, - "apps.api.tasks.schedule_cache_for_alert_group": {"queue": "default"}, "apps.heartbeat.tasks.heartbeat_checkup": {"queue": "default"}, "apps.heartbeat.tasks.integration_heartbeat_checkup": {"queue": "default"}, "apps.heartbeat.tasks.process_heartbeat_task": {"queue": "default"}, diff --git a/grafana-plugin/src/components/CursorPagination/CursorPagination.module.css b/grafana-plugin/src/components/CursorPagination/CursorPagination.module.css new file mode 100644 index 00000000..63d08ecc --- /dev/null +++ b/grafana-plugin/src/components/CursorPagination/CursorPagination.module.css @@ -0,0 +1,3 @@ +.root { + display: block; +} diff --git a/grafana-plugin/src/components/CursorPagination/CursorPagination.tsx b/grafana-plugin/src/components/CursorPagination/CursorPagination.tsx new file mode 100644 index 00000000..33b228e1 --- /dev/null +++ b/grafana-plugin/src/components/CursorPagination/CursorPagination.tsx @@ -0,0 +1,79 @@ +import React, { FC, useCallback, useEffect, useState } from 'react'; + +import { SelectableValue } from '@grafana/data'; +import { Button, HorizontalGroup, Icon, Select } from '@grafana/ui'; +import cn from 'classnames/bind'; + +import Text from 'components/Text/Text'; + +import styles from './CursorPagination.module.css'; + +interface CursorPaginationProps { + current: string; + onChange: (cursor: string, direction: 'prev' | 'next') => void; + itemsPerPageOptions: Array>; + itemsPerPage: number; + onChangeItemsPerPage: (value: number) => void; + prev: string; + next: string; +} + +const cx = cn.bind(styles); + +const CursorPagination: FC = (props) => { + const { current, onChange, prev, next, itemsPerPage, itemsPerPageOptions, onChangeItemsPerPage } = props; + + const [disabled, setDisabled] = useState(false); + + useEffect(() => { + setDisabled(false); + }, [prev, next]); + + const onChangeItemsPerPageCallback = useCallback((option) => { + setDisabled(true); + onChangeItemsPerPage(option.value); + }, []); + + return ( + + + Items per list + - - - - {current} - - - - ); -}; - -export default CursorPagination; diff --git a/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx b/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx index b21b0f1d..0990e8ce 100644 --- a/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx +++ b/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx @@ -37,7 +37,7 @@ const cx = cn.bind(styles); interface IncidentsFiltersProps extends WithStoreProps { value: IncidentsFiltersType; - onChange: (filters: { [key: string]: any }, isOnMount: boolean) => void; + onChange: (filters: { [key: string]: any }) => void; query: { [key: string]: any }; } interface IncidentsFiltersState { @@ -79,9 +79,7 @@ class IncidentsFilters extends Component { - this.onChange(true); - }); + this.setState({ filterOptions, filters, values }, this.onChange); } render() { @@ -423,11 +421,11 @@ class IncidentsFilters extends Component { + onChange = () => { const { onChange } = this.props; const { values } = this.state; - onChange(values, isOnMount); + onChange(values); }; debouncedOnChange = debounce(this.onChange, 500); diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.ts b/grafana-plugin/src/models/alertgroup/alertgroup.ts index d7c6fa4f..ba284236 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.ts @@ -36,10 +36,7 @@ export class AlertGroupStore extends BaseStore { initialQuery = qs.parse(window.location.search); @observable - incidentsCursor?: string; - - @observable - incidentsItemsPerPage?: number; + incidentsPage: any = this.initialQuery.p ? Number(this.initialQuery.p) : 1; @observable alertsSearchResult: any = {}; @@ -218,69 +215,54 @@ export class AlertGroupStore extends BaseStore { } @action - async updateIncidentFilters(params: any, keepCursor = false) { - if (!keepCursor) { - this.incidentsCursor = undefined; + async updateIncidentFilters(params: any, resetPage = true) { + if (resetPage) { + this.incidentsPage = 1; } - this.incidentFilters = params; this.updateIncidents(); } @action - async setIncidentsCursor(cursor: string) { - this.incidentsCursor = cursor; + async setIncidentsPage(page: number) { + this.incidentsPage = page; this.updateAlertGroups(); } @action - async setIncidentsItemsPerPage(value: number) { - this.incidentsCursor = undefined; - this.incidentsItemsPerPage = value; + async updateAlertGroups(skip_slow_rendering = true) { + this.alertGroupsLoading = skip_slow_rendering; - this.updateAlertGroups(); - } - - @action - async updateAlertGroups() { - this.alertGroupsLoading = true; - - const { - results, - next: nextRaw, - previous: previousRaw, - } = await makeRequest(`${this.path}`, { + const result = await makeRequest(`${this.path}`, { params: { ...this.incidentFilters, - cursor: this.incidentsCursor, - perpage: this.incidentsItemsPerPage, + page: this.incidentsPage, is_root: true, + skip_slow_rendering, }, }).catch(refreshPageError); - const prevCursor = previousRaw ? qs.parse(qs.extract(previousRaw)).cursor : previousRaw; - const nextCursor = nextRaw ? qs.parse(qs.extract(nextRaw)).cursor : nextRaw; - - const newAlerts = new Map( - results.map((alert: Alert) => { - const oldAlert = this.alerts.get(alert.pk) || {}; - const mergedAlertData = { ...oldAlert, ...alert }; - return [alert.pk, mergedAlertData]; - }) - ); + const newAlerts = new Map(result.results.map((alert: Alert) => [alert.pk, alert])); // @ts-ignore this.alerts = new Map([...this.alerts, ...newAlerts]); this.alertsSearchResult['default'] = { - prev: prevCursor, - next: nextCursor, - results: results.map((alert: Alert) => alert.pk), + count: result.count, + results: result.results.map((alert: Alert) => alert.pk), }; this.alertGroupsLoading = false; + + if (skip_slow_rendering) { + const hasShortened = result.results.some((alert: Alert) => alert.short); + + if (hasShortened) { + this.updateAlertGroups(false); + } + } } getAlertSearchResult(query: string) { @@ -291,6 +273,27 @@ export class AlertGroupStore extends BaseStore { return this.alertsSearchResult[query].results.map((pk: Alert['pk']) => this.alerts.get(pk)); } + @action + async searchIncidents(search: string) { + const result = await makeRequest(`${this.path}`, { + params: { + search, + resolved: false, + is_root: true, + }, + }); + + const newAlerts = new Map(result.results.map((alert: Alert) => [alert.pk, alert])); + + // @ts-ignore + this.alerts = new Map([...this.alerts, ...newAlerts]); + + this.alertsSearchResult[search] = { + count: result.count, + results: result.results.map((alert: Alert) => alert.pk), + }; + } + @action getAlert(pk: Alert['pk']) { return makeRequest(`${this.path}${pk}`, {}).then((alert: Alert) => { diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.types.ts b/grafana-plugin/src/models/alertgroup/alertgroup.types.ts index 2ae7498c..c848990f 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.types.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.types.ts @@ -42,16 +42,17 @@ export interface Alert { title: string; message: string; image_url: string; - alerts?: any[]; + alerts: any[]; acknowledged: boolean; created_at: string; acknowledged_at: string; + acknowledged_at_verbose: string; acknowledged_by_user: User; acknowledged_on_source: boolean; channel: Channel; - permalink?: string; + permalink: string; related_users: User[]; - render_after_resolve_report_json?: TimeLineItem[]; + render_after_resolve_report_json: TimeLineItem[]; render_for_slack: { attachments: any[] }; render_for_web: { message: any; @@ -62,13 +63,17 @@ export interface Alert { inside_organization_number: number; resolved: boolean; resolved_at: string; + resolved_at_verbose: string; resolved_by: number; resolved_by_user: User; + resolved_by_verbose: string; silenced: boolean; silenced_at: string; + silenced_at_verbose: string; silenced_by_user: Partial; silenced_until: string; started_at: string; + started_at_verbose: string; last_alert_at: string; verbose_name: string; dependent_alert_groups: Alert[]; diff --git a/grafana-plugin/src/pages/incident/Incident.tsx b/grafana-plugin/src/pages/incident/Incident.tsx index 816dde2c..e1f6a2d2 100644 --- a/grafana-plugin/src/pages/incident/Incident.tsx +++ b/grafana-plugin/src/pages/incident/Incident.tsx @@ -92,7 +92,7 @@ class IncidentPage extends React.Component render() { const { store, - query: { id, cursor, start, perpage }, + query: { id }, } = this.props; const { showIntegrationSettings, showAttachIncidentForm, notFound } = this.state; @@ -112,7 +112,7 @@ class IncidentPage extends React.Component 404 Incident not found - + @@ -182,7 +182,7 @@ class IncidentPage extends React.Component renderHeader = () => { const { store, - query: { id, cursor, start, perpage }, + query: { id }, } = this.props; const { alerts } = store.alertGroupStore; @@ -197,7 +197,7 @@ class IncidentPage extends React.Component - + {/* @ts-ignore*/} @@ -293,13 +293,7 @@ class IncidentPage extends React.Component }; renderIncident = (incident: Alert) => { - let datetimeReference; - - if (incident.last_alert_at || incident.created_at) { - const m = moment(incident.last_alert_at || incident.created_at); - datetimeReference = `(${m.fromNow()}, ${m.toString()})`; - } - + const m = moment(incident.last_alert_at || incident.created_at); return (
@@ -308,7 +302,9 @@ class IncidentPage extends React.Component ? `#${incident.inside_organization_number} ${incident.render_for_web.title}` : incident.render_for_web.title} - {datetimeReference} + + ({m.fromNow()}, {m.toString()}) +
const incident = store.alertGroupStore.alerts.get(id); const alerts = incident.alerts; - if (!alerts) { - return null; - } const latestAlert = alerts[alerts.length - 1]; const latestAlertMoment = moment(latestAlert.created_at); @@ -414,10 +407,6 @@ class IncidentPage extends React.Component const incident = store.alertGroupStore.alerts.get(id); - if (!incident.render_after_resolve_report_json) { - return null; - } - const timeline = this.filterTimeline(incident.render_after_resolve_report_json); const { timelineFilter, resolutionNoteText } = this.state; const isResolutionNoteTextEmpty = resolutionNoteText === ''; diff --git a/grafana-plugin/src/pages/incidents/Incidents.module.css b/grafana-plugin/src/pages/incidents/Incidents.module.css index 06bf0f13..1871dbc3 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.module.css +++ b/grafana-plugin/src/pages/incidents/Incidents.module.css @@ -34,8 +34,3 @@ height: 24px; margin-right: 0; } - -.pagination { - width: 100%; - margin-top: 20px; -} diff --git a/grafana-plugin/src/pages/incidents/Incidents.tsx b/grafana-plugin/src/pages/incidents/Incidents.tsx index 18d718af..6e8f3f72 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.tsx +++ b/grafana-plugin/src/pages/incidents/Incidents.tsx @@ -11,7 +11,6 @@ import moment from 'moment'; import Emoji from 'react-emoji-render'; import CardButton from 'components/CardButton/CardButton'; -import CursorPagination from 'components/CursorPagination/CursorPagination'; import GTable from 'components/GTable/GTable'; import IntegrationLogo from 'components/IntegrationLogo/IntegrationLogo'; import PluginLink from 'components/PluginLink/PluginLink'; @@ -36,10 +35,7 @@ import styles from './Incidents.module.css'; const cx = cn.bind(styles); -interface Pagination { - start: number; - end: number; -} +const ITEMS_PER_PAGE = 50; function withSkeleton(fn: (alert: AlertType) => ReactElement | ReactElement[]) { return (alert: AlertType) => { @@ -57,41 +53,28 @@ interface IncidentsPageState { selectedIncidentIds: Array; affectedRows: { [key: string]: boolean }; filters?: IncidentsFiltersType; - pagination: Pagination; } -const ITEMS_PER_PAGE = 25; - @observer class Incidents extends React.Component { constructor(props: IncidentsPageProps) { super(props); - const { - store, - query: { id, cursor: cursorQuery, start: startQuery, perpage: perpageQuery }, - } = props; - - const cursor = cursorQuery || undefined; - const start = !isNaN(startQuery) ? Number(startQuery) : 1; - const itemsPerPage = !isNaN(perpageQuery) ? Number(perpageQuery) : ITEMS_PER_PAGE; - - store.alertGroupStore.incidentsCursor = cursor; - store.alertGroupStore.incidentsItemsPerPage = itemsPerPage; + const { store } = props; this.state = { selectedIncidentIds: [], affectedRows: {}, - pagination: { - start, - end: start + itemsPerPage - 1, - }, }; store.alertGroupStore.updateBulkActions(); store.alertGroupStore.updateSilenceOptions(); } + async componentDidMount() {} + + componentDidUpdate() {} + render() { return (
@@ -112,55 +95,24 @@ class Incidents extends React.Component ); } - handleFiltersChange = (filters: IncidentsFiltersType, isOnMount: boolean) => { + handleFiltersChange = (filters: IncidentsFiltersType) => { const { store } = this.props; - this.setState({ - filters, - selectedIncidentIds: [], - }); + this.setState({ filters, selectedIncidentIds: [] }); - if (!isOnMount) { - this.setState({ - pagination: { - start: 1, - end: store.alertGroupStore.incidentsItemsPerPage, - }, - }); - } + store.alertGroupStore.updateIncidentFilters(filters, true); - store.alertGroupStore.updateIncidentFilters(filters, isOnMount); - - getLocationSrv().update({ query: { page: 'incidents', ...store.alertGroupStore.incidentFilters } }); + getLocationSrv().update({ query: { page: 'incidents', ...store.incidentFilters, p: store.incidentsPage } }); // todo fix }; - onChangeCursor = (cursor: string, direction: 'prev' | 'next') => { + onChangePagination = (page: number) => { const { store } = this.props; - store.alertGroupStore.setIncidentsCursor(cursor); + store.alertGroupStore.setIncidentsPage(page); - this.setState({ - selectedIncidentIds: [], - pagination: { - start: - this.state.pagination.start + store.alertGroupStore.incidentsItemsPerPage * (direction === 'prev' ? -1 : 1), - end: this.state.pagination.end + store.alertGroupStore.incidentsItemsPerPage * (direction === 'prev' ? -1 : 1), - }, - }); - }; + this.setState({ selectedIncidentIds: [] }); - handleChangeItemsPerPage = (value: number) => { - const { store } = this.props; - - store.alertGroupStore.setIncidentsItemsPerPage(value); - - this.setState({ - selectedIncidentIds: [], - pagination: { - start: 1, - end: store.alertGroupStore.incidentsItemsPerPage, - }, - }); + getLocationSrv().update({ partial: true, query: { p: store.incidentsPage } }); }; renderBulkActions = () => { @@ -262,7 +214,7 @@ class Incidents extends React.Component }; renderTable() { - const { selectedIncidentIds, affectedRows, pagination } = this.state; + const { selectedIncidentIds, affectedRows } = this.state; const { store } = this.props; const { teamStore: { currentTeam }, @@ -270,8 +222,7 @@ class Incidents extends React.Component const { alertGroupsLoading } = store.alertGroupStore; const results = store.alertGroupStore.getAlertSearchResult('default'); - const prev = get(store.alertGroupStore.alertsSearchResult, `default.prev`); - const next = get(store.alertGroupStore.alertsSearchResult, `default.next`); + const count = get(store.alertGroupStore.alertsSearchResult, `default.count`); if (results && !results.length) { return ( @@ -368,22 +319,12 @@ class Incidents extends React.Component data={results} columns={columns} // rowClassName={getUserRowClassNameFn(userPkToEdit, userStore.currentUserPk)} + pagination={{ + page: store.incidentsPage, + total: Math.ceil((count || 0) / ITEMS_PER_PAGE), + onChange: this.onChangePagination, + }} /> -
- -
); } @@ -397,20 +338,9 @@ class Incidents extends React.Component } renderTitle = (record: AlertType) => { - const { store } = this.props; - const { - pagination: { start }, - } = this.state; - - const { incidentsItemsPerPage, incidentsCursor } = store.alertGroupStore; - return ( - - {record.render_for_web.title} - + {record.render_for_web.title} {Boolean(record.dependent_alert_groups.length) && `+ ${record.dependent_alert_groups.length} attached`} ); @@ -436,6 +366,48 @@ class Incidents extends React.Component renderStatus(record: AlertType) { return getIncidentStatusTag(record); + + /*if (record.resolved) { + return ( +
+ + + +
+ ); + } + + if (record.acknowledged) { + return ( +
+ + + +
+ ); + } + + if (record.silenced) { + const silencedUntilText = record.silenced_until + ? `Silenced until ${moment(record.silenced_until).toLocaleString()}` + : 'Silenced forever'; + + return ( +
+ + + +
+ ); + } + + return ( +
+ + + +
+ );*/ } renderStartedAt(alert: AlertType) { From c37bfe45b1ad7b7fb9c78c005e47dd1051537dc1 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Thu, 21 Jul 2022 15:49:17 +0100 Subject: [PATCH 43/49] Fix timezone naming clash (#271) --- engine/apps/api/views/schedule.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 1de11b85..1b587210 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -190,9 +190,10 @@ class ScheduleView( return user_tz, date - def _filter_events(self, schedule, timezone, starting_date, days, with_empty, with_gap): + def _filter_events(self, schedule, user_timezone, starting_date, days, with_empty, with_gap): shifts = ( - list_of_oncall_shifts_from_ical(schedule, starting_date, timezone, with_empty, with_gap, days=days) or [] + list_of_oncall_shifts_from_ical(schedule, starting_date, user_timezone, with_empty, with_gap, days=days) + or [] ) events = [] # for start, end, users, priority_level, source in shifts: From f216c9a84ad3976c8fa1a6994fe7c196ef2fd096 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Thu, 21 Jul 2022 16:00:38 +0100 Subject: [PATCH 44/49] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21d998c6..fd7332e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,9 @@ # Change Log -## v1.0.8 (2022-07-21) +## v1.0.9 (2022-07-21) - Frontend bug fixes & improvements - Support regex_replace() in templates +- Bring back alert group caching and list view ## v1.0.7 (2022-07-18) - Backend & frontend bug fixes From 5205ceeede361d95ac1679c3330eb9a6159a695b Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 22 Jul 2022 14:58:27 +0400 Subject: [PATCH 45/49] More accurate invalidating of alert group web cache (#277) * Disable invalidate ag web cache on user save * Tweak invalidate_ag_web_cache on AlertReceiveChannel save --- engine/apps/alerts/models/alert_receive_channel.py | 13 +++++++++---- engine/apps/user_management/models/user.py | 9 --------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 308686c0..2f0cc016 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -694,14 +694,19 @@ def listen_for_alertreceivechannel_model_save(sender, instance, created, *args, instance.organization, None, OrganizationLogType.TYPE_HEARTBEAT_CREATED, description ) else: - logger.info(f"Drop AG cache. Reason: save alert_receive_channel {instance.pk}") if kwargs is not None: if "update_fields" in kwargs: if kwargs["update_fields"] is not None: + fields_to_not_to_invalidate_cache = [ + "rate_limit_message_task_id", + "rate_limited_in_slack_at", + "reason_to_skip_escalation", + ] # Hack to not to invalidate web cache on AlertReceiveChannel.start_send_rate_limit_message_task - if "rate_limit_message_task_id" in kwargs["update_fields"]: - return - + for f in fields_to_not_to_invalidate_cache: + if f in kwargs["update_fields"]: + return + logger.info(f"Drop AG cache. Reason: save alert_receive_channel {instance.pk}") invalidate_web_cache_for_alert_group.apply_async(kwargs={"channel_pk": instance.pk}) if instance.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING: diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index ca769243..6e8b27dc 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -8,7 +8,6 @@ from django.db.models.signals import post_save from django.dispatch import receiver from emoji import demojize -from apps.alerts.tasks import invalidate_web_cache_for_alert_group from apps.schedules.tasks import drop_cached_ical_for_custom_events_for_organization from common.constants.role import Role from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length @@ -255,14 +254,6 @@ class User(models.Model): # TODO: check whether this signal can be moved to save method of the model @receiver(post_save, sender=User) def listen_for_user_model_save(sender, instance, created, *args, **kwargs): - # if kwargs is not None: - # if "update_fields" in kwargs: - # if kwargs["update_fields"] is not None: - # if "username" not in kwargs["update_fields"]: - # return - drop_cached_ical_for_custom_events_for_organization.apply_async( (instance.organization_id,), ) - logger.info(f"Drop AG cache. Reason: save user {instance.pk}") - invalidate_web_cache_for_alert_group.apply_async(kwargs={"org_pk": instance.organization_id}) From c9c6df88c5d0f6b46eb9f0fd0f2e8aec2b5c0346 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 22 Jul 2022 15:24:24 +0400 Subject: [PATCH 46/49] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd7332e3..3546a5a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Change Log +## v1.0.10 (2022-07-22) +- Speed-up of alert group web caching +- Internal api for OnCall shifts + ## v1.0.9 (2022-07-21) - Frontend bug fixes & improvements - Support regex_replace() in templates From fdba32a15fd3cc66457e3016019008d3c2710e43 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 21 Jul 2022 10:24:47 -0300 Subject: [PATCH 47/49] Update schedule filter_events to resolve final schedule shifts --- engine/apps/api/tests/test_schedules.py | 228 +++++++++++++++++++++++- engine/apps/api/views/schedule.py | 112 +++++++++++- 2 files changed, 332 insertions(+), 8 deletions(-) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 8cb0319d..907b9d45 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -469,13 +469,13 @@ def test_filter_events_calendar( "by_day": ["MO", "FR"], "schedule": schedule, } - on_call_shift = make_on_call_shift( organization=organization, shift_type=CustomOnCallShift.TYPE_RECURRENT_EVENT, **data ) on_call_shift.users.add(user) url = reverse("api-internal:schedule-filter-events", kwargs={"pk": schedule.public_primary_key}) + url += "?type=rotation" response = client.get(url, format="json", **make_user_auth_headers(user, token)) # current week events are expected @@ -525,6 +525,7 @@ def test_filter_events_calendar( @pytest.mark.django_db def test_filter_events_range_calendar( make_organization_and_user_with_plugin_token, + make_user_for_organization, make_user_auth_headers, make_schedule, make_on_call_shift, @@ -540,6 +541,9 @@ def test_filter_events_range_calendar( now = timezone.now().replace(microsecond=0) start_date = now - timezone.timedelta(days=7) + mon_start = now - timezone.timedelta(days=start_date.weekday()) + request_date = mon_start + timezone.timedelta(days=2) + data = { "start": start_date, "rotation_start": start_date, @@ -549,17 +553,26 @@ def test_filter_events_range_calendar( "by_day": ["MO", "FR"], "schedule": schedule, } - on_call_shift = make_on_call_shift( organization=organization, shift_type=CustomOnCallShift.TYPE_RECURRENT_EVENT, **data ) on_call_shift.users.add(user) - mon_start = now - timezone.timedelta(days=start_date.weekday()) - request_date = mon_start + timezone.timedelta(days=2) + # add override shift + override_start = request_date + timezone.timedelta(seconds=3600) + override_data = { + "start": override_start, + "duration": timezone.timedelta(seconds=3600), + "schedule": schedule, + } + override = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data + ) + other_user = make_user_for_organization(organization) + override.users.add(other_user) url = reverse("api-internal:schedule-filter-events", kwargs={"pk": schedule.public_primary_key}) - url += "?date={}&days=3".format(request_date.strftime("%Y-%m-%d")) + url += "?date={}&days=3&type=rotation".format(request_date.strftime("%Y-%m-%d")) response = client.get(url, format="json", **make_user_auth_headers(user, token)) # only friday occurrence is expected @@ -590,6 +603,211 @@ def test_filter_events_range_calendar( assert response.data == expected_result +@pytest.mark.django_db +def test_filter_events_overrides( + make_organization_and_user_with_plugin_token, + make_user_for_organization, + make_user_auth_headers, + make_schedule, + make_on_call_shift, +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_web_schedule", + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - timezone.timedelta(days=7) + mon_start = now - timezone.timedelta(days=start_date.weekday()) + request_date = mon_start + timezone.timedelta(days=2) + + data = { + "start": start_date, + "duration": timezone.timedelta(seconds=7200), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_WEEKLY, + "by_day": ["MO", "FR"], + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_RECURRENT_EVENT, **data + ) + on_call_shift.users.add(user) + + # add override shift + override_start = request_date + timezone.timedelta(seconds=3600) + override_data = { + "start": override_start, + "duration": timezone.timedelta(seconds=3600), + "schedule": schedule, + } + override = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data + ) + other_user = make_user_for_organization(organization) + override.users.add(other_user) + + url = reverse("api-internal:schedule-filter-events", kwargs={"pk": schedule.public_primary_key}) + url += "?date={}&days=3&type=override".format(request_date.strftime("%Y-%m-%d")) + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + + # only override occurrence is expected + expected_result = { + "id": schedule.public_primary_key, + "name": "test_web_schedule", + "type": 2, + "events": [ + { + "all_day": False, + "start": override_start, + "end": override_start + override.duration, + "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "missing_users": [], + "priority_level": None, + "source": "api", + "calendar_type": OnCallSchedule.OVERRIDES, + "is_empty": False, + "is_gap": False, + "shift": { + "pk": override.public_primary_key, + }, + } + ], + } + assert response.status_code == status.HTTP_200_OK + assert response.data == expected_result + + +@pytest.mark.django_db +def test_filter_events_final_schedule( + make_organization_and_user_with_plugin_token, + make_user_for_organization, + make_user_auth_headers, + make_schedule, + make_on_call_shift, +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_web_schedule", + ) + + now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) + start_date = now - timezone.timedelta(days=7) + request_date = start_date + + user_a, user_b, user_c, user_d, user_e = (make_user_for_organization(organization, username=i) for i in "ABCDE") + + shifts = ( + # user, priority, start time (h), duration (hs) + (user_a, 1, 10, 5), # r1-1: 10-15 / A + (user_b, 1, 11, 2), # r1-2: 11-13 / B + (user_a, 1, 16, 3), # r1-3: 16-19 / A + (user_a, 1, 21, 1), # r1-4: 21-22 / A + (user_b, 1, 22, 2), # r1-5: 22-00 / B + (user_c, 2, 12, 2), # r2-1: 12-14 / C + (user_d, 2, 14, 1), # r2-2: 14-15 / D + (user_d, 2, 17, 1), # r2-3: 17-18 / D + (user_d, 2, 20, 3), # r2-4: 20-23 / D + ) + for user, priority, start_h, duration in shifts: + data = { + "start": start_date + timezone.timedelta(hours=start_h), + "duration": timezone.timedelta(hours=duration), + "priority_level": priority, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_RECURRENT_EVENT, **data + ) + on_call_shift.users.add(user) + + # override: 22-23 / E + override_data = { + "start": start_date + timezone.timedelta(hours=22), + "duration": timezone.timedelta(hours=1), + "schedule": schedule, + } + override = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data + ) + override.users.add(user_e) + + url = reverse("api-internal:schedule-filter-events", kwargs={"pk": schedule.public_primary_key}) + url += "?date={}&days=1".format(request_date.strftime("%Y-%m-%d")) + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + + expected = ( + # start (h), duration (H), user, priority, is_gap, is_override + (0, 10, None, None, True, False), # 0-10 gap + (10, 2, "A", 1, False, False), # 10-12 A + (11, 1, "B", 1, False, False), # 11-12 B + (12, 2, "C", 2, False, False), # 12-14 C + (14, 1, "D", 2, False, False), # 14-15 D + (15, 1, None, None, True, False), # 15-16 gap + (16, 1, "A", 1, False, False), # 16-17 A + (17, 1, "D", 2, False, False), # 17-18 D + (18, 1, "A", 1, False, False), # 18-19 A + (19, 1, None, None, True, False), # 19-20 gap + (20, 2, "D", 2, False, False), # 20-22 D + (22, 1, "E", None, False, True), # 22-23 E (override) + (23, 1, "B", 1, False, False), # 23-00 B + ) + expected_events = [ + { + "calendar_type": 1 if is_override else None if is_gap else 0, + "end": start_date + timezone.timedelta(hours=start + duration), + "is_gap": is_gap, + "priority_level": priority, + "start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0), + "user": user, + } + for start, duration, user, priority, is_gap, is_override in expected + ] + returned_events = [ + { + "calendar_type": e["calendar_type"], + "end": e["end"], + "is_gap": e["is_gap"], + "priority_level": e["priority_level"], + "start": e["start"], + "user": e["users"][0]["display_name"] if e["users"] else None, + } + for e in response.data["events"] + ] + assert returned_events == expected_events + + +@pytest.mark.django_db +def test_filter_events_invalid_type( + make_organization_and_user_with_plugin_token, + make_user_auth_headers, + make_schedule, +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_web_schedule", + ) + + url = reverse("api-internal:schedule-filter-events", kwargs={"pk": schedule.public_primary_key}) + url += "?type=invalid" + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db @pytest.mark.parametrize( "role,expected_status", diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 1b587210..066cde21 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -257,8 +257,11 @@ class ScheduleView( @action(detail=True, methods=["get"]) def filter_events(self, request, pk): user_tz, date = self.get_request_timezone() - with_empty = self.request.query_params.get("with_empty", False) == "true" - with_gap = self.request.query_params.get("with_gap", False) == "true" + filter_by = self.request.query_params.get("type") + + if filter_by is not None and filter_by not in ("override", "rotation", "final"): + raise BadRequest(detail="Invalid type value") + resolve_schedule = filter_by is None or filter_by == "final" starting_date = date if self.request.query_params.get("date") else None if starting_date is None: @@ -272,9 +275,16 @@ class ScheduleView( schedule = self.original_get_object() events = self._filter_events( - schedule, user_tz, starting_date, days=days, with_empty=with_empty, with_gap=with_gap + schedule, user_tz, starting_date, days=days, with_empty=True, with_gap=resolve_schedule ) + if filter_by == "override": + events = [e for e in events if e["calendar_type"] == OnCallSchedule.OVERRIDES] + elif filter_by == "rotation": + events = [e for e in events if e["calendar_type"] == OnCallSchedule.PRIMARY] + else: # resolve_schedule + events = self._resolve_schedule(events) + result = { "id": schedule.public_primary_key, "name": schedule.name, @@ -283,6 +293,102 @@ class ScheduleView( } return Response(result, status=status.HTTP_200_OK) + def _resolve_schedule(self, events): + """Calculate final schedule shifts considering rotations and overrides.""" + if not events: + return [] + + # sort schedule events by (type desc, priority desc, start timestamp asc) + events.sort( + key=lambda e: ( + -e["calendar_type"] if e["calendar_type"] else 0, # overrides: 1, shifts: 0, gaps: None + -e["priority_level"] if e["priority_level"] else 0, + e["start"], + ) + ) + + def _merge_intervals(evs): + """Keep track of scheduled intervals.""" + if not evs: + return [] + intervals = [[e["start"], e["end"]] for e in evs] + result = [intervals[0]] + for e in intervals[1:]: + if result[-1][0] <= e[0] <= result[-1][1]: + result[-1][1] = max(result[-1][1], e[1]) + else: + result.append(e) + return result + + # iterate over events, reserving schedule slots based on their priority + # if the expected slot was already scheduled for a higher priority event, + # split the event, or fix start/end timestamps accordingly + + # include overrides from start + resolved = [e for e in events if e["calendar_type"] == OnCallSchedule.TYPE_ICAL_OVERRIDES] + intervals = _merge_intervals(resolved) + + pending = events[len(resolved) :] + if not pending: + return resolved + + current_event_idx = 0 # current event to resolve + current_interval_idx = 0 # current scheduled interval being checked + current_priority = pending[0]["priority_level"] # current priority level being resolved + + while current_event_idx < len(pending): + ev = pending[current_event_idx] + + if ev["priority_level"] != current_priority: + # update scheduled intervals on priority change + # and start from the beginning for the new priority level + resolved.sort(key=lambda e: e["start"]) + intervals = _merge_intervals(resolved) + current_interval_idx = 0 + current_priority = ev["priority_level"] + + if current_interval_idx >= len(intervals): + # event outside scheduled intervals, add to resolved + resolved.append(ev) + current_event_idx += 1 + elif ev["start"] < intervals[current_interval_idx][0] and ev["end"] <= intervals[current_interval_idx][0]: + # event starts and ends outside an already scheduled interval, add to resolved + resolved.append(ev) + current_event_idx += 1 + elif ev["start"] < intervals[current_interval_idx][0] and ev["end"] > intervals[current_interval_idx][0]: + # event starts outside interval but overlaps with an already scheduled interval + # 1. add a split event copy to schedule the time before the already scheduled interval + to_add = ev.copy() + to_add["end"] = intervals[current_interval_idx][0] + resolved.append(to_add) + # 2. check if there is still time to be scheduled after the current scheduled interval ends + if ev["end"] > intervals[current_interval_idx][1]: + # event ends after current interval, update event start timestamp to match the interval end + # and process the updated event as any other event + ev["start"] = intervals[current_interval_idx][1] + else: + # done, go to next event + current_event_idx += 1 + elif ev["start"] >= intervals[current_interval_idx][0] and ev["end"] <= intervals[current_interval_idx][1]: + # event inside an already scheduled interval, ignore (go to next) + current_event_idx += 1 + elif ( + ev["start"] >= intervals[current_interval_idx][0] + and ev["start"] < intervals[current_interval_idx][1] + and ev["end"] > intervals[current_interval_idx][1] + ): + # event starts inside a scheduled interval but ends out of it + # update the event start timestamp to match the interval end + ev["start"] = intervals[current_interval_idx][1] + # move to next interval and process the updated event as any other event + current_interval_idx += 1 + elif ev["start"] >= intervals[current_interval_idx][1]: + # event starts after the current interval, move to next interval and go through it + current_interval_idx += 1 + + resolved.sort(key=lambda e: e["start"]) + return resolved + @action(detail=False, methods=["get"]) def type_options(self, request): # TODO: check if it needed From 1ded234d428550682948b1a3790e70a6d7459b34 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 22 Jul 2022 09:13:24 -0300 Subject: [PATCH 48/49] Updates from review, improve readability --- engine/apps/api/views/schedule.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 066cde21..f7d25ab5 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -38,6 +38,10 @@ from common.api_helpers.mixins import ( ) from common.api_helpers.utils import create_engine_url +EVENTS_FILTER_BY_ROTATION = "rotation" +EVENTS_FILTER_BY_OVERRIDE = "override" +EVENTS_FILTER_BY_FINAL = "final" + class ScheduleView( PublicPrimaryKeyMixin, ShortSerializerMixin, CreateSerializerMixin, UpdateSerializerMixin, ModelViewSet @@ -259,9 +263,10 @@ class ScheduleView( user_tz, date = self.get_request_timezone() filter_by = self.request.query_params.get("type") - if filter_by is not None and filter_by not in ("override", "rotation", "final"): + valid_filters = (EVENTS_FILTER_BY_ROTATION, EVENTS_FILTER_BY_OVERRIDE, EVENTS_FILTER_BY_FINAL) + if filter_by is not None and filter_by not in valid_filters: raise BadRequest(detail="Invalid type value") - resolve_schedule = filter_by is None or filter_by == "final" + resolve_schedule = filter_by is None or filter_by == EVENTS_FILTER_BY_FINAL starting_date = date if self.request.query_params.get("date") else None if starting_date is None: @@ -278,9 +283,9 @@ class ScheduleView( schedule, user_tz, starting_date, days=days, with_empty=True, with_gap=resolve_schedule ) - if filter_by == "override": + if filter_by == EVENTS_FILTER_BY_OVERRIDE: events = [e for e in events if e["calendar_type"] == OnCallSchedule.OVERRIDES] - elif filter_by == "rotation": + elif filter_by == EVENTS_FILTER_BY_ROTATION: events = [e for e in events if e["calendar_type"] == OnCallSchedule.PRIMARY] else: # resolve_schedule events = self._resolve_schedule(events) @@ -313,11 +318,12 @@ class ScheduleView( return [] intervals = [[e["start"], e["end"]] for e in evs] result = [intervals[0]] - for e in intervals[1:]: - if result[-1][0] <= e[0] <= result[-1][1]: - result[-1][1] = max(result[-1][1], e[1]) + for interval in intervals[1:]: + previous_interval = result[-1] + if previous_interval[0] <= interval[0] <= previous_interval[1]: + previous_interval[1] = max(previous_interval[1], interval[1]) else: - result.append(e) + result.append(interval) return result # iterate over events, reserving schedule slots based on their priority From 4649f1faaa984ecb910e4a477f168c5b6735594a Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 22 Jul 2022 09:29:34 -0300 Subject: [PATCH 49/49] Update schedule tests for missing shift rotation_start --- engine/apps/api/tests/test_schedules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 907b9d45..6f766998 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -562,6 +562,7 @@ def test_filter_events_range_calendar( override_start = request_date + timezone.timedelta(seconds=3600) override_data = { "start": override_start, + "rotation_start": override_start, "duration": timezone.timedelta(seconds=3600), "schedule": schedule, } @@ -627,6 +628,7 @@ def test_filter_events_overrides( data = { "start": start_date, + "rotation_start": start_date, "duration": timezone.timedelta(seconds=7200), "priority_level": 1, "frequency": CustomOnCallShift.FREQUENCY_WEEKLY, @@ -642,6 +644,7 @@ def test_filter_events_overrides( override_start = request_date + timezone.timedelta(seconds=3600) override_data = { "start": override_start, + "rotation_start": override_start, "duration": timezone.timedelta(seconds=3600), "schedule": schedule, } @@ -720,6 +723,7 @@ def test_filter_events_final_schedule( for user, priority, start_h, duration in shifts: data = { "start": start_date + timezone.timedelta(hours=start_h), + "rotation_start": start_date, "duration": timezone.timedelta(hours=duration), "priority_level": priority, "frequency": CustomOnCallShift.FREQUENCY_DAILY, @@ -733,6 +737,7 @@ def test_filter_events_final_schedule( # override: 22-23 / E override_data = { "start": start_date + timezone.timedelta(hours=22), + "rotation_start": start_date, "duration": timezone.timedelta(hours=1), "schedule": schedule, }