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

updateTiKVSendReqHistogram is always called with nil resp #558

Closed
zyguan opened this issue Jul 27, 2022 · 2 comments
Closed

updateTiKVSendReqHistogram is always called with nil resp #558

zyguan opened this issue Jul 27, 2022 · 2 comments

Comments

@zyguan
Copy link
Contributor

zyguan commented Jul 27, 2022

Here we try to pass resp to updateTiKVSendReqHistogram, however we never assign value to the resp since we always return directly in sendRequest (try this). As a result, metrics like TiKVRPCNetLatencyHistogram is actully unavailable.

@sticnarf
Copy link
Collaborator

To my understanding, the key is not about whether we assign to resp... Whether we use a closure matters.

Use a closure but no assignment: https://go.dev/play/p/pnM8dhPd4qj

Assign the return variable but don't use a closure: https://go.dev/play/p/YExnvOLOBF2

@zyguan
Copy link
Contributor Author

zyguan commented Jul 28, 2022

Ah, I see, sorry for bothering.

@zyguan zyguan closed this as completed Jul 28, 2022
zyguan added a commit to zyguan/client-go that referenced this issue Jul 28, 2022
sticnarf pushed a commit that referenced this issue Jul 29, 2022
* support adding `ExecDetailsV2` to tracing

Signed-off-by: zyguan <[email protected]>

* rename `TraceExecEnabled` to `TraceExecDetailsEnabled`

Signed-off-by: zyguan <[email protected]>

* revert changes for #558

Signed-off-by: zyguan <[email protected]>

* address comments

Signed-off-by: zyguan <[email protected]>

* add unit test

Signed-off-by: zyguan <[email protected]>
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

No branches or pull requests

2 participants