Skip to content

Commit 321e24f

Browse files
wrn14897wrn14897
authored
fix: alerting time range filtering bug (#814)
Ref: HDX-1701 1. fix alerting time range filtering 2. add time range info to the alert body <img width="620" alt="image" src="https://github.com/user-attachments/assets/205d6537-e177-4be9-888f-a9328c8a2b8a" />
1 parent a825961 commit 321e24f

File tree

12 files changed

+187
-119
lines changed

12 files changed

+187
-119
lines changed

‎.changeset/few-mails-check.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
"@hyperdx/api": patch
4+
"@hyperdx/app": patch
5+
---
6+
7+
fix: alerting time range filtering bug

‎.changeset/twelve-dolls-yell.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
---
4+
5+
feat: support 'dateRangeEndInclusive' in timeFilterExpr

‎packages/api/src/tasks/__tests__/checkAlerts.test.ts

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ describe('checkAlerts', () => {
420420
'*<http://app:8080/search/fake-saved-search-id?from=1679091183103&to=1679091239103&isLive=false | Alert for "My Search" - 10 lines found>*',
421421
'Group: "http"',
422422
'10 lines found, expected less than 1 lines',
423+
'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)',
423424
'Custom body ',
424425
'```',
425426
'',
@@ -481,6 +482,7 @@ describe('checkAlerts', () => {
481482
'*<http://app:8080/search/fake-saved-search-id?from=1679091183103&to=1679091239103&isLive=false | Alert for "My Search" - 10 lines found>*',
482483
'Group: "http"',
483484
'10 lines found, expected less than 1 lines',
485+
'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)',
484486
'Custom body ',
485487
'```',
486488
'',
@@ -585,6 +587,7 @@ describe('checkAlerts', () => {
585587
'*<http://app:8080/search/fake-saved-search-id?from=1679091183103&to=1679091239103&isLive=false | Alert for "My Search" - 10 lines found>*',
586588
'Group: "http"',
587589
'10 lines found, expected less than 1 lines',
590+
'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)',
588591
'',
589592
' Runbook URL: https://example.com',
590593
' hi i matched',
@@ -613,6 +616,7 @@ describe('checkAlerts', () => {
613616
'*<http://app:8080/search/fake-saved-search-id?from=1679091183103&to=1679091239103&isLive=false | Alert for "My Search" - 10 lines found>*',
614617
'Group: "http"',
615618
'10 lines found, expected less than 1 lines',
619+
'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)',
616620
'',
617621
' Runbook URL: https://example.com',
618622
' hi i matched',
@@ -657,25 +661,33 @@ describe('checkAlerts', () => {
657661
const team = await createTeam({ name: 'My Team' });
658662

659663
const now = new Date('2023-11-16T22:12:00.000Z');
660-
// Send events in the last alert window 22:05 - 22:10
661-
const eventMs = now.getTime() - ms('5m');
664+
const eventMs = new Date('2023-11-16T22:05:00.000Z');
665+
const eventNextMs = new Date('2023-11-16T22:10:00.000Z');
662666

663667
await bulkInsertLogs([
668+
// logs from 22:05 - 22:10
664669
{
665670
ServiceName: 'api',
666-
Timestamp: new Date(eventMs),
671+
Timestamp: eventMs,
667672
SeverityText: 'error',
668673
Body: 'Oh no! Something went wrong!',
669674
},
670675
{
671676
ServiceName: 'api',
672-
Timestamp: new Date(eventMs),
677+
Timestamp: eventMs,
673678
SeverityText: 'error',
674679
Body: 'Oh no! Something went wrong!',
675680
},
676681
{
677682
ServiceName: 'api',
678-
Timestamp: new Date(eventMs),
683+
Timestamp: eventMs,
684+
SeverityText: 'error',
685+
Body: 'Oh no! Something went wrong!',
686+
},
687+
// logs from 22:10 - 22:15
688+
{
689+
ServiceName: 'api',
690+
Timestamp: eventNextMs,
679691
SeverityText: 'error',
680692
Body: 'Oh no! Something went wrong!',
681693
},
@@ -745,6 +757,11 @@ describe('checkAlerts', () => {
745757
const nextWindow = new Date('2023-11-16T22:16:00.000Z');
746758
await processAlert(nextWindow, enhancedAlert);
747759
// alert should be in ok state
760+
expect(enhancedAlert.state).toBe('ALERT');
761+
762+
const nextNextWindow = new Date('2023-11-16T22:20:00.000Z');
763+
await processAlert(nextNextWindow, enhancedAlert);
764+
// alert should be in ok state
748765
expect(enhancedAlert.state).toBe('OK');
749766

750767
// check alert history
@@ -753,19 +770,25 @@ describe('checkAlerts', () => {
753770
}).sort({
754771
createdAt: 1,
755772
});
756-
expect(alertHistories.length).toBe(2);
773+
expect(alertHistories.length).toBe(3);
757774
expect(alertHistories[0].state).toBe('ALERT');
758775
expect(alertHistories[0].counts).toBe(1);
759776
expect(alertHistories[0].createdAt).toEqual(
760777
new Date('2023-11-16T22:10:00.000Z'),
761778
);
762-
expect(alertHistories[1].state).toBe('OK');
763-
expect(alertHistories[1].counts).toBe(0);
779+
expect(alertHistories[1].state).toBe('ALERT');
780+
expect(alertHistories[1].counts).toBe(1);
764781
expect(alertHistories[1].createdAt).toEqual(
765782
new Date('2023-11-16T22:15:00.000Z'),
766783
);
784+
expect(alertHistories[2].state).toBe('OK');
785+
expect(alertHistories[2].counts).toBe(0);
786+
expect(alertHistories[2].createdAt).toEqual(
787+
new Date('2023-11-16T22:20:00.000Z'),
788+
);
767789

768790
// check if webhook was triggered
791+
// We're only checking the general structure here since the exact text includes timestamps
769792
expect(slack.postMessageToWebhook).toHaveBeenNthCalledWith(
770793
1,
771794
'https://hooks.slack.com/services/123',
@@ -779,6 +802,19 @@ describe('checkAlerts', () => {
779802
],
780803
},
781804
);
805+
expect(slack.postMessageToWebhook).toHaveBeenNthCalledWith(
806+
2,
807+
'https://hooks.slack.com/services/123',
808+
{
809+
text: 'Alert for "My Search" - 1 lines found',
810+
blocks: [
811+
{
812+
text: expect.any(Object),
813+
type: 'section',
814+
},
815+
],
816+
},
817+
);
782818
});
783819

784820
it('TILE alert (events) - slack webhook', async () => {
@@ -931,6 +967,7 @@ describe('checkAlerts', () => {
931967
`*<http://app:8080/dashboards/${dashboard._id}?from=1700170200000&granularity=5+minute&to=1700174700000 | Alert for "Logs Count" in "My Dashboard" - 3 exceeds 1>*`,
932968
'',
933969
'3 exceeds 1',
970+
'Time Range (UTC): [Nov 16 10:05:00 PM - Nov 16 10:10:00 PM)',
934971
'',
935972
].join('\n'),
936973
type: 'mrkdwn',
@@ -1248,6 +1285,7 @@ describe('checkAlerts', () => {
12481285
`*<http://app:8080/dashboards/${dashboard._id}?from=1700170200000&granularity=5+minute&to=1700174700000 | Alert for "CPU" in "My Dashboard" - 6.25 exceeds 1>*`,
12491286
'',
12501287
'6.25 exceeds 1',
1288+
'Time Range (UTC): [Nov 16 10:05:00 PM - Nov 16 10:10:00 PM)',
12511289
'',
12521290
].join('\n'),
12531291
type: 'mrkdwn',

‎packages/api/src/tasks/checkAlerts.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
ChartConfigWithOptDateRange,
99
DisplayType,
1010
} from '@hyperdx/common-utils/dist/types';
11+
import { formatDate } from '@hyperdx/common-utils/dist/utils';
1112
import * as fns from 'date-fns';
1213
import Handlebars, { HelperOptions } from 'handlebars';
1314
import _ from 'lodash';
@@ -477,6 +478,11 @@ export const renderAlertTemplate = async ({
477478
);
478479
};
479480

481+
const timeRangeMessage = `Time Range (UTC): [${formatDate(view.startTime, {
482+
isUTC: true,
483+
})} - ${formatDate(view.endTime, {
484+
isUTC: true,
485+
})})`;
480486
let rawTemplateBody;
481487

482488
// TODO: support advanced routing with template engine
@@ -538,7 +544,7 @@ ${value} lines found, expected ${
538544
alert.thresholdType === AlertThresholdType.ABOVE
539545
? 'less than'
540546
: 'greater than'
541-
} ${alert.threshold} lines
547+
} ${alert.threshold} lines\n${timeRangeMessage}
542548
${targetTemplate}
543549
\`\`\`
544550
${truncatedResults}
@@ -556,7 +562,7 @@ ${value} ${
556562
: alert.thresholdType === AlertThresholdType.ABOVE
557563
? 'falls below'
558564
: 'exceeds'
559-
} ${alert.threshold}
565+
} ${alert.threshold}\n${timeRangeMessage}
560566
${targetTemplate}`;
561567
}
562568

@@ -716,6 +722,8 @@ export const processAlert = async (now: Date, alert: EnhancedAlert) => {
716722
connection: connectionId,
717723
displayType: DisplayType.Line,
718724
dateRange: [checkStartTime, checkEndTime],
725+
dateRangeStartInclusive: true,
726+
dateRangeEndInclusive: false,
719727
from: source.from,
720728
granularity: `${windowSizeInMins} minute`,
721729
select: [
@@ -772,6 +780,8 @@ export const processAlert = async (now: Date, alert: EnhancedAlert) => {
772780
chartConfig = {
773781
connection: connectionId,
774782
dateRange: [checkStartTime, checkEndTime],
783+
dateRangeStartInclusive: true,
784+
dateRangeEndInclusive: false,
775785
displayType: firstTile.config.displayType,
776786
from: source.from,
777787
granularity: `${windowSizeInMins} minute`,

‎packages/app/src/__tests__/utils.test.ts

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,57 +5,12 @@ import { MetricsDataType, NumberFormat } from '../types';
55
import * as utils from '../utils';
66
import {
77
formatAttributeClause,
8-
formatDate,
98
formatNumber,
109
getMetricTableName,
1110
stripTrailingSlash,
1211
useQueryHistory,
1312
} from '../utils';
1413

15-
describe('utils', () => {
16-
it('12h utc', () => {
17-
const date = new Date('2021-01-01T12:00:00Z');
18-
expect(
19-
formatDate(date, {
20-
clock: '12h',
21-
isUTC: true,
22-
}),
23-
).toEqual('Jan 1 12:00:00 PM');
24-
});
25-
26-
it('24h utc', () => {
27-
const date = new Date('2021-01-01T12:00:00Z');
28-
expect(
29-
formatDate(date, {
30-
clock: '24h',
31-
isUTC: true,
32-
format: 'withMs',
33-
}),
34-
).toEqual('Jan 1 12:00:00.000');
35-
});
36-
37-
it('12h local', () => {
38-
const date = new Date('2021-01-01T12:00:00');
39-
expect(
40-
formatDate(date, {
41-
clock: '12h',
42-
isUTC: false,
43-
}),
44-
).toEqual('Jan 1 12:00:00 PM');
45-
});
46-
47-
it('24h local', () => {
48-
const date = new Date('2021-01-01T12:00:00');
49-
expect(
50-
formatDate(date, {
51-
clock: '24h',
52-
isUTC: false,
53-
format: 'withMs',
54-
}),
55-
).toEqual('Jan 1 12:00:00.000');
56-
});
57-
});
58-
5914
describe('formatAttributeClause', () => {
6015
it('should format SQL attribute clause correctly', () => {
6116
expect(

‎packages/app/src/timeQuery.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
withDefault,
2727
} from 'use-query-params';
2828
import { DateRange } from '@hyperdx/common-utils/dist/types';
29+
import { formatDate } from '@hyperdx/common-utils/dist/utils';
2930

3031
import { parseTimeRangeInput } from './components/TimePicker/utils';
3132
import { useUserPreferences } from './useUserPreferences';
@@ -34,18 +35,16 @@ import { usePrevious } from './utils';
3435
const LIVE_TAIL_TIME_QUERY = 'Live Tail';
3536
const LIVE_TAIL_REFRESH_INTERVAL_MS = 1000;
3637

37-
const formatDate = (
38-
date: Date,
39-
isUTC: boolean,
40-
strFormat = 'MMM d HH:mm:ss',
41-
) => {
42-
return isUTC
43-
? formatInTimeZone(date, 'Etc/UTC', strFormat)
44-
: format(date, strFormat);
45-
};
46-
4738
export const dateRangeToString = (range: [Date, Date], isUTC: boolean) => {
48-
return `${formatDate(range[0], isUTC)} - ${formatDate(range[1], isUTC)}`;
39+
return `${formatDate(range[0], {
40+
isUTC,
41+
format: 'normal',
42+
clock: '24h',
43+
})} - ${formatDate(range[1], {
44+
isUTC,
45+
format: 'normal',
46+
clock: '24h',
47+
})}`;
4948
};
5049

5150
function isInputTimeQueryLive(inputTimeQuery: string) {

‎packages/app/src/useFormatTime.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from 'react';
2+
import { formatDate } from '@hyperdx/common-utils/dist/utils';
23

34
import { useUserPreferences } from './useUserPreferences';
4-
import { formatDate } from './utils';
55

66
type DateLike = number | string | Date;
77

‎packages/app/src/utils.ts

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -646,44 +646,6 @@ export const legacyMetricNameToNameAndDataType = (metricName?: string) => {
646646
};
647647

648648
// Date formatting
649-
const TIME_TOKENS = {
650-
normal: {
651-
'12h': 'MMM d h:mm:ss a',
652-
'24h': 'MMM d HH:mm:ss',
653-
},
654-
short: {
655-
'12h': 'MMM d h:mma',
656-
'24h': 'MMM d HH:mm',
657-
},
658-
withMs: {
659-
'12h': 'MMM d h:mm:ss.SSS a',
660-
'24h': 'MMM d HH:mm:ss.SSS',
661-
},
662-
time: {
663-
'12h': 'h:mm:ss a',
664-
'24h': 'HH:mm:ss',
665-
},
666-
};
667-
668-
export const formatDate = (
669-
date: Date,
670-
{
671-
isUTC = false,
672-
format = 'normal',
673-
clock = '12h',
674-
}: {
675-
isUTC?: boolean;
676-
format?: 'normal' | 'short' | 'withMs' | 'time';
677-
clock?: '12h' | '24h';
678-
},
679-
) => {
680-
const formatStr = TIME_TOKENS[format][clock];
681-
682-
return isUTC
683-
? formatInTimeZone(date, 'Etc/UTC', formatStr)
684-
: fnsFormat(date, formatStr);
685-
};
686-
687649
export const mergePath = (path: string[]) => {
688650
const [key, ...rest] = path;
689651
if (rest.length === 0) {

0 commit comments

Comments
 (0)