The Wayback Machine - https://web.archive.org/web/20201020031139/https://github.com/google/mtail/pull/266
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

make timestamp() return float64 #266

Open
wants to merge 1 commit into
base: master
from
Open

make timestamp() return float64 #266

wants to merge 1 commit into from

Conversation

@antong
Copy link
Contributor

@antong antong commented Aug 31, 2019

This changes the return type of the built-in timestamp() function to
float64 instead of int64. The unit remains the same, that is seconds
since the Unix epoch. Before this change, timestamps were truncated to
integer seconds, which made it impossible to measure durations less than
a second. Unix seconds as a float64 give better than microsecond
resolution.

Fixes #263

This changes the return type of the built-in timestamp() function to
float64 instead of int64. The unit remains the same, that is seconds
since the Unix epoch. Before this change, timestamps were truncated to
integer seconds, which made it impossible to measure durations less than
a second. Unix seconds as a float64 give better than microsecond
resolution.

Fixes #263
@antong
Copy link
Contributor Author

@antong antong commented Aug 31, 2019

I couldn't figure out how to declare in the golden format that a metric that has no value is a float.

@jaqx0r
Copy link
Contributor

@jaqx0r jaqx0r commented Jun 22, 2020

I appreciate you taking the time to make the PR but I think changing time from int to float isn't the right fix; I think it something that looks like time.Time should be exposed in the mtail language.

@antong
Copy link
Contributor Author

@antong antong commented Jun 25, 2020

Yes, I understand. This PR was more to just open the discussion and experiment what the minimal change that timestamp() returned a float would look like, and to see how backwards compatible it is. And it actually quite does work. Existing programs automatically get improved resolution, and even the tests suite mostly passes (couldn't figure out how to articulate that a metric is a float in the test result "golden" format.)

It clearly would be more elegant if there was a Time type, but I don't see how that could be backwards compatible with existing mtail programs in case that is a requirement. Perhaps by making a timestamp2() function that returns the new type? If there is a Time type, it might also benefit from a Duration/Interval type as there is in Go, if we want to distinguish between calendar time and intervals using type safety.

Or, perhaps nanotimestamp() function, that is like timestamp() but returns nanoseconds since epoch instead of seconds, could be an acceptable stopgap for new programs that require higher resolution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.