Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues due to direct dependency on prom-client library when used for nodejs cluster #52

Open
pp05 opened this issue Aug 2, 2021 · 2 comments
Labels
dependencies Pull requests that update a dependency file

Comments

@pp05
Copy link

pp05 commented Aug 2, 2021

Hello @lance
First of all thanks for the https://github.com/nodeshift/opossum library which seems to have work pretty well for us so far as a circuit breaker library. 👍
We have also opted to use the https://github.com/nodeshift/opossum-prometheus for monitoring the circuit breakers.
Our nodejs application runs on a cluster mode with a configurable number of worker threads.

  • When we tested we found that opossum-prometheus collects the metrics per worker thread
  • Where as in a cluster one would be probably interested in the aggregated metrics of the cluster
  • Which means doing something like this https://github.com/siimon/prom-client/blob/master/example/cluster.js
    When we brought in the direct dependency of the prom-client library in our application we noticed the data coming back from the prom-client AggregateRegistry::getClusterMetrics() was in coherent and not the aggregated value as expected.
    On further debugging we found that the worker threads were responding multiple times to a single message from the cluster master.

The root cause seems to be the multiple dependency on prom-client brought in because of direct dependency of opossum-prometheus & our own dependency to use the AggregateRegistry.
This seems to be a known problem e.g. if you notice this one slanatech/swagger-stats#114 express-prom-bundle have made a fix to the same effect and made prom-client a peer dependency.

A similar approach here would work as well.
I might give it a shot and open a PR for this as well a bit later when I have some time.

@lance
Copy link
Member

lance commented Aug 2, 2021

@pp05 thanks for that detailed explanation. If you have the time and energy for a PR it would definitely be appreciated!

@lance lance added the dependencies Pull requests that update a dependency file label Aug 2, 2021
@helio-frota helio-frota self-assigned this Aug 6, 2021
@helio-frota helio-frota removed their assignment Jan 19, 2022
@iamelevich
Copy link

You can check my implementation with no direct dependency on prom-client, so you can provide that you need https://github.com/iamelevich/opossum-prometheus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

4 participants