From df1517573e0ac8c9bd590f5cd1c8cd6e25090b6f Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Thu, 2 Feb 2023 11:37:52 +0800 Subject: [PATCH] Cache web template rendered fields for alert and alertgroup endpoints (#1261) # What this PR does This PR adds same approach as introduced [here](https://github.com/grafana/oncall/pull/1236) to all alert and alertgroup endpoints ## Which issue(s) this PR fixes ## Checklist - [ ] Tests updated - [ ] Documentation added - [ ] `CHANGELOG.md` updated --------- Co-authored-by: Joey Orlando --- CHANGELOG.md | 1 + engine/apps/api/serializers/alert.py | 38 ++++++++- engine/apps/api/serializers/alert_group.py | 91 ++++++++++++---------- 3 files changed, 88 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 295cadc6..15932fff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add [`django-dbconn-retry` library](https://github.com/jdelic/django-dbconn-retry) to `INSTALLED_APPS` to attempt to alleviate occasional `django.db.utils.OperationalError` errors +- Improve alerts and alert group endpoint response time in internal API with caching ([1261](https://github.com/grafana/oncall/pull/1261)) ### Changed diff --git a/engine/apps/api/serializers/alert.py b/engine/apps/api/serializers/alert.py index 1fcd0e3d..9216a281 100644 --- a/engine/apps/api/serializers/alert.py +++ b/engine/apps/api/serializers/alert.py @@ -1,10 +1,40 @@ +from django.core.cache import cache +from django.utils import timezone from rest_framework import serializers from apps.alerts.incident_appearance.renderers.web_renderer import AlertWebRenderer from apps.alerts.models import Alert -class AlertSerializer(serializers.ModelSerializer): +class AlertFieldsCacheSerializerMixin: + @classmethod + def get_or_set_web_template_field( + cls, + obj, + field_name, + renderer_class, + cache_lifetime=60 * 60 * 24, + ): + CACHE_KEY = f"{field_name}_alert_{obj.id}" + cached_field = cache.get(CACHE_KEY, None) + + web_templates_modified_at = obj.group.channel.web_templates_modified_at + + # use cache only if cache exists + # and either web templates never modified + # or cache was created after templates were modified + if cached_field is not None and ( + web_templates_modified_at is None or cached_field.get("cache_created_at") > web_templates_modified_at + ): + field = cached_field.get(field_name) + else: + field = renderer_class(obj).render() + cache.set(CACHE_KEY, {"cache_created_at": timezone.now(), field_name: field}, cache_lifetime) + + return field + + +class AlertSerializer(AlertFieldsCacheSerializerMixin, serializers.ModelSerializer): id = serializers.CharField(read_only=True, source="public_primary_key") render_for_web = serializers.SerializerMethodField() @@ -18,7 +48,11 @@ class AlertSerializer(serializers.ModelSerializer): ] def get_render_for_web(self, obj): - return AlertWebRenderer(obj).render() + return AlertFieldsCacheSerializerMixin.get_or_set_web_template_field( + obj, + "render_for_web", + AlertWebRenderer, + ) class AlertRawSerializer(serializers.ModelSerializer): diff --git a/engine/apps/api/serializers/alert_group.py b/engine/apps/api/serializers/alert_group.py index 5f3c854e..58c58fdd 100644 --- a/engine/apps/api/serializers/alert_group.py +++ b/engine/apps/api/serializers/alert_group.py @@ -18,7 +18,39 @@ logger = logging.getLogger(__name__) logger.setLevel(logging.DEBUG) -class ShortAlertGroupSerializer(serializers.ModelSerializer): +class AlertGroupFieldsCacheSerializerMixin: + @classmethod + def get_or_set_web_template_field( + cls, + obj, + field_name, + renderer_class, + cache_lifetime=60 * 60 * 24, + ): + CACHE_KEY = f"{field_name}_alert_group_{obj.id}" + cached_field = cache.get(CACHE_KEY, None) + + web_templates_modified_at = obj.channel.web_templates_modified_at + last_alert_created_at = obj.last_alert.created_at + + # use cache only if cache exists + # and cache was created after the last alert created + # and either web templates never modified + # or cache was created after templates were modified + if ( + cached_field is not None + and cached_field.get("cache_created_at") > last_alert_created_at + and (web_templates_modified_at is None or cached_field.get("cache_created_at") > web_templates_modified_at) + ): + field = cached_field.get(field_name) + else: + field = renderer_class(obj, obj.last_alert).render() + cache.set(CACHE_KEY, {"cache_created_at": timezone.now(), field_name: field}, cache_lifetime) + + return field + + +class ShortAlertGroupSerializer(AlertGroupFieldsCacheSerializerMixin, serializers.ModelSerializer): pk = serializers.CharField(read_only=True, source="public_primary_key") alert_receive_channel = FastAlertReceiveChannelSerializer(source="channel") render_for_web = serializers.SerializerMethodField() @@ -28,10 +60,16 @@ class ShortAlertGroupSerializer(serializers.ModelSerializer): fields = ["pk", "render_for_web", "alert_receive_channel", "inside_organization_number"] def get_render_for_web(self, obj): - return AlertGroupWebRenderer(obj).render() + if not obj.last_alert: + return {} + return AlertGroupFieldsCacheSerializerMixin.get_or_set_web_template_field( + obj, + "render_for_web", + AlertGroupWebRenderer, + ) -class AlertGroupListSerializer(EagerLoadingMixin, serializers.ModelSerializer): +class AlertGroupListSerializer(EagerLoadingMixin, AlertGroupFieldsCacheSerializerMixin, serializers.ModelSerializer): pk = serializers.CharField(read_only=True, source="public_primary_key") alert_receive_channel = FastAlertReceiveChannelSerializer(source="channel") status = serializers.ReadOnlyField() @@ -91,41 +129,22 @@ class AlertGroupListSerializer(EagerLoadingMixin, serializers.ModelSerializer): ] def get_render_for_web(self, obj): - # alert group has no alerts if not obj.last_alert: return {} - - web_templates_modified_at = obj.channel.web_templates_modified_at - last_alert_created_at = obj.last_alert.created_at - - CACHE_KEY = f"render_for_web_alert_group_{obj.id}" - CACHE_LIFEIME = 60 * 60 * 24 - cached_render_for_web = cache.get(CACHE_KEY, None) - - # use cache only if cache exists - # and cache was created after the last alert created - # and either web templates never modified - # or cache was created after templates were modified - if ( - cached_render_for_web is not None - and cached_render_for_web.get("cache_created_at") > last_alert_created_at - and ( - web_templates_modified_at is None - or cached_render_for_web.get("cache_created_at") > web_templates_modified_at - ) - ): - render_for_web = cached_render_for_web.get("render_for_web") - else: - render_for_web = AlertGroupWebRenderer(obj, obj.last_alert).render() - cache.set(CACHE_KEY, {"cache_created_at": timezone.now(), "render_for_web": render_for_web}, CACHE_LIFEIME) - - return render_for_web + return AlertGroupFieldsCacheSerializerMixin.get_or_set_web_template_field( + obj, + "render_for_web", + AlertGroupWebRenderer, + ) def get_render_for_classic_markdown(self, obj): if not obj.last_alert: return {} - - return AlertGroupClassicMarkdownRenderer(obj, obj.last_alert).render() + return AlertGroupFieldsCacheSerializerMixin.get_or_set_web_template_field( + obj, + "render_for_classic_markdown", + AlertGroupClassicMarkdownRenderer, + ) def get_related_users(self, obj): users_ids = set() @@ -167,14 +186,6 @@ class AlertGroupSerializer(AlertGroupListSerializer): "paged_users", ] - def get_render_for_web(self, obj): - # alert group has no alerts - alert = obj.alerts.last() - if not alert: - return {} - - return AlertGroupWebRenderer(obj).render() - def get_last_alert_at(self, obj): last_alert = obj.alerts.last()