search

logstash-plugins / logstash-output-gelf

Apache License 2.0
15 stars 18 forks source link

Gelf crashes with numeric "level" fields #10

Closed c10l closed 8 years ago

c10l commented 8 years ago

LogStash::Outputs::Gelf tries to convert word-based syslog levels to numbers. If the level being parsed is already a number, the class crashes with a NoMethodError because Fixnum doesn't have a .downcase method.

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

untergeek commented 8 years ago

Can you add a test in the spec directory to test for this functionality, please?

Thanks for signing the SLA.

c10l commented 8 years ago

Sure, I'll look into it. I have a question though. Why do you mutate a field on the output? Shouldn't that be left for the user to decide?

The GELF specs don't seem to enforce any format for this field. At least I was unable to find it but I might be looking in the wrong place. :smile:

untergeek commented 8 years ago

I wish I could comment on that directly, but I did not write the original code. I can only guess at the original author's intent. You're the first person to mention it, to my knowledge.

c10l commented 8 years ago

Fair enough. It's not bothering me but I can't help but wonder. It might bother others though, if they rely on word-based severities for aggregations, filters and whatnot.

Anyway, I have added the spec. Let me know if I should change anything.

suyograo commented 8 years ago

Fixed via https://github.com/logstash-plugins/logstash-output-gelf/pull/9. Thanks @cassianoleal

c10l commented 8 years ago

:+1: Glad this was solved! :smile: