From 08b5729fc9351508f9620700557c230546094e16 Mon Sep 17 00:00:00 2001 From: Pavel Zotikov Date: Tue, 14 Apr 2026 19:16:02 +0300 Subject: [PATCH 1/3] fix(metrics): round Redis TimeSeries timestamps to bucket boundaries --- workers/grouper/src/index.ts | 51 +++++++++++++---------------- workers/grouper/src/redisHelper.ts | 6 ++-- workers/grouper/tests/index.test.ts | 9 +++-- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/workers/grouper/src/index.ts b/workers/grouper/src/index.ts index f016c850..45156f8c 100644 --- a/workers/grouper/src/index.ts +++ b/workers/grouper/src/index.ts @@ -325,6 +325,20 @@ export default class GrouperWorker extends Worker { return `ts:project-${metricType}:${projectId}:${granularity}`; } + /** + * Returns the current time truncated to the start of the given granularity + * bucket in milliseconds. All events within the same bucket share one + * timestamp so ON_DUPLICATE SUM accumulates them into a single sample. + */ + private bucketTimestampMs(granularity: 'minutely' | 'hourly' | 'daily'): number { + const now = Date.now(); + switch (granularity) { + case 'hourly': return now - (now % TimeMs.HOUR); + case 'daily': return now - (now % TimeMs.DAY); + default: return now - (now % TimeMs.MINUTE); // minutely + } + } + /** * Record project metrics to Redis TimeSeries. * @@ -343,37 +357,18 @@ export default class GrouperWorker extends Worker { }; const series = [ - { - key: minutelyKey, - label: 'minutely', - retentionMs: TimeMs.DAY, - }, - { - key: hourlyKey, - label: 'hourly', - retentionMs: TimeMs.WEEK, - }, - { - key: dailyKey, - label: 'daily', - // eslint-disable-next-line @typescript-eslint/no-magic-numbers - retentionMs: 90 * TimeMs.DAY, - }, + { key: minutelyKey, label: 'minutely', retentionMs: TimeMs.DAY, timestampMs: this.bucketTimestampMs('minutely') }, + { key: hourlyKey, label: 'hourly', retentionMs: TimeMs.WEEK, timestampMs: this.bucketTimestampMs('hourly') }, + { key: dailyKey, label: 'daily', retentionMs: 90 * TimeMs.DAY, timestampMs: this.bucketTimestampMs('daily') }, ]; - const operations = series.map(({ key, label, retentionMs }) => ({ - label, - promise: this.redis.safeTsAdd(key, 1, labels, retentionMs), - })); - - const results = await Promise.allSettled(operations.map((op) => op.promise)); - - results.forEach((result, index) => { - if (result.status === 'rejected') { - const { label } = operations[index]; - this.logger.error(`Failed to add ${label} TS for ${metricType}`, result.reason); + for (const { key, label, retentionMs, timestampMs } of series) { + try { + await this.redis.safeTsAdd(key, 1, labels, retentionMs, timestampMs); + } catch (error) { + this.logger.error(`Failed to add ${label} TS for ${metricType}`, error); } - }); + } } /** diff --git a/workers/grouper/src/redisHelper.ts b/workers/grouper/src/redisHelper.ts index 57a35543..242b74f8 100644 --- a/workers/grouper/src/redisHelper.ts +++ b/workers/grouper/src/redisHelper.ts @@ -237,14 +237,16 @@ export default class RedisHelper { * @param value - value to add * @param labels - labels to attach to the time series * @param retentionMs - optional retention in milliseconds + * @param timestampMs - timestamp in milliseconds; defaults to current time */ public async safeTsAdd( key: string, value: number, labels: Record, - retentionMs = 0 + retentionMs = 0, + timestampMs = 0 ): Promise { - const timestamp = Date.now(); + const timestamp = timestampMs === 0 ? Date.now() : timestampMs; /** * Create key if not exists — then call increment diff --git a/workers/grouper/tests/index.test.ts b/workers/grouper/tests/index.test.ts index 6ad01812..c75351a9 100644 --- a/workers/grouper/tests/index.test.ts +++ b/workers/grouper/tests/index.test.ts @@ -763,21 +763,24 @@ describe('GrouperWorker', () => { `ts:project-events-accepted:${projectIdMock}:minutely`, 1, expectedLabels, - TimeMs.DAY + TimeMs.DAY, + expect.any(Number), ); expect(safeTsAddSpy).toHaveBeenNthCalledWith( 2, `ts:project-events-accepted:${projectIdMock}:hourly`, 1, expectedLabels, - TimeMs.WEEK + TimeMs.WEEK, + expect.any(Number), ); expect(safeTsAddSpy).toHaveBeenNthCalledWith( 3, `ts:project-events-accepted:${projectIdMock}:daily`, 1, expectedLabels, - 90 * TimeMs.DAY + 90 * TimeMs.DAY, + expect.any(Number), ); } finally { safeTsAddSpy.mockRestore(); From 5db739f5bbf8d49cab4700b1094dbb9ec733d1a2 Mon Sep 17 00:00:00 2001 From: Pavel Zotikov Date: Tue, 14 Apr 2026 19:24:59 +0300 Subject: [PATCH 2/3] refactor(metrics): extract bucketTimestampMs to utils and add unit tests --- workers/grouper/src/index.ts | 21 ++---- workers/grouper/src/utils/bucketTimestamp.ts | 17 +++++ workers/grouper/tests/bucketTimestamp.test.ts | 65 +++++++++++++++++++ 3 files changed, 86 insertions(+), 17 deletions(-) create mode 100644 workers/grouper/src/utils/bucketTimestamp.ts create mode 100644 workers/grouper/tests/bucketTimestamp.test.ts diff --git a/workers/grouper/src/index.ts b/workers/grouper/src/index.ts index 45156f8c..60e1933f 100644 --- a/workers/grouper/src/index.ts +++ b/workers/grouper/src/index.ts @@ -23,6 +23,7 @@ import TimeMs from '../../../lib/utils/time'; import DataFilter from './data-filter'; import RedisHelper from './redisHelper'; import { computeDelta } from './utils/repetitionDiff'; +import { bucketTimestampMs } from './utils/bucketTimestamp'; import { rightTrim } from '../../../lib/utils/string'; import { hasValue } from '../../../lib/utils/hasValue'; @@ -325,20 +326,6 @@ export default class GrouperWorker extends Worker { return `ts:project-${metricType}:${projectId}:${granularity}`; } - /** - * Returns the current time truncated to the start of the given granularity - * bucket in milliseconds. All events within the same bucket share one - * timestamp so ON_DUPLICATE SUM accumulates them into a single sample. - */ - private bucketTimestampMs(granularity: 'minutely' | 'hourly' | 'daily'): number { - const now = Date.now(); - switch (granularity) { - case 'hourly': return now - (now % TimeMs.HOUR); - case 'daily': return now - (now % TimeMs.DAY); - default: return now - (now % TimeMs.MINUTE); // minutely - } - } - /** * Record project metrics to Redis TimeSeries. * @@ -357,9 +344,9 @@ export default class GrouperWorker extends Worker { }; const series = [ - { key: minutelyKey, label: 'minutely', retentionMs: TimeMs.DAY, timestampMs: this.bucketTimestampMs('minutely') }, - { key: hourlyKey, label: 'hourly', retentionMs: TimeMs.WEEK, timestampMs: this.bucketTimestampMs('hourly') }, - { key: dailyKey, label: 'daily', retentionMs: 90 * TimeMs.DAY, timestampMs: this.bucketTimestampMs('daily') }, + { key: minutelyKey, label: 'minutely', retentionMs: TimeMs.DAY, timestampMs: bucketTimestampMs('minutely') }, + { key: hourlyKey, label: 'hourly', retentionMs: TimeMs.WEEK, timestampMs: bucketTimestampMs('hourly') }, + { key: dailyKey, label: 'daily', retentionMs: 90 * TimeMs.DAY, timestampMs: bucketTimestampMs('daily') }, ]; for (const { key, label, retentionMs, timestampMs } of series) { diff --git a/workers/grouper/src/utils/bucketTimestamp.ts b/workers/grouper/src/utils/bucketTimestamp.ts new file mode 100644 index 00000000..29dbd8aa --- /dev/null +++ b/workers/grouper/src/utils/bucketTimestamp.ts @@ -0,0 +1,17 @@ +import TimeMs from '../../../../lib/utils/time'; + +/** + * Returns the current time truncated to the start of the given granularity + * bucket in milliseconds (UTC). All events within the same bucket share one + * timestamp so ON_DUPLICATE SUM accumulates them into a single sample. + * + * @param granularity - time granularity level + * @param now - current timestamp in ms, defaults to Date.now() + */ +export function bucketTimestampMs(granularity: 'minutely' | 'hourly' | 'daily', now = Date.now()): number { + switch (granularity) { + case 'hourly': return now - (now % TimeMs.HOUR); + case 'daily': return now - (now % TimeMs.DAY); + default: return now - (now % TimeMs.MINUTE); // minutely + } +} diff --git a/workers/grouper/tests/bucketTimestamp.test.ts b/workers/grouper/tests/bucketTimestamp.test.ts new file mode 100644 index 00000000..53ebac45 --- /dev/null +++ b/workers/grouper/tests/bucketTimestamp.test.ts @@ -0,0 +1,65 @@ +import '../../../env-test'; +import { bucketTimestampMs } from '../src/utils/bucketTimestamp'; + +describe('bucketTimestampMs', () => { + /** + * 2026-04-14T15:37:42.500Z + * minute start: 2026-04-14T15:37:00.000Z + * hour start: 2026-04-14T15:00:00.000Z + * day start: 2026-04-14T00:00:00.000Z + */ + const now = new Date('2026-04-14T15:37:42.500Z').getTime(); + + it('truncates to the start of the current minute', () => { + const expected = new Date('2026-04-14T15:37:00.000Z').getTime(); + + expect(bucketTimestampMs('minutely', now)).toBe(expected); + }); + + it('truncates to the start of the current hour', () => { + const expected = new Date('2026-04-14T15:00:00.000Z').getTime(); + + expect(bucketTimestampMs('hourly', now)).toBe(expected); + }); + + it('truncates to the start of the current day (UTC midnight)', () => { + const expected = new Date('2026-04-14T00:00:00.000Z').getTime(); + + expect(bucketTimestampMs('daily', now)).toBe(expected); + }); + + it('returns the same value for two calls within the same minute', () => { + const t1 = new Date('2026-04-14T15:37:00.000Z').getTime(); + const t2 = new Date('2026-04-14T15:37:59.999Z').getTime(); + + expect(bucketTimestampMs('minutely', t1)).toBe(bucketTimestampMs('minutely', t2)); + }); + + it('returns different values for two calls in different minutes', () => { + const t1 = new Date('2026-04-14T15:37:59.999Z').getTime(); + const t2 = new Date('2026-04-14T15:38:00.000Z').getTime(); + + expect(bucketTimestampMs('minutely', t1)).not.toBe(bucketTimestampMs('minutely', t2)); + }); + + it('returns the same value for two calls within the same hour', () => { + const t1 = new Date('2026-04-14T15:00:00.000Z').getTime(); + const t2 = new Date('2026-04-14T15:59:59.999Z').getTime(); + + expect(bucketTimestampMs('hourly', t1)).toBe(bucketTimestampMs('hourly', t2)); + }); + + it('returns the same value for two calls within the same day', () => { + const t1 = new Date('2026-04-14T00:00:00.000Z').getTime(); + const t2 = new Date('2026-04-14T23:59:59.999Z').getTime(); + + expect(bucketTimestampMs('daily', t1)).toBe(bucketTimestampMs('daily', t2)); + }); + + it('returns different values for two calls on different days', () => { + const t1 = new Date('2026-04-14T23:59:59.999Z').getTime(); + const t2 = new Date('2026-04-15T00:00:00.000Z').getTime(); + + expect(bucketTimestampMs('daily', t1)).not.toBe(bucketTimestampMs('daily', t2)); + }); +}); From f895b525d91eae74209d84f14279156d9e04f751 Mon Sep 17 00:00:00 2001 From: Pavel Zotikov Date: Tue, 14 Apr 2026 19:31:18 +0300 Subject: [PATCH 3/3] fix(metrics): replace magic number 90 with named constant --- workers/grouper/src/index.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/workers/grouper/src/index.ts b/workers/grouper/src/index.ts index 60e1933f..ea74084b 100644 --- a/workers/grouper/src/index.ts +++ b/workers/grouper/src/index.ts @@ -49,6 +49,11 @@ const CACHE_CLEANUP_INTERVAL_SECONDS = 30; */ const DB_DUPLICATE_KEY_ERROR = '11000'; +/** + * Retention period for daily Redis TimeSeries metrics in days + */ +const DAILY_METRICS_RETENTION_DAYS = 90; + /** * Maximum length for backtrace code line or title */ @@ -346,7 +351,7 @@ export default class GrouperWorker extends Worker { const series = [ { key: minutelyKey, label: 'minutely', retentionMs: TimeMs.DAY, timestampMs: bucketTimestampMs('minutely') }, { key: hourlyKey, label: 'hourly', retentionMs: TimeMs.WEEK, timestampMs: bucketTimestampMs('hourly') }, - { key: dailyKey, label: 'daily', retentionMs: 90 * TimeMs.DAY, timestampMs: bucketTimestampMs('daily') }, + { key: dailyKey, label: 'daily', retentionMs: DAILY_METRICS_RETENTION_DAYS * TimeMs.DAY, timestampMs: bucketTimestampMs('daily') }, ]; for (const { key, label, retentionMs, timestampMs } of series) {