Skip to content
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

Fix bug in suppress_emit_error_log_interval #2069

Merged

Conversation

patrobinson
Copy link
Contributor

Issue

When setting --emit-error-log-interval command line parameter, fluent instead prints the following error which masks other errors:

fluentd_1       | 2018-07-12 23:18:26 +0000 [error]: #0 failed to emit fluentd's log event tag="fluent.error" event={"host"=>"172.20.0.1", "port"=>42300, "error"=>"#<ArgumentError: comparison of Time with 1531437277 failed>", "message"=>"unexpected error on reading data host=\"172.20.0.1\" port=42300 error_class=ArgumentError error=\"comparison of Time with 1531437277 failed\"", "time"=>"2018-07-12T23:18:26Z"} error_class=ArgumentError error="comparison of Time with 1531437277 failed"

Fix

Ensure time is in epoch format before comparison
Add tests

@patrobinson patrobinson force-pushed the fix-emit-error-log-interval branch from c25393d to 1835090 Compare July 13, 2018 02:09
Ensure time is in epoch format before comparison

Signed-off-by: Patrick Robinson <[email protected]>
@patrobinson patrobinson force-pushed the fix-emit-error-log-interval branch from 1835090 to 4bfbf9a Compare July 14, 2018 10:38
@patrobinson
Copy link
Contributor Author

I'm not sure why the Windows tests are failing, it doesn't seem to have anything to do with my changes

@repeatedly
Copy link
Member

Yes. Test failure is caused by appveyor's unstable execution.

@repeatedly repeatedly merged commit d2137c2 into fluent:master Jul 17, 2018
@repeatedly
Copy link
Member

Thanks for the patch!

@patrobinson patrobinson deleted the fix-emit-error-log-interval branch July 17, 2018 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants