-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Channelz should use milliseconds for timestamps #5494
Comments
I'm going to disagree a little here. Two reasons:
|
One more factor to consider: the channelz service uses |
Started Analysis and going through previous comments/details. |
I agree with @dapengzhang0's last comment. It is more convenient to use nanos to create |
The TimeProvider API is already using nanos, so there's no change needed here. It's just what resolution the actual values use. |
#4883 changed the
TimeProvider.currentTimeNanos()
's precision to milliseconds, because there isn't a way to get the current time in nanoseconds precision. After #5056 is fixed, Channelz/ChannelTracer will be the only users ofTimeProvider
, and they all convert the time to protoTimestamp
which doesn't mandate the unit. It will only be misleading to keepTimeProvider.currentTimeNanos()
as is. It should be changed tocurrentTimeMillis()
.The text was updated successfully, but these errors were encountered: