Skip to content

Conversation

@zhumazhenis
Copy link

This is for accessing gRPC Metadata in RPC implementation code.

Usage:

  • Attach MetadataCoroutineContextInterceptor to gRPC server.
  • Call function grpcMetadata() for accessing RPC request's Metadata.

Copy link
Collaborator

@duckladydinh duckladydinh left a comment

Choose a reason for hiding this comment

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

Just to update you, my next review will be next Wednesday, I am OOO now :).

Copy link
Collaborator

@duckladydinh duckladydinh left a comment

Choose a reason for hiding this comment

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

Ok, I agree that this metadata is quite a useful thing to have and especially given that grpc rust also has it. So we will just format the CL to Google Style and we can merge it.

BUT... pls note that we can merge but for this type of change, we need to wait for the real admins to release :) because I am not sure if the authors want to get into the business of providing a set of reusable interceptors and if they do, maybe they should be in some package like io.grpc.kotlin.interceptors or something. If we make a release now, it will be fixated on the current path. That's why I'll merge but I'll leave the release to the admins to leave room for modification and restructuring.

And my next review will wait a bit longer btw.

@duckladydinh duckladydinh self-assigned this Sep 24, 2025
@zhumazhenis
Copy link
Author

Ok, I agree that this metadata is quite a useful thing to have and especially given that grpc rust also has it. So we will just format the CL to Google Style and we can merge it.

BUT... pls note that we can merge but for this type of change, we need to wait for the real admins to release :) because I am not sure if the authors want to get into the business of providing a set of reusable interceptors and if they do, maybe they should be in some package like io.grpc.kotlin.interceptors or something. If we make a release now, it will be fixated on the current path. That's why I'll merge but I'll leave the release to the admins to leave room for modification and restructuring.

And my next review will wait a bit longer btw.

Valid concern!

Let's me clarify the goal for avoiding the confusion. The goal is accessing Metadata in gRPC Server Implementation code.

Currently, I see 2 ways for achieving that:

  1. Exposing a method grpcMetadata() for accessing the Metadata (this PR implements it).
  • Ideally, the only API that should exposed is the function grpcMetadata(). MetadataCoroutineContextInterceptor should not be exposed to the users. This interceptor should be implicitly injected without requiring the user to explicitely inject it to the gRPC Server object. Implementing without exposing MetadataCoroutineContextInterceptor, or even without any interceptor requires more investigation. I think that as an initial step we can start with exposing MetadataCoroutineContextInterceptor. Later, if we find a way for not exposing the interceptor, then we can deprecate the interceptor by noting that the interceptor will be removed in the next releases. After that, we can make it private, and not expose it to the user. It would be great if you merge and release it. But, just merging it is also fine for now.
  1. Add a second parameter Metadata to the methods of generated gRPC Kotlin Server code
  • This is a breaking change, and requires much more effort than the first one.

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

Successfully merging this pull request may close these issues.

2 participants