search

discourse / prometheus_exporter

A framework for collecting and aggregating prometheus metrics
MIT License
525 stars 153 forks source link

Add hostname to puma instrumentation #172

Open codez opened 3 years ago

codez commented 3 years ago

Other than e.g. the process instrumentation, the puma metrics do not include a hostname. In our setup, multiple puma containers report towards the same prometheus exporter. Without the hostname, they overwrite each others values.

There is also no option to add custom labels to the puma instrumentation either. This would have allowed us to add a hostname label on our own.

I am willing to work on a pull request for this. Which option do you prefer, always adding a hostname label, add custom labels or both?

SamSaffron commented 3 years ago

I think it makes sense to add unconditionally

On Thu, 17 Jun 2021 at 7:37 pm, z @.***> wrote:

Other than e.g. the process instrumentation, the puma metrics do not include a hostname. In our setup, multiple puma containers report towards the same prometheus exporter. Without the hostname, they overwrite each others values.

There is also no option to add custom labels to the puma instrumentation either. This would have allowed us to add a hostname label on our own.

I am willing to work on a pull request for this. Which option do you prefer, always adding a hostname label, add custom labels or both?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/discourse/prometheus_exporter/issues/172, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABIXK6M74UNLJHNLCEEQTTTG67FANCNFSM463I3XXQ .

codez commented 3 years ago

Oops, I'm sorry I don't quite understand your answer as a non-native English speaker. Do you think it's best to add both an option for custom labels and the hostname unconditionally?

By the way, I saw that while other instrumentations add the hostname, it is not propagated by the server but only used to distinguish different values. This should not be needed for the puma metrics IMHO.

SamSaffron commented 3 years ago

Sorry, I was way too short there. :pensive:

I think it is fine to add hostname label for Puma unconditionally in a PR

I am also not against adding support for custom label support in puma in a different PR.

codez commented 3 years ago

That's ok, I will try to start work on it in the next couple of days.

bgmat commented 2 years ago

Hi there, Any plans to release this? I really need it. Thanks