Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cschneid opened this issue Jun 12, 2020 · 4 comments

Comments

@cschneid
Copy link
Contributor

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.

@dzirtusss
Copy link

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.

@knu
Copy link

knu commented Jan 24, 2021

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.

@Japestrale
Copy link

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.

@syedaminx
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants