Skip to content

Commit

Permalink
fix: poor performance of incidents detail page (#1565)
Browse files Browse the repository at this point in the history
  • Loading branch information
VladimirFilonov authored Aug 8, 2024
1 parent a39c34c commit 2fa066e
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 46 deletions.
54 changes: 48 additions & 6 deletions keep-ui/app/incidents/[id]/incident-alerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,49 @@ import {
import AlertName from "app/alerts/alert-name";
import { ExclamationTriangleIcon } from "@radix-ui/react-icons";
import IncidentAlertMenu from "./incident-alert-menu";
import IncidentPagination from "../incident-pagination";
import React, {Dispatch, SetStateAction, useEffect, useState} from "react";

interface Props {
incidentId: string;
}

interface Pagination {
limit: number;
offset: number;
}


const columnHelper = createColumnHelper<AlertDto>();

export default function IncidentAlerts({ incidentId }: Props) {
const { data: alerts } = useIncidentAlerts(incidentId);
const [alertsPagination, setAlertsPagination] = useState<Pagination>({
limit: 20,
offset: 0,
});

const { data: alerts, isLoading } = useIncidentAlerts(incidentId, alertsPagination.limit, alertsPagination.offset);

const [pagination, setTablePagination] = useState({
pageIndex: alerts? Math.ceil(alerts.offset / alerts.limit) : 0,
pageSize: alerts? alerts.limit : 20,
});

useEffect(() => {
if (alerts && alerts.limit != pagination.pageSize) {
setAlertsPagination({
limit: pagination.pageSize,
offset: 0,
})
}
const currentOffset = pagination.pageSize * pagination.pageIndex;
if (alerts && alerts.offset != currentOffset) {
setAlertsPagination({
limit: pagination.pageSize,
offset: currentOffset,
})
}
}, [pagination])
usePollIncidentAlerts(incidentId);

const columns = [
Expand Down Expand Up @@ -104,12 +138,16 @@ export default function IncidentAlerts({ incidentId }: Props) {

const table = useReactTable({
columns: columns,
data: alerts ?? [],
manualPagination: true,
state: { pagination },
rowCount: alerts ? alerts.count : 0,
onPaginationChange: setTablePagination,
data: alerts?.items ?? [],
getCoreRowModel: getCoreRowModel(),
});
return (
<>
{(alerts ?? []).length === 0 && (
{!isLoading && (alerts?.items ?? []).length === 0 && (
<Callout
className="mt-4 w-full"
title="Missing Alerts"
Expand Down Expand Up @@ -137,7 +175,7 @@ export default function IncidentAlerts({ incidentId }: Props) {
</TableRow>
))}
</TableHead>
{alerts && alerts.length > 0 && (
{alerts && alerts?.items?.length > 0 && (
<TableBody>
{table.getRowModel().rows.map((row) => (
<TableRow key={row.id} className="hover:bg-slate-100">
Expand All @@ -152,9 +190,9 @@ export default function IncidentAlerts({ incidentId }: Props) {
)}
{
// Skeleton
(alerts ?? []).length === 0 && (
(isLoading || (alerts?.items ?? []).length === 0) && (
<TableBody>
{Array(5)
{Array(pagination.pageSize)
.fill("")
.map((index) => (
<TableRow key={index}>
Expand All @@ -169,6 +207,10 @@ export default function IncidentAlerts({ incidentId }: Props) {
)
}
</Table>

<div className="mt-4 mb-8">
<IncidentPagination table={table} isRefreshAllowed={true}/>
</div>
</>
);
}
2 changes: 2 additions & 0 deletions keep-ui/app/incidents/[id]/incident.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ import {
import IncidentAlerts from "./incident-alerts";
import { ArrowUturnLeftIcon } from "@heroicons/react/24/outline";
import { useRouter } from "next/navigation";
import {useState} from "react";

interface Props {
incidentId: string;
}

export default function IncidentView({ incidentId }: Props) {
const { data: incident, isLoading, error } = useIncident(incidentId);

const router = useRouter();

if (isLoading || !incident) return <Loading />;
Expand Down
61 changes: 31 additions & 30 deletions keep-ui/app/incidents/incident-pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ import { StylesConfig, SingleValueProps, components, GroupBase } from 'react-sel
import Select from 'react-select';
import { Table } from "@tanstack/react-table";
import {IncidentDto} from "./model";
import {AlertDto} from "../alerts/models";

interface Props {
table: Table<IncidentDto>;
table: Table<IncidentDto> | Table<AlertDto>;
isRefreshAllowed: boolean;
}

Expand All @@ -22,36 +23,36 @@ interface OptionType {
label: string;
}

const customStyles: StylesConfig<OptionType, false, GroupBase<OptionType>> = {
control: (provided, state) => ({
...provided,
borderColor: state.isFocused ? 'orange' : provided.borderColor,
'&:hover': { borderColor: 'orange' },
boxShadow: state.isFocused ? '0 0 0 1px orange' : provided.boxShadow,
}),
singleValue: (provided) => ({
...provided,
display: 'flex',
alignItems: 'center',
}),
menu: (provided) => ({
...provided,
color: 'orange',
}),
option: (provided, state) => ({
...provided,
backgroundColor: state.isSelected ? 'orange' : provided.backgroundColor,
'&:hover': { backgroundColor: state.isSelected ? 'orange' : '#f5f5f5' },
color: state.isSelected ? 'white' : provided.color,
}),
};
const customStyles: StylesConfig<OptionType, false, GroupBase<OptionType>> = {
control: (provided, state) => ({
...provided,
borderColor: state.isFocused ? 'orange' : provided.borderColor,
'&:hover': { borderColor: 'orange' },
boxShadow: state.isFocused ? '0 0 0 1px orange' : provided.boxShadow,
}),
singleValue: (provided) => ({
...provided,
display: 'flex',
alignItems: 'center',
}),
menu: (provided) => ({
...provided,
color: 'orange',
}),
option: (provided, state) => ({
...provided,
backgroundColor: state.isSelected ? 'orange' : provided.backgroundColor,
'&:hover': { backgroundColor: state.isSelected ? 'orange' : '#f5f5f5' },
color: state.isSelected ? 'white' : provided.color,
}),
};

const SingleValue = ({ children, ...props }: SingleValueProps<OptionType, false, GroupBase<OptionType>>) => (
<components.SingleValue {...props}>
{children}
<TableCellsIcon className="w-4 h-4 ml-2" />
</components.SingleValue>
);
const SingleValue = ({ children, ...props }: SingleValueProps<OptionType, false, GroupBase<OptionType>>) => (
<components.SingleValue {...props}>
{children}
<TableCellsIcon className="w-4 h-4 ml-2" />
</components.SingleValue>
);


export default function IncidentPagination({ table, isRefreshAllowed }: Props) {
Expand Down
10 changes: 10 additions & 0 deletions keep-ui/app/incidents/model.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {AlertDto} from "../alerts/models";

export interface IncidentDto {
id: string;
name: string;
Expand All @@ -18,3 +20,11 @@ export interface PaginatedIncidentsDto {
count: number;
items: IncidentDto[];
}

export interface PaginatedIncidentAlertsDto {
limit: number;
offset: number;
count: number;
items: AlertDto[];
}

8 changes: 5 additions & 3 deletions keep-ui/utils/hooks/useIncidents.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AlertDto } from "app/alerts/models";
import { IncidentDto, PaginatedIncidentsDto } from "app/incidents/model";
import {IncidentDto, PaginatedIncidentAlertsDto, PaginatedIncidentsDto} from "app/incidents/model";
import { useSession } from "next-auth/react";
import useSWR, { SWRConfiguration } from "swr";
import { getApiURL } from "utils/apiUrl";
Expand Down Expand Up @@ -33,14 +33,16 @@ export const useIncidents = (

export const useIncidentAlerts = (
incidentId: string,
limit: number = 20,
offset: number = 0,
options: SWRConfiguration = {
revalidateOnFocus: false,
}
) => {
const apiUrl = getApiURL();
const { data: session } = useSession();
return useSWR<AlertDto[]>(
() => (session ? `${apiUrl}/incidents/${incidentId}/alerts` : null),
return useSWR<PaginatedIncidentAlertsDto>(
() => (session ? `${apiUrl}/incidents/${incidentId}/alerts?limit=${limit}&offset=${offset}` : null),
(url) => fetcher(url, session?.accessToken),
options
);
Expand Down
6 changes: 4 additions & 2 deletions keep/api/core/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -2082,7 +2082,7 @@ def get_incidents_count(
)


def get_incident_alerts_by_incident_id(tenant_id: str, incident_id: str) -> List[Alert]:
def get_incident_alerts_by_incident_id(tenant_id: str, incident_id: str, limit: int, offset: int) -> (List[Alert], int):
with Session(engine) as session:
query = (
session.query(
Expand All @@ -2096,7 +2096,9 @@ def get_incident_alerts_by_incident_id(tenant_id: str, incident_id: str) -> List
)
)

return query.all()
total_count = query.count()

return query.limit(limit).offset(offset).all(), total_count


def get_alerts_data_for_incident(
Expand Down
15 changes: 11 additions & 4 deletions keep/api/routes/incidents.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
)
from keep.api.models.alert import AlertDto, IncidentDto, IncidentDtoIn
from keep.api.utils.enrichment_helpers import convert_db_alerts_to_dto_alerts
from keep.api.utils.pagination import IncidentsPaginatedResultsDto
from keep.api.utils.pagination import IncidentsPaginatedResultsDto, AlertPaginatedResultsDto

router = APIRouter()
logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -229,8 +229,10 @@ def delete_incident(
)
def get_incident_alerts(
incident_id: str,
limit: int = 25,
offset: int = 0,
authenticated_entity: AuthenticatedEntity = Depends(AuthVerifier(["read:alert"])),
) -> List[AlertDto]:
) -> AlertPaginatedResultsDto:
tenant_id = authenticated_entity.tenant_id
logger.info(
"Fetching incident",
Expand All @@ -250,7 +252,12 @@ def get_incident_alerts(
"tenant_id": tenant_id,
},
)
db_alerts = get_incident_alerts_by_incident_id(tenant_id, incident_id)
db_alerts, total_count = get_incident_alerts_by_incident_id(
tenant_id=tenant_id,
incident_id=incident_id,
limit=limit,
offset=offset,
)

enriched_alerts_dto = convert_db_alerts_to_dto_alerts(db_alerts)
logger.info(
Expand All @@ -260,7 +267,7 @@ def get_incident_alerts(
},
)

return enriched_alerts_dto
return AlertPaginatedResultsDto(limit=limit, offset=offset, count=total_count, items=enriched_alerts_dto)


@router.post(
Expand Down
6 changes: 5 additions & 1 deletion keep/api/utils/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from pydantic import BaseModel

from keep.api.models.alert import IncidentDto
from keep.api.models.alert import IncidentDto, AlertDto


class PaginatedResultsDto(BaseModel):
Expand All @@ -14,3 +14,7 @@ class PaginatedResultsDto(BaseModel):

class IncidentsPaginatedResultsDto(PaginatedResultsDto):
items: list[IncidentDto]


class AlertPaginatedResultsDto(PaginatedResultsDto):
items: list[AlertDto]

0 comments on commit 2fa066e

Please sign in to comment.