diff --git a/.changeset/few-mails-check.md b/.changeset/few-mails-check.md new file mode 100644 index 00000000..068b998c --- /dev/null +++ b/.changeset/few-mails-check.md @@ -0,0 +1,7 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/api": patch +"@hyperdx/app": patch +--- + +fix: alerting time range filtering bug diff --git a/.changeset/twelve-dolls-yell.md b/.changeset/twelve-dolls-yell.md new file mode 100644 index 00000000..5d248adf --- /dev/null +++ b/.changeset/twelve-dolls-yell.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/common-utils": patch +--- + +feat: support 'dateRangeEndInclusive' in timeFilterExpr diff --git a/packages/api/src/tasks/__tests__/checkAlerts.test.ts b/packages/api/src/tasks/__tests__/checkAlerts.test.ts index b0556dc7..4292c529 100644 --- a/packages/api/src/tasks/__tests__/checkAlerts.test.ts +++ b/packages/api/src/tasks/__tests__/checkAlerts.test.ts @@ -420,6 +420,7 @@ describe('checkAlerts', () => { '**', 'Group: "http"', '10 lines found, expected less than 1 lines', + 'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)', 'Custom body ', '```', '', @@ -481,6 +482,7 @@ describe('checkAlerts', () => { '**', 'Group: "http"', '10 lines found, expected less than 1 lines', + 'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)', 'Custom body ', '```', '', @@ -585,6 +587,7 @@ describe('checkAlerts', () => { '**', 'Group: "http"', '10 lines found, expected less than 1 lines', + 'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)', '', ' Runbook URL: https://example.com', ' hi i matched', @@ -613,6 +616,7 @@ describe('checkAlerts', () => { '**', 'Group: "http"', '10 lines found, expected less than 1 lines', + 'Time Range (UTC): [Mar 17 10:13:03 PM - Mar 17 10:13:59 PM)', '', ' Runbook URL: https://example.com', ' hi i matched', @@ -657,25 +661,33 @@ describe('checkAlerts', () => { const team = await createTeam({ name: 'My Team' }); const now = new Date('2023-11-16T22:12:00.000Z'); - // Send events in the last alert window 22:05 - 22:10 - const eventMs = now.getTime() - ms('5m'); + const eventMs = new Date('2023-11-16T22:05:00.000Z'); + const eventNextMs = new Date('2023-11-16T22:10:00.000Z'); await bulkInsertLogs([ + // logs from 22:05 - 22:10 { ServiceName: 'api', - Timestamp: new Date(eventMs), + Timestamp: eventMs, SeverityText: 'error', Body: 'Oh no! Something went wrong!', }, { ServiceName: 'api', - Timestamp: new Date(eventMs), + Timestamp: eventMs, SeverityText: 'error', Body: 'Oh no! Something went wrong!', }, { ServiceName: 'api', - Timestamp: new Date(eventMs), + Timestamp: eventMs, + SeverityText: 'error', + Body: 'Oh no! Something went wrong!', + }, + // logs from 22:10 - 22:15 + { + ServiceName: 'api', + Timestamp: eventNextMs, SeverityText: 'error', Body: 'Oh no! Something went wrong!', }, @@ -745,6 +757,11 @@ describe('checkAlerts', () => { const nextWindow = new Date('2023-11-16T22:16:00.000Z'); await processAlert(nextWindow, enhancedAlert); // alert should be in ok state + expect(enhancedAlert.state).toBe('ALERT'); + + const nextNextWindow = new Date('2023-11-16T22:20:00.000Z'); + await processAlert(nextNextWindow, enhancedAlert); + // alert should be in ok state expect(enhancedAlert.state).toBe('OK'); // check alert history @@ -753,19 +770,25 @@ describe('checkAlerts', () => { }).sort({ createdAt: 1, }); - expect(alertHistories.length).toBe(2); + expect(alertHistories.length).toBe(3); 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].state).toBe('ALERT'); + expect(alertHistories[1].counts).toBe(1); expect(alertHistories[1].createdAt).toEqual( new Date('2023-11-16T22:15:00.000Z'), ); + expect(alertHistories[2].state).toBe('OK'); + expect(alertHistories[2].counts).toBe(0); + expect(alertHistories[2].createdAt).toEqual( + new Date('2023-11-16T22:20:00.000Z'), + ); // check if webhook was triggered + // We're only checking the general structure here since the exact text includes timestamps expect(slack.postMessageToWebhook).toHaveBeenNthCalledWith( 1, 'https://hooks.slack.com/services/123', @@ -779,6 +802,19 @@ describe('checkAlerts', () => { ], }, ); + expect(slack.postMessageToWebhook).toHaveBeenNthCalledWith( + 2, + 'https://hooks.slack.com/services/123', + { + text: 'Alert for "My Search" - 1 lines found', + blocks: [ + { + text: expect.any(Object), + type: 'section', + }, + ], + }, + ); }); it('TILE alert (events) - slack webhook', async () => { @@ -931,6 +967,7 @@ describe('checkAlerts', () => { `**`, '', '3 exceeds 1', + 'Time Range (UTC): [Nov 16 10:05:00 PM - Nov 16 10:10:00 PM)', '', ].join('\n'), type: 'mrkdwn', @@ -1248,6 +1285,7 @@ describe('checkAlerts', () => { `**`, '', '6.25 exceeds 1', + 'Time Range (UTC): [Nov 16 10:05:00 PM - Nov 16 10:10:00 PM)', '', ].join('\n'), type: 'mrkdwn', diff --git a/packages/api/src/tasks/checkAlerts.ts b/packages/api/src/tasks/checkAlerts.ts index 67b50d46..315f7d97 100644 --- a/packages/api/src/tasks/checkAlerts.ts +++ b/packages/api/src/tasks/checkAlerts.ts @@ -8,6 +8,7 @@ import { ChartConfigWithOptDateRange, DisplayType, } from '@hyperdx/common-utils/dist/types'; +import { formatDate } from '@hyperdx/common-utils/dist/utils'; import * as fns from 'date-fns'; import Handlebars, { HelperOptions } from 'handlebars'; import _ from 'lodash'; @@ -477,6 +478,11 @@ export const renderAlertTemplate = async ({ ); }; + const timeRangeMessage = `Time Range (UTC): [${formatDate(view.startTime, { + isUTC: true, + })} - ${formatDate(view.endTime, { + isUTC: true, + })})`; let rawTemplateBody; // TODO: support advanced routing with template engine @@ -538,7 +544,7 @@ ${value} lines found, expected ${ alert.thresholdType === AlertThresholdType.ABOVE ? 'less than' : 'greater than' - } ${alert.threshold} lines + } ${alert.threshold} lines\n${timeRangeMessage} ${targetTemplate} \`\`\` ${truncatedResults} @@ -556,7 +562,7 @@ ${value} ${ : alert.thresholdType === AlertThresholdType.ABOVE ? 'falls below' : 'exceeds' - } ${alert.threshold} + } ${alert.threshold}\n${timeRangeMessage} ${targetTemplate}`; } @@ -716,6 +722,8 @@ export const processAlert = async (now: Date, alert: EnhancedAlert) => { connection: connectionId, displayType: DisplayType.Line, dateRange: [checkStartTime, checkEndTime], + dateRangeStartInclusive: true, + dateRangeEndInclusive: false, from: source.from, granularity: `${windowSizeInMins} minute`, select: [ @@ -772,6 +780,8 @@ export const processAlert = async (now: Date, alert: EnhancedAlert) => { chartConfig = { connection: connectionId, dateRange: [checkStartTime, checkEndTime], + dateRangeStartInclusive: true, + dateRangeEndInclusive: false, displayType: firstTile.config.displayType, from: source.from, granularity: `${windowSizeInMins} minute`, diff --git a/packages/app/src/__tests__/utils.test.ts b/packages/app/src/__tests__/utils.test.ts index 5af3ad79..059415a8 100644 --- a/packages/app/src/__tests__/utils.test.ts +++ b/packages/app/src/__tests__/utils.test.ts @@ -5,57 +5,12 @@ import { MetricsDataType, NumberFormat } from '../types'; import * as utils from '../utils'; import { formatAttributeClause, - formatDate, formatNumber, getMetricTableName, stripTrailingSlash, useQueryHistory, } from '../utils'; -describe('utils', () => { - it('12h utc', () => { - const date = new Date('2021-01-01T12:00:00Z'); - expect( - formatDate(date, { - clock: '12h', - isUTC: true, - }), - ).toEqual('Jan 1 12:00:00 PM'); - }); - - it('24h utc', () => { - const date = new Date('2021-01-01T12:00:00Z'); - expect( - formatDate(date, { - clock: '24h', - isUTC: true, - format: 'withMs', - }), - ).toEqual('Jan 1 12:00:00.000'); - }); - - it('12h local', () => { - const date = new Date('2021-01-01T12:00:00'); - expect( - formatDate(date, { - clock: '12h', - isUTC: false, - }), - ).toEqual('Jan 1 12:00:00 PM'); - }); - - it('24h local', () => { - const date = new Date('2021-01-01T12:00:00'); - expect( - formatDate(date, { - clock: '24h', - isUTC: false, - format: 'withMs', - }), - ).toEqual('Jan 1 12:00:00.000'); - }); -}); - describe('formatAttributeClause', () => { it('should format SQL attribute clause correctly', () => { expect( diff --git a/packages/app/src/timeQuery.ts b/packages/app/src/timeQuery.ts index 31be40b2..a903838a 100644 --- a/packages/app/src/timeQuery.ts +++ b/packages/app/src/timeQuery.ts @@ -26,6 +26,7 @@ import { withDefault, } from 'use-query-params'; import { DateRange } from '@hyperdx/common-utils/dist/types'; +import { formatDate } from '@hyperdx/common-utils/dist/utils'; import { parseTimeRangeInput } from './components/TimePicker/utils'; import { useUserPreferences } from './useUserPreferences'; @@ -34,18 +35,16 @@ import { usePrevious } from './utils'; const LIVE_TAIL_TIME_QUERY = 'Live Tail'; const LIVE_TAIL_REFRESH_INTERVAL_MS = 1000; -const formatDate = ( - date: Date, - isUTC: boolean, - strFormat = 'MMM d HH:mm:ss', -) => { - return isUTC - ? formatInTimeZone(date, 'Etc/UTC', strFormat) - : format(date, strFormat); -}; - export const dateRangeToString = (range: [Date, Date], isUTC: boolean) => { - return `${formatDate(range[0], isUTC)} - ${formatDate(range[1], isUTC)}`; + return `${formatDate(range[0], { + isUTC, + format: 'normal', + clock: '24h', + })} - ${formatDate(range[1], { + isUTC, + format: 'normal', + clock: '24h', + })}`; }; function isInputTimeQueryLive(inputTimeQuery: string) { diff --git a/packages/app/src/useFormatTime.tsx b/packages/app/src/useFormatTime.tsx index 8d884e33..46949b8c 100644 --- a/packages/app/src/useFormatTime.tsx +++ b/packages/app/src/useFormatTime.tsx @@ -1,7 +1,7 @@ import React from 'react'; +import { formatDate } from '@hyperdx/common-utils/dist/utils'; import { useUserPreferences } from './useUserPreferences'; -import { formatDate } from './utils'; type DateLike = number | string | Date; diff --git a/packages/app/src/utils.ts b/packages/app/src/utils.ts index b5fd5f76..6f12bbab 100644 --- a/packages/app/src/utils.ts +++ b/packages/app/src/utils.ts @@ -646,44 +646,6 @@ export const legacyMetricNameToNameAndDataType = (metricName?: string) => { }; // Date formatting -const TIME_TOKENS = { - normal: { - '12h': 'MMM d h:mm:ss a', - '24h': 'MMM d HH:mm:ss', - }, - short: { - '12h': 'MMM d h:mma', - '24h': 'MMM d HH:mm', - }, - withMs: { - '12h': 'MMM d h:mm:ss.SSS a', - '24h': 'MMM d HH:mm:ss.SSS', - }, - time: { - '12h': 'h:mm:ss a', - '24h': 'HH:mm:ss', - }, -}; - -export const formatDate = ( - date: Date, - { - isUTC = false, - format = 'normal', - clock = '12h', - }: { - isUTC?: boolean; - format?: 'normal' | 'short' | 'withMs' | 'time'; - clock?: '12h' | '24h'; - }, -) => { - const formatStr = TIME_TOKENS[format][clock]; - - return isUTC - ? formatInTimeZone(date, 'Etc/UTC', formatStr) - : fnsFormat(date, formatStr); -}; - export const mergePath = (path: string[]) => { const [key, ...rest] = path; if (rest.length === 0) { diff --git a/packages/common-utils/src/__tests__/utils.test.ts b/packages/common-utils/src/__tests__/utils.test.ts index b8ea1c02..d09ccc67 100644 --- a/packages/common-utils/src/__tests__/utils.test.ts +++ b/packages/common-utils/src/__tests__/utils.test.ts @@ -1,6 +1,50 @@ -import { splitAndTrimCSV, splitAndTrimWithBracket } from '../utils'; +import { formatDate, splitAndTrimCSV, splitAndTrimWithBracket } from '../utils'; describe('utils', () => { + describe('formatDate', () => { + it('12h utc', () => { + const date = new Date('2021-01-01T12:00:00Z'); + expect( + formatDate(date, { + clock: '12h', + isUTC: true, + }), + ).toEqual('Jan 1 12:00:00 PM'); + }); + + it('24h utc', () => { + const date = new Date('2021-01-01T12:00:00Z'); + expect( + formatDate(date, { + clock: '24h', + isUTC: true, + format: 'withMs', + }), + ).toEqual('Jan 1 12:00:00.000'); + }); + + it('12h local', () => { + const date = new Date('2021-01-01T12:00:00'); + expect( + formatDate(date, { + clock: '12h', + isUTC: false, + }), + ).toEqual('Jan 1 12:00:00 PM'); + }); + + it('24h local', () => { + const date = new Date('2021-01-01T12:00:00'); + expect( + formatDate(date, { + clock: '24h', + isUTC: false, + format: 'withMs', + }), + ).toEqual('Jan 1 12:00:00.000'); + }); + }); + describe('splitAndTrimCSV', () => { it('should split a comma-separated string and trim whitespace', () => { expect(splitAndTrimCSV('a, b, c')).toEqual(['a', 'b', 'c']); diff --git a/packages/common-utils/src/renderChartConfig.ts b/packages/common-utils/src/renderChartConfig.ts index dd6a7f57..d835d2b6 100644 --- a/packages/common-utils/src/renderChartConfig.ts +++ b/packages/common-utils/src/renderChartConfig.ts @@ -449,25 +449,27 @@ function timeBucketExpr({ } async function timeFilterExpr({ - timestampValueExpression, - dateRange, - dateRangeStartInclusive, - databaseName, - tableName, - metadata, connectionId, - with: withClauses, + databaseName, + dateRange, + dateRangeEndInclusive, + dateRangeStartInclusive, includedDataInterval, + metadata, + tableName, + timestampValueExpression, + with: withClauses, }: { - timestampValueExpression: string; - dateRange: [Date, Date]; - dateRangeStartInclusive: boolean; - metadata: Metadata; connectionId: string; databaseName: string; - tableName: string; - with?: ChartConfigWithDateRange['with']; + dateRange: [Date, Date]; + dateRangeEndInclusive: boolean; + dateRangeStartInclusive: boolean; includedDataInterval?: string; + metadata: Metadata; + tableName: string; + timestampValueExpression: string; + with?: ChartConfigWithDateRange['with']; }) { const valueExpressions = splitAndTrimWithBracket(timestampValueExpression); const startTime = dateRange[0].getTime(); @@ -507,11 +509,15 @@ async function timeFilterExpr({ if (columnMeta?.type === 'Date') { return chSql`(${unsafeTimestampValueExpression} ${ dateRangeStartInclusive ? '>=' : '>' - } toDate(${startTimeCond}) AND ${unsafeTimestampValueExpression} <= toDate(${endTimeCond}))`; + } toDate(${startTimeCond}) AND ${unsafeTimestampValueExpression} ${ + dateRangeEndInclusive ? '<=' : '<' + } toDate(${endTimeCond}))`; } else { return chSql`(${unsafeTimestampValueExpression} ${ dateRangeStartInclusive ? '>=' : '>' - } ${startTimeCond} AND ${unsafeTimestampValueExpression} <= ${endTimeCond})`; + } ${startTimeCond} AND ${unsafeTimestampValueExpression} ${ + dateRangeEndInclusive ? '<=' : '<' + } ${endTimeCond})`; } }), ); @@ -701,6 +707,7 @@ async function renderWhere( timestampValueExpression: chartConfig.timestampValueExpression, dateRange: chartConfig.dateRange, dateRangeStartInclusive: chartConfig.dateRangeStartInclusive ?? true, + dateRangeEndInclusive: chartConfig.dateRangeEndInclusive ?? true, metadata, connectionId: chartConfig.connection, databaseName: chartConfig.from.databaseName, diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 89e998fe..a75d265a 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -399,6 +399,7 @@ export type ChartConfig = z.infer; export type DateRange = { dateRange: [Date, Date]; dateRangeStartInclusive?: boolean; // default true + dateRangeEndInclusive?: boolean; // default true }; export type ChartConfigWithDateRange = ChartConfig & DateRange; diff --git a/packages/common-utils/src/utils.ts b/packages/common-utils/src/utils.ts index 8e5a513b..a34d29d2 100644 --- a/packages/common-utils/src/utils.ts +++ b/packages/common-utils/src/utils.ts @@ -1,5 +1,6 @@ // Port from ChartUtils + source.ts -import { add } from 'date-fns'; +import { add as fnsAdd, format as fnsFormat } from 'date-fns'; +import { formatInTimeZone } from 'date-fns-tz'; import type { SQLInterval } from '@/types'; @@ -230,7 +231,7 @@ export function timeBucketByGranularity( const granularitySeconds = convertGranularityToSeconds(granularity); while (current < end) { buckets.push(current); - current = add(current, { + current = fnsAdd(current, { seconds: granularitySeconds, }); } @@ -254,3 +255,42 @@ export const parseJSON = (json: string) => { const [error, result] = _useTry(() => JSON.parse(json)); return result; }; + +// Date formatting +const TIME_TOKENS = { + normal: { + '12h': 'MMM d h:mm:ss a', + '24h': 'MMM d HH:mm:ss', + }, + short: { + '12h': 'MMM d h:mma', + '24h': 'MMM d HH:mm', + }, + withMs: { + '12h': 'MMM d h:mm:ss.SSS a', + '24h': 'MMM d HH:mm:ss.SSS', + }, + time: { + '12h': 'h:mm:ss a', + '24h': 'HH:mm:ss', + }, +}; + +export const formatDate = ( + date: Date, + { + isUTC = false, + format = 'normal', + clock = '12h', + }: { + isUTC?: boolean; + format?: 'normal' | 'short' | 'withMs' | 'time'; + clock?: '12h' | '24h'; + }, +) => { + const formatStr = TIME_TOKENS[format][clock]; + + return isUTC + ? formatInTimeZone(date, 'Etc/UTC', formatStr) + : fnsFormat(date, formatStr); +};