search

magnusbaeck / logstash-filter-verifier

Apache License 2.0
192 stars 27 forks source link

Support for logstash 5.0 #8

Closed matejzero closed 6 years ago

matejzero commented 7 years ago

Hello,

I tried running your verifier on Logstash 5.0, since I wan't to migrate to LS 5.0, but there were some problems.

Some long parameters have changed and as a result, starting logstash-filter-verifier fails to start.

I made it work on my end by changing parameters, that logstash-filter-verifier calls in process.go file:

All, except settings path are quickly fixable and shouldn't break the app, but for settings path, we would need to check the version of logstash. I'm not sure how to do it the right way. Running logstash with --version takes too much time. Maybe looking in logstash-core/lib/logstash-core/version.rb for version?

magnusbaeck commented 7 years ago

Thanks for trying out LFV with Logstash 5.0! I haven't had the time to do it but I suspected LFV wouldn't work out of the box. Looking at version.rb would probably be doable but feels a bit fragile. Could we check for the presence of /etc/logstash/logstash.yml (or whatever the file is called) and offer a command line option override?

matejzero commented 7 years ago

I've been loving LFV since the day I discovered it and can't imagine keeping logstash and all the filters working without it:)

Your idea is probably better. So in case, it doesn't find the file or there is no override option set, it will assume we are using LS<5.0? That will probably work and be less fragile, since not everybody install logstash to default path.

With your idea, we also have an option to pick default logstash path, so we cover that too.

choffee commented 7 years ago

Maybe the next release could support V5 by default and add a command line option for versions less than 5?

matejzero commented 7 years ago

That could work too...

Also, option to specify location of logstash.yml would also be useful in case, logstash in installed in some other folder (Windows install? other-than-linux-os? Not sure if those are supported by LFV).

breml commented 7 years ago

While looking into #25, I found that in my case it was enough to rename --log to -l and --config to -f. The nice thing about this is, that both short options (-l and -f) are also supported by Logstash v2.4.1 (and I guess also older versions of Logstash). This means, we could achieve Logstash 5.x compatibility quite easily without breaking backwards compatibility and without checking / parsing version.rb.

matejzero commented 7 years ago

Yea, I noticed today that --path.settings is not necessary anymore. I know that when 5.0.0 was released, path.settings was a must.

I'm off work now, but will do some testing tomorrow or later today and report back.

magnusbaeck commented 7 years ago

Judging by a report at https://discuss.elastic.co/t/logstash-filter-verifier-any-working-example/41880/21 this is still a problem, even with Logstash 5.2.

breml commented 7 years ago

@magnusbaeck I tested this again and I it works if I use Logstash (v5.2.1) distributed as .tar.gz. In this case, there is a config folder in the application root and I guess, that logstash picks this folder as a default if --path.settings is not provided.

Could it therefore be a problem with the deb or rpm versions of Logstash, because the binaries and the configuration are in different directory trees and Logstash is in this case not able to find the configuration files?

magnusbaeck commented 7 years ago

Could it therefore be a problem with the deb or rpm versions of Logstash, because the binaries and the configuration are in different directory trees and Logstash is in this case not able to find the configuration files?

Yes, that's probably exactly what's going on.

breml commented 7 years ago

So in this case, the question is, if it is the responsibility of LFV to handle this situation?

One possible solution I could think of would be to offer a flag like --logstash-args, which would be then be added to the flags used to start Logstash.

breml commented 7 years ago

As we are in the process of the migration to Logstash 5.x, I looked into the related issues.

Starting with version 5.x, the version flags (-V or --version) are handled by the bin/logstash shell script (on *nix systems), without starting the (slow) jvm Logstash process (see: https://github.com/elastic/logstash/blob/v5.0.0/bin/logstash#L49-L56). This is obviously very fast. To evaluate the version, the command is relying on the file ${LOGSTASH_HOME}/logstash-core/lib/logstash/version.rb. This file does not exist in Logstash < 5.0, so the absence of this file could be used as an indicator for the Logstash version.

An other way to retrieve the version of Logstash would be to process the Gemfile.*.lock and query the version the logstash-core gem, which corresponds to the Logstash version.

Example command:

cat Gemfile.*.lock | sed -ne 's/^\s*logstash-core [(]= \([^)]*\)[)]/\1/p'^

@magnusbaeck What is your current state of thinking regarding Logstash 5.x compatibility for LFV?

magnusbaeck commented 7 years ago

What is your current state of thinking regarding Logstash 5.x compatibility for LFV?

I'd prefer using official interfaces for determining the version, i.e. using the --version flag and parsing the output, but with an LFV flag for explicitly setting the version. That way pre-5.0 users have the option to avoid the version autodetection and have the same startup time as today. So, getting LFV to work with Logstash 5.x would require something like this: