Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize alert and alert group public api endpoints, add filter by id #1274

Merged
merged 6 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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))
- Optimize alert and alert group public api endpoints and add filter by id ([1274](https://github.com/grafana/oncall/pull/1274)

### Changed

Expand Down
1 change: 1 addition & 0 deletions docs/sources/oncall-api-reference/alertgroups.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ The above command returns JSON structured in the following way:

These available filter parameters should be provided as `GET` arguments:

- `id`
- `route_id`
- `integration_id`
- `state`
Expand Down
1 change: 1 addition & 0 deletions docs/sources/oncall-api-reference/alerts.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ The above command returns JSON structured in the following way:

The following available filter parameters should be provided as `GET` arguments:

- `id`
- `alert_group_id`
- `search`—string-based inclusion search by alert payload

Expand Down
10 changes: 3 additions & 7 deletions engine/apps/public_api/serializers/incidents.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ class IncidentSerializer(EagerLoadingMixin, serializers.ModelSerializer):
route_id = serializers.SerializerMethodField()
created_at = serializers.DateTimeField(source="started_at")
alerts_count = serializers.SerializerMethodField()
title = serializers.SerializerMethodField()
title = serializers.CharField(source="web_title_cache")
state = serializers.SerializerMethodField()

SELECT_RELATED = ["channel", "channel_filter", "slack_message"]
SELECT_RELATED = ["channel", "channel_filter", "slack_message", "channel__organization"]
PREFETCH_RELATED = [
"alerts",
Prefetch(
"telegram_messages",
TelegramMessage.objects.filter(chat_id__startswith="-", message_type=TelegramMessage.ALERT_GROUP_MESSAGE),
Expand All @@ -42,10 +41,7 @@ class Meta:
]

def get_alerts_count(self, obj):
return len(obj.alerts.all())

def get_title(self, obj):
return obj.alerts.all()[0].title
return obj.alerts_count
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: probably makes sense to add a comment that alerts_count is an annotated field which was added in get_queryset


def get_state(self, obj):
return obj.state
Expand Down
8 changes: 8 additions & 0 deletions engine/apps/public_api/views/alerts.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.db.models import CharField
from django.db.models.functions import Cast
from django_filters import rest_framework as filters
from rest_framework import mixins
from rest_framework.permissions import IsAuthenticated
from rest_framework.viewsets import GenericViewSet
Expand All @@ -12,6 +13,10 @@
from common.api_helpers.paginators import FiftyPageSizePaginator


class AlertFilter(filters.FilterSet):
id = filters.CharFilter(field_name="public_primary_key")


class AlertView(RateLimitHeadersMixin, mixins.ListModelMixin, GenericViewSet):
authentication_classes = (ApiTokenAuthentication,)
permission_classes = (IsAuthenticated,)
Expand All @@ -22,6 +27,9 @@ class AlertView(RateLimitHeadersMixin, mixins.ListModelMixin, GenericViewSet):
serializer_class = AlertSerializer
pagination_class = FiftyPageSizePaginator

filter_backends = (filters.DjangoFilterBackend,)
filterset_class = AlertFilter

def get_queryset(self):
alert_group_id = self.request.query_params.get("alert_group_id", None)
search = self.request.query_params.get("search", None)
Expand Down
6 changes: 4 additions & 2 deletions engine/apps/public_api/views/incidents.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from django.db.models import Q
from django.db.models import Count, Q
from django_filters import rest_framework as filters
from rest_framework import mixins, status
from rest_framework.exceptions import NotFound
Expand Down Expand Up @@ -29,6 +29,8 @@ class IncidentByTeamFilter(ByTeamModelFieldFilterMixin, filters.FilterSet):
method=ByTeamModelFieldFilterMixin.filter_model_field_with_single_value.__name__,
)

id = filters.CharFilter(field_name="public_primary_key")


class IncidentView(RateLimitHeadersMixin, mixins.ListModelMixin, mixins.DestroyModelMixin, GenericViewSet):
authentication_classes = (ApiTokenAuthentication,)
Expand Down Expand Up @@ -77,7 +79,7 @@ def get_queryset(self):
raise BadRequest(detail={"state": f"Must be one of the following: {valid_choices_text}"})

queryset = self.serializer_class.setup_eager_loading(queryset)

queryset = queryset.annotate(alerts_count=Count("alerts"))
return queryset

def get_object(self):
Expand Down