Add the idea of a jar manager to handle jar_dependencies

[purbon]

In #2937 and #2954 we discussed the idea to build a jar manager in order to change the way we're loading the jar dependencies within our code base.

This PR add the concept of a jar manager responsible of finding and loading jars as dependencies.

Collaterally this PR also removed the load_elasticsearch_jars! method as it's a legacy code that was there from the early days when the ES jars were shipped with LS and not in the plugins.

This new jar manager is in use in

• Remove the build time jar dependency installation and embedded the jars instead logstash-plugins/logstash-input-log4j#8
• Remove the build time jar dependency installation and embedded the jars instead logstash-plugins/logstash-output-elasticsearch#101
• https://github.com/logstash-plugins/logstash-output-elasticsearch-ec2/pull/6
• Remove the build time jar dependency installation and embedded the jars instead logstash-plugins/logstash-input-drupal_dblog#7

Relates to #2974

For now it still need a bit more test to be sure everything is working as expected.

[purbon]

This test scenario has been successful for me.

• Install all plugins
• Update the plugins that has been changed in this PR.
• Run the all plugins test.

🍏

[wiibaa]

I suppose you could also remove this line, JAR_DIR is not used anymore in 1.5

[purbon]

makes sense @wiibaa

[colinsurprenant]

• why not keep load_jars! in the Envronment module? this is pretty much the idea of this module...
• why default to a new dependencies/jar path? we already have the vendor dir for that? just vendor/jar no?

[purbon]

• I wanted to create something separate in order to let this be loaded without the overhead of the hole LS-Environment, I don't specifically see this as LS env, while loading JARS we don't need stuff like bundle for example. But this was just my point or initial idea.
• I wanted to made this generic enough so you can change this if you want, this are just the default parameters I thought interesting. But the important question here is, do we want the jars to live within the lib dir? or within the vendors? I don't have strong opinions, but I always use them throw lib and don't strongly dislike the idea.

[colinsurprenant]

Agreed about bundler code not belonging in the Environment module and PR #2752 solves that.

On the other hand since the Environment module is already loaded in all plugins I don't think we need to created a separate module for a single jar loading method which we basically already had with the Environment,load_elasticsearch_jars! method. Also, I this it is specifically about the environment: it is basically just a path convention and checking that we are running JRuby.

For the jars dir, first, let's make the default relevant to the logstash environment. Once we agree and where we should store the jars, let's change the defaults to that. For the vendor/ dir, I am actually on the fence about it, since typically you expect the be able to delete the vendor dir and recreate it using some rake tasks or something. On the other hand, this is exactly what the vendor dir is about, to store external dependencies. So I think I'd vote for putting jars into vendor/jar unless we see a potential problem with that?

[purbon]

• About Environment ok, I like the idea to make it there.
• About the directory, I see the point and the regular usage of the vendor dir, as you said I'm on the fence, but find the idea interesting ... so lets use vendor too. Is this something we all like?

[jsvd]

+1 for vendor/jars

[colinsurprenant]

@jsvd did you mean vendor/jar ?

[purbon]

why not vendor/jar-dependencies/runtime ?

[jsvd]

yes. yes I did.

[colinsurprenant]

@purbon vendor/jar-dependencies/runtime wat?

[purbon]

what happen when we have also other dependencies for test? for example.

[purbon]

All related code in here has been adapted to the last review comments, including the dependants PR's on the plugins side.

[jsvd]

[purbon]

@jsvd I agree I should add them, but before I wanted us to agree on the API, although it seems there is no concern with it.

[purbon]

@jsvd simple unit test suite added.

[jsvd]

In the plugins we do:

ROOT_DIR = File.expand_path(File.join(File.dirname(__FILE__), "..")) LogStash::Environment.load_runtime_jars! File.join(ROOT_DIR, "vendor")

But that's a lot of unecessary code we need on each plugin. At most, we need LogStash::Environment.load_runtime_jars! File.dirname(__FILE__) and even that we could somehow find by ourselves.

If we reduce this to just LogStash::Environment.load_runtime_jars!, we no longer need this extra file and can just include the require call in the plugin code. thoughts?

[purbon]

I agree with you in part, however I did it this way in order to provide more flexibility, keep in mind now this file has a ROOT_DIR just one level below the file that load the jars, but it can happen to be in another location. Or that for some jars, we aim to store them in another place.

In my opinion, even if It might make sense, I find it too restrictive.

[jsvd]

I understand the flexibility argument, but don't really see a practical use case for it. The jars should be in a predetermined folder, and all .jar files we find in that folder and its subfolders get loaded.

[jsvd]

After discussing with @purbon, let's go with the current strategy. The amount of code plugins need to load their jars should either be a one liner or it could be detected automatically. However this requires a different strategy, so I've opened an issue to deal with this: #3005

[purbon]

Good @jsvd, this is a good discussion :-)

[ph]

double dragon quotes +1

[ph]

So if I understand correctly this code, this will load all the jars in the vendor directory?
Even if we don't actually use the plugin?

[purbon]

@ph it load the plugins whenever this functions are used, in the current case from a file within each plugins, so no, it will load the jars whenever the plugins are used for first time.

[purbon]

@ph can you make sure the code I took from here related to maven makes sense?

[ph]

Well, in my current checkout this PR breaks logstash-*-kafka

~/e/logstash git:master ❯❯❯ bin/plugin install logstash-input-kafka ⬆ ✱ Validating logstash-input-kafka Installing logstash-input-kafka Installation successful ~/e/logstash git:master ❯❯❯ tree vendor/bundle/jruby/1.9/gems/logstash-input-kafka-0.1.12 ⬆ ✱ vendor/bundle/jruby/1.9/gems/logstash-input-kafka-0.1.12 ├── CONTRIBUTORS ├── DEVELOPER.md ├── Gemfile ├── LICENSE ├── README.md ├── Rakefile ├── lib │   └── logstash │   └── inputs │   └── kafka.rb ├── logstash-input-kafka.gemspec └── spec └── inputs └── kafka_spec.rb 5 directories, 9 files 

[purbon]

@ph good you have test with kafka now, might be is because i removed the maven code, because I tested before that and it was working ... see thanks for the review man

[purbon]

@ph I checked with the change and with master and I see the same number of files for the kafka input, I see no change in this plugin ... I must say there was actually no change with this plugin for this PR
... If this is an issue, I see it as something happening not because of this PR.

[ph]

Did a test with this PR, Jars are in jruby-kafka.

LGTM.

[purbon]

ok, merging this in

[elasticsearch-bot]

Merged sucessfully into master 1.5!

[codekitchen]

Related to this change, it looks like the documentation describing how to add jar dependencies to a logstash plugin is now out of date: https://www.elastic.co/guide/en/logstash/current/_how_to_write_a_logstash_input_plugin.html#_jar_dependencies

Is there guidance on the new correct way of doing things? I'm trying to update my plugin https://github.com/codekitchen/logstash-input-kinesis to work with logstash 1.5.1, but I'm not clear on how to get my jars properly vendored now. Thanks.

[purbon]

@codekitchen you are right, documentation should be updated. Do you mind opening an issue for that?

On the other side, you can see how to add you own jars if you take a look for example at https://github.com/logstash-plugins/logstash-input-log4j/tree/master/lib , there you will see a final named logstash-input-log4j_jars.rb who has the code necessary to load the jars.

Jars are then placed into the gem, in the case of the logstash-input-log4j you can find them at https://github.com/logstash-plugins/logstash-input-log4j/tree/master/vendor/jar-dependencies/runtime-jars . You can apply the same pattern in your plugin.

Makes sense?

/cheers

[codekitchen]

Thanks @purbon I filed #3455

I very rarely work with the JVM, so it actually took me a little while to figure out a good method for getting the jars I need into vendor -- I found it a lot more difficult than the previous jar-dependencies based method where everything just seemed to work based on a gemspec declaration. I also found no scripts or clues in the other plugins such as elasticsearch or log4j, which surprised me a bit. How were those jars vendored?

Eventually I ended up getting it working by adding a pom.xml file to my project and using mvn to vendor the jars, in codekitchen/logstash-input-kinesis@816a106 , but I have no idea if that's the best option.

[purbon]

@codekitchen good you figure it out, basically nowadays we just drop all need jars in a folder, even if not the best solution, jar-dependencies was causing us some random hassle during plugin management and installation. We're working with the maintainer of the gem to address this issues sooner or later and go back to it as it's clear a better way to handle jar dependencies.

In the meanwhile this is a necessary pain point to handle plugins who need jars. In a nutshell you should grab all jars you need and put them in the runtime folder.

Thanks a lot for filling #3455, this would help our documentation efforts.