Out of box support for resque

[kbaum]

No description provided.

[kbaum]

Created a simple project which runs resque jobs and includes coverband.

https://github.com/kbaum/resque_coverband_test/

[danmayer]

awesome this is a great way to work on it, I use sidekiq most of the time so thanks for getting it setup!

[danmayer]

ok so digging in a bit my basic thinking...

We detect if Reque is loaded... if it is we configure job hooks... then it could work similar to the rack_middleware and report coverage...

https://github.com/resque/resque/blob/master/docs/HOOKS.md#job-hooks

module CoveredJob
  def after_perform_coverage(*args)
    Coverband::Collectors::Coverage.instance.report_coverage(true)
  end
end

class Job
  extend CoveredJob

The issue I see is resque doesn't have a base class or other easy way to extend all jobs... as they can just be any generic Ruby class.

[danmayer]

another option would be to document or enable resque to run our at_exit hook

https://www.rubydoc.info/gems/resque/Resque/Worker#run_at_exit_hooks-instance_method

or we patch the runner

module Resque
  class Worker
    def perform(job)
      begin
        if fork_per_job?
          reconnect
          run_hook :after_fork, job
        end
        job.perform
      rescue Object => e
        report_failed_job(job,e)
      else
        log_with_severity :info, "done: #{job.inspect}"
      ensure
        Coverband::Collectors::Coverage.instance.report_coverage(true)
        yield job if block_given?
      end
    end

[kbaum]

Those are all great suggestions. I think the challenge here is that resque workers are plain ruby objects and don't include a module. Resque has an after_fork hook that we can use to start coverband. We can also leverage the at_exit ruby hook but an environment variable RUN_AT_EXIT_HOOKS has to be set for this to work.

https://github.com/resque/resque/blob/master/docs/HOOKS.md#worker-hooks

Threw this together and it seems to work:

https://github.com/kbaum/resque_coverband_test/blob/at_exit_example/Rakefile

The only issue with this approach is that all at_exit hooks will run for each job.

[kbaum]

Removed the extra at_exit hook since coverband already has an at_exit:

kbaum/resque_coverband_test@45874b8

Though on a side note, i think we should probably move that hook out of the Background class as I am thinking it's not related.

https://github.com/danmayer/coverband/blob/master/lib/coverband/integrations/background.rb

At the very least, it shouldn't be dependent on Configuration#background_reporting_enabled

[danmayer]

ok so if we detect resque, don't start background reporting, but use the after_fork to start coverband and add the at_exit.... Then the resque primary process does no coverband and the forked processes start/report once per cycle...

I think this works... it is a bit slow as we can't only report in the background so they take the full network reporting hit per job. Which isn't ideal, but I don't see a good way around that.

[kbaum]

Right. I don’t think we need to do anything special for resque for at_exit as I think this makes sense for all coverband ruby processes.

We could also introduce the idea of only reporting a certain percentage of job executions with resque if we are worried about performance.

[kbaum]

RE: percentage of job executions. Ideally this would be configurable per job. Some jobs might be running non-stop and are fast while other jobs might run once every 3 months and take minutes.

[kbaum]

Thought it would be helpful to look at what newrelic does for resque:

https://github.com/newrelic/rpm/blob/master/lib/new_relic/agent/instrumentation/resque.rb#L25

Looks like they wrap calls to Resque::Job#perform using alias_method. The advantage of this approach is that we do not have to rely on all jobs running at_exit which is off by default in resque. I can see why the default is off because at_exit might be expensive for every job.

If we followed this approach we could use module prepend since we only support ruby 2.3+. That feels a bit cleaner although it has about the same downsides which is that we are depending on the internals of resque. I am leaning toward this approach though since new relic does it and it feels better for users of resque since it does not depend on at_exit.

[danmayer]

yeah I like going with prepend. I think percentage as an option would be good to add in as we used to have for our middleware / tracepoint version. Do you want to knock this out? looks like you have a good spike and idea going.

[kbaum]

Sounds good. I’ll knock this one out.

[kbaum]

Merged in this: #177

That being said noticing some sporadic test failures with the resque test. Going to open a new issue for that and close this.

[danmayer]

This is available for testing in the 4.1.0.alpha gem release. https://rubygems.org/gems/coverband/versions/4.1.0.alpha