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

proposal: time: add UnixMilli and UnixMicro #44412

Closed
ConradIrwin opened this issue Feb 19, 2021 · 2 comments
Closed

proposal: time: add UnixMilli and UnixMicro #44412

ConradIrwin opened this issue Feb 19, 2021 · 2 comments
Labels
Projects
Milestone

Comments

@ConradIrwin
Copy link
Contributor

@ConradIrwin ConradIrwin commented Feb 19, 2021

This is a follow on from #44196.

It is common for go services to interact with external applications that use timestamps in milliseconds and microseconds, and currently conversion between these formats and Go's internal time is fiddly and error prone.

In #44196 we added Time.UnixMillis() and Time.UnixMicros() to allow go programs to convert to milliseconds and microseconds in an obvious, easy way. I propose we add the symmetrical methods to allow go programs to convert from these formats.

Correctly implementing the conversion from milliseconds to time is something people struggle with. Assuming that you only care about the range supported by UnixNano, and you have read and understood the documentation of the Unix(sec, nsec), you know that you could do Unix(0, msec*1e6); but it's common to see time.Unix(msec/1e3, (msec%1e3)*1e6) (in cases where the extended range really doesn't matter), or even time.Unix(msec/1e3, 0) (where simplicity was considered more important than precision). I also found an incorrect conversion (time.Unix(ms/1000, (ms%1000)*1000)) that made it past code-review.

Instead of pushing the burden for figuring out to end-users of the language, I propose we provide a robust implementation of time.UnixMilli with a simple API.

For symmetry with Time.UnixMicro I propose we also add a time.UnixMicro that does the same.

// UnixMilli returns the local Time corresponding to the given Unix time,
// msec milliseconds since January 1, 1970 UTC.
func UnixMilli(msec int64) Time {
       if msec%1e3 < 0 {
               return unixTime(msec/1e3-1, int32((msec%1e3)*1e6)+1e9)
       }
       return unixTime(msec/1e3, int32((msec%1e3)*1e6))
}

// UnixMicro returns the local Time corresponding to the given Unix time,
// usec milliseconds since January 1, 1970 UTC.
func UnixMicro(usec int64) Time {
       if usec%1e6 < 0 {
               return unixTime(usec/1e6-1, int32((usec%1e6)*1e3)+1e9)
       }
       return unixTime(usec/1e6, int32((usec%1e6)*1e3))
}

(NOTE: The current package has time.Unix which does both conversion from a unix timestamp in seconds (by passing a 0 as the second argument), or for conversion from a unix timestamp in nanoseconds (by passing a 0 as the first argument). It would be more consistent, and subjectively simpler, if we added time.UnixNano and deprecated passing values outside of the range of 0-999999999 to the nsec parameter of time.Unix(), but I am not proposing we make that change as I don't know of a compelling reason to beyond "it would be nice")

@gopherbot gopherbot added this to the Proposal milestone Feb 19, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Feb 19, 2021
@dtmirizzi
Copy link

@dtmirizzi dtmirizzi commented Feb 19, 2021

Awesome! I was just thinking this while playing with the Atlassian API (https://developer.atlassian.com/cloud/confluence/rest/api-group-audit/#api-api-audit-get)

Thanks for all the work 🎉 🎉 🎉

@ConradIrwin
Copy link
Contributor Author

@ConradIrwin ConradIrwin commented Mar 25, 2021

Closing this now #44196 was approvied with both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants