Support for dynamic labels/tags (set in runtime) (continuation)

[raivil]

Hey @Envek,
I've started using the exporter and I'm trying to set it up to use dynamic tags. This is a continuation of yabeda-rb/yabeda#12.

Using the existing method def yabeda_tags(*params) works but it has one drawback.
If the job needs to retrieve the tag from a database or other external source, it may do repeated queries that would be executed on the perform method already.

Example of duplicated database calls below. It could get more complicated depending on the job.

class MyJobs::JobName
  include Sidekiq::Worker

  def yabeda_tags(*params)
    { importante: Customer.find(params.first).importance }
  end

  def perform(id)
    customer = Customer.find(id)
    customer.process_something
  end
end

Ideally, if the Yabeda.with_tags worked inside the job, it would solve all problems, but since Sidekiq uses a middleware chain, I'm not sure if it's possible.

I checked the Yabeda middlewares on this project and changed ServerMiddleware a little bit to work with Thread.current[:yabeda_temporary_tags], the same strategy as with_tags.

module Testing
  class ServerMiddleware
    # CODE FROM https://github.com/yabeda-rb/yabeda-sidekiq/blob/master/lib/yabeda/sidekiq/server_middleware.rb
    def call(worker, job, queue)
      start = Time.now
      begin
        latency = ::Sidekiq::Job.new(job).latency
        # removed any calls to metrics from before the actual job execution.
        yield
        labels = Yabeda::Sidekiq.labelize(worker, job, queue)
        Yabeda.sidekiq_job_latency.measure(labels, latency)
        Yabeda.sidekiq_jobs_success_total.increment(labels)
      rescue Exception # rubocop: disable Lint/RescueException
        Yabeda.sidekiq_jobs_failed_total.increment(labels)
        raise
      ensure
        Yabeda.sidekiq_job_runtime.measure(labels, elapsed(start))
        Yabeda.sidekiq_jobs_executed_total.increment(labels)
        Thread.current[:yabeda_temporary_tags] = {}
      end
    end

    private

    def elapsed(start)
      (Time.now - start).round(3)
    end
  end
end

Sidekiq config to remove existing middleware and add the new one.

config.server_middleware do |chain|
    chain.remove Yabeda::Sidekiq::ServerMiddleware
    chain.add Testing::ServerMiddleware
end

class MyJobs::JobName
  include Sidekiq::Worker

  def perform(id)
   # make sure that discovering tags is at the beginning of the method.
    Thread.current[:yabeda_temporary_tags] = { importance: 'super important' } # this could be simplified with a concern maybe?
    Customer.find(id).do_super_important_stuff
  end
end

What do you think about this strategy? Any points that draw your attention?
Can you see alternatives to it?

Best,

[Envek]

For now I think you should be able to use Yabeda.with_tags and memoization in yabeda_tags to achieve the same result without changes in middlewares. Something like this:

class MyJobs::JobName
  include Sidekiq::Worker

  def perform(id)
    Yabeda.with_tags(yabeda_tags) do
      Customer.find(id).do_super_important_stuff
    end
  end

  def yabeda_tags
    @yabeda_tags ||= { importance: 'super important' }
  end
end

[Envek]

I'm not sure whether or not yabeda-sidekiq should use Yabeda.with_tags by default in server middleware. Inside Yabeda.with_tags these tags will be added to all metrics, not only to sidekiq-specific ones. And some users tracks app-specific things from code executed within sidekiq jobs. I just not sure which behavior is more expected.

[Envek]

@raivil, try 0.7.0 with Yabeda.with_tags used in middleware (I decided to change this behavior).