Support for JRuby - Githubissues

Adithya-copart commented 5 years ago

These are the minor changes that I had to add to get instana working on JRuby.

Ideally, it'll be easier for us to use instana if the changes in this PR are made on the gem directly as that would eliminate the need for maintaining a forked version and also enable other JRuby users to use this gem.

The GC metrics are not available directly in the ruby process dashboard but instana agent taps onto the JVM and displays GC metrics on a separate dashboard for us.

Added conditional loading of gems that require C-extensions in JRuby. I added another file that is responsible for loading OJ when the gem is available, so that MRI users need not take any performance hits.
Bumped the version of sys-proctable to resolve JRuby+Mac issue: https://github.com/djberg96/sys-proctable/issues/77. We could also consider loading sys-proctable conditionally in OSX if I understand https://github.com/instana/ruby-sensor/issues/108 correctly.
Updated the logic of checking the host OS in agent.rb.
Added socket.closed? guard in the ensure block to get rid of the socket related errors while trying to call socket.close on a already closed socket.

@pglombardo Thanks for the help and pointing in the right direction.

Adithya-copart commented 5 years ago

I did not change travis config to run the tests on JRuby.

However, I manually verified it in my workstation. It’ll be nice if JRuby is included in the travis pipeline.

pglombardo commented 5 years ago

This is great work @Adithya-copart! I'm glad you got something useable for JRuby now.

Unfortunately, we can't merge this into the mainline until we decide to officially support JRuby - meaning we put effort into on-going support, development, updates, bug fixes, documentation etc..

Also note that tracing with this setup, you are likely to receive broken or incomplete traces between the JRuby and Java parts since they are not aware of each other.

We have the feature request filed in our backlog but as of yet, there hasn't been any concrete plans to pursue this.

With that said, I may merge some of these changes via cherry-pick. I'll update as things develop.

Adithya-copart commented 5 years ago

Also note that tracing with this setup, you are likely to receive broken or incomplete traces between the JRuby and Java parts since they are not aware of each other.

Thanks for the insight and the response, we don't use a lot of Java code directly but we do use via the JDBC database adapter etc.

We will make sure to keep that in mind.

pglombardo commented 5 years ago

I spoke to @CodingFabian today and actually the tracing point should be non-issue as JRuby won't be touched by the Java instrumentation currently. We'll also aim to merge this PR sooner than later so at least we will work under JRuby for the time being until we add official support. 😎

pglombardo commented 5 years ago

This has been released in gem 1.10.1 - thanks again @Adithya-copart!

https://github.com/instana/ruby-sensor/releases/tag/1.10.1

instana / ruby-sensor

Support for JRuby #148