Hot Module Reload problem with prom-client

[cabrinoob]

Hi, I'am using your library and I love it, but I found this little problem (not really a big one, but ...).

I'am using your library in a NestJS project. This framework proposes to use HMR with webpack.
I have a provider class for prometheus-things which have this contructor :

constructor(){
        this.counter = new Counter({
            name: 'fred_total_method_calls',
            help: 'Example of a counter',
            labelNames: ['method','path']
        });

        this.gauge = new Gauge({
            name: 'fred_method_response_time',
            help: 'Example of a gauge',
            labelNames: ['method','path']
        });
    }

When I'am running my project in my develop environment, and when I modify something somewhere in the project I have this error :

[HMR] Updated modules:
[HMR]  - ./src/app.controller.ts
[HMR]  - ./src/app.module.ts
[HMR]  - ./src/main.hmr.ts
[HMR]  - ./src/app.service.ts
[HMR] Update applied.
[Nest] 8624   - 2018-5-31 15:22:03   [ExceptionHandler] A metric with the name fred_total_method_calls has already been registered.

When the Hot Module Reload event is handled, webpack regenerate the changing code but existing metrics are still there.

I have found a workarround :

constructor(){
       register.clear();
        this.counter = new Counter({
            name: 'fred_total_method_calls',
            help: 'Example of a counter',
            labelNames: ['method','path']
        });
...

I call register.clear() in my constructor and it works in HMR mode. But I don't like this solution because this line is only here for my comfort allowing me to use HMR in my development phase. It has no direct buisness intrest.

So, do you have a better workarround for this problem?

Thank you

[jeremija]

I have a similar problem when using express-prom-bundle and running my supertest tests via mocha --watch for the second time:

Error: A metric with the name up has already been registered.
    at Registry.registerMetric (node_modules/prom-client/lib/registry.js:71:10)
    at Gauge.config.registers.forEach.registryInstance (node_modules/prom-client/lib/gauge.js:72:21)
    at Array.forEach (<anonymous>)
    at new Gauge (node_modules/prom-client/lib/gauge.js:71:20)
    at Object.up (node_modules/express-prom-bundle/src/index.js:72:17)

[zbjornson]

I haven't used Webpack's HMR, but it looks like you might want to put that register.clear() bit in module.hot.accept, as shown here: https://webpack.js.org/guides/hot-module-replacement/#gotchas.

On principle I don't think there's a nice way for this to "just work." The registry (a singleton, if using the global reg) and metrics are both meant to be created once; HMR is creating the registry once but the metrics more than once.

[SimenB]

One thing we could have is like a Counter.createOrReuse which takes all the same options as the constructor and returns the old instance if it exists in the registry. Then throw if one with the sane name but different help text or labels is there already.

Or just make that the default in the constructor

[waisbrot]

I have a similar problem: a test framework that bypasses Node's module-caching in some cases, so I have to wrap prom-client with my own module that creates meters idempotently.

Is this something that would be considered if I wrote it as a PR? I was thinking just what @SimenB describes (I'd have called it Counter.create but don't have a strong opinion there).

[dfairaizl]

We just ran into this too, is there a preferred way to ensure counters and other metrics are only created once?

[slim-hmidi]

I don't know if it is the same problem as you but I got this error when I run the tests:

Boot failed Error: A metric with the name nodejs_gc_runs_total has already been registered.
    at Registry.registerMetric (/home/user/project/node_modules/prom-client/lib/registry.js:79:10)
    at Counter.config.registers.forEach.registryInstance (/home/user/project/node_modules/prom-client/lib/counter.js:80:21)
    at Array.forEach (<anonymous>)
    at new Counter (/home/user/project/node_modules/prom-client/lib/counter.js:79:20)
    at module.exports (/home/user/project/node_modules/prometheus-gc-stats/index.js:33:19)
    at module.exports.app (/home/user/project/node_modules/kaliida/index.js:39:24)
    at module.exports.app (/home/user/project/node_modules/rf-init/src/boot/routeKaliida.js:6:3)
    at iterator (/home/user/project/node_modules/rf-init/src/index.js:125:37)
    at pReduce (/home/user/project/node_modules/p-each-series/index.js:4:73)
    at Promise.all.then.value (/home/user/project/node_modules/p-reduce/index.js:16:10)

and the script to run tests in package.json:

{
  scripts: {
      "test": "mocha --recursive --exit"
  }
}

I modify the test script by:

{
  scripts: {
      "test": "NODE_ENV=test mocha --recursive --exit"
  }
}

Then the error was fixed. I hope that it will help you if you faced the same problem with test.

[cadorn]

I call client.register.removeSingleMetric('<name>') before I register metrics which seems to solve the issue.

[adambisek]

This works (you have to call clear on every HMR reload on SSR):

import promBundle from 'express-prom-bundle'
import prom from 'prom-client'
import { RequestHandler } from 'express'

const applyPrometheusMiddleware = (): RequestHandler => {
  prom.register.clear() // to avoid breaking SSR HMR
  return promBundle({ includeMethod: true, includePath: true })
}

export default applyPrometheusMiddleware

in express initialization:

const server = express()
server
  .disable('x-powered-by')
  .use(applyPrometheusMiddleware()) // don't put this middleware into global variable, or you'll break SSR HMR!

[theliuk]

I'm having the same issue with tap test framework.

client.register.removeSingleMetric('')

client.register.clear()

does the job.

[harry-gocity]

FWIW, we found that register.clear() looked like it fixed things locally. But in our production build (Next.js) clear() was clearing all metrics collected by collectDefaultMetrics too. I assume not visible locally due to HMR calling collectDefaultMetrics often enough. Switching to removeSingleMetric() was the solution for us.