interledgerjs / ilp-connector

Reference implementation of an Interledger connector.
Other
136 stars 53 forks source link

Cannot create multiple connectors in the same process #473

Closed karzak closed 5 years ago

karzak commented 5 years ago

Running ilp-connector at 22.1.3. If you call createApp on multiple connectors in the same process, the prometheus client in the stats middleware throws an error:

Error: A metric with the name ilp_connector_incoming_ilp_packets has already been registered.
    at Registry.registerMetric (/Users/kjd/ILP/ilp-test/node_modules/prom-client/lib/registry.js:79:10)
    at Counter.config.registers.forEach.registryInstance (/Users/kjd/ILP/ilp-test/node_modules/prom-client/lib/counter.js:79:21)
    at Array.forEach (<anonymous>)
    at new Counter (/Users/kjd/ILP/ilp-test/node_modules/prom-client/lib/counter.js:78:20)
    at new AccountCounter (/Users/kjd/ILP/ilp-test/node_modules/ilp-connector/src/services/stats.ts:15:5)
    at new Stats (/Users/kjd/ILP/ilp-test/node_modules/ilp-connector/src/services/stats.ts:34:32)
    at construct (/Users/kjd/ILP/ilp-test/node_modules/reduct/index.ts:55:12)
    at reduct (/Users/kjd/ILP/ilp-test/node_modules/reduct/index.ts:80:7)
    at new MiddlewareManager (/Users/kjd/ILP/ilp-test/node_modules/ilp-connector/src/services/middleware-manager.ts:77:18)
    at construct (/Users/kjd/ILP/ilp-test/node_modules/reduct/index.ts:55:12)

To reproduce: https://gist.github.com/karzak/351882b5cf265893ff46e0cc83bcc825

adrianhopebailie commented 5 years ago

That's an issue with Prometheus client. It has to be a singleton.

@karzak would it make more sense to combine the stats from all processes or disable Prometheus client on all but one process? (i.e. What works best for your use case?)

karzak commented 5 years ago

I don't have strong feelings on the issue, but I would say combined stats.

karzak commented 5 years ago

fixed by https://github.com/interledgerjs/ilp-connector/pull/475