Skip to content

Commit 778092d

Browse files
fix: long timeranges for task queries truncated (#1276)
Fixes HDX-2618 Related to ClickHouse/support-escalation#6113 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 2162a69 commit 778092d

File tree

4 files changed

+257
-9
lines changed

4 files changed

+257
-9
lines changed

.changeset/silver-forks-rhyme.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/api": patch
3+
---
4+
5+
fix: set a max size for alert timeranges

packages/api/src/tasks/__tests__/util.test.ts

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
calcAlertDateRange,
23
escapeJsonString,
34
roundDownTo,
45
roundDownToXMinutes,
@@ -335,4 +336,192 @@ describe('util', () => {
335336
expect(escapeJsonString('foo\u0000bar')).toBe('foo\\u0000bar');
336337
});
337338
});
339+
340+
describe('calcAlertDateRange', () => {
341+
const now = Date.now();
342+
const oneMinuteMs = 60 * 1000;
343+
const oneHourMs = 60 * oneMinuteMs;
344+
345+
it('should return unchanged dates when range is within limits', () => {
346+
const startTime = now - 10 * oneMinuteMs; // 10 minutes ago
347+
const endTime = now;
348+
const windowSizeInMins = 5;
349+
350+
const [start, end] = calcAlertDateRange(
351+
startTime,
352+
endTime,
353+
windowSizeInMins,
354+
);
355+
356+
expect(start.getTime()).toBe(startTime);
357+
expect(end.getTime()).toBe(endTime);
358+
});
359+
360+
it('should truncate start time when too many windows (> 50)', () => {
361+
const windowSizeInMins = 1;
362+
const maxWindows = 50;
363+
const tooManyWindowsMs =
364+
(maxWindows + 10) * windowSizeInMins * oneMinuteMs; // 60 minutes
365+
const startTime = now - tooManyWindowsMs;
366+
const endTime = now;
367+
368+
const [start, end] = calcAlertDateRange(
369+
startTime,
370+
endTime,
371+
windowSizeInMins,
372+
);
373+
374+
// Should truncate to exactly 50 windows
375+
const expectedStartTime =
376+
endTime - maxWindows * windowSizeInMins * oneMinuteMs;
377+
expect(start.getTime()).toBe(expectedStartTime);
378+
expect(end.getTime()).toBe(endTime);
379+
});
380+
381+
it('should truncate start time when time range exceeds 6 hours for short windows (< 15 mins)', () => {
382+
const windowSizeInMins = 10;
383+
const maxLookbackTime = 6 * oneHourMs; // 6 hours for windows < 15 minutes
384+
const tooLongRangeMs = maxLookbackTime + oneHourMs; // 7 hours
385+
const startTime = now - tooLongRangeMs;
386+
const endTime = now;
387+
388+
const [start, end] = calcAlertDateRange(
389+
startTime,
390+
endTime,
391+
windowSizeInMins,
392+
);
393+
394+
const expectedStartTime = endTime - maxLookbackTime;
395+
expect(start.getTime()).toBeGreaterThan(startTime);
396+
expect(start.getTime()).toBe(expectedStartTime);
397+
expect(end.getTime()).toBe(endTime);
398+
});
399+
400+
it('should truncate start time when time range exceeds 24 hours for long windows (>= 15 mins)', () => {
401+
const windowSizeInMins = 30;
402+
const maxLookbackTime = 24 * oneHourMs; // 24 hours for windows >= 15 minutes
403+
const tooLongRangeMs = maxLookbackTime + 2 * oneHourMs; // 26 hours
404+
const startTime = now - tooLongRangeMs;
405+
const endTime = now;
406+
407+
const [start, end] = calcAlertDateRange(
408+
startTime,
409+
endTime,
410+
windowSizeInMins,
411+
);
412+
413+
const expectedStartTime = endTime - maxLookbackTime;
414+
expect(start.getTime()).toBe(expectedStartTime);
415+
expect(end.getTime()).toBe(endTime);
416+
});
417+
418+
it('should apply the more restrictive truncation when both limits are exceeded', () => {
419+
const windowSizeInMins = 1;
420+
const maxWindows = 50;
421+
const maxLookbackTime = 6 * oneHourMs; // 6 hours for 1-minute windows
422+
423+
// Create a range that exceeds both limits
424+
const excessiveRangeMs = Math.max(
425+
(maxWindows + 100) * windowSizeInMins * oneMinuteMs, // 150 windows
426+
maxLookbackTime + 2 * oneHourMs, // 8 hours
427+
);
428+
const startTime = now - excessiveRangeMs;
429+
const endTime = now;
430+
431+
const [start, end] = calcAlertDateRange(
432+
startTime,
433+
endTime,
434+
windowSizeInMins,
435+
);
436+
437+
// Should use the more restrictive limit (maxWindows in this case)
438+
const expectedStartTime =
439+
endTime - maxWindows * windowSizeInMins * oneMinuteMs;
440+
expect(start.getTime()).toBe(expectedStartTime);
441+
expect(end.getTime()).toBe(endTime);
442+
});
443+
444+
it('should handle very large window sizes correctly', () => {
445+
const windowSizeInMins = 120; // 2 hours
446+
const maxLookbackTime = 24 * oneHourMs; // 24 hours for large windows
447+
const normalRange = 12 * oneHourMs; // 12 hours - within limit
448+
const startTime = now - normalRange;
449+
const endTime = now;
450+
451+
const [start, end] = calcAlertDateRange(
452+
startTime,
453+
endTime,
454+
windowSizeInMins,
455+
);
456+
457+
// Should remain unchanged since within limits
458+
expect(start.getTime()).toBe(startTime);
459+
expect(end.getTime()).toBe(endTime);
460+
});
461+
462+
it('should handle exactly 50 windows without truncation', () => {
463+
const windowSizeInMins = 5;
464+
const maxWindows = 50;
465+
const exactlyMaxWindowsMs = maxWindows * windowSizeInMins * oneMinuteMs; // 250 minutes
466+
const startTime = now - exactlyMaxWindowsMs;
467+
const endTime = now;
468+
469+
const [start, end] = calcAlertDateRange(
470+
startTime,
471+
endTime,
472+
windowSizeInMins,
473+
);
474+
475+
// Should remain unchanged since exactly at the limit
476+
expect(start.getTime()).toBe(startTime);
477+
expect(end.getTime()).toBe(endTime);
478+
});
479+
480+
it('should handle fractional windows correctly', () => {
481+
const windowSizeInMins = 7;
482+
const partialWindowsMs = 7.5 * windowSizeInMins * oneMinuteMs; // 7.5 windows
483+
const startTime = now - partialWindowsMs;
484+
const endTime = now;
485+
486+
const [start, end] = calcAlertDateRange(
487+
startTime,
488+
endTime,
489+
windowSizeInMins,
490+
);
491+
492+
// Should remain unchanged since well within limits
493+
expect(start.getTime()).toBe(startTime);
494+
expect(end.getTime()).toBe(endTime);
495+
});
496+
497+
it('should handle zero time range', () => {
498+
const startTime = now;
499+
const endTime = now;
500+
const windowSizeInMins = 5;
501+
502+
const [start, end] = calcAlertDateRange(
503+
startTime,
504+
endTime,
505+
windowSizeInMins,
506+
);
507+
508+
expect(start.getTime()).toBe(startTime);
509+
expect(end.getTime()).toBe(endTime);
510+
});
511+
512+
it('should return Date objects', () => {
513+
const startTime = now - 10 * oneMinuteMs;
514+
const endTime = now;
515+
const windowSizeInMins = 5;
516+
517+
const [start, end] = calcAlertDateRange(
518+
startTime,
519+
endTime,
520+
windowSizeInMins,
521+
);
522+
523+
expect(start).toBeInstanceOf(Date);
524+
expect(end).toBeInstanceOf(Date);
525+
});
526+
});
338527
});

packages/api/src/tasks/checkAlerts.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ import {
3636
renderAlertTemplate,
3737
} from '@/tasks/template';
3838
import { CheckAlertsTaskArgs, HdxTask } from '@/tasks/types';
39-
import { roundDownToXMinutes, unflattenObject } from '@/tasks/util';
39+
import {
40+
calcAlertDateRange,
41+
roundDownToXMinutes,
42+
unflattenObject,
43+
} from '@/tasks/util';
4044
import logger from '@/utils/logger';
4145

4246
import { tasksTracer } from './tracer';
@@ -171,18 +175,22 @@ export const processAlert = async (
171175
);
172176
return;
173177
}
174-
const checkStartTime = previous
175-
? previous.createdAt
176-
: fns.subMinutes(nowInMinsRoundDown, windowSizeInMins);
177-
const checkEndTime = nowInMinsRoundDown;
178+
const dateRange = calcAlertDateRange(
179+
(previous
180+
? previous.createdAt
181+
: fns.subMinutes(nowInMinsRoundDown, windowSizeInMins)
182+
).getTime(),
183+
nowInMinsRoundDown.getTime(),
184+
windowSizeInMins,
185+
);
178186

179187
let chartConfig: ChartConfigWithOptDateRange | undefined;
180188
if (details.taskType === AlertTaskType.SAVED_SEARCH) {
181189
const savedSearch = details.savedSearch;
182190
chartConfig = {
183191
connection: connectionId,
184192
displayType: DisplayType.Line,
185-
dateRange: [checkStartTime, checkEndTime],
193+
dateRange,
186194
dateRangeStartInclusive: true,
187195
dateRangeEndInclusive: false,
188196
from: source.from,
@@ -206,7 +214,7 @@ export const processAlert = async (
206214
if (tile.config.displayType === DisplayType.Line) {
207215
chartConfig = {
208216
connection: connectionId,
209-
dateRange: [checkStartTime, checkEndTime],
217+
dateRange,
210218
dateRangeStartInclusive: true,
211219
dateRangeEndInclusive: false,
212220
displayType: tile.config.displayType,
@@ -252,9 +260,10 @@ export const processAlert = async (
252260
logger.info(
253261
{
254262
alertId: alert.id,
263+
chartConfig,
255264
checksData,
256-
checkStartTime,
257-
checkEndTime,
265+
checkStartTime: dateRange[0],
266+
checkEndTime: dateRange[1],
258267
},
259268
`Received alert metric [${alert.source} source]`,
260269
);

packages/api/src/tasks/util.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { set } from 'lodash';
22

3+
import logger from '@/utils/logger';
4+
35
// transfer keys of attributes with dot into nested object
46
// ex: { 'a.b': 'c', 'd.e.f': 'g' } -> { a: { b: 'c' }, d: { e: { f: 'g' } } }
57
export const unflattenObject = (
@@ -38,3 +40,46 @@ export const roundDownToXMinutes = (x: number) => roundDownTo(1000 * 60 * x);
3840
export const escapeJsonString = (str: string) => {
3941
return JSON.stringify(str).slice(1, -1);
4042
};
43+
44+
const MAX_NUM_WINDOWS = 50;
45+
const maxLookbackTime = (windowSizeInMins: number) =>
46+
3600_000 * (windowSizeInMins < 15 ? 6 : 24);
47+
export function calcAlertDateRange(
48+
_startTime: number,
49+
_endTime: number,
50+
windowSizeInMins: number,
51+
): [Date, Date] {
52+
let startTime = _startTime;
53+
const endTime = _endTime;
54+
const numWindows = (endTime - startTime) / 60_000 / windowSizeInMins;
55+
// Truncate if too many windows are present
56+
if (numWindows > MAX_NUM_WINDOWS) {
57+
startTime = endTime - MAX_NUM_WINDOWS * 1000 * 60 * windowSizeInMins;
58+
logger.info(
59+
{
60+
requestedStartTime: _startTime,
61+
startTime,
62+
endTime,
63+
windowSizeInMins,
64+
numWindows,
65+
},
66+
'startTime truncated due to too many windows',
67+
);
68+
}
69+
// Truncate if time range is over threshold
70+
const MAX_LOOKBACK_TIME = maxLookbackTime(windowSizeInMins);
71+
if (endTime - startTime > MAX_LOOKBACK_TIME) {
72+
startTime = endTime - MAX_LOOKBACK_TIME;
73+
logger.info(
74+
{
75+
requestedStartTime: _startTime,
76+
startTime,
77+
endTime,
78+
windowSizeInMins,
79+
numWindows,
80+
},
81+
'startTime truncated due to long lookback time',
82+
);
83+
}
84+
return [new Date(startTime), new Date(endTime)];
85+
}

0 commit comments

Comments
 (0)