opentelemetry-ruby-contrib
opentelemetry-ruby-contrib copied to clipboard
open-telemetry
Rails instrumentation
Add instrumentation adapters for Rails components via ActiveSupport notifications. @martinisoft can fill in the details.
I'll be working on adding Rails instrumentation by implementing a Rack adapter first and adding on each individual component in Rails via ActiveSupport and friends. ActiveJob support would also be handy to add for background tasks. I think sidekiq does not use the ActiveJob interface so there would have to be a separate issue to implement that.
Some more cleanup and optimization can be done by unifying the Rack adapter and re-using it between the Sinatra and Rails adapters, but that feels like it is out of the scope of this issue.
@mwear I have a draft PR up now, but I am missing a couple things:
- Where/how do I get appraisal support working like it is done in circle-ci? I seem to be missing a step that isn't documented in the porting guide.
- There seems to be a split between examples and tests. Is there a reason to not just use tests only or do they serve a different purpose? I'm thinking it might be a little overkill since tests can verify coverage most of the time and there will be less duplication of code.
@martinisoft
Where/how do I get appraisal support working like it is done in circle-ci? I seem to be missing a step that isn't documented in the porting guide.
I believe you need to check in the generated Gemfiles as per: https://github.com/thoughtbot/appraisal#version-control. Other than that, the setup looks ok to me
There seems to be a split between examples and tests. Is there a reason to not just use tests only or do they serve a different purpose? I'm thinking it might be a little overkill since tests can verify coverage most of the time and there will be less duplication of code.
The examples are meant to be mini-projects that can be easily spun up to explore the spans generated by the instrumentation. They can be useful for QA and for users to see how to use the instrumentation and what the expected results should look like. The JS project has a similar setup: https://github.com/open-telemetry/opentelemetry-js/tree/master/examples.
One thing I have been hitting trying to work on the Rails adapter is getting a testing feedback loop going. I've got a potential solution by generalizing the docker compose configs, then using the rake tasks within each adapter to run the test suites for them. Right now the testing tasks seem to be an all or nothing approach, but they aren't directly accessible via docker compose run commands.
Instead of:
ex-adapter-rails:
<<: *base
working_dir: /app/adapters/rails/example
It would be:
adapter-rails:
<<: *base
working_dir: /app/adapters/rails
This would allow for running the tests the way you would typically in a ruby project via Rake docker-compose run adapter-rails bundle exec rake test
Just to clarify, will this work include: ActiveSupport, ActionPack, ActionView, ActionRecord, ActionCable, or should this be broken our more granularly?
I'm sticking to instrumenting all the components for now then I guess they can be broken out later.
Would it be possible to merge this and cut a beta release of this? I'd like to start experimenting w/ using it in my app.
Would it be possible to merge this and cut a beta release of this? I'd like to start experimenting w/ using it in my app.
Hey @johnnyshields, I'm in the process of getting a rails instrumentation gem setup. We're going to start with it scoped very minimally to just auto instrumenting controller requests. I'll ref this issue with the PR when it's up so you can track the progress.
If you want to experiment with otel on a rails app, you can inject the rack middleware instrumentation and get your requests traced that way. The approach I'm taking at this point is going to be very similar that (without having to manually inject the middleware).
FYI @johnnyshields you can track the discussions and progress around the rails instrumentation on this PR here https://github.com/open-telemetry/opentelemetry-ruby/pull/454
Hi, I'd like to instrument ActionView (ERB/HAML/SLIM etc template rendering.) It will use ActiveSupport::Notification hooks. Should it be done as part of the Rails instrumentation or be it's own separate instrumenter?
Hey @johnnyshields that would be awesome, let's do it in the Rails instrumentation gem.
I was wondering if this was already explored. I'm happy to see this is planned. But I wonder if doing Module#prepend on ActiveRecord subclasses isn't redundant with subscribing to its notifications? By adding opentelemetry to instrument activerecord, it now feels this is instrumented twice.
Once https://edgeguides.rubyonrails.org/active_support_instrumentation.html is implemented, should we be able to choose between the two via an option would truly be awesome.
if module#prepend is superior in all cases, then I want to look into disabling the default ActiveSupport::Notifications from ActiveRecord at least.
I was wondering if this was already explored. I'm happy to see this is planned. But I wonder if doing Module#prepend on ActiveRecord subclasses isn't redundant with subscribing to its notifications? By adding opentelemetry to instrument activerecord, it now feels this is instrumented twice.
I believe this comment should address your question regarding the persistence modules vs the sql.active_record notification topic https://github.com/open-telemetry/opentelemetry-ruby/pull/653#issuecomment-801153648
Once https://edgeguides.rubyonrails.org/active_support_instrumentation.html is implemented, should we be able to choose between the two via an option would truly be awesome.
if module#prepend is superior in all cases, then I want to look into disabling the default ActiveSupport::Notifications from ActiveRecord at least.
Once these different pieces of instrumentation exist we fully intend to allow for this level of configuration.