[usage] Round workspace runtime

[easyCZ]

Description

As requested in #10583 (review).

Related Issue(s)

Fixes #

How to test

Unit tests

Release Notes

NONE

Documentation

NONE

[easyCZ]

I personally think that rounding at this layer is losing precision for no good reason. The rounding can always happen before it's inserted into the DB, or before it's synced into stripe but the report should remain as accurate as possible. It is an intermediate representation.

[jankeromnes]

Thought: This could even be uint64 right? (I don't think we want to support "negative usage time", so we can double our max representable value 🙌)

[geropl]

If this really is only about "usage"/"runtime", I agree. If we want to represent "credits" here (which I'm not sure we want/should..?) then int64 it is.
Another thought: Not sure how (type) safe uint64/int64 interactions are in Go: Is it easy to mess it up? If yes, that might be a good argument too keep one type everywhere (most likely in64 if we need the "positive" side)

[jankeromnes]

Thanks again!

I personally think that rounding at this layer is losing precision for no good reason. The rounding can always happen before it's inserted into the DB, or before it's synced into stripe but the report should remain as accurate as possible. It is an intermediate representation.

Thanks for voicing these concerns. I do understand your point, however I still believe that the extra readability and sanity of integer values for seconds outweighs any benefits given by sub-second precision. Still, if we really do want sub-second precision one day, I think it will be easy to adjust. 😊

Approving, but holding due to one optional suggestion.

/hold

[geropl]

I do understand your point, however I still believe that the extra readability and sanity of integer values for seconds outweighs any benefits given by sub-second precision

My 2 cents:

• we should definitely store time durations in integer values (upvoting the uint64 comment above!)
• whether we want to store ms vs s:
  • for precisions/consistencies sake it's sometimes beneficial to keep whatever we get as input (ms here), e.g. if we need to be able to exactly identify time-range in the opposite direction (usage -> workspace instance)
  • but as all clients are not interested in anything below s precision, I think it's a great way to keep things simple, and use the same datatype everywhere 🧘

[jankeromnes]

Since Milan is on holidays, let's merge this as is and address the review comments in follow-up PRs.

/unhold