Be consistent with how we handle configuration options

[purbon]

This issue is open to propose a unification process toward the way we handle certain options in plugins, I'm going to focus for now only on host and port options, but I'm sure there are a few others that might benefit from it.

In the case of host connection management, we got different behaviour:

• Elasticsearch Output, moved to a unifed hosts option, see https://github.com/logstash-plugins/logstash-output-elasticsearch/blob/master/lib/logstash/outputs/elasticsearch/common_configs.rb#L82 for more details.
• Redis Output, has something similar, (https://github.com/logstash-plugins/logstash-output-redis/blob/master/lib/logstash/outputs/redis.rb#L36) , but include the option to have a default port (https://github.com/logstash-plugins/logstash-output-redis/blob/master/lib/logstash/outputs/redis.rb#L42) that can be overriten by hostname.
• Redis input, has a host and port options.
• Kafka Input, got a hosts option (https://github.com/logstash-plugins/logstash-input-kafka/blob/master/lib/logstash/inputs/kafka.rb#L34, named zk_connect ) but that is string and not an array.
• TCP Input, gets a host and port options.
• Zabbix Output, got a host and port options, (https://github.com/logstash-plugins/logstash-output-zabbix/blob/master/lib/logstash/outputs/zabbix.rb#L48)

this is an small subset of plugins, just trying to show the situation

This give user a contradictory experience as most of people expect to have similar behaviour across logstash plugins, the proposal here is to:

• See what options can be make consistent.
• Unify the common options across the set of plugins, we can start with host and and port option to make the interface consistent.
• Provide a common class, that might be living in core, or in a plugin mixin, to encapsulate this behaviour across plugins.
• Update the plugins to use this common helper.

What do you think?

Related to: logstash-plugins/logstash-input-redis#24 logstash-plugins/logstash-input-redis#25

[LucaWintergerst]

I fully agree with Pere.
Different configuration options take up a lot of time, since you often have to look up the way to configure the plugins.
Log output in case of problems is often misleading at first as well.

In case of a redis input, you even have to use a second input if you want to use a different port. Using the way it is done on the output would solve this problem (host => 127.0.0.1:6379 instead of host => 127.0.0.1 port => 6379)

[colinsurprenant]

related to #3773 and #2151

[purbon]

Good additions,
my thoughts goes now to the minimal necessary changes to start us having
this inconsistencies being removed. Do you have a proposal? or the points I
said in the issue description are good for you?

On Thu, Apr 14, 2016 at 4:09 PM Colin Surprenant [email protected]
wrote:

related to #3773 #3773 and
#2151 #2151

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#5117 (comment)

[guyboertje]

👍
With the redis input though, if you need two connections then you need two connections regardless of the config format.

[colinsurprenant]

@purbon I pointed out #2151 because it also identifies options inconsistencies related to ssl, so you can add that to your: See what options can be make consistent. action item.

I pointed out #3773 because you suggested to: Provide a common class, that might be living in core, or in a plugin mixin, to encapsulate this behaviour across plugins. Since this infer a refactor of options handling, there has already be some thoughts about that in #3773 and maybe it is worth looking at this from a little bit higher level perspective and see if we should move in that direction and/or go ahead with what was proposed?

[guyboertje]

After rereading the links from @colinsurprenant - I see we spoke about Java config declarations. I recently did some preliminary work on feasibility of a pure Java plugin + JRuby wrapper - I discovered Typesafe/Lightbend config java lib - this issue shows more, config lib readme