protoc: Change protoc to include static method call option #6960
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6960 +/- ##
==========================================
- Coverage 83.72% 82.53% -1.19%
==========================================
Files 287 300 +13
Lines 30915 31351 +436
==========================================
- Hits 25884 25876 -8
- Misses 3969 4422 +453
+ Partials 1062 1053 -9 |
cmd/protoc-gen-go-grpc/grpc.go
Outdated
| @@ -170,7 +170,7 @@ func generateFileContent(gen *protogen.Plugin, file *protogen.File, g *protogen. | |||
|
|
|||
| g.P("// This is a compile-time assertion to ensure that this generated file") | |||
| g.P("// is compatible with the grpc package it is being compiled against.") | |||
| g.P("// Requires gRPC-Go v1.32.0 or later.") | |||
| g.P("// Requires gRPC-Go v1.32.0 or later.") // TODO: Update to latest released gRPC version | |||
| g.P("const _ = ", grpcPackage.Ident("SupportPackageIsVersion7")) // When changing, update version number above. | |||
There was a problem hiding this comment.
This needs to be updated to Version8 now. The comment just lets users discover more easily when Version8 became available.
cmd/protoc-gen-go-grpc/grpc.go
Outdated
| g.P("cOpts := make([]", grpcPackage.Ident("CallOption"), ", len(opts))") | ||
| g.P("for i, opt := range opts {") | ||
| g.P(" cOpts[i] = opt") | ||
| g.P("}") | ||
| g.P("cOpts = append(cOpts, ", grpcPackage.Ident("RegisteredMethod()"), "\"") |
There was a problem hiding this comment.
Can be simplified to:
opts := append([]CallOption{grpc.RegisteredMethod()}, opts...) // shadows `opts` so you don't need the subsequent diffs evenOr use copy instead of a loop (and also pre-allocate len(opts)+1).
There was a problem hiding this comment.
Changed to former. Chose that since less lines of code and get rid of subsequent diffs.
|
Finished comments, as discussed offline, let's run this by Eric to see if we even want to go down this route, or if we should preregister on ClientConn. |
|
Also cannot merge until 1.62 release. |
|
Reopened this since I think this is ready to be merged as the release is out. |
zasweq
changed the title
This PR changes protoc to include generated call option. This will be used in stats handlers. This cannot be merged until gRPC is released with #6926, and we will have to update the minimum version of gRPC required for new versions of protoc-gen-go, and features which use this will need to regenerate their protos (such as Envoy's protos). Per offline discussion, OpenTelemetry e2e testing will be sufficient enough for testing this feature, as protoc-gen-go is implicitly tested in our tests which all use generated code from protoc-gen-go.
RELEASE NOTES: