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

Add alert page for indexing and seeing details about alerts #154

Merged
merged 20 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d426106
Add basic routes for getting alert details
jaggederest Dec 13, 2023
0df06be
Tweak format of alert list/get
jaggederest Dec 19, 2023
65ee0ef
Clean up typing of alert list/get routes
jaggederest Dec 19, 2023
f3f667b
Merge remote-tracking branch 'origin/main' into jag/alert_page
jaggederest Dec 21, 2023
c9f7d63
WIP alerts page
jaggederest Dec 21, 2023
8c0db18
Tweak and update alerts page, link to dashboard/search, working spark…
jaggederest Dec 21, 2023
b195109
Split alert layout into 3 sections (4 once alerts can be insufficient…
jaggederest Dec 21, 2023
c451753
add lastValue attribute to AlertHistory, display it on alert page if …
jaggederest Dec 21, 2023
85556fb
Icons for alert statuses, dummy disable/enable button for alerts
jaggederest Dec 21, 2023
db92c01
commentary on alert history in alert controller
jaggederest Dec 21, 2023
8363a33
Update with some alert page tweaks and type corrections
jaggederest Dec 22, 2023
75df136
Alerts page updates - using lastValues as an array
jaggederest Dec 23, 2023
b9b5491
Merge remote-tracking branch 'origin/main' into jag/alert_page
jaggederest Dec 23, 2023
1ad6c78
basic smoke tests for the alerts api index
jaggederest Dec 23, 2023
43484fc
Merge branch 'main' into jag/alert_page
wrn14897 Dec 28, 2023
0a72739
style: cleanup GET alerts endpoint
wrn14897 Dec 28, 2023
8de4edb
fix: cleanup UI + remove unused fields (alert history)
wrn14897 Dec 28, 2023
353882d
style: cleanup AlertHistory type
wrn14897 Dec 28, 2023
113848f
fix: LogView ref bug (alert model)
wrn14897 Dec 28, 2023
cb56756
Merge branch 'main' into jag/alert_page
wrn14897 Dec 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ dev-int:
docker compose -p int -f ./docker-compose.ci.yml run --rm api dev:int $(FILE)
docker compose -p int -f ./docker-compose.ci.yml down

.PHONY: dev-int-build
dev-int-build:
docker compose -p int -f ./docker-compose.ci.yml build

.PHONY: ci-int
ci-int:
docker compose -p int -f ./docker-compose.ci.yml run --rm api ci:int
Expand Down
1 change: 1 addition & 0 deletions packages/api/src/controllers/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export const createAlert = async (alertInput: AlertInput) => {

// create an update alert function based off of the above create alert function
export const updateAlert = async (id: string, alertInput: AlertInput) => {
// should consider clearing AlertHistory when updating an alert?
return Alert.findByIdAndUpdate(id, makeAlert(alertInput), {
returnDocument: 'after',
});
Expand Down
13 changes: 13 additions & 0 deletions packages/api/src/models/alertHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface IAlertHistory {
counts: number;
createdAt: Date;
state: AlertState;
lastValues: { startTime: Date; count: number }[];
}

const AlertHistorySchema = new Schema<IAlertHistory>({
Expand All @@ -27,6 +28,18 @@ const AlertHistorySchema = new Schema<IAlertHistory>({
enum: Object.values(AlertState),
required: true,
},
lastValues: [
{
startTime: {
type: Date,
required: true,
},
count: {
type: Number,
required: true,
},
},
],
});

AlertHistorySchema.index(
Expand Down
96 changes: 96 additions & 0 deletions packages/api/src/routers/api/__tests__/alerts.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import {
clearDBCollections,
closeDB,
getLoggedInAgent,
getServer,
} from '@/fixtures';

const randomId = () => Math.random().toString(36).substring(7);

const makeChart = () => ({
id: randomId(),
name: 'Test Chart',
x: 1,
y: 1,
w: 1,
h: 1,
series: [
{
type: 'time',
table: 'metrics',
},
],
});

const makeAlert = ({
dashboardId,
chartId,
}: {
dashboardId: string;
chartId: string;
}) => ({
channel: {
type: 'webhook',
webhookId: 'test-webhook-id',
},
interval: '15m',
threshold: 8,
type: 'presence',
source: 'CHART',
dashboardId,
chartId,
});

const MOCK_DASHBOARD = {
name: 'Test Dashboard',
charts: [makeChart(), makeChart(), makeChart(), makeChart(), makeChart()],
query: 'test query',
};

describe('alerts router', () => {
const server = getServer();

it('index has alerts attached to dashboards', async () => {
const { agent } = await getLoggedInAgent(server);

await agent.post('/dashboards').send(MOCK_DASHBOARD).expect(200);
const initialDashboards = await agent.get('/dashboards').expect(200);

// Create alerts for all charts
const dashboard = initialDashboards.body.data[0];
await Promise.all(
dashboard.charts.map(chart =>
agent
.post('/alerts')
.send(
makeAlert({
dashboardId: dashboard._id,
chartId: chart.id,
}),
)
.expect(200),
),
);

const alerts = await agent.get(`/alerts`).expect(200);
expect(alerts.body.data.alerts.length).toBe(5);
for (const alert of alerts.body.data.alerts) {
expect(alert.dashboardId).toBe(dashboard._id);
expect(alert.chartId).toBeDefined();
expect(alert.dashboard).toBeDefined();
}
});

beforeAll(async () => {
await server.start();
});

afterEach(async () => {
await clearDBCollections();
});

afterAll(async () => {
await server.closeHttpServer();
await closeDB();
});
});
69 changes: 68 additions & 1 deletion packages/api/src/routers/api/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import {
validateGroupByProperty,
} from '@/controllers/alerts';
import { getTeam } from '@/controllers/team';
import Alert from '@/models/alert';
import Alert, { IAlert } from '@/models/alert';
import AlertHistory, { IAlertHistory } from '@/models/alertHistory';
import Dashboard from '@/models/dashboard';
import LogView, { ILogView } from '@/models/logView';

const router = express.Router();

Expand All @@ -18,6 +21,8 @@ const zChannel = z.object({
webhookId: z.string().min(1),
});

const validateGet = validateRequest({ params: z.object({ id: z.string() }) });

const zLogAlert = z.object({
source: z.literal('LOG'),
groupBy: z.string().optional(),
Expand All @@ -43,6 +48,29 @@ const zAlert = z

const zAlertInput = zAlert;

const getHistory = async (alert: IAlert, teamId: string) => {
const histories = await AlertHistory.find({ alert: alert._id, team: teamId })
.sort({ createdAt: -1 })
.limit(20);
jaggederest marked this conversation as resolved.
Show resolved Hide resolved
return histories;
};

const getDashboard = async (alert: IAlert, teamId: string) => {
const dashboard = await Dashboard.findOne({
_id: alert.dashboardId,
team: teamId,
});
return dashboard;
};

const getLogView = async (alert: IAlert, teamId: string) => {
const logView = await LogView.findOne({
_id: alert.logView,
team: teamId,
});
return logView;
};
jaggederest marked this conversation as resolved.
Show resolved Hide resolved

// Validate groupBy property
const validateGroupBy = async (req, res, next) => {
const { groupBy, source } = req.body || {};
Expand Down Expand Up @@ -71,6 +99,45 @@ const validateGroupBy = async (req, res, next) => {
};

// Routes
router.get('/', async (req, res, next) => {
try {
const teamId = req.user?.team;
if (teamId == null) {
return res.sendStatus(403);
}
const alerts = await Alert.find({ team: teamId });
jaggederest marked this conversation as resolved.
Show resolved Hide resolved
const alertsWithHistory = await Promise.all(
alerts.map(async alert => {
const history = (await getHistory(alert, teamId.toString())) ?? [];
if (!alert.source) throw new Error('Alert source is undefined');
if (alert.source === 'LOG') {
const logView = await getLogView(alert, teamId.toString());
// had to rename because logView is an ObjectID
return {
logViewObj: logView,
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume logView is the redundant info. why don't we overwrite logView with object here ?

history,
...alert.toObject(),
};
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think its better to check the source here as well and throw at the end if necessary

const dashboard = await getDashboard(alert, teamId.toString());
return {
dashboard,
history,
...alert.toObject(),
};
}
}),
);
res.json({
data: {
alerts: alertsWithHistory,
},
});
} catch (e) {
next(e);
}
});

router.post(
'/',
validateRequest({ body: zAlertInput }),
Expand Down
21 changes: 11 additions & 10 deletions packages/api/src/tasks/__tests__/checkAlerts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,16 +360,17 @@ describe('checkAlerts', () => {
createdAt: 1,
});
expect(alertHistories.length).toBe(2);
expect(alertHistories[0].state).toBe('ALERT');
expect(alertHistories[0].counts).toBe(1);
expect(alertHistories[0].createdAt).toEqual(
new Date('2023-11-16T22:10:00.000Z'),
);
expect(alertHistories[1].state).toBe('OK');
expect(alertHistories[1].counts).toBe(0);
expect(alertHistories[1].createdAt).toEqual(
new Date('2023-11-16T22:15:00.000Z'),
);
const [history1, history2] = alertHistories;
expect(history1.state).toBe('ALERT');
expect(history1.counts).toBe(1);
expect(history1.createdAt).toEqual(new Date('2023-11-16T22:10:00.000Z'));
expect(history1.lastValues.length).toBe(1);
expect(history1.lastValues.length).toBeGreaterThan(0);
expect(history1.lastValues[0].count).toBeGreaterThanOrEqual(1);

expect(history2.state).toBe('OK');
expect(history2.counts).toBe(0);
expect(history2.createdAt).toEqual(new Date('2023-11-16T22:15:00.000Z'));

// check if getLogsChart query + webhook were triggered
expect(clickhouse.getLogsChart).toHaveBeenNthCalledWith(1, {
Expand Down
3 changes: 2 additions & 1 deletion packages/api/src/tasks/checkAlerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ export const processAlert = async (now: Date, alert: AlertDocument) => {
const totalCount = isString(checkData.data)
? parseInt(checkData.data)
: checkData.data;
const bucketStart = new Date(checkData.ts_bucket * 1000);
if (doesExceedThreshold(alert, totalCount)) {
alertState = AlertState.ALERT;
logger.info({
Expand All @@ -484,7 +485,6 @@ export const processAlert = async (now: Date, alert: AlertDocument) => {
totalCount,
checkData,
});
const bucketStart = new Date(checkData.ts_bucket * 1000);

await fireChannelEvent({
alert,
Expand All @@ -498,6 +498,7 @@ export const processAlert = async (now: Date, alert: AlertDocument) => {
});
history.counts += 1;
}
history.lastValues.push({ count: totalCount, startTime: bucketStart });
}

history.state = alertState;
Expand Down
3 changes: 3 additions & 0 deletions packages/app/pages/alerts.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import AlertsPage from '../src/AlertsPage';

export default AlertsPage;