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

Add Trailer to client-side stats.End #2639

Merged
merged 1 commit into from
Mar 13, 2019
Merged

Add Trailer to client-side stats.End #2639

merged 1 commit into from
Mar 13, 2019

Conversation

mklencke
Copy link
Contributor

Currently, it is not possible to access trailers from within a
stats.Handler. The reason is that both stats.Handler and
ClientStream.Trailer require a lock on the ClientStream.

A workaround would be to start a separate goroutine that will call
ClientStream.Trailer asynchronously, but that requires careful
coordination and we can quite easily make the trailer metadata available
to the stats.Handler directly.

Use case: an interceptor that processes trailer metadata for each
streaming RPC after the stream has finished. Note that a
StreamClientInterceptor returns immediately, before the stream has
finished and before the trailer metadata is available.

@thelinuxfoundation
Copy link

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,
CLA GitHub bot

Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM but you need to make sure to make the CLA bot happy. Thanks!

@dfawley dfawley assigned mklencke and unassigned mklencke and dfawley Feb 28, 2019
@mklencke
Copy link
Contributor Author

mklencke commented Mar 5, 2019

I added my google.com account and the CLA should now be good. Please retry CLA verification.

@dfawley dfawley closed this Mar 5, 2019
@dfawley dfawley reopened this Mar 5, 2019
@dfawley
Copy link
Member

dfawley commented Mar 5, 2019

@mklencke that doesn't seem to have worked. Is the email address on your commit your @google.com email address?

Currently, it is not possible to access trailers from within a
stats.Handler. The reason is that both stats.Handler and
ClientStream.Trailer require a lock on the ClientStream.

A workaround would be to start a separate goroutine that will call
ClientStream.Trailer asynchronously, but that requires careful
coordination and we can quite easily make the trailer metadata available
to the stats.Handler directly.

Use case: an interceptor that processes trailer metadata for each
streaming RPC after the stream has finished. Note that a
StreamClientInterceptor returns immediately, before the stream has
finished and before the trailer metadata is available.
@thelinuxfoundation
Copy link

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,
CLA GitHub bot

@dfawley
Copy link
Member

dfawley commented Mar 6, 2019

That looks like a better error than the first one. I'm not sure it's required for Google employees to sign the CLA directly, but that is what we did on the gRPC team and it made everything work. So if you can't find some other way to take care of it, it's probably best to go through the process directly (it's pretty quick: click the link above, sign up as an employee, sign in with google using your corp account, link your github account, agree to the CLA, then drop a comment here to wake the bot up).

@mklencke
Copy link
Contributor Author

bot, please retry :)

@mklencke
Copy link
Contributor Author

I signed it

@dfawley dfawley merged commit 9c3a959 into grpc:master Mar 13, 2019
@dfawley dfawley added the Type: Feature New features or improvements in behavior label Mar 13, 2019
@dfawley dfawley added this to the 1.20 Release milestone Mar 13, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants