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

grpc: Change server stream context handling #6598

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented Aug 29, 2023

This PR changes some plumbing with respect to the server streams context once HandleStream() called. These changes are logical no-ops. First, it gets rid of the old trace info plumbing into context, since it's just used to read out trace info at a later point, and now I construct trace info inline. Second, it reads stream.Context() at the beginning of HandleStream(). This is to help the future changes.

The next steps are to:

  1. Move all stats handler callouts (now in http/2 transport and server stream) to server.go
  2. Pass a pointer to grpc.Server inside the context, and add a method to server for if a method is registered or not. Stats handler will call this to determine if method is registered or not.

RELEASE NOTES: N/A

@zasweq zasweq added the Type: Internal Cleanup Refactors, etc label Aug 29, 2023
@zasweq zasweq added this to the 1.59 Release milestone Aug 29, 2023
@zasweq zasweq requested a review from dfawley August 29, 2023 21:44
@dfawley dfawley assigned zasweq and unassigned dfawley Aug 31, 2023
@zasweq
Copy link
Contributor Author

zasweq commented Sep 1, 2023

I will merge this. The TagRPC discussion we had earlier about whether it gets passed and verifying that is verified from the stats/opencensus package, which continue to pass. The TagRPC call populates it with information that is needed for HandleRPC calls to work correctly and subsequent e2e assertions, and it does. I will think about how to test this once all callouts are moved around to gRPC layer.

@zasweq zasweq changed the title grpc: Change server stream context reading grpc: Change server stream context handling Sep 1, 2023
@zasweq zasweq merged commit 8eb4ac4 into grpc:master Sep 1, 2023
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants