logstash-plugins / logstash-input-gelf

Apache License 2.0
20 stars 39 forks source link

force encoding for source host #23

Closed gmoskovicz closed 6 years ago

gmoskovicz commented 8 years ago

Running tests on windows showed that potentially the encoding for the source host could come with Windows Encoding rather than UTF 8, which caused the configuration to fail.

gmoskovicz commented 8 years ago

Fixes #22

gmoskovicz commented 8 years ago

@ph @purbon @jsvd Can i have a review?

jsvd commented 8 years ago

The fix does solve the problem but I'm trying to find out why https://github.com/elastic/logstash/blob/master/lib/logstash/patches/bugfix_jruby_2558.rb doesn't fix this in the first place, since it was added to core because of this problem: some of the strings windows produces aren't UTF8 If I can't find a proper fix soon at the core level we can just merge this.

gmoskovicz commented 8 years ago

@jsvd makes sense to me!

jsvd commented 8 years ago

Apparently the encoding issue on windows still exists in jruby 1.7.22: https://github.com/jruby/jruby/issues/2558#issuecomment-151102656

The proper way to address this would be through extending the monkeypatch we do in core to process all output of IPSocket.recvfrom calls, but since we have frozen logstash-core, I'll open an issue and let this in :)

LGTM

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'.

jsvd commented 6 years ago

merged in 8b003fe7f2386ab67e1f62f9e7fe4aa5b994cb4c 604c42cb69e1c53467c2f0a22a69429c88b95a46