Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve riemann-redis client #29

Merged
merged 5 commits into from Mar 29, 2013
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 30 additions & 4 deletions bin/riemann-redis
Expand Up @@ -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 => ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to set a default value here, maybe redis://localhost:6379 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm trying to say is: if you use a redis URL, it will use that and no other parameter.

opt :redis_socket, "Redis socket", :default => ''
opt :redis_section, "Redis INFO section", :default => 'default'

STRING_VALUES = %w{ redis_version redis_git_sha1 redis_mode os
multiplexing_api gcc_version run_id used_memory_human
used_memory_peak_human mem_allocator
rdb_last_bgsave_status aof_last_bgrewrite_status role }

def initialize
@redis = ::Redis.new(:host => opts[:redis_host], :port => opts[:redis_port])
options = if opts[:redis_url] != ''
{ :url => opts[:redis_url] }
elsif opts[:redis_socket] != ''
{ :path => opts[:redis_socket] }
else
{ :host => opts[:redis_host], :port => opts[:redis_port] }
end
@redis = ::Redis.new(options)
@redis.auth(opts[:redis_password]) unless opts[:redis_password] == ''
@section = opts[:redis_section]
end

def tick
@redis.info.each do |property, value|
report(
@redis.info(@section).each do |property, value|
data = {
:host => opts[:redis_host],
:service => "redis #{property}",
:metric => value.to_f,
:state => 'ok',
:tags => ['redis']
)
}

if STRING_VALUES.include?(property) || property.match(/^db\d+/)
if %w{ rdb_last_bgsave_status aof_last_bgrewrite_status }.include?(property)
data[:state] = value
else
data[:description] = value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing, the description always gets set to nil. Do want to set it to the property name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, @gsandie, I just did a test run and I'm the description as it's supposed to. Are you here at Monitorama?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I am. In the R workshop but just about to leave it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sitting to the left of the registration table, come and see me if you like :)

end
end

report(data)
end
end

Expand Down