Optional queue args: link to RabbitMQ docs instead

[michaelklishin]

The current example for optional arguments [1] is not
great for a number of reasons:

• It suggests something without any explanation as to what the feature is.
• Policies [2] is a far better option operationally for many extra arguments.
  Application-provided arguments are rigid and require queue deletion and redeclaration to change.
• The example suggests classic mirrored queues. They are superseded by Quorum Queues [3].

I suggest including a few external links to relevant RabbitMQ doc guides
instead of a "just copy this" single line advice that can be counterproductive.

1. https://www.rabbitmq.com/queues.html#optional-arguments
2. https://www.rabbitmq.com/parameters.html#policies
3. https://www.rabbitmq.com/quorum-queues.html

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

[michaelklishin]

@jsvd sorry to bother you directly but it's been a week without any feedback :)

[michaelklishin]

@jsvd done. I have updated documentation for a couple settings. I don't see where this plugin would overriden March Hare's default heartbeat timeout and with stock defaults it use 60 seconds instead of "disabled". I verified this from a JRuby REPL with March Hare and Wireshark.
Let me know if you'd like me to post a capture file as evidence.

[jsvd]

I don't see where this plugin would overriden March Hare's default heartbeat timeout and with stock defaults it use 60 seconds instead of "disabled".

Nice catch, so you see it being 60 seconds? the default is set in https://github.com/rabbitmq/rabbitmq-java-client/blob/master/src/main/java/com/rabbitmq/client/ConnectionFactory.java#L66, but it should be overriden with zero in the mixin in https://github.com/logstash-plugins/logstash-mixin-rabbitmq_connection/blob/master/lib/logstash/plugin_mixins/rabbitmq_connection.rb#L105

[michaelklishin]

It is 60 seconds in the Java client and March Hare. I'm not sure why would anyone disable heartbeats by default? I wouldn't.

[michaelklishin]

I'd be happy to submit another PR that changes the default or even push it to this one.

[michaelklishin]

@jsvd any more suggestions? :)

[michaelklishin]

Bump.

[jsvd]

LGTM

[michaelklishin]

@jsvd so what happens now, more reviews?

[michaelklishin]

@jsvd I'd really appreciate if the docs were updated as currently they are recommending things that Team RabbitMQ does not.

[jsvd]

@yaauie with the move of this plugin to an integration repo, please take note of carrying this change over to https://github.com/logstash-plugins/logstash-integration-rabbitmq

[yaauie]

@jsvd I have filed logstash-plugins/logstash-integration-rabbitmq#3 (including absorbing these upstream changes) to enumerate the steps being taken, and added it to the top of my planned work.

@michaelklishin I'm sorry for dropping this. I'l work to ensure that docs build has what it needs to include these changes shortly.