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

Support microsecond timestamp format #1426

Merged
merged 1 commit into from
Dec 19, 2019
Merged

Conversation

wphan
Copy link

@wphan wphan commented Dec 16, 2019

Which issue(s) this PR fixes:
Fixes #1407

Adds microsecond timestamp support.

grafana#1407
@claassistantio
Copy link

claassistantio commented Dec 16, 2019

CLA assistant check
All committers have signed the CLA.

if err != nil {
return time.Time{}, err
}
return time.Unix(0, i*int64(time.Microsecond)), nil
Copy link
Member

@pstibrany pstibrany Dec 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just use *1000 instead. This has implicit requirement on durations being in nanosecond, while us -> ns is simple *1000 conversion.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the time.Millisecond for the UnixMs case be changed to *1000000 also then? I'm also a bit confused about the implicit requirement here, isn't the time package already based on the notion of durations being in nanosecond?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the time.Millisecond for the UnixMs case be changed to *1000000 also then?

That would make sense.

I'm also a bit confused about the implicit requirement here, isn't the time package already based on the notion of durations being in nanosecond?

Yes it is, and that is the implicit part I'm talking about. Correct expression would be (time.Duration(i) * time.Microsecond).Nanoseconds(), which will work even if time.Duration starts to use seconds one day.

This is just a tiny nitpick. If you leave it as is, I'm fine with it.

@cyriltovena cyriltovena merged commit d627cda into grafana:master Dec 19, 2019
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
Add GCS object client specific timeout
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 this pull request may close these issues.

Support microsecond timestamp format
5 participants