Add TCP protocol for GELF

logstash-plugins / logstash-input-gelf

Apache License 2.0

20 stars 39 forks source link

Add TCP protocol for GELF #33

Closed edefaria closed 7 years ago

edefaria commented 8 years ago

Add TCP protocol support.

Based on #6,

elasticsearch-release commented 8 years ago

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

suyograo commented 8 years ago

@edefaria thanks for this! can you please add some tests?

jordansissel commented 8 years ago

Are there docs on the TCP protocol for GELF? I don't see any on the gelf site.

edefaria commented 8 years ago

Gelf TCP is the same as UDP. The only thing is you need a delimit for your payload, by default on graylog/gelf is nul '\0'. Also Graylog GELF TCP does not support compression, so most of the GELF library over TCP disable compression for TCP.

I will see to add test and new api support.

ddelnano commented 8 years ago

Is there a timetable for if / when this will be merged?? Is there anything left to do, wouldn't mind helping if its needed.

jordansissel commented 7 years ago

@ddelnano It could help if some folks helped with any of the following:

Test this in production/staging environments
Evaluate it for edge cases
Check that the new features are supported by new tests
Check that the code is understandable, and ask questions in places where it is not.
Check that the patch doesn't contain unrelated changes to the proposal
For new features, do you agree that the setting names are the right names to choose? The right validation?
For new features, are there docs that accompany it? Are the docs clear to you?

ddelnano commented 7 years ago

@jordansissel since July we have moved away from logstash, I appreciate the response tho.

jordansissel commented 7 years ago

This PR makes a number of changes that seem unrelated to adding TCP support:

Several changelog, readme, etc changes.
Many new specs that seem to cover things other than TCP support

I'm not sure what to make of this.

olqs commented 7 years ago

What's the status of this PR? If it gets merged in the official plugin we don't have to build a custom one for us.

acchen97 commented 7 years ago

Anyone who is using this plugin extensively want to step up and volunteer to help maintain this plugin?

https://discuss.elastic.co/t/logstash-plugins-community-maintainers/35953

jordansissel commented 7 years ago

This PR's been idle for some time. I'm going to close it because I don't know how to get clarity on the proposed change as compared to the patch provided.

We can still add TCP support for GELF input, but for whoever does, please include rspec tests, some indication of what product(s) you used to test it.