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

Feature Request: Add ActiveJob Support #201

Open
jtcontreras90 opened this issue Jul 9, 2018 · 8 comments
Open

Feature Request: Add ActiveJob Support #201

jtcontreras90 opened this issue Jul 9, 2018 · 8 comments
Assignees

Comments

@jtcontreras90
Copy link

There should be Rails' pure ActiveJob support for different queue adapters based in it. In particular, I had a lot of trouble configuring this gem with active_elastic_job. Finally, I configured my own instrument like this (based in the gist avaliable in documentation under Sneakers section)

@mathieujobin
Copy link
Contributor

Thanks for this @jtcontreras90

I really need this myself as well

does it work for you OK?

Couldn't we not call it once on a Active::Job monkey patch, instead of modifying every job files?

@jtcontreras90
Copy link
Author

jtcontreras90 commented Apr 9, 2019

You are welcome @mathieujobin

It does work fine for me and it has been working fine on production environment for more than 10 months.

The thing about the comment on the gist is that is not entirely honest. I did not include the instrument on every job file. Instead, I created an initializer named scout.rb with the following code:

if Rails.application.config.active_job.queue_adapter == :active_elastic_job
 Rails.application.config.after_initialize do
   ActiveJob::Base.descendants.each do |subclass|
     subclass.include(ScoutApm::BackgroundJobIntegrations::ActiveElasticJob::Instruments)
   end
 end
end

@ioquatix ioquatix self-assigned this Apr 11, 2019
@ioquatix
Copy link
Contributor

@cschneid @dlanderson would you like me to implement this?

@cschneid
Copy link
Contributor

I thought I wrote a piece about ActiveJob in another issue, but I can't locate it now. The gist of it is:

ActiveJob doesn't give us the information needed to generically instrument jobs. It's a really thin wrapper that makes nearly no assumptions about the data being sent to the job runner. Which means that even basic things like extracting the name of the job being requested aren't possible to extract in a general way.

In addition, it also doesn't specify execution style (correctly, but inconvenient for us), so delayed-job runs in thread, sidekiq creates a new threads and resque forks. Those all require their own handling to be sure the data is captured correctly, and can't be totally generic.

I can take another look at it in a while, but I'm pretty sure totally generic AJ support isn't possible with how the Scout agent works.

@ioquatix
Copy link
Contributor

That makes sense.

If that's the case, why don't I raise the issue with the Rails team and see if we can get the appropriate hooks for instrumentation?

@cschneid
Copy link
Contributor

That would be fine - we've mostly not looked into instrumentation hooks because we have to support back to before the hooks existed, so we need a specialized monkey patch approach anyway. But if you want to look into what hooks exist, and if new ones should be implemented, that would be excellent.

@ioquatix
Copy link
Contributor

I will have limited time until after RubyKaigi, but later this month I'll dig into this issue and see how we can move forward.

@mathieujobin
Copy link
Contributor

mathieujobin commented Apr 11, 2019

Exactly as suggested by @jtcontreras90 I deployed this to production today and it works magically

https://gist.github.com/mathieujobin/2a705d7820f9f84bbcebdfc495872b38

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

4 participants