Various fixes to Sidekiq classes (#193)

Various fixes to Sidekiq classes (#193)

  • Use proper class in SidekiqProcess test

  • Remove duplicated test

  • Cosmetic changes to Sidekiq tests

  • Fix unused variable in SidekiqProcessCollector

diff --git a/lib/prometheus_exporter/server/sidekiq_process_collector.rb b/lib/prometheus_exporter/server/sidekiq_process_collector.rb
index c3f563b..0180df9 100644
--- a/lib/prometheus_exporter/server/sidekiq_process_collector.rb
+++ b/lib/prometheus_exporter/server/sidekiq_process_collector.rb
@@ -26,7 +26,7 @@ module PrometheusExporter::Server
         SIDEKIQ_PROCESS_GAUGES.map do |name, help|
           if (value = metric[name])
             gauge = gauges[name] ||= PrometheusExporter::Metric::Gauge.new("sidekiq_process_#{name}", help)
-            gauges[name].observe(value, labels)
+            gauge.observe(value, labels)
           end
         end
       end
diff --git a/test/server/collector_test.rb b/test/server/collector_test.rb
index 531e436..1baef4c 100644
--- a/test/server/collector_test.rb
+++ b/test/server/collector_test.rb
@@ -325,36 +325,20 @@ class PrometheusCollectorTest < Minitest::Test
     assert(result.include?('sidekiq_jobs_total{job_name="Sidekiq::Extensions::DelayedClass",queue="default",service="service1"} 1'), "has sidekiq delayed class")
   end
 
-  def test_it_can_collect_sidekiq_queue_metrics
+  def test_it_can_collect_sidekiq_queue_metrics_for_all_queues
     collector = PrometheusExporter::Server::Collector.new
     client = PipedClient.new(collector, custom_labels: { service: 'service1' })
     instrument = PrometheusExporter::Instrumentation::SidekiqQueue.new(all_queues: true)
 
     mocks_for_sidekiq_que_all = 2.times.map do |i|
       mock = Minitest::Mock.new
-      mock.expect(
-        :name,
-        "que_#{i}",
-      )
-      mock.expect(
-        :size,
-        10 + i,
-      )
-      mock.expect(
-        :latency,
-        1.to_f + i,
-      )
-      mock.expect(
-        :name,
-        "que_#{i}",
-      )
+      mock.expect(:name, "que_#{i}")
+      mock.expect(:size, 10 + i)
+      mock.expect(:latency, 1.to_f + i)
     end
 
     mock_sidekiq_que = Minitest::Mock.new
-    mock_sidekiq_que.expect(
-      :all,
-      mocks_for_sidekiq_que_all,
-    )
+    mock_sidekiq_que.expect(:all, mocks_for_sidekiq_que_all)
 
     Object.stub_const(:Sidekiq, Module) do
       ::Sidekiq.stub_const(:Queue, mock_sidekiq_que) do
@@ -370,7 +354,7 @@ class PrometheusCollectorTest < Minitest::Test
     assert(result.include?('sidekiq_queue_backlog{queue="que_1",service="service1"} 11'), "has number of backlog")
     assert(result.include?('sidekiq_queue_latency_seconds{queue="que_0",service="service1"} 1'), "has latency")
     assert(result.include?('sidekiq_queue_latency_seconds{queue="que_1",service="service1"} 2'), "has latency")
-    mocks_for_sidekiq_que_all.each { |m| m.verify }
+    mocks_for_sidekiq_que_all.each(&:verify)
     mock_sidekiq_que.verify
   end
 
@@ -381,29 +365,14 @@ class PrometheusCollectorTest < Minitest::Test
 
     mocks_for_sidekiq_que_all = 2.times.map do |i|
       mock = Minitest::Mock.new
-      mock.expect(
-        :name,
-        "que_#{i}",
-      )
-      mock.expect(
-        :size,
-        10 + i,
-      )
-      mock.expect(
-        :latency,
-        1.to_f + i,
-      )
-      mock.expect(
-        :name,
-        "que_#{i}",
-      )
+      mock.expect(:name, "que_#{i}")
+      mock.expect(:size, 10 + i)
+      mock.expect(:latency, 1.to_f + i)
+      mock.expect(:name, "que_#{i}")
     end
 
     mock_sidekiq_que = Minitest::Mock.new
-    mock_sidekiq_que.expect(
-      :all,
-      mocks_for_sidekiq_que_all,
-    )
+    mock_sidekiq_que.expect(:all, mocks_for_sidekiq_que_all)
 
     Object.stub_const(:Sidekiq, Module) do
       ::Sidekiq.stub_const(:Queue, mock_sidekiq_que) do
@@ -419,56 +388,7 @@ class PrometheusCollectorTest < Minitest::Test
     refute(result.include?('sidekiq_queue_backlog{queue="que_1",service="service1"} 11'), "has number of backlog")
     assert(result.include?('sidekiq_queue_latency_seconds{queue="que_0",service="service1"} 1'), "has latency")
     refute(result.include?('sidekiq_queue_latency_seconds{queue="que_1",service="service1"} 2'), "has latency")
-    mocks_for_sidekiq_que_all.each { |m| m.verify }
-    mock_sidekiq_que.verify
-  end
-
-  def test_it_can_collect_sidekiq_queue_metrics
-    collector = PrometheusExporter::Server::Collector.new
-    client = PipedClient.new(collector, custom_labels: { service: 'service1' })
-    instrument = PrometheusExporter::Instrumentation::SidekiqQueue.new
-
-    mocks_for_sidekiq_que_all = 2.times.map do |i|
-      mock = Minitest::Mock.new
-      mock.expect(
-        :name,
-        "que_#{i}",
-      )
-      mock.expect(
-        :size,
-        10 + i,
-      )
-      mock.expect(
-        :latency,
-        1.to_f + i,
-      )
-      mock.expect(
-        :name,
-        "que_#{i}",
-      )
-    end
-
-    mock_sidekiq_que = Minitest::Mock.new
-    mock_sidekiq_que.expect(
-      :all,
-      mocks_for_sidekiq_que_all,
-    )
-
-    Object.stub_const(:Sidekiq, Module) do
-      ::Sidekiq.stub_const(:Queue, mock_sidekiq_que) do
-        instrument.stub(:collect_current_process_queues, ["que_0", "que_1"]) do
-          metric = instrument.collect
-          client.send_json metric
-        end
-      end
-    end
-
-    result = collector.prometheus_metrics_text
-    assert(result.include?('sidekiq_queue_backlog{queue="que_0",service="service1"} 10'), "has number of backlog")
-    assert(result.include?('sidekiq_queue_backlog{queue="que_1",service="service1"} 11'), "has number of backlog")
-    assert(result.include?('sidekiq_queue_latency_seconds{queue="que_0",service="service1"} 1'), "has latency")
-    assert(result.include?('sidekiq_queue_latency_seconds{queue="que_1",service="service1"} 2'), "has latency")
-    mocks_for_sidekiq_que_all.each { |m| m.verify }
+    mocks_for_sidekiq_que_all.each(&:verify)
     mock_sidekiq_que.verify
   end
 
diff --git a/test/server/sidekiq_process_collector_test.rb b/test/server/sidekiq_process_collector_test.rb
index b31fa66..a4623dd 100644
--- a/test/server/sidekiq_process_collector_test.rb
+++ b/test/server/sidekiq_process_collector_test.rb
@@ -55,7 +55,7 @@ class PrometheusSidekiqProcessCollectorTest < Minitest::Test
       )
     end
 
-    Process.stub(:clock_gettime, 2.0 + PrometheusExporter::Server::SidekiqQueueCollector::MAX_SIDEKIQ_METRIC_AGE) do
+    Process.stub(:clock_gettime, 2.0 + PrometheusExporter::Server::SidekiqProcessCollector::MAX_SIDEKIQ_METRIC_AGE) do
       collector.collect(
         'process' => {
           'busy' => 2,

GitHub sha: 58593dfb79d6db7638538158f16d0defc9b4655b

This commit appears in #193 which was merged by SamSaffron.