Contribution of plugin logstash-output-loginsight

[ellieayla]

I have authored and want to contribute https://github.com/alanjcastonguay/logstash-output-loginsight

CLA was signed. Tests pass locally with rspec. This is my first attempt at writing ruby, so it definitely needs another set of eyes.

[ellieayla]

@andrewvc How do the tests look here?

[andrewvc]

This is awesome. /cc @acchen97 & @suyograo WDYT about adding this to the logstash-plugins org after some changes are made?

First, I want to say this is very clean code for someone writing ruby the very first time.

I do see some changes that need to be made.

The main one is that we're trying to move all outputs away from stud::buffer. The good news is that this would primarily mean deleting code for you.

Instead of def receive(event) you'll want def receive(events) which gives you an array. Logstash as of 2.2+ now does its own batching. This is critical when used with the persistent queue, because if an output does its own buffering the PQ cannot make any guarantees.

The second change I would make is moving to use the http client mixin. This is a standard interface for inputs/filters/outputs that will create a manticore client for you with all sorts of esoteric options baked in. See the source of https://github.com/logstash-plugins/logstash-output-http for an example.

Looking forward to seeing this plugin move forward . Thanks for the excellent work @alanjcastonguay

[andrewvc]

@alanjcastonguay you may even consider making your output a subclass of the HTTP output which does a lot of work for you. Does that make sense? You'd only need to add the logic required to modify the HTTP request for loginsight.

[acchen97]

@alanjcastonguay thanks for the great contribution. +1 to the changes @andrewvc mentioned and it's encouraged to proceed with them. We're happy to work with you on getting this plugin moved over and docs created, but it will likely have to wait until closer to mid-June due to priorities.

[PhaedrusTheGreek]

@alanjcastonguay , thanks for the contribution.

[ellieayla]

@andrewvc I appreciate your feedback. The things you brought up are areas I didn't feel comfortable with. This API expects to handle 1-500 events in a batch, but I wasn't sure the best way to drain that many off a queue for submission. And I don't grok subclassing (assuming that's what you mean by mixin) in ruby. Can you provide more specific guidance on each of these (in mid-June)?

[ellieayla]

@andrewvc I took a crack at implementing your suggestions above. I didn't find a way to batch submissions in this style.

Can you take a look at https://github.com/alanjcastonguay/logstash-output-loginsight/tree/subclass-http and let me know if that's in-line with your thinking?

[ellieayla]

@andrewvc I merged those changes and pushed version 0.3.0 to rubygems on 2017-09-14. Have you had a chance to review?