Skip to content

Bump jruby 9.4.13.0 #17696

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 35 commits into from
Jun 24, 2025
Merged

Bump jruby 9.4.13.0 #17696

merged 35 commits into from
Jun 24, 2025

Conversation

donoghuc
Copy link
Member

@donoghuc donoghuc commented Jun 2, 2025 •
edited
Loading

Release notes

Logstash now ships with JRuby 9.4.13.0 to leveragle latest features and improvements in the 9.4 series.

What does this PR do?

Here are the biggest updates we need to deal with to take up 9.4.12.1 (and 9.4.13.0):

  1. We had been explicitly overriding some default gems for security compliance, with the latest jruby release we can remove some of those pins ✅
  2. The default gem set changed and there are some license checker updates required ✅
  3. File loading behavior changes require some explicity requires where previously those were already loaded (for example where previously we could call a to_set on an Array assuming require 'set' was already executed, changes in loading invalidated that and now require 'set' is required. ✅
  4. Some error handling needs to be updated based on a refactor in jruby rubygems/rubygems@23b828c Specifically https://github.com/elastic/logstash/pull/17696/files#r2136596098 ✅
  5. There has been some changes to bundler we need to adapt to. We are monkey patching some code that was changed and predictably that is breaking the pluginmanager functionality. The PR that breaks us is https://github.com/rubygems/rubygems/pull/8027/files. In addition to this change there have been a couple other changes to that code base that we need to consider: https://github.com/rubygems/rubygems/pull/8027/files has landed and rubygems/rubygems@152331a is coming (not yet released). We continue to patch to preserve the functionality. NAMELY that we will download/cache gems when they cannot be found. This supports preparing offline packs and installing plugins from packs.

Why is it important/What is the impact to the user?

Ideally there is no direct user impact, though consumers will benefit from the latest and greatest in jruby and its deps.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

For testing please focus on plugin manager operations. I have been comparing the effect of the cache between jruby versions and from what i can tell i've got something pretty comparable. The rubygems (and importantly bundler) version changed from 3.3.26 to 3.6.3 which had considerable changes in bundler. This was the most challenging part of taking up the latest jruby version.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 2, 2025

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 2, 2025

This pull request does not have a backport label. Could you fix it @donoghuc? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • If no backport is necessary, please add the backport-skip label

This was referenced Jun 2, 2025
Closed
Open
donoghuc added 2 commits June 4, 2025 15:48
@donoghuc
Explicitly require 'set' to make to_set available
383cdaf 
Something appears to have changed in code loading where this require
is no longer in the call chain. Explicitly require it here.
@donoghuc
Fix up dependency mapper
973ff98
@donoghuc donoghuc force-pushed the bump-jruby-9.4.12.1 branch from 6604658 to 973ff98 Compare June 4, 2025 23:52
@donoghuc
Copy link
Member Author

donoghuc commented Jun 5, 2025

I think i've got PR tests green, kicked off an acceptance test build https://buildkite.com/elastic/logstash-exhaustive-tests-pipeline/builds/1944

Next step is trying to understand the bundler and gem changes ported over from 9.4.12.0 branch.

donoghuc
donoghuc commented Jun 9, 2025
View reviewed changes
lib/bootstrap/bundler.rb
@@ -165,7 +185,7 @@ def execute_bundler_with_retry(options)
begin
execute_bundler(options)
break
rescue ::Bundler::VersionConflict => e
rescue ::Bundler::SolveFailure => e
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See rubygems/rubygems@23b828c

donoghuc
donoghuc commented Jun 9, 2025
View reviewed changes
lib/bootstrap/patches/gems.rb Outdated
@@ -29,3 +29,16 @@ def get(path, data = {}, content_type = 'application/x-www-form-urlencoded', req
end
end
end

module ::Bundler
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of patching this, i think we were just sending an object instead of a string. Modified to send along a string instead of a Gem::Platform instance.

`Caused by: org.jruby.exceptions.NoMethodError: (NoMethodError) undefined method `empty?' for #<Gem::Platform:0x637b58c9>`

@donoghuc donoghuc force-pushed the bump-jruby-9.4.12.1 branch from 81ea787 to 8f1cca4 Compare June 9, 2025 23:10
@donoghuc
Update bundler code with latest rubygems release
7f97c7b 
This commit makes 2 changes:

1. We were sending a `Gem::Platform` instance to Thor parser instead of a
string. Convert to string version of a platform name. With newly added checks in
Thor, we were getting `noMethodError` for `Gem::Platform`.

2.  It looks like there was a patch to get around a bug in bundler that was
reaching out to the network for local gems. This appears to have been fixed
upstream, this removes the patch.
@donoghuc donoghuc force-pushed the bump-jruby-9.4.12.1 branch from 8f1cca4 to 7f97c7b Compare June 9, 2025 23:14
@donoghuc donoghuc changed the title Bump jruby 9.4.12.1 Bump jruby 9.4.13.0 Jun 24, 2025
@donoghuc donoghuc marked this pull request as ready for review June 24, 2025 06:01
donoghuc
donoghuc commented Jun 24, 2025
View reviewed changes
qa/acceptance/spec/shared_examples/cli/logstash-plugin/install.rb Outdated
@@ -44,7 +44,7 @@
it "successfully install the plugin" do
command = logstash.run_sudo_command_in_path("bin/logstash-plugin install #{gem_tmp_path}")
expect(command).to install_successfully
expect(logstash).to have_installed?("logstash-filter-dns")
expect(logstash).to have_installed?("logstash-filter-qatest")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODEREVIEW: I think this was just a bug that existed before... The ONLY failing test i've got now https://buildkite.com/elastic/logstash-exhaustive-tests-pipeline/builds/1993 is this.

On my test VM i print out the command and it has installed correctly:

  behaves like logstash install
    on [Ubuntu 22.04]
      with a direct internet connection
        when the plugin exist
          from a local `.GEM` file
sleep
#<Command:0x8b1bfdf @stdout="Validating /tmp/logstash-filter-qatest-0.1.1.gem\nInstalling logstash-filter-qatest\nInstallation successful", @exit_status=0, @stderr="Using bundled JDK: /usr/share/logstash/jdk">

You can see that it has cached it correctly:

buildkite-agent@cas-donoghue-logstash-ubuntu-2204:/usr/share/logstash$ sudo find . -name "*qatest*"
./vendor/cache/logstash-filter-qatest-0.1.1.gem
./vendor/local_gems/26f39b85/logstash-filter-qatest-0.1.1
./vendor/local_gems/c53f3e2d/logstash-filter-qatest-0.1.1

When I run again (even after clearning cache) it reports success:

buildkite-agent@cas-donoghue-logstash-ubuntu-2204:~$ sudo JARS_SKIP='true' /usr/share/logstash/bin/logstash-plugin install /tmp/logstash-filter-qatest-0.1.1.gem
Using bundled JDK: /usr/share/logstash/jdk
Validating /tmp/logstash-filter-qatest-0.1.1.gem
Installing logstash-filter-qatest
Installation successful

I see the same behavior on main (IE i think this same commit woudl break what we have today)

➜  logstash git:(909d66c46) bin/logstash-plugin install /Users/cas/Downloads/logstash-filter-qatest-0.1.1.gem
Using system java: /Users/cas/.jenv/shims/java
Validating /Users/cas/Downloads/logstash-filter-qatest-0.1.1.gem
Installing logstash-filter-qatest
Installation successful
➜  logstash git:(909d66c46) bin/logstash-plugin list
Using system java: /Users/cas/.jenv/shims/java
logstash-codec-avro
logstash-codec-cef
logstash-codec-collectd
logstash-codec-dots
logstash-codec-edn
logstash-codec-edn_lines
logstash-codec-es_bulk
logstash-codec-fluent
logstash-codec-graphite
logstash-codec-json
logstash-codec-json_lines
logstash-codec-line
logstash-codec-msgpack
logstash-codec-multiline
logstash-codec-netflow
logstash-codec-plain
logstash-codec-rubydebug
logstash-filter-aggregate
logstash-filter-anonymize
logstash-filter-cidr
logstash-filter-clone
logstash-filter-csv
logstash-filter-date
logstash-filter-de_dot
logstash-filter-dissect
logstash-filter-dns
logstash-filter-drop
logstash-filter-elastic_integration
logstash-filter-elasticsearch
logstash-filter-fingerprint
logstash-filter-geoip
logstash-filter-grok
logstash-filter-http
logstash-filter-json
logstash-filter-kv
logstash-filter-memcached
logstash-filter-metrics
logstash-filter-mutate
logstash-filter-prune
logstash-filter-ruby
logstash-filter-sleep
logstash-filter-split
logstash-filter-syslog_pri
logstash-filter-throttle
logstash-filter-translate
logstash-filter-truncate
logstash-filter-urldecode
logstash-filter-useragent
logstash-filter-uuid
logstash-filter-xml
logstash-input-azure_event_hubs
logstash-input-beats
 └── logstash-input-elastic_agent (alias)
logstash-input-couchdb_changes
logstash-input-dead_letter_queue
logstash-input-elastic_serverless_forwarder
logstash-input-elasticsearch
logstash-input-exec
logstash-input-file
logstash-input-ganglia
logstash-input-gelf
logstash-input-generator
logstash-input-graphite
logstash-input-heartbeat
logstash-input-http
logstash-input-http_poller
logstash-input-jms
logstash-input-pipe
logstash-input-redis
logstash-input-stdin
logstash-input-syslog
logstash-input-tcp
logstash-input-twitter
logstash-input-udp
logstash-input-unix
logstash-integration-aws
 ├── logstash-codec-cloudfront
 ├── logstash-codec-cloudtrail
 ├── logstash-input-cloudwatch
 ├── logstash-input-s3
 ├── logstash-input-sqs
 ├── logstash-output-cloudwatch
 ├── logstash-output-s3
 ├── logstash-output-sns
 └── logstash-output-sqs
logstash-integration-jdbc
 ├── logstash-input-jdbc
 ├── logstash-filter-jdbc_streaming
 └── logstash-filter-jdbc_static
logstash-integration-kafka
 ├── logstash-input-kafka
 └── logstash-output-kafka
logstash-integration-logstash
 ├── logstash-input-logstash
 └── logstash-output-logstash
logstash-integration-rabbitmq
 ├── logstash-input-rabbitmq
 └── logstash-output-rabbitmq
logstash-integration-snmp
 ├── logstash-input-snmp
 └── logstash-input-snmptrap
logstash-output-csv
logstash-output-elasticsearch
logstash-output-email
logstash-output-file
logstash-output-graphite
logstash-output-http
logstash-output-lumberjack
logstash-output-nagios
logstash-output-null
logstash-output-pipe
logstash-output-redis
logstash-output-stdout
logstash-output-tcp
logstash-output-udp
logstash-output-webhdfs
logstash-patterns-core
➜  logstash git:(909d66c46) bin/logstash-plugin list | grep qa
Using system java: /Users/cas/.jenv/shims/java
➜  logstash git:(909d66c46) bin/logstash-plugin list --verbose
Using system java: /Users/cas/.jenv/shims/java
logstash-codec-avro (3.4.1)
logstash-codec-cef (6.2.8)
logstash-codec-collectd (3.1.0)
logstash-codec-dots (3.0.6)
logstash-codec-edn (3.1.0)
logstash-codec-edn_lines (3.1.0)
logstash-codec-es_bulk (3.1.0)
logstash-codec-fluent (3.4.3)
logstash-codec-graphite (3.0.6)
logstash-codec-json (3.1.1)
logstash-codec-json_lines (3.2.2)
logstash-codec-line (3.1.1)
logstash-codec-msgpack (3.1.0)
logstash-codec-multiline (3.1.2)
logstash-codec-netflow (4.3.2)
logstash-codec-plain (3.1.0)
logstash-codec-rubydebug (3.1.0)
logstash-filter-aggregate (2.10.0)
logstash-filter-anonymize (3.0.7)
logstash-filter-cidr (3.1.3)
logstash-filter-clone (4.2.0)
logstash-filter-csv (3.1.1)
logstash-filter-date (3.1.15)
logstash-filter-de_dot (1.1.0)
logstash-filter-dissect (1.2.5)
logstash-filter-dns (3.2.0)
logstash-filter-drop (3.0.5)
logstash-filter-elastic_integration (9.0.0)
logstash-filter-elasticsearch (4.2.0)
logstash-filter-fingerprint (3.4.4)
logstash-filter-geoip (7.3.1)
logstash-filter-grok (4.4.3)
logstash-filter-http (2.0.0)
logstash-filter-json (3.2.1)
logstash-filter-kv (4.7.0)
logstash-filter-memcached (1.2.0)
logstash-filter-metrics (4.0.7)
logstash-filter-mutate (3.5.8)
logstash-filter-prune (3.0.4)
logstash-filter-ruby (3.1.8)
logstash-filter-sleep (3.0.7)
logstash-filter-split (3.1.8)
logstash-filter-syslog_pri (3.2.1)
logstash-filter-throttle (4.0.4)
logstash-filter-translate (3.4.2)
logstash-filter-truncate (1.0.6)
logstash-filter-urldecode (3.0.6)
logstash-filter-useragent (3.3.5)
logstash-filter-uuid (3.0.5)
logstash-filter-xml (4.3.1)
logstash-input-azure_event_hubs (1.5.1)
logstash-input-beats (7.0.2)
 └── logstash-input-elastic_agent (alias)
logstash-input-couchdb_changes (3.1.6)
logstash-input-dead_letter_queue (2.0.1)
logstash-input-elastic_serverless_forwarder (2.0.0)
logstash-input-elasticsearch (5.2.0)
logstash-input-exec (3.6.0)
logstash-input-file (4.4.6)
logstash-input-ganglia (3.1.4)
logstash-input-gelf (3.3.2)
logstash-input-generator (3.1.0)
logstash-input-graphite (3.0.6)
logstash-input-heartbeat (3.1.1)
logstash-input-http (4.1.2)
logstash-input-http_poller (6.0.0)
logstash-input-jms (3.3.0)
logstash-input-pipe (3.1.0)
logstash-input-redis (3.7.1)
logstash-input-stdin (3.4.0)
logstash-input-syslog (3.7.1)
logstash-input-tcp (7.0.2)
logstash-input-twitter (4.1.1)
logstash-input-udp (3.5.0)
logstash-input-unix (3.1.2)
logstash-integration-aws (7.2.1)
 ├── logstash-codec-cloudfront
 ├── logstash-codec-cloudtrail
 ├── logstash-input-cloudwatch
 ├── logstash-input-s3
 ├── logstash-input-sqs
 ├── logstash-output-cloudwatch
 ├── logstash-output-s3
 ├── logstash-output-sns
 └── logstash-output-sqs
logstash-integration-jdbc (5.6.0)
 ├── logstash-input-jdbc
 ├── logstash-filter-jdbc_streaming
 └── logstash-filter-jdbc_static
logstash-integration-kafka (11.6.3)
 ├── logstash-input-kafka
 └── logstash-output-kafka
logstash-integration-logstash (1.0.4)
 ├── logstash-input-logstash
 └── logstash-output-logstash
logstash-integration-rabbitmq (7.4.0)
 ├── logstash-input-rabbitmq
 └── logstash-output-rabbitmq
logstash-integration-snmp (4.0.6)
 ├── logstash-input-snmp
 └── logstash-input-snmptrap
logstash-output-csv (3.0.10)
logstash-output-elasticsearch (12.0.3)
logstash-output-email (4.1.3)
logstash-output-file (4.3.0)
logstash-output-graphite (3.1.6)
logstash-output-http (6.0.0)
logstash-output-lumberjack (3.1.9)
logstash-output-nagios (3.0.6)
logstash-output-null (3.0.5)
logstash-output-pipe (3.0.6)
logstash-output-redis (5.2.0)
logstash-output-stdout (3.1.4)
logstash-output-tcp (7.0.1)
logstash-output-udp (3.2.0)
logstash-output-webhdfs (3.1.0)
logstash-patterns-core (4.3.4)
➜  logstash git:(909d66c46) find . -name "*qatest*"
./qa/integration/fixtures/logstash-filter-qatest-0.1.1.gem
./qa/support/logstash-filter-qatest
./qa/support/logstash-filter-qatest/logstash-filter-qatest-newer.gemspec
./qa/support/logstash-filter-qatest/logstash-filter-qatest-old.gemspec
./qa/support/logstash-filter-qatest/logstash-filter-qatest-0.1.1.gem
./vendor/cache/logstash-filter-qatest-0.1.1.gem
./vendor/local_gems/a51773e7/logstash-filter-qatest-0.1.1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're correct given

let(:gem_name) { "logstash-filter-qatest-0.1.1.gem" }

the presence of logstash-filter-dns looks out of place.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what the issue is here is that the gemspec does not include itself in the test gem it builds https://github.com/elastic/logstash/blob/main/qa/support/logstash-filter-qatest/logstash-filter-qatest-old.gemspec for local gem installation I think that is required to be shown in the logstash-plugin list output. I beleive we see it when we download from rubygems because of some magic there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this commit should fix it. I'll kick off acceptance test run 6245cef

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦🏼 you're right that locally-installed gems don't have a gemspec unless they explicitly include one.

A couple months ago I also found this, and discovered that when a locally-installed gem does include a gemspec, it gets evaluated each time bundler looks at it, so it isn't necessarily going to generate a consistent spec if it references things on disk or in the environment. Unfortunately I failed to follow up with a PR to fix it.

This patch would also fix what you're seeing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, i forgot the test downloads the gem directly from rubygems instead of using the vendored one. Making that update and testing locally. Will push an update in addition to the other code review suggestions.

donoghuc
donoghuc commented Jun 24, 2025
View reviewed changes
robbavey
robbavey reviewed Jun 24, 2025
View reviewed changes
donoghuc added 2 commits June 24, 2025 08:26
@donoghuc
Fix missing version update
1c0e333
@donoghuc
Fix qatest filter plugin
6245cef 
Without including a gemspec with a locally built gem logstash-plugin list
will not show it in the output.
@donoghuc
Copy link
Member Author

exhaustive test run testing my test fix https://buildkite.com/elastic/logstash-exhaustive-tests-pipeline/builds/2003

jsvd
jsvd approved these changes Jun 24, 2025
View reviewed changes
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been testing this locally and I couldn't find any issues with the plugin manager, mainly around offline pack generation and installation and local .gem plugin file installation.
With CI green I'm good with merging this, just don't forget to add the reasoning around the patch in bundler.rb where you marked the TODO.

As soon as the exhaustive CI finished successfully please merge it 🚢

yaauie
yaauie reviewed Jun 24, 2025
View reviewed changes
Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty confident in this. I've left a couple of comments, but assume you understand the specifics better than me.

qa/acceptance/spec/shared_examples/cli/logstash-plugin/install.rb Outdated
@@ -44,7 +44,7 @@
it "successfully install the plugin" do
command = logstash.run_sudo_command_in_path("bin/logstash-plugin install #{gem_tmp_path}")
expect(command).to install_successfully
expect(logstash).to have_installed?("logstash-filter-dns")
expect(logstash).to have_installed?("logstash-filter-qatest")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦🏼 you're right that locally-installed gems don't have a gemspec unless they explicitly include one.

A couple months ago I also found this, and discovered that when a locally-installed gem does include a gemspec, it gets evaluated each time bundler looks at it, so it isn't necessarily going to generate a consistent spec if it references things on disk or in the environment. Unfortunately I failed to follow up with a PR to fix it.

This patch would also fix what you're seeing.

donoghuc added 3 commits June 24, 2025 11:37
@donoghuc
Use vendored gem instead of reaching out to rubygems
54d774a 
We vendor an udpated test gem for local install. Use that instead of
reaching out to rubygems.
@donoghuc
Document and comment on the patched bundler code
8bd5f5f
@donoghuc
Fix test
1050f61 
instead of downloading the "broken" one from rubygems (downloading the `.gem` file directly
does show up in plugin-list), using a vendored one polutes the cache. Use a new vendored
gem
@elastic-sonarqube Elastic SonarQube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@donoghuc donoghuc mentioned this pull request Jun 24, 2025
Open
@elasticmachine
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

History

@donoghuc
Copy link
Member Author

donoghuc commented Jun 24, 2025 •
edited
Loading

I would like to explore the strange behavior where locally installed gems who's specs are not self referencial further (separately). While it is certainly a pretty niche case, i do find it surprising.

Filed #17734

@donoghuc donoghuc mentioned this pull request Jun 24, 2025
Open
@donoghuc
Copy link
Member Author

Green! https://buildkite.com/elastic/logstash-exhaustive-tests-pipeline/builds/2004#_

@donoghuc donoghuc merged commit 0e99f52 into elastic:main Jun 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robbavey robbavey robbavey left review comments

@jsvd jsvd jsvd approved these changes

@yaauie yaauie yaauie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@donoghuc @elasticmachine @jsvd @yaauie @robbavey