search

logstash-plugins / logstash-input-snmptrap

Apache License 2.0
12 stars 20 forks source link

explict convert gbk chartset to utf-8,resolved chinese character disp… #20

Closed laywin closed 7 years ago

laywin commented 7 years ago

problem detail please see https://discuss.elastic.co/t/logstash-snmptrap-chinese-incorrect-codes/69509

karmi commented 7 years ago

Hi @laywin, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

laywin commented 7 years ago

@karmi ok!

laywin commented 7 years ago

@karmi have aready added,please try again.

andrewvc commented 7 years ago

@laywin looks like it passed the CLA check! I'll review your PR this week.

andrewvc commented 7 years ago

@laywin , this patch would convert all UTF-8 to gbk. We definitely don't want that. @jordansissel what is your preferred approach here? Should we have a new encoding option that lets users deal with non-utf encodings?

andrewvc commented 7 years ago

This should probably use the same approach as the plain-codec uses here: https://github.com/logstash-plugins/logstash-codec-plain/blob/master/lib/logstash/codecs/plain.rb#L25

laywin commented 7 years ago

@andrewvc,yeah,but i tested codec,set charset to utf-8 or gbk,it doesn't work.

andrewvc commented 7 years ago

@laywin I think you misunderstand. I'm asking that you amend your patch to add a charset option directly to the plugin. A codec won't be used by the snmptrap plugin because snmp data is already structured. I was merely referencing the plain codec because its code could be copied directly into this plugin.

andrewvc commented 7 years ago

Closing this PR due to inactivity.