search

StreamMachine / prometheus_client_nodejs

Prometheus metrics client for Node.js
Apache License 2.0
30 stars 16 forks source link

Client does not pick up counters in sub modules #4

Open mansona opened 9 years ago

mansona commented 9 years ago

I'm not 100% sure if this is my fault because I'm working with a linked library locally but the documentation say:

By default, the Prometheus client will use a global namespace. That means that any metrics registered inside your app (even by libraries) will show up in your client without any need to pass around a client object.

Can you go into more detail on how this is supposed to work and what part of the implementation is supposed to do this?

This is currently not working for me, I am only getting counters that exposed in the "parent" project i.e. no sub modules

ewr commented 9 years ago

I think that should fix it for you. I was creating the global registry incorrectly.

mansona commented 9 years ago

Eh... well that was fast! :boom: I'll check it out now and see how it fares :+1:

mansona commented 9 years ago

Ok so i've tried this now and it still doesn't work (I don't know if I am able to to reopen this issue?)

I think this might be a case of a Documentation issue again or if not then I can help you achieve what you are trying to do here from a NodeJS perspective (I hope).

Essentially I am interpreting your documentation line (even by libraries) to mean that: even in node libraries (i.e. node_modules) that this application includes and exist in the node_modules folder.

From your attempted fix I am able to see how you are trying to achieve this and It won't work (to achieve what I think you are trying to achieve) unless you're doing something using the process keyword or the global keyword. I.e. check if process.common_prometheus_registry is set and if it is use that.

Now that might not be the best idea because it will cause all manor of issues if the versions differ between sub modules, and I think the better way is for each of the submodules that need to expose counters should have prometheus-client as a peerDependancy. I can't do that because I've hit on an edge case that makes this not work for npm 2 (see this and this )

In the mean time i'm just going to do some magic about passing the Prometheus object around :+1:

ewr commented 9 years ago

Interesting. You're right that I forgot to account for the fact that included libraries would ship in their own prometheus-client lib, which would create its own client class, etc.

I think you're on the right path with the common registry idea. Versioning is a concern, but one that hopefully you could address with semver and a strict sense of what the data model is (so that helper functions could change while still honoring the data model and compatibility).

I've reopened this in the meantime.