scout_apm_ruby icon indicating copy to clipboard operation
scout_apm_ruby copied to clipboard

Allow ActiveJob before_perform callbacks to call ignore! and similar Scout APIs

Open cschneid opened this issue 4 years ago • 4 comments

A customer running Sidekiq via ActiveJob reports that calling ignore from a before_perform callback is not working. Calling the ignore from inside the job's perform method appears to be fine however.

The sequence a task gets executed appears to be:

  • Sidekiq runner pops off a job, and calls 'process'
  • Instantiates a ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper
  • Runs server_middleware array
  • Calls perform
  • ActiveJob::Base.execute is called (defined in execution.rb).
  • Execute callbacks are run
  • The wrapped job is unwrapped, and perform_now is called (also defined in ActiveJob's execution.rb)
  • Perform callbacks are run
  • Job#perform is finally called

Scout injects itself as a server_middleware, so we should have the state necessary to ignore or rename or whatever API call you need by then.

cschneid avatar Jun 12 '20 20:06 cschneid

Same here. This bug was misleading, as before_perform is the right place to put such a logic. At least keeping common configuration in ApplicationJob with before_perform looks much easier vs inserting it into all perform methods.

@cschneid thanks for reporting this, as it is good to know that this is a gem bug that smb else has, and not code bug.

dzirtusss avatar Nov 27 '20 19:11 dzirtusss

Thanks for this piece of information.

Here's our workaround:

# app/concerns/apm_sampling
module ApmSampling
  extend ActiveSupport::Concern

  private def apm_sample
    ScoutApm::Transaction.ignore! unless apm_sample?
  end

  private def apm_sample?
    true
  end

  def perform_with_apm_sampling(*args)
    apm_sample
    perform_without_apm_sampling(*args)
  end

  included do
    case
    when self < ActiveJob::Base || self < Sidekiq::Worker
      def self.method_added(method_name)
        case method_name
        when :perform
          # Avoid reentrance
          unless method_defined?(:perform_without_apm_sampling)
            alias_method :perform_without_apm_sampling, :perform
            alias_method :perform, :perform_with_apm_sampling # This invokes method_added(:perform)
          end
        end
      end
    when respond_to?(:before_action)
      before_action :apm_sample
    else
      # There is no before_action callback in ActionController::Metal.
      # You need to manually call apm_sampling from within each action method.
    end
  end
end

# app/controllers/application_controller.rb
class ApplicationController < ActionController::Base
  include ApmSampling

  private def apm_sample?
    rand < 0.05
  end
end

# app/jobs/application_job.rb
class ApplicationJob < ActiveJob::Base
  # Make sure to include this first, especially before adding authentication callbacks.
  # Otherwise all authentication errors are submitted without sampling.
  include ApmSampling

  private def apm_sample?
    rand < 0.01
  end
end

We didn't use Module prepend because it didn't work nicely with rspec-mocks.

knu avatar Jan 24 '21 13:01 knu

I've run into the same problem and the workaround proposed by @knu was a great help.

The only issue i'm still trying to get around is that there are still certain jobs that are harder to implement sampling on, such as the ActionMailer::MailDeliveryJob.

Japestrale avatar Jul 27 '22 15:07 Japestrale

To introduce sampling to jobs that inherit from ActiveJob::Base instead of ApplicationJob like ActionMailer::MailDeliveryJob, you can use lazy-loaded hooks to add your sampling code as a concern to ActiveJob::Base.

# config/initializers/scout_apm_job_sampling.rb

module ScoutApmJobSampling
  extend ActiveSupport::Concern

  included do
    before_perform :sample_requests_for_scout
  end

  private

    def sample_requests_for_scout
      sample_rate = 0.01

      if rand > sample_rate
        ScoutApm::Transaction.ignore!
      end
    end
end

ActiveSupport.on_load(:active_job) do
  include ScoutApmJobSampling
end

syedaminx avatar May 02 '23 15:05 syedaminx