Improve riemann-redis client #29
Conversation
Add additional Redis connection parameters URL and socket, in case the user wants to connect to monitor a Redis server by using a Redis URL that can have the following format: * redis://server:port * redis://:password@server:port/db * unix:///path/to/redis.sock
Send the values of the different Redis INFO metrics that are strings in the description field of the report.
Send the values of the persistence last status metrics (rdb_last_bgsave_status and aof_last_bgrewrite_status) in the state field of the report.
|
This is still a work in progress |
|
So far this completes the list of things I was planning to add. It'd be nice that metrics were properly tagged. If there's interest in doing that, please let me know. |
| if %w{ rdb_last_bgsave_status aof_last_bgrewrite_status }.include?(property) | ||
| data[:state] = value | ||
| else | ||
| data[:description] = value |
There was a problem hiding this comment.
In my testing, the description always gets set to nil. Do want to set it to the property name?
There was a problem hiding this comment.
Hey, @gsandie, I just did a test run and I'm the description as it's supposed to. Are you here at Monitorama?
There was a problem hiding this comment.
Yeah I am. In the R workshop but just about to leave it.
There was a problem hiding this comment.
I'm sitting to the left of the registration table, come and see me if you like :)
|
👍 This is neat. |
| @@ -11,21 +11,47 @@ class Riemann::Tools::Redis | |||
| opt :redis_host, "Redis hostname", :default => 'localhost' | |||
| opt :redis_port, "Redis port", :default => 6379 | |||
| opt :redis_password, "Redis password", :default => '' | |||
| opt :redis_url, "Redis URL", :default => '' | |||
There was a problem hiding this comment.
Do you want to set a default value here, maybe redis://localhost:6379 ?
There was a problem hiding this comment.
I thought about it, but that would break backwards compatibility with existing clients that rely on using --redis-host and redis-port: if you check line 24 it says if opts[:redis_url] != '' and host and port are set in the else block.
There was a problem hiding this comment.
What I'm trying to say is: if you use a redis URL, it will use that and no other parameter.
|
Cool, all looks good I think :) ✨ |
|
Nice work guys! BTW feel free to add yourselves to the thank you page whenever you make changes! Fork https://github.com/aphyr/riemann, git checkout gh-pages, open thanks.html, and make a note in "contributors to the next release". :) Thanks for all your hard work. |
|
Thank you, Kyle! |
Currently
riemann-redistool only accepts host, port and (optional) password to connect to a Redis server. However it's better if it would additionally would accept an URL in case the user want to connect using via TCP or via socket. Auth is also possible to be specified from there for TCP connections.Also it currently issues an
INFOand sends all the data converted to float by using#to_f. While this conversion is possible for most of the values returned byINFO, some values are actually strings, which will be converted to0in that case, most notablyaof_last_bgrewrite_statusandrdb_last_bgsave_status, which reportsokorerror.Last but not least, Redis 2.6+ allows to query just for a section of the metrics, so it would be nice if each metric could be tagged properly depending on its section:
server: General information about the Redis serverclients: Client connections sectionmemory: Memory consumption related informationpersistence: RDB and AOF related informationstats: General statisticsreplication: Master/slave replication informationcpu: CPU consumption statisticscommandstats: Redis command statisticscluster: Redis Cluster sectionkeyspace: Database related statisticsIf anybody wants to join me, feel free to leave a comment, criticism or suggestion.