AIM-402 Refine test, address PR comments

README.md

@@ -10,6 +10,7 @@ Reusable plugins for Fastify.
   - [Split IO Plugin](#split-io-plugin)
   - [BugSnag Plugin](#bugsnag-plugin)
   - [Metrics Plugin](#metrics-plugin)
+  - [Bull MQ Metrics Plugin](#bullmq-metrics-plugin)
   - [NewRelic Transaction Manager Plugin](#newrelic-transaction-manager-plugin)
   - [UnhandledException Plugin](#unhandledexception-plugin)
 
@@ -133,7 +134,7 @@ Add the plugin to your Fastify instance by registering it with the following opt
 
 The plugin exposes a `GET /metrics` route in your Fastify app to retrieve Prometheus metrics. If something goes wrong while starting the Prometheus metrics server, an `Error` is thrown. Otherwise, a success message is displayed when the plugin has been loaded.
 
-### BullMQ Netrics Plugin
+### BullMQ Metrics Plugin
 
 Plugin to auto-discover BullMQ queues which regularly collects metrics for them and exposes them via `fastify-metrics` global Prometheus registry. If used together with `metricsPlugin`, it will show these metrics on `GET /metrics` route.
 

lib/plugins/bull-mq-metrics/MetricsCollector.ts

@@ -87,7 +87,9 @@ export class MetricsCollector {
 
   async start() {
     const queueNames = await this.options.queueDiscoverer.discoverQueues()
-    await Promise.all(
+
+    // `void` is used to run the `observeQueue` function without waiting for it to finish
+    void Promise.all(
       queueNames.map((name) => this.observeQueue(name, this.redis, this.metrics, this.options)),
     )
   }

lib/plugins/bull-mq-metrics/queueDiscoverers.ts

@@ -18,6 +18,7 @@ export class RedisBasedQueueDiscoverer implements QueueDiscoverer {
 
     const queues = new Set<string>()
     for await (const chunk of scanStream) {
+      // ESLint doesn't want a `;` here but Prettier does
       // eslint-disable-next-line no-extra-semi
       ;(chunk as string[])
         .map((key) => key.split(':')[1])

lib/plugins/bullMqMetricsPlugin.spec.ts

@@ -1,3 +1,5 @@
+import { setTimeout } from 'node:timers/promises'
+
 import { buildClient, sendGet, UNKNOWN_RESPONSE_SCHEMA } from '@lokalise/backend-http-client'
 import type {
   AbstractBackgroundJobProcessor,
@@ -8,24 +10,35 @@ import fastify from 'fastify'
 import type { Redis } from 'ioredis'
 
 import { TestBackgroundJobProcessor } from '../../test/mocks/TestBackgroundJobProcessor'
-import { DependencyMocks } from '../../test/mocks/dependencyMocks'
+import { TestDepedendencies } from '../../test/mocks/TestDepedendencies'
 
 import { RedisBasedQueueDiscoverer } from './bull-mq-metrics/queueDiscoverers'
+import type { BullMqMetricsPluginOptions } from './bullMqMetricsPlugin';
 import { bullMqMetricsPlugin } from './bullMqMetricsPlugin'
 import { metricsPlugin } from './metricsPlugin'
 
-async function initApp(redis: Redis, errorObjectResolver = (err: unknown) => err) {
+type TestOptions = {
+  enableMetricsPlugin: boolean
+}
+
+const DEFAULT_TEST_OPTIONS = { enableMetricsPlugin: true }
+
+export async function initAppWithBullMqMetrics(pluginOptions: BullMqMetricsPluginOptions, { enableMetricsPlugin }: TestOptions = DEFAULT_TEST_OPTIONS) {
+
   const app = fastify()
-  await app.register(metricsPlugin, {
-    bindAddress: '0.0.0.0',
-    loggerOptions: false,
-    errorObjectResolver,
-  })
+
+  if (enableMetricsPlugin) {
+    await app.register(metricsPlugin, {
+      bindAddress: '0.0.0.0',
+      loggerOptions: false,
+      errorObjectResolver: (err: unknown) => err,
+    })
+  }
 
   await app.register(bullMqMetricsPlugin, {
-    redisClient: redis,
-    queueDiscoverer: new RedisBasedQueueDiscoverer(redis, 'bull'),
+    queueDiscoverer: new RedisBasedQueueDiscoverer(pluginOptions.redisClient, 'bull'),
     collectionIntervalInMs: 100,
+    ...pluginOptions,
   })
 
   await app.ready()
@@ -38,46 +51,60 @@ type JobReturn = {
 
 describe('bullMqMetricsPlugin', () => {
   let app: FastifyInstance
-  let mocks: DependencyMocks
+  let dependencies: TestDepedendencies
   let processor: AbstractBackgroundJobProcessor<BaseJobPayload, JobReturn>
   let redis: Redis
 
   beforeEach(async () => {
-    mocks = new DependencyMocks()
-    redis = mocks.startRedis()
+    dependencies = new TestDepedendencies()
+    redis = dependencies.startRedis()
     await redis?.flushall('SYNC')
 
-    mocks.create()
-
     processor = new TestBackgroundJobProcessor<BaseJobPayload, JobReturn>(
-      mocks.create(),
+      dependencies.createMocksForBackgroundJobProcessor(),
       { result: 'done' },
       'test_job',
     )
     await processor.start()
-
-    app = await initApp(redis)
   })
 
   afterEach(async () => {
-    await app.close()
-    await mocks.dispose()
+    if (app) {
+      await app.close()
+    }
+    await dependencies.dispose()
     await processor.dispose()
   })
 
   it('adds BullMQ metrics to Prometheus metrics endpoint', async () => {
+    app = await initAppWithBullMqMetrics({
+      redisClient: redis,
+    })
+
     await processor.schedule({
       metadata: {
         correlationId: 'test',
       },
     })
 
+    await setTimeout(100)
+
     const response = await sendGet(buildClient('http://127.0.0.1:9080'), '/metrics', {
       requestLabel: 'test',
       responseSchema: UNKNOWN_RESPONSE_SCHEMA,
     })
 
     expect(response.result.statusCode).toBe(200)
-    expect(response.result.body).toContain('bullmq_jobs_completed{queue="test_job"}')
+    expect(response.result.body).toContain('bullmq_jobs_completed{queue="test_job"} 1')
+  })
+
+  it('throws if fastify-metrics was not initialized', async () => {
+    await expect(() => {
+      return initAppWithBullMqMetrics({
+        redisClient: redis,
+      }, {
+        enableMetricsPlugin: false
+      })
+    }).rejects.toThrowError('No Prometheus Client found, BullMQ metrics plugin requires `fastify-metrics` plugin to be registered')
   })
 })

lib/plugins/bullMqMetricsPlugin.ts

@@ -16,42 +16,32 @@ export type BullMqMetricsPluginOptions = {
   histogramBuckets?: number[]
 }
 
-function plugin(
+async function plugin(
   fastify: FastifyInstance,
   pluginOptions: BullMqMetricsPluginOptions,
-  next: (err?: Error) => void,
 ) {
-  try {
-    const promClient = fastify.metrics.client
-    if (!promClient) {
-      return next(
-        new Error(
-          'No Prometheus Client found, BullMQ metrics plugin requires `fastify-metrics` plugin to be registered',
-        ),
-      )
-    }
+  if (!fastify.metrics) {
+    throw new Error('No Prometheus Client found, BullMQ metrics plugin requires `fastify-metrics` plugin to be registered')
+  }
 
+  try {
     const collector = new MetricsCollector(
       pluginOptions.redisClient,
-      promClient.register,
+      fastify.metrics.client.register,
       fastify.log,
       pluginOptions,
     )
 
-    void collector.start()
-
     fastify.addHook('onClose', () => {
       collector.stop()
     })
+
+    await collector.start()
   } catch (err: unknown) {
-    return next(
-      err instanceof Error
+    throw err instanceof Error
         ? err
-        : new Error('Unknown error in bull-mq-metrics-plugin', { cause: err }),
-    )
+        : new Error('Unknown error in bull-mq-metrics-plugin', { cause: err })
   }
-
-  next()
 }
 
 export const bullMqMetricsPlugin = fp<BullMqMetricsPluginOptions>(plugin, {

test/mocks/dependencyMocks.ts → test/mocks/TestDepedendencies.ts

@@ -8,11 +8,11 @@ const testLogger = globalLogger
 export let lastInfoSpy: MockInstance
 export let lastErrorSpy: MockInstance
 
-export class DependencyMocks {
+export class TestDepedendencies {
   private client?: Redis
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  create(): BackgroundJobProcessorDependencies<any, any> {
+  createMocksForBackgroundJobProcessor(): BackgroundJobProcessorDependencies<any, any> {
     const originalChildFn = testLogger.child.bind(testLogger)
 
     const originalMethodSpy = vitest.spyOn(testLogger, 'child')
@@ -52,7 +52,6 @@ export class DependencyMocks {
     const commandTimeout = process.env.REDIS_COMMAND_TIMEOUT
       ? Number.parseInt(process.env.REDIS_COMMAND_TIMEOUT, 10)
       : undefined
-    // const keyPrefix = process.env.REDIS_KEY_PREFIX
     this.client = new Redis({
       host,
       db,