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

Switch from FastDateFormat for higher precision timestamps #364

Closed
kyrias opened this issue Oct 8, 2019 · 4 comments
Closed

Switch from FastDateFormat for higher precision timestamps #364

kyrias opened this issue Oct 8, 2019 · 4 comments

Comments

@kyrias
Copy link

kyrias commented Oct 8, 2019

Currently logstash-logback-encoder uses FastDateFormat to generate the timestamps, but it is limited to millisecond precision. Java 8 added java.time which has nanosecond precision. Since logstash-logback-encoder >= 6 requires Java 8, would it be possible to switch to java.time.Instant for generating the timestamps instead?

@Jimmy-653
Copy link

Sorry, I don't know how to like PR's to Issues, but take a look at #365

@kyrias
Copy link
Author

kyrias commented Oct 9, 2019

Nice, though I don't have enough Java experience or knowledge of the code to be able to really review it.

Though I guess LOGBACK-1374 would be necessary as well, since I hadn't noticed that the timestamp is gotten from ILoggingEvent which returns the timestamp in milliseconds.

@philsttr
Copy link
Collaborator

Thanks for the issue report @kyrias and PR @J-Jimmy !

I 100% agree with replacing FastDateFormat with DateTimeFormatter. In addition to supporting higher precision timestamps, it has the added benefit of removing a dependency on commons-lang3.

As you mentioned @kyrias , higher-precision timestamps won't be supported until logback supports them, but we can go ahead and make the necessary changes in logstash-logback-encoder in preparation.

I've added some review comments on the PR that need to be addressed before I can merge the PR.

Also, this change will require a major version bump of logstash-logback-encoder. So I might hold off on it for a bit. There's no real immediate need for it since logback doesn't support higher-precision timestamps yet.

philsttr added a commit that referenced this issue Nov 23, 2019
Remove dependency on apache commons-lang3 (since FastDateFormat was the only thing being used).

Switch default timestamp format to DateTimeFormatter.ISO_OFFSET_DATE_TIME

Added the ability to specify DateTimeFormatter constant names as patterns.

This change is NOT fully backwards compatible, since:
* the default timestamp pattern changed to DateTimeFormatter.ISO_OFFSET_DATE_TIME
* the pattern strings supported by DateTimeFormatter are not exactly the same as those supported by FastDateFormat
@philsttr
Copy link
Collaborator

Closing this in favor of #377

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants