Add an optional implementation of streams using generics

[aarongable]

Add six new interfaces, and two new structs implementing those interfaces, to the grpc package. These interfaces and implementations represent the client and server sides of client-streaming, server-streaming, and bidi-streaming RPCs.

Add a new "use_generic_streams" flag to protoc-gen-go-grpc, which causes the code generation to use these new experimental types instead of generating individual implementations for each and every streaming method. This simplifies both the code generation process, and the resulting generated code.

Release notes for protoc-gen-go-grpc:

• the new "use_generic_stream_experimental=true" option will cause the gRPC codegen to use prebuilt generic types to implement client and server stream objects, rather than generating new types and implementations for every RPC method.

Fixes #7030

RELEASE NOTES: none

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: aarongable / name: Aaron Gable (d551dad, f75c2d6, 06c9a00, ba0d4e3, 8a9ade3, f159599, 08f3f64, 8c66d69, 617090d, faab808, 2230a4e, 586bde9, 9f4fa15, 8f93711, 9ddc54d, cc25c38)

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 81.17%. Comparing base (adf976b) to head (8a9ade3).
Report is 31 commits behind head on master.

❗ Current head 8a9ade3 differs from pull request most recent head 8f93711. Consider uploading reports for the commit 8f93711 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7057      +/-   ##
==========================================
- Coverage   81.24%   81.17%   -0.07%     
==========================================
  Files         345      345              
  Lines       33941    33954      +13     
==========================================
- Hits        27574    27562      -12     
- Misses       5202     5228      +26     
+ Partials     1165     1164       -1     

Files	Coverage Δ
experimental/experimental.go	12.90% <0.00%> (-87.10%)	⬇️

... and 18 files with indirect coverage changes

[dfawley]

Thanks for the PR and sorry I haven't gotten to it yet. I like the way it looks, but I need to spend more time thinking about it than I currently have available. Hopefully I will have more time by early-mid next week.

[dfawley]

So even with the generics being generated, the route_guide server and client do not need any changes?

I thought we were saying things would not be backward compatible...can you remind me where the trouble comes in please?

[aarongable]

So even with the generics being generated, the route_guide server and client do not need any changes?

I thought we were saying things would not be backward compatible...can you remind me where the trouble comes in please?

Correct, as this change is written now, everything is backwards compatible. (This is why I asked about maybe putting the new type definitions directly into the grpc package, rather than into experimental, since technically this change could be made completely invisibly to everyone as long as they're on a version of go with generics.)

In my ideal world, the type aliases wouldn't exist. Rather than the generated code saying

type RouteGuideServer interface {
	RouteChat(RouteGuide_RouteChatServer) error
}
...
type RouteGuide_RouteChatServer = grpc.BidiStreamServer[RouteNote, RouteNote]

it would just say

type RouteGuideServer interface {
	RouteChat(grpc.BidiStreamServer[RouteNote, RouteNote]) error
}

But that change would be backwards incompatible, since everyone would need to update the function signatures of the types that they use to implement such servers.

---

One way to split the difference between how this PR currently looks and my imagined ideal would be to use the new types everywhere (including in the interface definitions, and in the client implementation function signatures) and still provide the type aliases purely for backwards compatibility:

type RouteGuideServer interface {
	RouteChat(grpc.BidiStreamServer[RouteNote, RouteNote]) error
}
...
type RouteGuide_RouteChatServer = grpc.BidiStreamServer[RouteNote, RouteNote]

This would be a slightly more visible change, as anyone who regenerates their protos will see the interface definitions change, but all of their existing code will still continue to work thanks to the type alias.

[dfawley]

Both of the two versions (this and the "split the difference") seem to be the same in practice. Is there anything you're missing from this current version that could be done with the other version (besides maybe looking nicer / being more understandable)?

[aarongable]

Nope, they're roughly equivalent to me. From a "will it compile" perspective, they're identical to me.

I do slightly prefer the "split the difference" version for one reason. The original motivation behind this change was allowing a single struct to implement two different gRPC Services that share the same streaming method, and both the current version and the "split the difference" version allow that. However, the current version makes it look a little awkward, because the function signature on the type you implement won't match at least one of the two interfaces you're implementing:

// Generated code
type GreeterServer interface {
	SayHello(*HelloRequest, Greeter_SayHelloServer) error
}

type Greeter_SayHelloServer = grpc.ServerStreamServer[HelloReply]

type GreeterReadOnlyServer interface {
	SayHello(*HelloRequest, GreeterReadOnly_SayHelloServer) error
}

type GreeterReadOnly_SayHelloServer = grpc.ServerStreamServer[HelloReply]

// Implementation
type GreeterImpl struct { ... }

// Any of the three lines below will compile, but none of them are identical to both interface lines above.
func (g *GreeterImpl) SayHello(req *emptypb.Empty, stream pb.Greeter_SayHelloServer) error { ... }
func (g *GreeterImpl) SayHello(req *emptypb.Empty, stream pb.GreeterReadOnly_SayHelloServer) error { ... }
func (g *GreeterImpl) SayHello(req *emptypb.Empty, stream grpc.ServerStreamServer[pb.HelloReply]) error { ... }

The "spllit the difference" version would cause the third of the three options above to exactly match both generated interfaces.

But as you said, that's just "looking nicer / being more understandable", and the current version completely works.

[dfawley]

This should be Res, not Req.

[aarongable]

Good catch, fixed, thanks!

[dfawley]

I love it. Thank you for the contribution. We just need a second pass review (and also please make the q->s change).

[arvindbr8]

Looks great. Modulo one comment on the new feature flag.

[arvindbr8]

Should the new flag be marked experimental in the next release of protoc-gen-go-grpc? By experimental, I mean flag string be use_generic_streams_experimental and the usage says something like This flag is EXPERIMENTAL and may be changed or removed in a later release.

Later when we stabilize the API we can switch the flag's default to true and remove the experimental tags.

@dfawley, wdyt?

[dfawley]

That makes sense - the idea would be to delete the flag entirely one release after it is on by default. So:

1. Add flag, disabled, release.
2. Switch default of flag, release.
3. Remove flag, release.

(All separated by some reasonable amount of time to find bugs/etc.)

[aarongable]

Makes sense to me! I've updated the name and description of the flag, updated the two places it is referenced in scripts in this repo, and updated the release notes in the PR description.

[arvindbr8]

Thanks for taking care of it. Looks great!