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

vhost leading '/' is not properly escaped #40

Closed bufadu closed 8 years ago

bufadu commented 8 years ago

Hi, I'm using collectd-rabbitmq with collectd 5.4.1 and I have a very strange behavior with vhost naming : '/' is url encoded like :

rabbitmq_%2F
rabbitmq_%2Ffoo

instead of

rabbitmq_default
rabbitmq_slash_foo

In rabbit.py, vhost name is url encoded

        for item in items:
            name = item.get('name', None)
            if name:
                name = urllib.quote(name, '')
                names.append(name)
        return names

Unless I missed something, there is no urllib.unquote() neither before nor in generate_vhost_name().

Thanks !

jimbydamonk commented 8 years ago

What version of collectd-rabbitmq are you using ? What are you vhost name are you expecting ?

I'm poking at it now.

bufadu commented 8 years ago

I'm using the collectd-rabbitmq version 1.7.1. My rabbitMQ contains two vhosts : / and /foo . My understanding is that they should be renamed as rabbitmq_default and rabbitmq_slash_foo when passed to collectd. Am I right ? Instead of that, both name appears url encoded as rabbitmq_%2F and rabbitmq_%2Ffoo in collectd output. For instance :

/var/lib/collectd/rrd/rabbitmq_%2F/queues-hello/get.rrd
redterror commented 8 years ago

This is also the behavior on current master, e.g. rabbitmq_%2F.

jimbydamonk commented 8 years ago

Thanks @bufadu and @redterror I just put a fix in.