RUBY-3714 Client metadata capture for TruffleRuby#2971
RUBY-3714 Client metadata capture for TruffleRuby#2971kyleburgess2025 merged 8 commits intomongodb:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the platform detection logic to generalize runtime identification beyond JRuby. Instead of specifically checking for JRuby, the code now identifies whether the runtime is standard Ruby (MRI) and handles all other Ruby implementations (including JRuby and TruffleRuby) generically using RUBY_ENGINE and RUBY_ENGINE_VERSION constants.
Key changes:
- Replaced JRuby-specific detection with generic Ruby engine detection
- Updated version reporting to use
RUBY_ENGINEandRUBY_ENGINE_VERSIONfor non-MRI runtimes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| end | ||
|
|
||
| # Queries whether the current runtime is JRuby or not. | ||
| # Queries whether the current runtime is Ruby or not. |
There was a problem hiding this comment.
The comment is unclear. Since this method is always called within a Ruby runtime, it should clarify that it checks for standard Ruby/MRI specifically, not whether Ruby is running. Consider: 'Queries whether the current runtime is standard Ruby (MRI).'
| # @return [ true | false ] whether the runtime is JRuby or not. | ||
| def jruby? | ||
| BSON::Environment.jruby? | ||
| # @return [ Boolean ] whether the current runtime is Ruby |
There was a problem hiding this comment.
The return value documentation is incomplete and unclear. It should specify 'whether the current runtime is standard Ruby (MRI)' to distinguish from other Ruby implementations.
spec/support/shared/app_metadata.rb
Outdated
| document[:client][:driver][:name].should == 'mongo-ruby-driver|' | ||
| document[:client][:driver][:version].should == "#{Mongo::VERSION}|" | ||
| document[:client][:platform].should =~ /\AJ?Ruby[^|]+\|\z/ | ||
| document[:client][:platform].should =~ /\Aj?[Rr]uby[^|]+\|\z/ |
There was a problem hiding this comment.
Changed this regex because the output of RUBY_ENGINE is all lowercase. Other potential solutions:
- Make this entire regex case-insensitive - decided against this because I'd prefer to tests as specific as possible
- Change "Ruby" to "ruby" in the platform metadata - would make the logging look more similar to logging for jruby and truffleruby, but seems unnecessary
- Capitalize JRuby and TruffleRuby - would look nicer, but seems unnecessary. Plus, would have to be implemented for each engine we support... for now, it's just JRuby, TruffleRuby, and standard Ruby, but in the future this list could grow
comandeo-mongo
changed the title
| # Queries whether the current runtime is standard Ruby or not. | ||
| # | ||
| # @return [ Boolean ] whether the current runtime is standard Ruby | ||
| def ruby? |
There was a problem hiding this comment.
"mri" ("matz's ruby implementation") is how the canonical implementation is usually called. It might be better to name this predicate mri? to reflect that, since ruby? is probably overly broad.
| def ruby? | |
| def mri? |
| def engine_name | ||
| case RUBY_ENGINE | ||
| when 'jruby' | ||
| 'JRuby' | ||
| when 'truffleruby' | ||
| 'TruffleRuby' | ||
| else | ||
| RUBY_ENGINE | ||
| end |
There was a problem hiding this comment.
This absolutely works, and is valid. Another approach that is frequently used in Ruby programs is to use a Hash to store all of the known options, and query it in the method. E.g.
ENGINE_NAMES = { 'jruby' => 'JRuby', 'truffleruby' => 'TruffleRuby' }.freeze
def engine_name
ENGINE_NAMES[RUBY_ENGINE] || RUBY_ENGINE
endIt really comes down to personal preference, though. Optimize for happiness. :D
There was a problem hiding this comment.
i like the hash better! replaced : )
| let(:platform_string) do | ||
| [ | ||
| "JRuby #{JRUBY_VERSION}", | ||
| "jruby #{JRUBY_VERSION}", |
There was a problem hiding this comment.
Don't forget to teach the tests the proper case on the engine names ☝️
jamis
left a comment
jamis
left a comment
There was a problem hiding this comment.
Looks good!
The failing tests appear to be due to infrastructure issues (Atlas not responding in time to create a cluster to test against, and another where the test seems to be hanging). Don't worry about them for now; we'll keep an eye on them in subsequent PRs and see if there's a pattern. 👍
Before this change,
mongoidlogs included only whether the engine was JRuby or not - other engines, such as truffleruby, were not specified (see https://jira.mongodb.org/browse/RUBY-3714). After this change, the Ruby engine is included in the logs. No testing has been added as of yet for this change.Below are the Ruby engines tested with the change, along with their values in the
platformfield of log metadata.truffleruby+graalvm-25.0.0:
"truffleruby 25.0.0, like Ruby 3.3.7, arm64-darwin20, arm64-unknown-darwin, A"3.2.6:
"Ruby 3.2.6, arm64-darwin25, aarch64-apple-darwin25.1.0, M"jruby-10.0.2.0:
"jruby 10.0.2.0, like Ruby 3.4.2, java, JVM 25.0.1, java21, A"