search

nytimes / collectd-rabbitmq

A collected plugin, written in python, to collect statistics from RabbitMQ.
https://collectd-rabbitmq.readthedocs.org/
Other
145 stars 79 forks source link

skip data collection on / vhost to address issue #32 #62

Open tnibert opened 7 years ago

tnibert commented 7 years ago

This may not be a good candidate to merge, but it does resolve issue #32 401 returned when using plugin - even though standard Basic Auth curls work using creds, so hopefully at least contributes to a better and less hacky solution.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.693% when pulling 02769fc19bd426a1d48ba669cbff675e79166b5c on tnibert:master into b6e3e59c997513447e90d00041fbbf9eca33c3d8 on NYTimes:master.

mrunge commented 7 years ago

The change itself looks ok for me, even if it reduces coverage.

supermitch commented 6 years ago

This pull request means you can never get stats on "/" vhost. My understanding is that vhost prefix (when configuring RabbitMQ) is optional, so what if I want to use the default? This code would never allow me to get stats on "/"?

tnibert commented 6 years ago

Yes I think that is probably true. I just ran into the issue #32 in my own application and this is the quick hack that I used for it to work with my application. I submitted this pull request mostly just as a pointer like "look here, clue to the root cause."