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

Timestamp is formatting milliseconds incorrectly #200

Closed
pcasaes opened this issue Nov 22, 2023 · 6 comments · Fixed by #204
Closed

Timestamp is formatting milliseconds incorrectly #200

pcasaes opened this issue Nov 22, 2023 · 6 comments · Fixed by #204
Assignees

Comments

@pcasaes
Copy link

pcasaes commented Nov 22, 2023

Describe the bug

When converting the unix timestamp in milliseconds 1700617928023 to ISO we are seeing
2023-11-22T01:52:08.23Z

But it should be
2023-11-22T01:52:08.023Z

To Reproduce
Steps to reproduce the behavior:

From the code in dateTimeNowString

   val sdf = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'.'Szzz", Locale.ROOT)
    val utc = TimeZone.getTimeZone("UTC");
    sdf.timeZone = utc;
    println(sdf.format(Date(1700617928023L)).replace("UTC", "Z"))

Expected behavior
The output should always present 3 digits for milliseconds.

Platform (please complete the following information):

  • Library Version in use: 1.23.4
  • Platform being tested: backend linux

Additional context
A quick fix would be to use the following pattern yyyy-MM-dd'T'HH:mm:ss'.'SSSzzz

But always instantiating a formatter most likely has performance implications.

Maybe use javax.time

Instant
                        .now()
                        .truncatedTo(ChronoUnit.MILLIS) // this guarantees only 3 decimals places
                        .toString()
@wenxi-zeng
Copy link
Contributor

hi @pcasaes, thanks for reporting this issue. we will take a look. we purposely walked away from javax.time because it requires desugaring to some customers even though they don't need it. we felt it's a bad experience to ask users to add something they don't want just because they use our library. so we'll see if we have other options to fix this.

@wenxi-zeng
Copy link
Contributor

@pcasaes this issue is fixed in 1.14.1. please have a try. we ended up with your quick fix, but made the formatter a property of a singleton to avoid instantiating it every time. this should eliminate the overhead (with trade off of negligible memory of course).

@pcasaes
Copy link
Author

pcasaes commented Dec 1, 2023

Hi, I took a look at the code and I see that the SimpleDateFormat instance is being reused. But that class is notoriously not thread safe. Has this been considered?

https://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html

Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.

@pcasaes
Copy link
Author

pcasaes commented Dec 1, 2023

btw. I miswrote. Instant is from java.time not javax.time.

Instant
                        .now()
                        .truncatedTo(ChronoUnit.MILLIS) // this guarantees only 3 decimals places
                        .toString()

It's been available since Java 8.

@wenxi-zeng
Copy link
Contributor

@pcasaes that's a good call out! no wonder people are staying away from SimpleDateFormat. we will have it fixed asap.

for Instant, please check out here. though Java 8+ APIs are supported, it requires desugaring in order to work in old platforms

@wenxi-zeng
Copy link
Contributor

@pcasaes we made it thread safe in 1.14.2. please have a try.

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 a pull request may close this issue.

2 participants