-
Notifications
You must be signed in to change notification settings - Fork 23
Log a warning when the JSON parsing fails #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -53,7 +53,8 @@ def encode(event) | |||
# from_json_parse uses the Event#from_json method to deserialize and directly produce events | |||
def from_json_parse(json, &block) | |||
LogStash::Event.from_json(json).each { |event| yield event } | |||
rescue LogStash::Json::ParserError | |||
rescue LogStash::Json::ParserError => e | |||
@logger.error("JSON parse error, original data now in message field", :error => e, :data => json) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this condition is automatically recovered by Logstash, I don't think logging it as an error is the right choice. Maybe info? or warn?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. I'll stick to a warning since that matches the default log level. Hiding what should be obvious signs of trouble behind a --verbose
invocation seems ill-advised.
0543987
to
54dbff8
Compare
This mirrors what the json codec already does.
54dbff8
to
90d1ab4
Compare
Updated according to your suggestion @jordansissel. Please have another look. |
LGTM |
Jordan Sissel merged this into the following branches!
|
This mirrors what the json codec already does. Fixes logstash-plugins#25
This mirrors what the json codec already does. Fixes #21.