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 plugin log time format if format is JSON #2356

Merged
merged 2 commits into from
Apr 1, 2019
Merged

Fix plugin log time format if format is JSON #2356

merged 2 commits into from
Apr 1, 2019

Conversation

ods94065
Copy link
Contributor

Fixes #2355

The time format for a plugin logger was out of sync with the parent logger, and the delegation of #time_format and #time_format= made it impossible to fix the plugin logger's time format without hacks.

This problem was not noticeable if the log format was text, because time formatting was delegated via #caller_line to the parent logger. However, in the case of JSON output, the plugin logger's #format_time method was called, yielding incorrectly-formatted timestamps.

Added tests for both text and JSON, and added tests for format changes in general while I was in there.

The time format for a plugin logger was out of sync with the parent
logger, and the delegation of #time_format and #time_format= made it
impossible to fix the plugin logger's time format without hacks.

This problem was not noticeable if the log format was text, because
time formatting was delegated via #caller_line to the parent logger.
However, in the case of JSON output, the plugin logger's #format_time
method was called, yielding incorrectly-formatted timestamps.

Added tests for both text and JSON, and added tests for format changes
in general while I was in there.

Signed-off-by: Owen Smith <[email protected]>
@repeatedly
Copy link
Member

Thanks for the patch. Will check.

@repeatedly repeatedly self-assigned this Mar 29, 2019
@repeatedly repeatedly merged commit 0e37e68 into fluent:master Apr 1, 2019
@repeatedly
Copy link
Member

I confirmed this patch works. Thanks!

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.

Plugin log time format not in sync with system config
2 participants