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

FormatBlock.Date.ISO8601 does not take offset into account #573

Open
megri opened this issue Apr 16, 2024 · 12 comments
Open

FormatBlock.Date.ISO8601 does not take offset into account #573

megri opened this issue Apr 16, 2024 · 12 comments

Comments

@megri
Copy link

megri commented Apr 16, 2024

I just noticed for the first time that my friend outputs the incorrect timestamp.

I'm in UTC+02:00 so at 12:00 UTC my local time is 14:00, and yet this formatter outputs "14:00Z". Basically the offset is not deducted from the timestamp before printing.

@megri megri changed the title FormatBlock.Date.ISO8601 outputs incorrect timestamp FormatBlock.Date.ISO8601 does not take offset into account Apr 16, 2024
@megri
Copy link
Author

megri commented Apr 16, 2024

Noticed two things:

  • This is a transitive issue from perfolation
  • Setting java.util.TimeZone.setDefault(java.util.TimeZone.getTimeZone("UTC")) gives me the desired behaviour, but as a JVM-wide global it's quite scary to set.. 😬

@megri
Copy link
Author

megri commented Apr 17, 2024

A better solution perhaps would be to add the zone offset at the end of the string according to ISO8601-standards 🤔

@darkfrog26
Copy link
Contributor

Can you clarify which log formatting you are using?

@darkfrog26
Copy link
Contributor

Also, are you logging on JVM, JS, or Native?

@megri
Copy link
Author

megri commented Apr 17, 2024

Can you clarify which log formatting you are using?
Also, are you logging on JVM, JS, or Native?

Using FormatBlock.Date.ISO8601on JVM, although I'm assuming the outcome should be identical across all platforms.

@darkfrog26
Copy link
Contributor

It should be, but the implementations are different.

@darkfrog26
Copy link
Contributor

Correct me if I'm wrong here, but the problem isn't Perfolation; it's that I'm hard-coding Z on the end of the formatted stamp when it's local time. Would it be acceptable to just fix the suffix to output the proper timezone? Then, if you want to use UTC you can set the default timezone, or just leave the default and it will be the machine's local zone.

@darkfrog26
Copy link
Contributor

my proposed solution is just to replace "Z" in FormatBlock:74 to be ${l.t.Z}

@darkfrog26
Copy link
Contributor

@megri please let me know if this solution is acceptable to you.

@megri
Copy link
Author

megri commented May 2, 2024

@darkfrog26 I wasn't paying attention to this. I'm sorry. Fixing the output to use the proper timezone would definitely be preferable.

my proposed solution is just to replace "Z" in FormatBlock:74 to be ${l.t.Z}

That would be great. Note I tried reading up on how to properly format this but my reference actually doesn't seem to acknowledge this format. It's definitely better than hard-coding a Z though.

As an aside I tried using ${l.t.z} to get a zone offset — which IS mentioned in the reference — but it seems the implementation is broken as I got output looking like 2024-05-02T14:09:43 +-2-120 [INFO] <empty>.JGit.JGit:37 - this is a test!

@megri
Copy link
Author

megri commented May 2, 2024

This works though:

 val z =
    val ms = l.t.timeZoneOffsetMillis
    val sign = if ms < 0 then "-" else "+"
    val HH = Math.abs(ms) / 3600000
    val MM = Math.abs(ms) / 60000 % 60
    f"$sign$HH%02d$MM%02d"

@darkfrog26
Copy link
Contributor

Hmm, I'll have to take a look and do some testing. Feel free to create a PR here: https://github.com/outr/perfolation if you're interested in helping fix it.

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

No branches or pull requests

2 participants