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 more properties to ClientContext and have the ClientTransport provide it #2158

Merged
merged 6 commits into from
Jan 16, 2025

Conversation

gjcairo
Copy link
Collaborator

@gjcairo gjcairo commented Jan 15, 2025

The ServerTransport provides the ServerContext, as it contains information that only the transport knows about (such as the remote peer's address).

For consistency and to allow the ClientContext to also hold some additional information (such as remote and local peer descriptions), this PR changes the ClientTransport protocol so that implementations also provide the corresponding ClientContext.

This PR also adds additional information to the context (which will be used by the tracing interceptor but can be useful for users in general): remote and local peer addresses, server hostname, and network transport.

@gjcairo gjcairo added the ⚠️ semver/major Breaks existing public API. label Jan 15, 2025
@gjcairo gjcairo requested a review from glbrntt January 15, 2025 13:25
@gjcairo
Copy link
Collaborator Author

gjcairo commented Jan 15, 2025

The API breaks are expected, since we've added the ClientContext to the closure signature:

💔 API breakage: func ClientTransport.withStream(descriptor:options:_:) has parameter 2 type change from (GRPCCore.RPCStream<Self.Inbound, Self.Outbound>) async throws -> T to (GRPCCore.RPCStream<Self.Inbound, Self.Outbound>, GRPCCore.ClientContext) async throws -> T

💔 API breakage: constructor ClientContext.init(descriptor:) has been removed

💔 API breakage: func InProcessTransport.Client.withStream(descriptor:options:_:) has parameter 2 type change from (GRPCCore.RPCStream<GRPCInProcessTransport.InProcessTransport.Client.Inbound, GRPCInProcessTransport.InProcessTransport.Client.Outbound>) async throws -> T to (GRPCCore.RPCStream<GRPCInProcessTransport.InProcessTransport.Client.Inbound, GRPCInProcessTransport.InProcessTransport.Client.Outbound>, GRPCCore.ClientContext) async throws -> T

@gjcairo gjcairo requested a review from glbrntt January 15, 2025 15:27
@gjcairo gjcairo requested a review from glbrntt January 15, 2025 16:06
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Fab, thanks Gus!

@glbrntt glbrntt merged commit 1fb1626 into grpc:main Jan 16, 2025
30 of 33 checks passed
@gjcairo gjcairo deleted the client-context-changes branch January 16, 2025 13:11
glbrntt pushed a commit to grpc/grpc-swift-nio-transport that referenced this pull request Jan 16, 2025
This PR adopts the changes introduced in
grpc/grpc-swift#2158, requiring client
transports to provide a `ClientContext` alongside the stream.
glbrntt pushed a commit that referenced this pull request Jan 17, 2025
This PR adds a `localPeer` property to the `ServerContext`, and renames
`peer` to `remotePeer`.

This is related to #2158 on the
client side and will be used to improve the server tracing interceptor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants