Fix excessive reconfigure

README.md

@@ -29,11 +29,11 @@ In your Gemfile:
 gem 'librato-sidekiq'
 ```
 
-In `config/environments/librato_sidekiq.rb`:
+In `config/initializers/librato_sidekiq.rb`:
 
 ```ruby
 # only needed for fine-tuning, gem will enable all metrics by default
-Librato::Sidekiq::Middleware.configure do |c|
+Librato::Sidekiq.configure do |c|
   # only enable for production
   c.enabled = Rails.env.production?
 

lib/librato-sidekiq.rb

@@ -1,5 +1,6 @@
+require 'librato-sidekiq/configuration'
+require 'librato-sidekiq/sidekiq'
 require 'librato-sidekiq/middleware'
 require 'librato-sidekiq/client_middleware'
 
-Librato::Sidekiq::Middleware.configure
-Librato::Sidekiq::ClientMiddleware.configure
+Librato::Sidekiq.configure

lib/librato-sidekiq/client_middleware.rb

@@ -1,21 +1,13 @@
+require 'librato-sidekiq/configuration'
+
 module Librato
   module Sidekiq
     class ClientMiddleware < Middleware
-      def reconfigure
-        # puts "Reconfiguring with: #{options}"
-        ::Sidekiq.configure_client do |config|
-          config.client_middleware do |chain|
-            chain.remove self.class
-            chain.add self.class, options
-          end
-        end
-      end
-
       protected
 
       def track(tracking_group, stats, worker_instance, msg, queue, elapsed)
         tracking_group.increment 'queued'
-        return unless allowed_to_submit queue, worker_instance
+        return unless config.allowed_to_submit queue, worker_instance
         # puts "doing Librato insert"
         tracking_group.group queue.to_s do |q|
           q.increment 'queued'

lib/librato-sidekiq/configuration.rb

@@ -0,0 +1,37 @@
+require 'active_support/core_ext/class/attribute_accessors'
+
+module Librato
+  module Sidekiq
+    class Configuration
+      ARRAY_OPTIONS = [:whitelist_queues, :blacklist_queues, :whitelist_classes, :blacklist_classes]
+      OPTIONS = [:enabled] + ARRAY_OPTIONS
+
+      attr_accessor :enabled, *ARRAY_OPTIONS
+
+      def initialize()
+        self.enabled = true
+        ARRAY_OPTIONS.each {|o| self.send("#{o}=", [])}
+      end
+
+      def queue_in_whitelist(queue)
+        whitelist_queues.nil? || whitelist_queues.empty? || whitelist_queues.include?(queue.to_s)
+      end
+
+      def queue_in_blacklist(queue)
+        blacklist_queues.include?(queue.to_s)
+      end
+
+      def class_in_whitelist(worker_instance)
+        whitelist_classes.nil? || whitelist_classes.empty? || whitelist_classes.include?(worker_instance.class.to_s)
+      end
+
+      def class_in_blacklist(worker_instance)
+        blacklist_classes.include?(worker_instance.class.to_s)
+      end
+
+      def allowed_to_submit(queue, worker_instance)
+        class_in_whitelist(worker_instance) && !class_in_blacklist(worker_instance) && queue_in_whitelist(queue) && !queue_in_blacklist(queue)
+      end
+    end
+  end
+end

lib/librato-sidekiq/middleware.rb

@@ -1,56 +1,12 @@
-require 'active_support/core_ext/class/attribute_accessors'
+require 'librato-sidekiq/configuration'
 
 module Librato
   module Sidekiq
     class Middleware
-      cattr_accessor :enabled do
-        true
-      end
-
-      cattr_accessor :whitelist_queues, :blacklist_queues, :whitelist_classes, :blacklist_classes do
-        []
-      end
+      attr_reader :config
 
       def initialize(options = {})
-        # hard dependency on one or the other being present
-        rails = !!defined?(Librato::Rails)
-        rack = !!defined?(Librato::Rack)
-        fail 'librato-sidekiq depends on having one of librato-rails or librato-rack installed' unless rails || rack
-
-        # librato-rails >= 0.10 changes behavior of reporting agent
-        if File.basename($PROGRAM_NAME) == 'sidekiq' && rails && Librato::Rails::VERSION.split('.')[1].to_i >= 10 && ENV['LIBRATO_AUTORUN'].nil?
-          puts 'NOTICE: --------------------------------------------------------------------'
-          puts 'NOTICE: THE REPORTING AGENT HAS NOT STARTED, AND NO METRICS WILL BE SENT'
-          puts 'NOTICE: librato-rails >= 0.10 requires LIBRATO_AUTORUN=1 in your environment'
-          puts 'NOTICE: --------------------------------------------------------------------'
-        end
-
-        reconfigure
-      end
-
-      def self.configure
-        yield(self) if block_given?
-        new # will call reconfigure
-      end
-
-      def options
-        {
-          enabled: enabled,
-          whitelist_queues: whitelist_queues,
-          blacklist_queues: blacklist_queues,
-          whitelist_classes: whitelist_classes,
-          blacklist_classes: blacklist_classes
-        }
-      end
-
-      def reconfigure
-        # puts "Reconfiguring with: #{options}"
-        ::Sidekiq.configure_server do |config|
-          config.server_middleware do |chain|
-            chain.remove self.class
-            chain.add self.class, options
-          end
-        end
+        @config = options[:config]
       end
 
       # redis_pool is needed for the sidekiq 3 upgrade
@@ -60,7 +16,7 @@ def call(worker_instance, msg, queue, redis_pool = nil)
         result = yield
         elapsed = (Time.now - start_time).to_f
 
-        return result unless enabled
+        return result unless config.enabled
         # puts "#{worker_instance} #{queue}"
 
         stats = ::Sidekiq::Stats.new
@@ -76,7 +32,7 @@ def call(worker_instance, msg, queue, redis_pool = nil)
 
       def track(tracking_group, stats, worker_instance, msg, queue, elapsed)
         submit_general_stats tracking_group, stats
-        return unless allowed_to_submit queue, worker_instance
+        return unless config.allowed_to_submit queue, worker_instance
         # puts "doing Librato insert"
         tracking_group.group queue.to_s do |q|
           q.increment 'processed'
@@ -102,26 +58,6 @@ def submit_general_stats(group, stats)
           group.measure((name || method).to_s, stats.send(method).to_i)
         end
       end
-
-      def queue_in_whitelist(queue)
-        whitelist_queues.nil? || whitelist_queues.empty? || whitelist_queues.include?(queue.to_s)
-      end
-
-      def queue_in_blacklist(queue)
-        blacklist_queues.include?(queue.to_s)
-      end
-
-      def class_in_whitelist(worker_instance)
-        whitelist_classes.nil? || whitelist_classes.empty? || whitelist_classes.include?(worker_instance.class.to_s)
-      end
-
-      def class_in_blacklist(worker_instance)
-        blacklist_classes.include?(worker_instance.class.to_s)
-      end
-
-      def allowed_to_submit(queue, worker_instance)
-        class_in_whitelist(worker_instance) && !class_in_blacklist(worker_instance) && queue_in_whitelist(queue) && !queue_in_blacklist(queue)
-      end
     end
   end
 end

lib/librato-sidekiq/sidekiq.rb

@@ -0,0 +1,62 @@
+module Librato
+  module Sidekiq
+    def self.config
+      @config ||= Librato::Sidekiq::Configuration.new
+    end
+
+    def self.configure
+      yield self.config if block_given?
+      self.register
+    end
+
+    def self.reset
+      @config = nil
+      self.register
+    end
+
+    def self.register
+      self.check_dependencies
+
+      # puts "Reconfiguring with: #{options}"
+      ::Sidekiq.configure_server do |config|
+        config.server_middleware do |chain|
+          chain.remove Librato::Sidekiq::Middleware
+          chain.add Librato::Sidekiq::Middleware, config: self.config
+        end
+      end
+
+      # puts "Reconfiguring with: #{options}"
+      ::Sidekiq.configure_client do |config|
+        config.client_middleware do |chain|
+          chain.remove Librato::Sidekiq::ClientMiddleware
+          chain.add Librato::Sidekiq::ClientMiddleware, config: self.config
+        end
+      end
+    end
+
+    # this is so we can stub it in the specs
+    def self.program_name
+      $PROGRAM_NAME
+    end
+
+    def self.check_librato_rails_version
+      parts = Librato::Rails::VERSION.split('.')
+      parts[0].to_i > 0 || parts[1].to_i >= 10
+    end
+
+    def self.check_dependencies
+      # hard dependency on one or the other being present
+      rails = !!defined?(Librato::Rails)
+      rack = !!defined?(Librato::Rack)
+      fail 'librato-sidekiq depends on having one of librato-rails or librato-rack installed' unless rails || rack
+
+      # librato-rails >= 0.10 changes behavior of reporting agent
+      if rails && File.basename(self.program_name) == 'sidekiq' && self.check_librato_rails_version && ENV['LIBRATO_AUTORUN'].nil?
+        puts 'NOTICE: --------------------------------------------------------------------'
+        puts 'NOTICE: THE REPORTING AGENT HAS NOT STARTED, AND NO METRICS WILL BE SENT'
+        puts 'NOTICE: librato-rails >= 0.10 requires LIBRATO_AUTORUN=1 in your environment'
+        puts 'NOTICE: --------------------------------------------------------------------'
+      end
+    end
+  end
+end

librato-sidekiq.gemspec

@@ -24,7 +24,7 @@ Gem::Specification.new do |s|
   s.add_dependency(%q<sidekiq>, [">= 0"])
   s.add_dependency(%q<activesupport>, [">= 0"])
 
-  s.add_development_dependency(%q<rspec>)
+  s.add_development_dependency(%q<rspec>, [">= 3.4"])
   s.add_development_dependency(%q<timecop>)
   s.add_development_dependency(%q<rake>)
 end

spec/spec_helper.rb

@@ -1,3 +1,5 @@
+require 'librato-sidekiq/configuration'
+require 'librato-sidekiq/sidekiq'
 require 'librato-sidekiq/middleware'
 require 'librato-sidekiq/client_middleware'
 require 'timecop'

spec/unit/client_middleware_spec.rb

@@ -2,51 +2,17 @@
 
 describe Librato::Sidekiq::ClientMiddleware do
 
-  before(:each) do
-    stub_const "Librato::Rails", Class.new
-    stub_const "Sidekiq", Module.new
-    stub_const "Sidekiq::Stats", Class.new
-  end
+  let(:config) { Librato::Sidekiq::Configuration.new }
+  let(:middleware) { described_class.new config: config }
+  let(:sidekiq_stats) { double('Sidekiq::Stats') }
 
-  let(:middleware) do
-    allow(Sidekiq).to receive(:configure_client)
-    Librato::Sidekiq::ClientMiddleware.new
+  before(:each) do
+    stub_const "Sidekiq::Stats", sidekiq_stats
   end
 
   describe '#intialize' do
-    it 'should call reconfigure' do
-      expect(Sidekiq).to receive(:configure_client)
-      Librato::Sidekiq::ClientMiddleware.new
-    end
-  end
-
-  describe '#configure' do
-
-    before(:each) { Sidekiq.should_receive(:configure_client) }
-
-    it 'should yield with it self as argument' do
-      expect { |b| Librato::Sidekiq::ClientMiddleware.configure &b }.to yield_with_args(Librato::Sidekiq::ClientMiddleware)
-    end
-
-    it 'should return a new instance' do
-      expect(Librato::Sidekiq::ClientMiddleware.configure).to be_an_instance_of Librato::Sidekiq::ClientMiddleware
-    end
-
-  end
-
-  describe '#reconfigure' do
-
-    let(:chain) { double() }
-    let(:config) { double() }
-
-    it 'should add itself to the server middleware chain' do
-      expect(chain).to receive(:remove).with Librato::Sidekiq::ClientMiddleware
-      expect(chain).to receive(:add).with Librato::Sidekiq::ClientMiddleware, middleware.options
-
-      expect(config).to receive(:client_middleware).once.and_yield(chain)
-      expect(Sidekiq).to receive(:configure_client).once.and_yield(config)
-
-      middleware.reconfigure
+    it 'should assign the config' do
+      expect(middleware.config).to eq(config)
     end
   end
 
@@ -59,32 +25,32 @@
     let(:some_message) { Hash['class', double(underscore: queue_name)] }
 
     let(:sidekiq_stats_instance_double) do
-      double("Sidekiq::Stats", :enqueued => 1, :failed => 2, :scheduled_size => 3)
+      instance_double("Sidekiq::Stats", :enqueued => 1, :failed => 2, :scheduled_size => 3)
     end
 
     context 'when middleware is not enabled' do
 
-      before(:each) { middleware.enabled = false }
+      before(:each) { config.enabled = false }
 
       it { expect { |b| middleware.call(1,2,3,&b) }.to yield_with_no_args }
 
       it 'should not send any metrics' do
-        Librato.should_not_receive(:group)
+        expect(Librato).to_not receive(:group)
       end
 
     end
 
     context 'when middleware is enabled but queue is blacklisted' do
 
       before(:each) do
-        allow(Sidekiq::Stats).to receive(:new).and_return(sidekiq_stats_instance_double)
+        allow(sidekiq_stats).to receive(:new).and_return(sidekiq_stats_instance_double)
         allow(Librato).to receive(:group).with('sidekiq').and_yield meter
       end
 
       before(:each) do
-        middleware.enabled = true
-        middleware.blacklist_queues = []
-        middleware.blacklist_queues << queue_name
+        config.enabled = true
+        config.blacklist_queues = []
+        config.blacklist_queues << queue_name
       end
 
       it { expect { |b| middleware.call(some_worker_instance, some_message, queue_name, &b) }.to yield_with_no_args }
@@ -103,8 +69,8 @@
       let(:class_group) { double(measure: nil, increment: nil, timing: nil, group: nil) }
 
       before(:each) do
-        middleware.enabled = true
-        middleware.blacklist_queues = []
+        config.enabled = true
+        config.blacklist_queues = []
       end
 
       before(:each) do