logstash-plugins / logstash-output-gelf

Apache License 2.0
15 stars 18 forks source link

Handle for cases when level is already numeric #9

Closed ssevertson closed 8 years ago

ssevertson commented 8 years ago

I believe this is related to https://github.com/elastic/logstash/issues/3888, but that issue hasn't been relocated to the correct project yet. My issue was discovered against 1.5.4-1, as provided by the packages.elasticsearch.org repo.

If Logstash is receiving messages where the "level" field is already numeric (from TCP/JSON in my case, from UDP/CollectD in the original issue's case), GELF output fails with the following error:

NoMethodError: undefined method `downcase' for 4:Fixnum
        receive at /opt/logstash/vendor/bundle/jruby/1.9/gems/logstash-output-gelf-1.0.0/lib/logstash/outputs/gelf.rb:200
         handle at /opt/logstash/vendor/bundle/jruby/1.9/gems/logstash-core-1.5.4-java/lib/logstash/outputs/base.rb:88
    output_func at (eval):300
   outputworker at /opt/logstash/vendor/bundle/jruby/1.9/gems/logstash-core-1.5.4-java/lib/logstash/pipeline.rb:244
  start_outputs at /opt/logstash/vendor/bundle/jruby/1.9/gems/logstash-core-1.5.4-java/lib/logstash/pipeline.rb:166

The line in question:

m["level"] = (@level_map[level.downcase] || level).to_i

Fix is to simply check if level.respond_to?(:downcase) before attempting it.

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

bjoernhaeuser commented 8 years ago

Hey @ssevertson,

I have the same problem. Is there any workaround? :-)

Thanks!

jordansissel commented 8 years ago

@ssevertson Thanks for your contribution! I don't see a CLA on file for you, so before we can review the code, can you resolve this? Link here: https://www.elastic.co/contributor-agreement/

ssevertson commented 8 years ago

Just signed the CLA.

aakso commented 8 years ago

Any chance getting this merged?

stdweird commented 8 years ago

:+1: on getting merged/resolved

elasticsearch-bot commented 8 years ago

Suyog Rao merged this into the following branches!

Branch Commits
master 8ea6bd2f1c7c257157e9b40c07f9330d0ed59c08