Skip to content

Commit 6f5a298

Browse files
authored
fix: replace hrtime with performance not to allocate (#1204)
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
1 parent 0f7a505 commit 6f5a298

6 files changed

Lines changed: 114 additions & 13 deletions

File tree

src/http/plugins/metrics.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,34 @@ import Fastify from 'fastify'
33
import { httpMetrics } from './metrics'
44

55
describe('httpMetrics plugin', () => {
6+
it('uses numeric monotonic timestamps for request duration metrics', async () => {
7+
const app = Fastify()
8+
const httpMetricsSpy = vi.spyOn(monitoringMetrics, 'recordHttpRequestMetrics')
9+
const performanceNowSpy = vi.spyOn(performance, 'now').mockReturnValue(0)
10+
let metricsStartTime: unknown
11+
12+
await app.register(httpMetrics())
13+
app.get('/objects/:bucket', async (request) => {
14+
metricsStartTime = request.metricsStartTime
15+
return 'ok'
16+
})
17+
18+
try {
19+
const response = await app.inject({
20+
method: 'GET',
21+
url: '/objects/bucket-a',
22+
})
23+
24+
expect(response.statusCode).toBe(200)
25+
expect(metricsStartTime).toBe(0)
26+
expect(httpMetricsSpy).toHaveBeenCalledWith(0, undefined, 2, 'GET', 'unknown', 200)
27+
} finally {
28+
performanceNowSpy.mockRestore()
29+
httpMetricsSpy.mockRestore()
30+
await app.close()
31+
}
32+
})
33+
634
it('records HTTP metric attributes without tenant id labels', async () => {
735
const app = Fastify()
836
const httpMetricsSpy = vi.spyOn(monitoringMetrics, 'recordHttpRequestMetrics')

src/http/plugins/metrics.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export const httpMetrics = (options: HttpMetricsOptions = {}) =>
6868
// Hook into request lifecycle to measure duration
6969
fastify.addHook('onRequest', (request, _reply, done) => {
7070
// Store start time on request for later use
71-
request.metricsStartTime = process.hrtime.bigint()
71+
request.metricsStartTime = performance.now()
7272
done()
7373
})
7474

@@ -82,15 +82,13 @@ export const httpMetrics = (options: HttpMetricsOptions = {}) =>
8282
}
8383

8484
const startTime = request.metricsStartTime
85-
if (!startTime) {
85+
if (startTime === undefined) {
8686
done()
8787
return
8888
}
8989

9090
// Calculate duration in seconds
91-
const endTime = process.hrtime.bigint()
92-
const durationNs = endTime - startTime
93-
const durationSeconds = Number(durationNs) / 1e9
91+
const durationSeconds = (performance.now() - startTime) / 1000
9492

9593
const operation =
9694
request.operation?.type || request.routeOptions?.config?.operation?.type || 'unknown'
@@ -120,6 +118,6 @@ export const httpMetrics = (options: HttpMetricsOptions = {}) =>
120118
// Extend FastifyRequest to include metricsStartTime
121119
declare module 'fastify' {
122120
interface FastifyRequest {
123-
metricsStartTime?: bigint
121+
metricsStartTime?: number
124122
}
125123
}

src/internal/queue/event.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { queueJobSchedulingTime } from '@internal/monitoring/metrics'
12
import { vi } from 'vitest'
23

34
type EventModule = typeof import('./event')
@@ -174,4 +175,33 @@ describe('Event payload versioning', () => {
174175
})
175176
)
176177
})
178+
179+
it('records queue scheduling duration from numeric monotonic timestamps', async () => {
180+
const { eventModule, queueModule } = await loadQueueModules({
181+
pgQueueEnable: true,
182+
})
183+
const TestEvent = defineTestEvent(eventModule.Event)
184+
const payload = createPayload()
185+
const send = vi.fn().mockResolvedValue('job-id')
186+
const recordSpy = vi.spyOn(queueJobSchedulingTime, 'record')
187+
const performanceNowSpy = vi
188+
.spyOn(performance, 'now')
189+
.mockReturnValueOnce(20)
190+
.mockReturnValueOnce(26)
191+
192+
vi.spyOn(queueModule.Queue, 'getInstance').mockReturnValue({
193+
send,
194+
} as unknown as ReturnType<typeof queueModule.Queue.getInstance>)
195+
196+
try {
197+
await TestEvent.send(payload)
198+
199+
expect(recordSpy).toHaveBeenCalledWith(0.006, {
200+
name: 'test-event',
201+
})
202+
} finally {
203+
performanceNowSpy.mockRestore()
204+
recordSpy.mockRestore()
205+
}
206+
})
177207
})

src/internal/queue/event.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export class Event<T extends Omit<BasePayload, '$version'>> {
281281
}
282282
}
283283

284-
const startTime = process.hrtime.bigint()
284+
const startTime = performance.now()
285285
const sendOptions = eventClass.getSendOptions(this.payload) || {}
286286

287287
if (this.payload.scheduleAt) {
@@ -360,7 +360,7 @@ export class Event<T extends Omit<BasePayload, '$version'>> {
360360
},
361361
})
362362
} finally {
363-
const duration = Number(process.hrtime.bigint() - startTime) / 1e9
363+
const duration = (performance.now() - startTime) / 1000
364364
queueJobSchedulingTime.record(duration, {
365365
name: eventClass.getQueueName(),
366366
})

src/storage/database/pg.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ class TestStoragePgDB extends StoragePgDB {
1010
return this.runUnscopedQuery('MetricWithoutTenantAttribute', async () => 'ok')
1111
}
1212

13+
runScopedMetricProbe(): Promise<string> {
14+
return this.runQuery('ScopedMetricDuration', async () => 'ok')
15+
}
16+
1317
runErrorMappingProbe(): Promise<unknown> {
1418
return this.runQuery('FindBucketById', (db) => {
1519
return this.query(db, 'SELECT * FROM storage.buckets WHERE id = $1')
@@ -47,18 +51,59 @@ describe('StoragePgDB metrics', () => {
4751
host: 'localhost',
4852
})
4953
const recordSpy = vi.spyOn(dbQueryPerformance, 'record')
54+
const performanceNowSpy = vi
55+
.spyOn(performance, 'now')
56+
.mockReturnValueOnce(10)
57+
.mockReturnValueOnce(15)
5058

5159
try {
5260
await expect(storage.runMetricProbe()).resolves.toBe('ok')
5361

54-
expect(recordSpy).toHaveBeenCalledWith(expect.any(Number), {
62+
expect(recordSpy).toHaveBeenCalledWith(0.005, {
5563
name: 'MetricWithoutTenantAttribute',
5664
requestAborted: false,
5765
requestAbortedBeforeStart: false,
5866
requestAbortedAfterStart: false,
5967
})
6068
expect(recordSpy.mock.calls[0]?.[1]).not.toHaveProperty('tenantId')
6169
} finally {
70+
performanceNowSpy.mockRestore()
71+
recordSpy.mockRestore()
72+
}
73+
})
74+
75+
test('records scoped DB query duration from numeric monotonic timestamps', async () => {
76+
const transaction = {
77+
commit: vi.fn(),
78+
rollback: vi.fn(),
79+
isCompleted: vi.fn().mockReturnValue(false),
80+
}
81+
const connection = {
82+
getAbortSignal: vi.fn().mockReturnValue(undefined),
83+
transaction: vi.fn().mockResolvedValue(transaction),
84+
setScope: vi.fn(),
85+
} as unknown as PgTenantConnection
86+
const storage = new TestStoragePgDB(connection, {
87+
tenantId: 'metric-cardinality-tenant',
88+
host: 'localhost',
89+
})
90+
const recordSpy = vi.spyOn(dbQueryPerformance, 'record')
91+
const performanceNowSpy = vi
92+
.spyOn(performance, 'now')
93+
.mockReturnValueOnce(2)
94+
.mockReturnValueOnce(9)
95+
96+
try {
97+
await expect(storage.runScopedMetricProbe()).resolves.toBe('ok')
98+
99+
expect(recordSpy).toHaveBeenCalledWith(0.007, {
100+
name: 'ScopedMetricDuration',
101+
requestAborted: false,
102+
requestAbortedBeforeStart: false,
103+
requestAbortedAfterStart: false,
104+
})
105+
} finally {
106+
performanceNowSpy.mockRestore()
62107
recordSpy.mockRestore()
63108
}
64109
})

src/storage/database/pg.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,7 +1754,7 @@ export class StoragePgDB implements Database {
17541754
queryName: string,
17551755
fn: (db: PgExecutor, signal?: AbortSignal) => Promise<T>
17561756
): Promise<T> {
1757-
const startTime = process.hrtime.bigint()
1757+
const startTime = performance.now()
17581758
const abortSignal = this.connection.getAbortSignal()
17591759
const recordDuration = this.createDurationRecorder(queryName, startTime, abortSignal)
17601760

@@ -1885,7 +1885,7 @@ export class StoragePgDB implements Database {
18851885
queryName: string,
18861886
fn: (db: PgExecutor, signal?: AbortSignal) => Promise<T>
18871887
): Promise<T> {
1888-
const startTime = process.hrtime.bigint()
1888+
const startTime = performance.now()
18891889
const abortSignal = this.connection.getAbortSignal()
18901890
const recordDuration = this.createDurationRecorder(queryName, startTime, abortSignal)
18911891

@@ -1912,13 +1912,13 @@ export class StoragePgDB implements Database {
19121912

19131913
private createDurationRecorder(
19141914
queryName: string,
1915-
startTime: bigint,
1915+
startTime: number,
19161916
abortSignal?: AbortSignal
19171917
): () => void {
19181918
const requestAbortedBeforeStart = Boolean(abortSignal?.aborted)
19191919

19201920
return () => {
1921-
const duration = Number(process.hrtime.bigint() - startTime) / 1e9
1921+
const duration = (performance.now() - startTime) / 1000
19221922
// This intentionally reads the signal after the query work settles. The
19231923
// attributes describe request abort observation, not proof that PostgreSQL
19241924
// cancelled this specific statement.

0 commit comments

Comments
 (0)