-
Notifications
You must be signed in to change notification settings - Fork 18
Redesign monitoring internals #30
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
module Monitoring | ||
class CheckResult | ||
attr_reader :resque_name, # Name of the resque the check checked | ||
:scope, # Refined scope inside this resque, if relavent (e.g. queue name or class name) | ||
:check_name, # Name of the thing checked | ||
:check_count # Count related to what was being checked | ||
|
||
def initialize(resque_name: nil,check_name: nil,check_count: nil, scope: nil) | ||
@resque_name = required! resque_name, "resque_name" | ||
@check_name = required! check_name, "check_name" | ||
@check_count = required! check_count, "check_count", :to_i | ||
@scope = optional scope | ||
end | ||
|
||
private | ||
|
||
def required!(value, name, conversion = :to_s) | ||
raise ArgumentError, "#{name} is required" if value.nil? | ||
value.send(conversion) | ||
end | ||
|
||
def optional(value) | ||
if value.nil? | ||
nil | ||
else | ||
value.to_s | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
module Monitoring | ||
class LibratoNotifier < Notifier | ||
def initialize(prefix: nil, logger: Rails.logger, type: :count, unit: "") | ||
@prefix = validate_prefix!(prefix) | ||
def initialize(logger: Rails.logger, type: :count, unit: "") | ||
@logger = logger | ||
@type = type | ||
@unit = unit || "" | ||
|
@@ -11,9 +10,10 @@ def initialize(prefix: nil, logger: Rails.logger, type: :count, unit: "") | |
# | ||
# results:: a hash where the keys represent the source (the resque instance name) and the values | ||
# are lists of items to be counted. The items won't be examined, just counted and used in the metric | ||
def notify!(results) | ||
results.each do |resque_name,items| | ||
log_to_librato(resque_name,@type,@prefix,items.size) | ||
def notify!(check_results) | ||
check_results.each do |check_result| | ||
source = [check_result.resque_name,check_result.scope].compact.join(".") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how the |
||
log_to_librato(source,@type,check_result.check_name,check_result.check_count) | ||
end | ||
end | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
module Monitoring | ||
class StaleWorkerCheck < Monitoring::Checker | ||
def check! | ||
Hash[@resques.all.map { |resque_instance| | ||
[resque_instance.name,resque_instance.jobs_running.select(&:too_long?)] | ||
}] | ||
@resques.all.map { |resque_instance| | ||
CheckResult.new(resque_name: resque_instance.name, | ||
check_name: "resque.stale_workers", | ||
check_count: resque_instance.jobs_running.select(&:too_long?).size) | ||
} | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,15 +3,15 @@ namespace :monitor do | |
task :failed => :environment do | ||
monitor = Monitoring::Monitor.new( | ||
checker: Monitoring::FailedJobCheck.new, | ||
notifier: Monitoring::LibratoNotifier.new(prefix: "resque.failed_jobs", unit: "jobs")) | ||
notifier: Monitoring::LibratoNotifier.new(unit: "jobs")) | ||
monitor.monitor! | ||
end | ||
|
||
desc "Check the number of stale workers and stat the results to the log in a way Librato can understand" | ||
task :stale_workers => :environment do | ||
monitor = Monitoring::Monitor.new( | ||
checker: Monitoring::StaleWorkerCheck.new, | ||
notifier: Monitoring::LibratoNotifier.new(prefix: "resque.stale_workers", type: :measure, unit: "workers")) | ||
notifier: Monitoring::LibratoNotifier.new(type: :measure, unit: "workers")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
monitor.monitor! | ||
end | ||
|
@@ -20,7 +20,7 @@ namespace :monitor do | |
task :queue_sizes => :environment do | ||
monitor = Monitoring::Monitor.new( | ||
checker: Monitoring::QueueSizeCheck.new, | ||
notifier: Monitoring::PerQueueLibratoNotifier.new(prefix: "resque.queue_size", type: :count, unit: "jobs")) | ||
notifier: Monitoring::LibratoNotifier.new(unit: "jobs")) | ||
|
||
monitor.monitor! | ||
end | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now captures the contract between checker and notifier. Before it was basically whatever and so it resulted in tight coupling between some checkers and notifiers.