-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
server: populate WireLength on stats.InPayload for unary RPCs #2932
server: populate WireLength on stats.InPayload for unary RPCs #2932
Conversation
Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement. After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.
Regards, |
de0849b
to
1a729ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. The change looks good.
For the test, can you see if it can be done in stats_test.go instead?
Maybe a WireLength check here for in payload and here for out payload
1a729ca
to
3e986df
Compare
Done. Much simpler. Any hope we could backport this and #2711 to 1.21? |
We can do a backport for this change. Isn't #2711 already in 1.21? |
Yes, sorry, I've been getting my versions mixed up. I was looking at 1.18 but if we get this into 1.21 I'm happy. I'll make the PR. |
Fixes #2692 which was incompletely fixed by #2711.
Also adds a basic unit test which provides scaffolding for further testing of
the stats logic.