Skip to content

Commit

Permalink
[bugfix] Silenced should be false for the alerts API
Browse files Browse the repository at this point in the history
* Force dry-run alerts to be silent to prevent false alerts
* Use the unmute_monitor API call to unmute silenced alerts. The
  update_alerts API call will not unmute even when setting silenced to
  false or null.
* Fix for timeout calculation
* Support aliases in the filesystem group_sources. This will enable
  groups to have alises during renaming.
* Require nokogiri to < 1.7.0 for Ruby 1.9.3
* Bump to 0.0.18
  • Loading branch information
Jimmy Ngo authored and Jimmy Ngo committed Jan 3, 2017
1 parent e56c90f commit 8ed8cdb
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 18 deletions.
1 change: 1 addition & 0 deletions interferon.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Gem::Specification.new do |gem|
gem.add_runtime_dependency "dogstatsd-ruby", "~> 1.4", ">= 1.4.1"
gem.add_runtime_dependency "diffy", "~> 3.1.0", ">= 3.1.0"
gem.add_runtime_dependency "parallel", "~> 1.9", ">= 1.9.0"
gem.add_runtime_dependency "nokogiri", "< 1.7.0"

gem.add_development_dependency "rspec", "~> 3.2"
gem.add_development_dependency "pry", "~> 0.10"
Expand Down
23 changes: 12 additions & 11 deletions lib/interferon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,11 @@ def do_dry_run_update(dest, hosts, alerts, existing_alerts, groups)
end
end

alerts_queue = build_alerts_queue(hosts, alerts, groups)
updates_queue = alerts_queue.reject do |name, alert_people_pair|
!Interferon::need_update(dest, alert_people_pair, existing_alerts)
end

# Add dry-run prefix to alerts and delete id to avoid impacting real alerts
existing_alerts.keys.each do |name|
existing_alert = existing_alerts[name]
Expand All @@ -200,16 +205,12 @@ def do_dry_run_update(dest, hosts, alerts, existing_alerts, groups)
existing_alerts[dry_run_alert_name] = existing_alerts.delete(name)
end

# Build new queue with dry-run prefixes
alerts_queue = build_alerts_queue(hosts, alerts, groups)
# Build new queue with dry-run prefixes and ensure they are silenced
alerts_queue.each do |name, alert_people_pair|
alert = alert_people_pair[0]
dry_run_alert_name = DRY_RUN_ALERTS_NAME_PREFIX + alert['name']
alert.change_name(dry_run_alert_name)
end

updates_queue = alerts_queue.reject do |name, alert_people_pair|
!Interferon::need_update(dest, alert_people_pair, existing_alerts)
alert.silence
end

# Create alerts in destination
Expand All @@ -221,7 +222,7 @@ def do_dry_run_update(dest, hosts, alerts, existing_alerts, groups)
alert = alert_people_pair[0]
old_alerts = to_remove[alert['name']]

if not old_alerts.nil?
if !old_alerts.nil?
if old_alerts['id'].length == 1
to_remove.delete(alert['name'])
else
Expand Down Expand Up @@ -261,8 +262,8 @@ def do_regular_update(dest, hosts, alerts, existing_alerts, groups)
alert = alert_people_pair[0]
old_alerts = to_remove[alert['name']]

if not old_alerts.nil?
if alert['id'].length == 1
if !old_alerts.nil?
if old_alerts['id'].length == 1
to_remove.delete(alert['name'])
else
old_alerts['id'] = old_alerts['id'].drop(1)
Expand Down Expand Up @@ -354,7 +355,7 @@ def build_alerts_queue(hosts, alerts, groups)
end

# did the alert fail to evaluate on all hosts?
if counters[:errors] == counters[:hosts] and !last_eval_error.nil?
if counters[:errors] == counters[:hosts] && !last_eval_error.nil?
log.error "alert #{alert} failed to evaluate in the context of all hosts!"
log.error "last error on alert #{alert}: #{last_eval_error}"

Expand Down Expand Up @@ -407,7 +408,7 @@ def self.same_alerts(dest, alert_people_pair, alert_api_json)
:message => dest.generate_message(alert['message'], people).strip,
:notify_no_data => alert['notify_no_data'],
:silenced => alert['silenced'] || alert['silenced_until'] > Time.now,
:timeout => alert['timeout'] || nil && [1, alert['timeout'].to_i / 3600].max,
:timeout => alert['timeout'] ? [1, alert['timeout'].to_i / 3600].max : nil,
:no_data_timeframe => alert['no_data_timeframe'] || nil
}

Expand Down
8 changes: 8 additions & 0 deletions lib/interferon/alert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ def change_name(name)
@dsl.name(name)
end

def silence
unless @dsl
raise "This alert has not yet been evaluated"
end

@dsl.silenced(true)
end

def [](attr)
unless @dsl
raise "This alert has not yet been evaluated"
Expand Down
14 changes: 10 additions & 4 deletions lib/interferon/destinations/datadog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,14 @@ def create_alert(alert, people)
alert_opts = {
:name => alert['name'],
:message => message,
:silenced => false,
:notify_no_data => alert['notify_no_data'],
:timeout_h => nil,
}

# Set alert to be silenced if there is a silenced set or silenced_until set
if alert['silenced'] or alert['silenced_until'] > Time.now
alert_opts[:silenced] = {"*" => nil}
if alert['silenced'] || alert['silenced_until'] > Time.now
alert_opts[:silenced] = true
end

# allow an optional timeframe for "no data" alerts to be specified
Expand Down Expand Up @@ -160,6 +161,11 @@ def create_alert(alert, people)
alert['metric']['datadog_query'].strip,
alert_opts
)
# Unmute existing alerts that have been unsilenced.
# Datadog does not allow updates to silencing via the update_alert API call.
if existing_alert['silenced'] && !alert_opts[:silenced]
@dog.unmute_monitor(id)
end
end
end

Expand All @@ -185,7 +191,7 @@ def remove_alert(alert)
@stats[:alerts_to_be_deleted] += 1
log.info("deleting alert: #{alert['name']}")

if not @dry_run
if !@dry_run
alert['id'].each do |alert_id|
resp = @dog.delete_alert(alert_id)
code = resp[0].to_i
Expand Down Expand Up @@ -227,7 +233,7 @@ def remove_alert_by_id(alert_id)

def log_datadog_response_code(resp, code, action, alert=nil)
# log whenever we've encountered errors
if code != 200 and !alert.nil?
if code != 200 && !alert.nil?
api_errors << "#{code.to_s} on alert #{alert['name']}"
end

Expand Down
19 changes: 17 additions & 2 deletions lib/interferon/group_sources/filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def initialize(options)

def list_groups
groups = {}
aliases = {}

@paths.each do |path|
path = File.expand_path(path)
Expand All @@ -19,7 +20,7 @@ def list_groups
next
end

Dir.glob(File.join(path, '*.{json,yml,yaml}')) do |group_file|
Dir.glob(File.join(path, '*.{json,yml,yaml}')).each do |group_file|
begin
group = YAML::parse(File.read(group_file))
rescue YAML::SyntaxError => e
Expand All @@ -28,11 +29,25 @@ def list_groups
log.warn "error reading group file #{group_file}: #{e}"
else
group = group.to_ruby
groups[group['name']] = group['people'] || []
if group['people']
groups[group['name']] = group['people'] || []
elsif group['alias_for']
aliases[group['name']] = {:group => group['alias_for'], :group_file => group_file}
end
end
end
end

aliases.each do |aliased_group, group_info|
group = group_info[:group]
group_file = group_info[:group_file]
if groups.include?(group)
groups[aliased_group] = groups[group]
else
log.warn "Alias not found for #{group} but used by #{aliased_group} in #{group_file}"
end
end

return groups
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/interferon/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Interferon
VERSION = "0.0.16"
VERSION = "0.0.18"
end
57 changes: 57 additions & 0 deletions spec/lib/interferon/group_sources/filesystem_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
require 'spec_helper'
require 'interferon/group_sources/filesystem'

describe Interferon::GroupSources::Filesystem do
let (:fs_loader) { Interferon::GroupSources::Filesystem.new({'paths' => ['/tmp']}) }

describe 'list_groups' do
context "with basic groups" do
before do
group_a = double()
expect(File).to receive(:read).with('group_a.yaml').and_return('group_a_text')
expect(Psych).to receive(:parse).and_return(group_a)
expect(group_a).to receive(:to_ruby).and_return({'name' => 'group_a',
'people' => ['Alice', 'Bob']})

group_b = double()
expect(File).to receive(:read).with('group_b.yaml').and_return('group_b_text')
expect(Psych).to receive(:parse).and_return(group_b)
expect(group_b).to receive(:to_ruby).and_return({'name' => 'group_b',
'people' => ['Carol', 'Dave']})
end

it 'loads groups defined by YAML' do
expect(Dir).to receive(:glob).and_return(['group_a.yaml', 'group_b.yaml'].each)

groups = fs_loader.list_groups()
expect(groups).to eq({'group_a' => ['Alice', 'Bob'], 'group_b' => ['Carol', 'Dave']})
end

it 'allows groups to be aliased in YAML' do
expect(Dir).to receive(:glob).and_return(['group_a.yaml', 'group_b.yaml', 'group_c.yaml'].each)
group_c = double()
expect(File).to receive(:read).with('group_c.yaml').and_return('group_c_text')
expect(Psych).to receive(:parse).and_return(group_c)
expect(group_c).to receive(:to_ruby).and_return({'name' => 'group_c', 'alias_for' => 'group_b'})

groups = fs_loader.list_groups()
expect(groups).to eq({'group_a' => ['Alice', 'Bob'],
'group_b' => ['Carol', 'Dave'],
'group_c' => ['Carol', 'Dave']})
end

it 'skips bad aliases in YAML' do
expect(Dir).to receive(:glob).and_return(['group_a.yaml', 'group_b.yaml', 'group_c.yaml'].each)
group_c = double()
expect(File).to receive(:read).with('group_c.yaml').and_return('group_c_text')
expect(Psych).to receive(:parse).and_return(group_c)
expect(group_c).to receive(:to_ruby).and_return({'name' => 'group_c', 'alias_for' => 'group_d'})

groups = fs_loader.list_groups()
expect(groups).to eq({'group_a' => ['Alice', 'Bob'],
'group_b' => ['Carol', 'Dave']})
end
end
end

end

0 comments on commit 8ed8cdb

Please sign in to comment.