From 4a0dbe674c32847c1ac502181f8b75f85b4ded83 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 22 Jul 2022 14:25:50 +0400 Subject: [PATCH 01/12] Add TestInsightLogsAPIView --- engine/apps/api/urls.py | 2 ++ engine/apps/api/views/test_insight_logs.py | 32 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 engine/apps/api/views/test_insight_logs.py diff --git a/engine/apps/api/urls.py b/engine/apps/api/urls.py index a82ee4e985..221b8472e9 100644 --- a/engine/apps/api/urls.py +++ b/engine/apps/api/urls.py @@ -38,6 +38,7 @@ from .views.subscription import SubscriptionView from .views.team import TeamViewSet from .views.telegram_channels import TelegramChannelViewSet +from .views.test_insight_logs import TestInsightLogsAPIView from .views.user import CurrentUserView, UserView from .views.user_group import UserGroupViewSet @@ -106,6 +107,7 @@ "preview_template_options", PreviewTemplateOptionsView.as_view(), name="preview_template_options" ), optional_slash_path("route_regex_debugger", RouteRegexDebuggerView.as_view(), name="route_regex_debugger"), + optional_slash_path("insight_logs_test", TestInsightLogsAPIView.as_view(), name="insight-logs-test"), ] urlpatterns += [ diff --git a/engine/apps/api/views/test_insight_logs.py b/engine/apps/api/views/test_insight_logs.py new file mode 100644 index 0000000000..ca3d591213 --- /dev/null +++ b/engine/apps/api/views/test_insight_logs.py @@ -0,0 +1,32 @@ +import logging + +from django.apps import apps +from rest_framework.response import Response +from rest_framework.views import APIView + +from apps.auth_token.auth import PluginAuthentication + +logger = logging.getLogger(__name__) + + +class TestInsightLogsAPIView(APIView): + """ + TestInsightLogsAPIView is used to test insight-logs infra setup. + It will be removed once proper insight-logs will be instrumented. + """ + + authentication_classes = (PluginAuthentication,) + + def post(self, request): + DynamicSetting = apps.get_model("base", "DynamicSetting") + org_id_to_enable_insight_logs, _ = DynamicSetting.objects.get_or_create( + name="org_id_to_enable_insight_logs", + defaults={"json_value": []}, + ) + org = self.request.user.organization + insight_logs_enabled = org.id in org_id_to_enable_insight_logs.json_value + if insight_logs_enabled: + message = request.data.get("message", "hello world") + logger.info(f"insight_logs=true tenant_id={self.request.user.organization.stack_id} message={message}") + return Response() + return Response(status=418) From d08425e17db2359ca1c9b30b38c5b8e36b283695 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Fri, 22 Jul 2022 15:09:52 +0300 Subject: [PATCH 02/12] Add logger for insight logs --- engine/apps/api/views/test_insight_logs.py | 3 ++- engine/settings/base.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/engine/apps/api/views/test_insight_logs.py b/engine/apps/api/views/test_insight_logs.py index ca3d591213..092536e63a 100644 --- a/engine/apps/api/views/test_insight_logs.py +++ b/engine/apps/api/views/test_insight_logs.py @@ -7,6 +7,7 @@ from apps.auth_token.auth import PluginAuthentication logger = logging.getLogger(__name__) +insight_logger = logging.getLogger("insight_logger") class TestInsightLogsAPIView(APIView): @@ -27,6 +28,6 @@ def post(self, request): insight_logs_enabled = org.id in org_id_to_enable_insight_logs.json_value if insight_logs_enabled: message = request.data.get("message", "hello world") - logger.info(f"insight_logs=true tenant_id={self.request.user.organization.stack_id} message={message}") + insight_logger.info(f"tenant_id={self.request.user.organization.stack_id} message={message}") return Response() return Response(status=418) diff --git a/engine/settings/base.py b/engine/settings/base.py index 515d1317c7..33e95a63c5 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -154,6 +154,7 @@ "filters": {"request_id": {"()": "log_request_id.filters.RequestIDFilter"}}, "formatters": { "standard": {"format": "source=engine:app google_trace_id=%(request_id)s logger=%(name)s %(message)s"}, + "insight_logger": {"format": "insight_logs=true %(message)s"}, }, "handlers": { "console": { @@ -161,8 +162,17 @@ "filters": ["request_id"], "formatter": "standard", }, + "insight_logger": { + "class": "logging.StreamHandler", + "formatter": "insight_logger", + }, }, "loggers": { + "insight_logger": { + "handlers": ["insight_logger"], + "level": "INFO", + "propagate": True, + }, "": { "handlers": ["console"], "level": "INFO", From fdba32a15fd3cc66457e3016019008d3c2710e43 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 21 Jul 2022 10:24:47 -0300 Subject: [PATCH 03/12] 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 8cb0319dea..907b9d4540 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 1b587210d5..066cde2134 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -257,8 +257,11 @@ def events(self, request, pk): @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 @@ def filter_events(self, request, pk): 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 @@ def filter_events(self, request, pk): } 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 04/12] 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 066cde2134..f7d25ab545 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -38,6 +38,10 @@ ) 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 @@ def filter_events(self, request, pk): 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 @@ def filter_events(self, request, pk): 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 @@ def _merge_intervals(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]) + 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 05/12] 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 907b9d4540..6f76699819 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, } From cf6a43c71e04c4d5dc393d728c8377a298259367 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 22 Jul 2022 11:54:53 -0300 Subject: [PATCH 06/12] Override shifts use rolling_users --- engine/apps/api/tests/test_schedules.py | 4 ++-- engine/apps/schedules/models/custom_on_call_shift.py | 2 +- engine/apps/schedules/tests/test_custom_on_call_shift.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 6f76699819..7e49a98700 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -652,7 +652,7 @@ def test_filter_events_overrides( organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data ) other_user = make_user_for_organization(organization) - override.users.add(other_user) + override.add_rolling_users([[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")) @@ -744,7 +744,7 @@ def test_filter_events_final_schedule( override = make_on_call_shift( organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data ) - override.users.add(user_e) + override.add_rolling_users([[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")) diff --git a/engine/apps/schedules/models/custom_on_call_shift.py b/engine/apps/schedules/models/custom_on_call_shift.py index 64d72ae85c..c19d225036 100644 --- a/engine/apps/schedules/models/custom_on_call_shift.py +++ b/engine/apps/schedules/models/custom_on_call_shift.py @@ -246,7 +246,7 @@ def convert_to_ical(self, time_zone="UTC"): # use shift time_zone if it exists, otherwise use schedule or default time_zone time_zone = self.time_zone if self.time_zone is not None else time_zone # rolling_users shift converts to several ical events - if self.type == CustomOnCallShift.TYPE_ROLLING_USERS_EVENT: + if self.type in (CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, CustomOnCallShift.TYPE_OVERRIDE): event_ical = None users_queue = self.get_rolling_users() for counter, users in enumerate(users_queue, start=1): diff --git a/engine/apps/schedules/tests/test_custom_on_call_shift.py b/engine/apps/schedules/tests/test_custom_on_call_shift.py index 1782e48350..70ec55b051 100644 --- a/engine/apps/schedules/tests/test_custom_on_call_shift.py +++ b/engine/apps/schedules/tests/test_custom_on_call_shift.py @@ -48,7 +48,7 @@ def test_get_on_call_users_from_web_schedule_override(make_organization_and_user } on_call_shift = make_on_call_shift(organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **data) - on_call_shift.users.add(user) + on_call_shift.add_rolling_users([[user]]) # user is on-call date = date + timezone.timedelta(minutes=5) From ff577753689838eca126970f7e272771ac633594 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 22 Jul 2022 10:02:00 -0600 Subject: [PATCH 07/12] Change slack URL to be built the same as other engine URLs (#280) * Change slack login url to be built the same way as other engine urls * Fix tests * Change how to override base url in create_engine_url * Change how to override base url in create_engine_url --- engine/apps/alerts/tests/test_alert_receiver_channel.py | 7 ++----- engine/apps/social_auth/live_setting_django_strategy.py | 5 +++-- engine/common/api_helpers/utils.py | 4 +++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/engine/apps/alerts/tests/test_alert_receiver_channel.py b/engine/apps/alerts/tests/test_alert_receiver_channel.py index b8b70cb608..20d5528d17 100644 --- a/engine/apps/alerts/tests/test_alert_receiver_channel.py +++ b/engine/apps/alerts/tests/test_alert_receiver_channel.py @@ -5,6 +5,7 @@ from django.urls import reverse from apps.alerts.models import AlertReceiveChannel +from common.api_helpers.utils import create_engine_url @pytest.mark.django_db @@ -26,11 +27,7 @@ def test_integration_url(make_organization, make_alert_receive_channel, url, set kwargs={"alert_channel_key": alert_receive_channel.token}, ) - # remove trailing / if present - if url[-1] == "/": - url = url[:-1] - - assert alert_receive_channel.integration_url == f"{url}{path}" + assert alert_receive_channel.integration_url == create_engine_url(path) @pytest.mark.django_db diff --git a/engine/apps/social_auth/live_setting_django_strategy.py b/engine/apps/social_auth/live_setting_django_strategy.py index d2cf0fe14b..a8deb5d8d1 100644 --- a/engine/apps/social_auth/live_setting_django_strategy.py +++ b/engine/apps/social_auth/live_setting_django_strategy.py @@ -7,6 +7,7 @@ from social_django.strategy import DjangoStrategy from apps.base.utils import live_settings +from common.api_helpers.utils import create_engine_url logger = logging.getLogger(__name__) @@ -37,9 +38,9 @@ def build_absolute_uri(self, path=None): Overridden DjangoStrategy's method to substitute and force the host value from ENV """ if live_settings.SLACK_INSTALL_RETURN_REDIRECT_HOST is not None and path is not None: - return live_settings.SLACK_INSTALL_RETURN_REDIRECT_HOST + path + return create_engine_url(path, override_base=live_settings.SLACK_INSTALL_RETURN_REDIRECT_HOST) if settings.SLACK_INSTALL_RETURN_REDIRECT_HOST is not None and path is not None: - return settings.SLACK_INSTALL_RETURN_REDIRECT_HOST + path + return create_engine_url(path, override_base=settings.SLACK_INSTALL_RETURN_REDIRECT_HOST) if self.request: return self.request.build_absolute_uri(path) else: diff --git a/engine/common/api_helpers/utils.py b/engine/common/api_helpers/utils.py index 06b17f0fc5..9c974a3783 100644 --- a/engine/common/api_helpers/utils.py +++ b/engine/common/api_helpers/utils.py @@ -54,8 +54,10 @@ def validate_ical_url(url): return None -def create_engine_url(path): +def create_engine_url(path, override_base=None): base = settings.BASE_URL + if override_base: + base = override_base if not base.endswith("/"): base += "/" trimmed_path = path.lstrip("/") From 9c72924881f245dfc30f003110ba4d12b43dabab Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Fri, 22 Jul 2022 22:22:59 +0300 Subject: [PATCH 08/12] Use forked push-notifications library with fixed migration --- engine/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/requirements.txt b/engine/requirements.txt index e0bfcd0122..d0896ae020 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -36,7 +36,7 @@ django-log-request-id==1.6.0 django-polymorphic==3.0.0 django-rest-polymorphic==0.1.9 pre-commit==2.15.0 -django-push-notifications==3.0.0 +https://github.com/iskhakov/django-push-notifications/archive/refs/tags/3.0.0-fix-migration.tar.gz django-mirage-field==1.3.0 django-mysql==4.6.0 PyMySQL==1.0.2 From 972bfd9c68dff540e86aa86d9bc1f134eb9eb183 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Mon, 25 Jul 2022 12:59:12 +0400 Subject: [PATCH 09/12] Fix propagation for the insight logger --- engine/apps/api/views/test_insight_logs.py | 1 - engine/settings/base.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/engine/apps/api/views/test_insight_logs.py b/engine/apps/api/views/test_insight_logs.py index 092536e63a..da6b4d179d 100644 --- a/engine/apps/api/views/test_insight_logs.py +++ b/engine/apps/api/views/test_insight_logs.py @@ -6,7 +6,6 @@ from apps.auth_token.auth import PluginAuthentication -logger = logging.getLogger(__name__) insight_logger = logging.getLogger("insight_logger") diff --git a/engine/settings/base.py b/engine/settings/base.py index 33e95a63c5..7e3ef38de5 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -154,7 +154,7 @@ "filters": {"request_id": {"()": "log_request_id.filters.RequestIDFilter"}}, "formatters": { "standard": {"format": "source=engine:app google_trace_id=%(request_id)s logger=%(name)s %(message)s"}, - "insight_logger": {"format": "insight_logs=true %(message)s"}, + "insight_logger": {"format": "insight_logs=true logger=%(name)s %(message)s"}, }, "handlers": { "console": { @@ -171,7 +171,7 @@ "insight_logger": { "handlers": ["insight_logger"], "level": "INFO", - "propagate": True, + "propagate": False, }, "": { "handlers": ["console"], From 76f67b171add0de73bd57d606b9b0e6d67170be8 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 25 Jul 2022 11:09:06 +0100 Subject: [PATCH 10/12] Show only maintenances for current team (#287) --- engine/apps/api/views/maintenance.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engine/apps/api/views/maintenance.py b/engine/apps/api/views/maintenance.py index bfb4c4f788..aa69dd9b75 100644 --- a/engine/apps/api/views/maintenance.py +++ b/engine/apps/api/views/maintenance.py @@ -43,9 +43,11 @@ class MaintenanceAPIView(APIView): def get(self, request): organization = self.request.auth.organization + team = self.request.user.current_team + response = [] integrations_under_maintenance = AlertReceiveChannel.objects.filter( - maintenance_mode__isnull=False, organization=organization + maintenance_mode__isnull=False, organization=organization, team=team ).order_by("maintenance_started_at") if organization.maintenance_mode is not None: From ce8f4e53fabbc9b15b3503c31e29ebaa4140b5bb Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 25 Jul 2022 09:12:50 -0600 Subject: [PATCH 11/12] Conform URLs (#281) * Make any URLs build from env vars tolerant of path prefix, trailing/leading slashes * Add comment * Lint --- .../metadata/heartbeat/_heartbeat_text_creator.py | 5 +++-- .../integrations/mixins/browsable_instruction_mixin.py | 5 +++-- engine/apps/integrations/views.py | 6 +++--- engine/apps/oss_installation/cloud_heartbeat.py | 3 ++- engine/apps/oss_installation/models/cloud_connector.py | 10 +++++++--- engine/apps/telegram/client.py | 4 ++-- engine/apps/twilioapp/models/phone_call.py | 4 ++-- engine/apps/twilioapp/models/sms_message.py | 4 ++-- engine/common/api_helpers/utils.py | 9 +++++++++ 9 files changed, 33 insertions(+), 17 deletions(-) diff --git a/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py b/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py index 84bd12353d..859957a16c 100644 --- a/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py +++ b/engine/apps/integrations/metadata/heartbeat/_heartbeat_text_creator.py @@ -1,8 +1,9 @@ from dataclasses import dataclass -from urllib.parse import urljoin from django.conf import settings +from common.api_helpers.utils import create_engine_url + @dataclass class IntegrationHeartBeatText: @@ -31,7 +32,7 @@ def _get_heartbeat_expired_title(self): return heartbeat_expired_title def _get_heartbeat_expired_message(self): - heartbeat_docs_url = urljoin(settings.DOCS_URL, "/#/integrations/heartbeat") + heartbeat_docs_url = create_engine_url("/#/integrations/heartbeat", override_base=settings.DOCS_URL) heartbeat_expired_message = ( f"Amixr was waiting for a heartbeat from {self.integration_verbal}. " f"Heartbeat is missing. That could happen because {self.integration_verbal} stopped or" diff --git a/engine/apps/integrations/mixins/browsable_instruction_mixin.py b/engine/apps/integrations/mixins/browsable_instruction_mixin.py index 823303befd..c87de5c71e 100644 --- a/engine/apps/integrations/mixins/browsable_instruction_mixin.py +++ b/engine/apps/integrations/mixins/browsable_instruction_mixin.py @@ -1,16 +1,17 @@ import json -from urllib.parse import urljoin from django.conf import settings from django.http import HttpResponse from django.template import loader +from common.api_helpers.utils import create_engine_url + class BrowsableInstructionMixin: def get(self, request, alert_receive_channel, *args, **kwargs): template = loader.get_template("integration_link.html") # TODO Create associative array for integrations - base_integration_docs_url = urljoin(settings.DOCS_URL, "/#/integrations/") + base_integration_docs_url = create_engine_url("/#/integrations/", override_base=settings.DOCS_URL) docs_url = f'{base_integration_docs_url}{request.get_full_path().split("/")[3]}' show_button = True if request.get_full_path().split("/")[3] == "amazon_sns": diff --git a/engine/apps/integrations/views.py b/engine/apps/integrations/views.py index 1aa164b432..4c975b1570 100644 --- a/engine/apps/integrations/views.py +++ b/engine/apps/integrations/views.py @@ -1,6 +1,5 @@ import json import logging -from urllib.parse import urljoin from django.apps import apps from django.conf import settings @@ -28,6 +27,7 @@ from apps.integrations.tasks import create_alert, create_alertmanager_alerts from apps.sendgridapp.parse import Parse from apps.sendgridapp.permissions import AllowOnlySendgrid +from common.api_helpers.utils import create_engine_url logger = logging.getLogger(__name__) @@ -76,7 +76,7 @@ def handle_message(self, message, payload): raw_request_data = message title = message.get("AlarmName", "Alert") else: - docs_amazon_sns_url = urljoin(settings.DOCS_URL, "/#/integrations/amazon_sns") + docs_amazon_sns_url = create_engine_url("/#/integrations/amazon_sns", override_base=settings.DOCS_URL) title = "Alert" message_text = ( "Non-JSON payload received. Please make sure you publish monitoring Alarms to SNS," @@ -272,7 +272,7 @@ def post(self, request, alert_receive_channel, *args, **kwargs): class HeartBeatAPIView(AlertChannelDefiningMixin, APIView): def get(self, request, alert_receive_channel): template = loader.get_template("heartbeat_link.html") - docs_url = urljoin(settings.DOCS_URL, "/#/integrations/heartbeat") + docs_url = create_engine_url("/#/integrations/heartbeat", override_base=settings.DOCS_URL) return HttpResponse( template.render( { diff --git a/engine/apps/oss_installation/cloud_heartbeat.py b/engine/apps/oss_installation/cloud_heartbeat.py index b75d529900..f61e249b46 100644 --- a/engine/apps/oss_installation/cloud_heartbeat.py +++ b/engine/apps/oss_installation/cloud_heartbeat.py @@ -8,6 +8,7 @@ from rest_framework import status from apps.base.utils import live_settings +from common.api_helpers.utils import create_engine_url logger = logging.getLogger(__name__) @@ -23,7 +24,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 = create_engine_url("api/v1/integrations/", override_base=settings.GRAFANA_CLOUD_ONCALL_API_URL) try: headers = {"Authorization": api_token} r = requests.post(url=url, data=data, headers=headers, timeout=5) diff --git a/engine/apps/oss_installation/models/cloud_connector.py b/engine/apps/oss_installation/models/cloud_connector.py index fefc640e23..07eb672435 100644 --- a/engine/apps/oss_installation/models/cloud_connector.py +++ b/engine/apps/oss_installation/models/cloud_connector.py @@ -7,6 +7,7 @@ from apps.base.utils import live_settings from apps.oss_installation.models.cloud_user_identity import CloudUserIdentity from apps.user_management.models import User +from common.api_helpers.utils import create_engine_url from common.constants.role import Role from settings.base import GRAFANA_CLOUD_ONCALL_API_URL @@ -33,7 +34,7 @@ def sync_with_cloud(cls, token=None): logger.warning("Unable to sync with cloud. GRAFANA_CLOUD_ONCALL_TOKEN is not set") error_msg = "GRAFANA_CLOUD_ONCALL_TOKEN is not set" else: - info_url = urljoin(GRAFANA_CLOUD_ONCALL_API_URL, "api/v1/info/") + info_url = create_engine_url("api/v1/info/", override_base=GRAFANA_CLOUD_ONCALL_API_URL) try: r = requests.get(info_url, headers={"AUTHORIZATION": api_token}, timeout=5) if r.status_code == 200: @@ -62,7 +63,7 @@ def sync_users_with_cloud(self) -> tuple[bool, str]: existing_emails = list(User.objects.filter(role__in=(Role.ADMIN, Role.EDITOR)).values_list("email", flat=True)) matching_users = [] - users_url = urljoin(GRAFANA_CLOUD_ONCALL_API_URL, "api/v1/users") + users_url = create_engine_url("api/v1/users", override_base=GRAFANA_CLOUD_ONCALL_API_URL) fetch_next_page = True users_fetched = True @@ -115,7 +116,10 @@ def sync_user_with_cloud(self, user): logger.warning(f"Unable to sync_user_with cloud user_id {user.id}. GRAFANA_CLOUD_ONCALL_TOKEN is not set") error_msg = "GRAFANA_CLOUD_ONCALL_TOKEN is not set" else: - url = urljoin(GRAFANA_CLOUD_ONCALL_API_URL, f"api/v1/users/?email={user.email}&roles=0&roles=1&short=true") + url = create_engine_url( + f"api/v1/users/?email={user.email}&roles=0&roles=1&short=true", + override_base=GRAFANA_CLOUD_ONCALL_API_URL, + ) try: r = requests.get(url, headers={"AUTHORIZATION": api_token}, timeout=5) if r.status_code != 200: diff --git a/engine/apps/telegram/client.py b/engine/apps/telegram/client.py index 4de25de168..3856d3718a 100644 --- a/engine/apps/telegram/client.py +++ b/engine/apps/telegram/client.py @@ -1,5 +1,4 @@ from typing import Optional, Tuple, Union -from urllib.parse import urljoin from telegram import Bot, InlineKeyboardMarkup, Message, ParseMode from telegram.error import InvalidToken, Unauthorized @@ -10,6 +9,7 @@ from apps.telegram.models import TelegramMessage from apps.telegram.renderers.keyboard import TelegramKeyboardRenderer from apps.telegram.renderers.message import TelegramMessageRenderer +from common.api_helpers.utils import create_engine_url class TelegramClient: @@ -34,7 +34,7 @@ def is_chat_member(self, chat_id: Union[int, str]) -> bool: return False def register_webhook(self, webhook_url: Optional[str] = None) -> None: - webhook_url = webhook_url or urljoin(live_settings.TELEGRAM_WEBHOOK_HOST, "/telegram/") + webhook_url = webhook_url or create_engine_url("/telegram/", override_base=live_settings.TELEGRAM_WEBHOOK_HOST) if webhook_url is None: webhook_url = live_settings.TELEGRAM_WEBHOOK_URL diff --git a/engine/apps/twilioapp/models/phone_call.py b/engine/apps/twilioapp/models/phone_call.py index 64b4304e8d..69893b8a56 100644 --- a/engine/apps/twilioapp/models/phone_call.py +++ b/engine/apps/twilioapp/models/phone_call.py @@ -1,5 +1,4 @@ import logging -from urllib.parse import urljoin import requests from django.apps import apps @@ -14,6 +13,7 @@ from apps.base.utils import live_settings from apps.twilioapp.constants import TwilioCallStatuses from apps.twilioapp.twilio_client import twilio_client +from common.api_helpers.utils import create_engine_url from common.utils import clean_markup, escape_for_twilio_phone_call logger = logging.getLogger(__name__) @@ -158,7 +158,7 @@ def created_for_slack(self): @classmethod def _make_cloud_call(cls, user, message_body): - url = urljoin(settings.GRAFANA_CLOUD_ONCALL_API_URL, "api/v1/make_call") + url = create_engine_url("api/v1/make_call", override_base=settings.GRAFANA_CLOUD_ONCALL_API_URL) auth = {"Authorization": live_settings.GRAFANA_CLOUD_ONCALL_TOKEN} data = { "email": user.email, diff --git a/engine/apps/twilioapp/models/sms_message.py b/engine/apps/twilioapp/models/sms_message.py index 00e98e4bef..55aea7e8a0 100644 --- a/engine/apps/twilioapp/models/sms_message.py +++ b/engine/apps/twilioapp/models/sms_message.py @@ -1,5 +1,4 @@ import logging -from urllib.parse import urljoin import requests from django.apps import apps @@ -13,6 +12,7 @@ from apps.base.utils import live_settings from apps.twilioapp.constants import TwilioMessageStatuses from apps.twilioapp.twilio_client import twilio_client +from common.api_helpers.utils import create_engine_url from common.utils import clean_markup logger = logging.getLogger(__name__) @@ -123,7 +123,7 @@ def created_for_slack(self): @classmethod def _send_cloud_sms(cls, user, message_body): - url = urljoin(settings.GRAFANA_CLOUD_ONCALL_API_URL, "api/v1/send_sms") + url = create_engine_url("api/v1/send_sms", override_base=settings.GRAFANA_CLOUD_ONCALL_API_URL) auth = {"Authorization": live_settings.GRAFANA_CLOUD_ONCALL_TOKEN} data = { "email": user.email, diff --git a/engine/common/api_helpers/utils.py b/engine/common/api_helpers/utils.py index 9c974a3783..7ecd5d47d1 100644 --- a/engine/common/api_helpers/utils.py +++ b/engine/common/api_helpers/utils.py @@ -54,6 +54,15 @@ def validate_ical_url(url): return None +""" +This utility function is for building a URL when we don't know if the base URL +has been given a trailing / such as reading from environment variable or user +input. If the base URL is coming from a validated model field urljoin can be used +instead. Do not use this function to append query parameters since a / is added +to the end of the base URL if there isn't one. +""" + + def create_engine_url(path, override_base=None): base = settings.BASE_URL if override_base: From 879e157f05740a94a9ea959ac2222333d33b8143 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Tue, 26 Jul 2022 10:13:02 -0300 Subject: [PATCH 12/12] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3546a5a504..b31cf7863b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Change Log +## v1.0.12 (2022-07-26) +- Update push-notifications dependency +- Rework how absolute URLs are built +- Fix to show maintenance windows per team +- Logging improvements +- Internal api to get a schedule final events + ## v1.0.10 (2022-07-22) - Speed-up of alert group web caching - Internal api for OnCall shifts