grpc-java
grpc-java copied to clipboard
Channelz should use milliseconds for timestamps
#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 of TimeProvider, and they all convert the time to proto Timestamp which doesn't mandate the unit. It will only be misleading to keep TimeProvider.currentTimeNanos() as is. It should be changed to currentTimeMillis().
I'm going to disagree a little here. Two reasons:
- When we (eventually) move to Java 8, the time API does provide nanosecond precise timestamp
- Keeping all timestamps in gRPC as the same unit simplifies reading the code, and no conversions are necessary. Nanoseconds everywhere (except on API boundaries) are the most reasonable choice.
One more factor to consider: the channelz service uses timestamp.proto, so when you create a timestamp message, you have to use its nano precision API, and if you have a millis variable, you still have to cast it to nano at some point.
Started Analysis and going through previous comments/details.
I agree with @dapengzhang0's last comment. It is more convenient to use nanos to create timestamp.proto timestamps. We can now change SYSTEM_TIME_PROVIDER to use Java 8 Instant.getNano() to get nano precision. I didn't find a way to get the current epoch nanos directly, so you'd still have to combine the seconds part and the nanos part from Instant.
It is more convenient to use nanos to create timestamp.proto timestamps
The TimeProvider API is already using nanos, so there's no change needed here. It's just what resolution the actual values use.