fix worker metrics to use sidekiq redis (#22)

Co-authored-by: [email protected] <[email protected]>

.github/workflows/ci.yml

@@ -15,7 +15,7 @@ jobs:
     strategy:
       matrix:
         ruby-version: [2.6, 2.7, 3.0]
-        redis-version: [7]
+        redis-version: [4, 5, 6, 7]
 
     steps:
       - name: Checkout project

README.md

@@ -67,6 +67,9 @@ Sidekiq.configure_client do |config|
     chain.add Sidekiq::Instrument::ClientMiddleware
   end
 end
+
+Sidekiq::Instrument::WorkerMetrics.enabled = true # Set true to enable worker metrics
+Sidekiq::Instrument::WorkerMetrics.namespace = <APP_NAME>
 ```
 
 ## StatsD Keys
@@ -78,15 +81,18 @@ For each job, the following metrics will be reported:
    worker begins performing a job.
 3. **shared.sidekiq._queue_._job_.runtime**: timer of the total time spent
    in `perform`, in milliseconds.
-3. **shared.sidekiq._queue_._job_.error**: counter incremented each time a
+4. **shared.sidekiq._queue_._job_.error**: counter incremented each time a
    job fails.
 
 The metric names can be changed by overriding the `statsd_metric_name`
 method in your worker classes.
 
 For each queue, the following metrics will be reported:
 1. **shared.sidekiq._queue_.size**: gauge of how many jobs are in the queue
-1. **shared.sidekiq._queue_.latency**: gauge of how long the oldest job has been in the queue
+2. **shared.sidekiq._queue_.latency**: gauge of how long the oldest job has been in the queue
+
+For each worker, the following metrics and tags will be reported:
+1. **sidekiq.worker_metrics.inqueue.#{key}**: number of jobs "in queue" per worker, uses redis to track increment/decrement
 
 ## DogStatsD Keys
 For each job, the following metrics and tags will be reported:
@@ -97,12 +103,15 @@ For each job, the following metrics and tags will be reported:
    worker begins performing a job.
 3. **sidekiq.runtime (tags: {queue: _queue_, worker: _job_})**: timer of the total time spent
    in `perform`, in milliseconds.
-3. **sidekiq.error (tags: {queue: _queue_, worker: _job_})**: counter incremented each time a
+4. **sidekiq.error (tags: {queue: _queue_, worker: _job_})**: counter incremented each time a
    job fails.
 
 For each queue, the following metrics and tags will be reported:
 1. **sidekiq.queue.size (tags: {queue: _queue_})**: gauge of how many jobs are in the queue
-1. **sidekiq.queue.latency (tags: {queue: _queue_})**: gauge of how long the oldest job has been in the queue
+2. **sidekiq.queue.latency (tags: {queue: _queue_})**: gauge of how long the oldest job has been in the queue
+
+For each worker, the following metrics and tags will be reported:
+1. **sidekiq.worker_metrics.inqueue.#{key}**: number of jobs "in queue" per worker, uses redis to track increment/decrement
 
 ## Worker
 There is a worker, `Sidekiq::Instrument::Worker`, that submits gauges

lib/sidekiq/instrument/version.rb

@@ -1,5 +1,5 @@
 module Sidekiq
   module Instrument
-    VERSION = '0.5.6'
+    VERSION = '0.6.0'
   end
 end

lib/sidekiq/instrument/worker.rb

@@ -64,8 +64,8 @@ def send_worker_metrics
       return unless WorkerMetrics.enabled
 
       WorkerMetrics.workers_in_queue.each do |key, value|
-        Statter.statsd.gauge("shared.sidekiq.trace.inqueue.#{key}", value)
-        Statter.dogstatsd&.gauge("shared.sidekiq.trace.inqueue.#{key}", value)
+        Statter.statsd.gauge("shared.sidekiq.worker_metrics.inqueue.#{key}", value)
+        Statter.dogstatsd&.gauge("shared.sidekiq.worker_metrics.inqueue.#{key}", value)
       end
     end
   end

lib/sidekiq/instrument/worker_metrics.rb

@@ -7,93 +7,46 @@ module Instrument
     # Stores worker count with a key sidekiq_instrument_trace_workers:#{namespace}:in_queue
     # Values are hash having keys as worker names.
     class WorkerMetrics
-      class_attribute :enabled, :namespace, :redis_config
-
-      class_attribute :redis_password
+      class_attribute :enabled, :namespace
 
       class << self
-        def redis_pool_user
-          @redis_pool_user ||= begin
-            redis_client_config = RedisClient.config(**redis_config)
-            @redis = redis_client_config.new_pool(
-              timeout: 0.5, size: Integer(ENV.fetch('RAILS_MAX_THREADS', 5))
-            )
-          end
-        end
-
-        def reset_redis
-          @redis = nil
-        end
-
         def trace_workers_increment_counter(klass_name, sidekiq_redis_pool_user)
           return unless enabled?
 
-          if redis_config?
-            redis_pool_user.with do |redis|
-              redis.call 'HINCRBY', worker_metric_name, klass_name, 1
-            end
-          else
-            sidekiq_redis_pool_user.with do |redis|
-              redis.hincrby worker_metric_name, klass_name, 1
-            end
+          Sidekiq.redis do |redis|
+            redis.hincrby(worker_metric_name, klass_name, 1)
           end
         end
 
         def trace_workers_decrement_counter(klass_name)
           return unless enabled?
 
-          if redis_config?
-            redis_pool_user.with do |redis|
-              redis.call 'HINCRBY', worker_metric_name, klass_name, -1
-            end
-          else
-            Sidekiq.redis do |redis|
-              redis.hincrby worker_metric_name, klass_name, -1
-            end
+          Sidekiq.redis do |redis|
+            redis.hincrby(worker_metric_name, klass_name, -1)
           end
         end
 
         def reset_counters
           return unless enabled?
 
-          if redis_config?
-            redis_pool_user.with do |redis|
-              all_keys = redis.call 'HGETALL', worker_metric_name
-              redis.call 'HDEL', worker_metric_name, all_keys.keys
-            end
-          else
-            Sidekiq.redis do |redis|
-              all_keys = redis.hgetall worker_metric_name
-              redis.hdel worker_metric_name, all_keys.keys
-            end
+          Sidekiq.redis do |redis|
+            all_keys = redis.hgetall(worker_metric_name)
+            redis.hdel(worker_metric_name, all_keys.keys)
           end
         end
 
         def reset_counter(key)
           return unless enabled?
 
-          if redis_config?
-            redis_pool_user.with do |redis|
-              redis.call 'HDEL', worker_metric_name, key
-            end
-          else
-            Sidekiq.redis do |redis|
-              redis.hdel worker_metric_name, key
-            end
+          Sidekiq.redis do |redis|
+            redis.hdel(worker_metric_name, key)
           end
         end
 
         def workers_in_queue
           return unless enabled?
-
-          if redis_config?
-            redis_pool_user.with do |redis|
-              redis.call 'HGETALL', worker_metric_name
-            end
-          else
-            Sidekiq.redis do |redis|
-              redis.hgetall worker_metric_name
-            end
+          Sidekiq.redis do |redis|
+            redis.hgetall(worker_metric_name)
           end
         end
 

sidekiq-instrument.gemspec

@@ -20,7 +20,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'statsd-instrument', '>= 2.0.4'
   spec.add_dependency 'dogstatsd-ruby', '~> 5.5'
   spec.add_dependency 'activesupport', '>= 5.1', '< 7'
-  spec.add_dependency "redis-client", ">= 0.11.0"
+  spec.add_dependency "redis-client", ">= 0.14.1"
 
   spec.add_development_dependency 'bundler', '~> 2.0', '>= 2.0.2'
   spec.add_development_dependency 'rake', '~> 12.0'

spec/sidekiq-instrument/client_middleware_spec.rb

@@ -46,11 +46,6 @@
       it 'increments the enqueue counter' do
           Sidekiq::Instrument::WorkerMetrics.enabled = true
           Redis.new.hdel worker_metric_name ,'my_other_worker'
-          Sidekiq::Instrument::WorkerMetrics.redis_config = {
-            host:        ENV['REDIS_HOST'],
-            port:        ENV['REDIS_PORT'],
-            db:          0
-          }
           MyOtherWorker.perform_async
           expect(
           Redis.new.hget worker_metric_name ,'my_other_worker'

spec/sidekiq-instrument/server_middleware_spec.rb

@@ -45,11 +45,6 @@
         end
         it 'increments the enqueue counter' do
             Sidekiq::Instrument::WorkerMetrics.enabled = true
-            Sidekiq::Instrument::WorkerMetrics.redis_config = {
-              host:        ENV['REDIS_HOST'],
-              port:        ENV['REDIS_PORT'],
-              db:          0
-            }
             Redis.new.hdel worker_metric_name ,'my_other_worker'
             MyOtherWorker.perform_async
             expect(

spec/sidekiq-instrument/worker_spec.rb

@@ -104,7 +104,7 @@
       end
 
       it_behaves_like 'worker behavior', %w[
-        shared.sidekiq.trace.inqueue.my_other_worker
+        shared.sidekiq.worker_metrics.inqueue.my_other_worker
         sidekiq.processed
         sidekiq.workers
         sidekiq.pending