Conversation
e895070 to
1bf1615
Compare
1bf1615 to
82b19f0
Compare
olavloite
left a comment
There was a problem hiding this comment.
Do we have any way to test this? There are a lot of corner cases where we are retrying the query stream etc., and it is very hard to tell from just looking at the code that we actually cover all corner cases, and that it actually records the latency that we want it to record.
| } | ||
| } | ||
| defer func() { | ||
| if mt.method != "" { |
There was a problem hiding this comment.
This could use a comment. Does mt.method != "" mean that the operation completed? Why?
| } | ||
| statusCode, _ := convertToGrpcStatusErr(r.err) | ||
| mt.currOp.setStatus(statusCode.String()) | ||
| recordOperationCompletion(&mt) |
There was a problem hiding this comment.
This seems to indicate that we will record the latency of the last attempt that finished as the operation latency. That means that an operation that is retried will not contain the entire latency, but only the latency of the last attempt. Is that intentional? If so, could we document that in code to clarify?
There was a problem hiding this comment.
Actually the currOp start time is initalised in first line of Next() and in recordOperationCompletion we capture to total time including multiple attempts latencies
There was a problem hiding this comment.
Ack. I see now that there is a separate field for the operation start time.
…otal latency till decoding of protos.
82b19f0 to
d9edbe3
Compare
df2c835 to
fea6538
Compare
fea6538 to
2404e56
Compare
No description provided.