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

protoc-gen-go: allow preservation of SourceCodeInfo in generated descriptors #1134

Closed
AnatolyRugalev opened this issue May 17, 2020 · 8 comments
Labels
generator-proto-option involves generators respecting a proto option to control generated source output

Comments

@AnatolyRugalev
Copy link

I am trying to build dynamic gRPC gateway which can call any gRPC service using protoreflect. While I am still using APIv1, I see that this issue could relate to both versions.

I want to build dynamic OpenAPI 3 document based on user-defined REST -> gRPC mappings in my gateway but unfortunately I cannot get any SourceCodeInfo via Google Reflection API. I understand that this particular field is stripped on .pb.go generation stage for performance and readability reasons:

...
func genFileDescriptor(gen *protogen.Plugin, g *protogen.GeneratedFile, f *fileInfo) {
	descProto := proto.Clone(f.Proto).(*descriptorpb.FileDescriptorProto)
	descProto.SourceCodeInfo = nil // drop source code information
...

But it seems to me that there could be an option which disables this behavior for heavy protoreflect users. Currently the only option I see is to use clunky protobuf annotations when I really could use just source code comments. Anyway who would import my custom .proto files to describe gRPC method twice. Using comments as a documentation source would be really really great. Also I predict surge of protoreflect based solutions due to official support but this particular limitation really could put a stick in this wheel.

I was going to propose adding an option flag to protoc-gen-go for this matter but it seems like you have pretty strict policy about them (the only non-deprecated flag is --version).

What do you think about preserving SourceCodeInfo in compiled code in general? Is it too much data?

@dsnet
Copy link
Member

dsnet commented May 17, 2020

Currently the only option I see is to use clunky protobuf annotations when I really could use just source code comments.

While I can see some usefulness for preserving comments, I should note that this specific use-case should probably use annotations. They may be more "clunky" than a simple comment, but they provide more type safety and is the generally advocated way to extend the protobuf language.

@dsnet dsnet changed the title Allow preservation of SourceCodeInfo in generated descriptors protoc-gen-go: allow preservation of SourceCodeInfo in generated descriptors May 17, 2020
@dsnet dsnet added the generator-proto-option involves generators respecting a proto option to control generated source output label May 17, 2020
@AnatolyRugalev
Copy link
Author

AnatolyRugalev commented May 17, 2020

Thank you for reply.

I am fully agree that extention of protobuf capabilites should be done using custom options, but I don't think that preserving comments is any kind of extension.

Using custom options will bring unnecessary dependency for the end-user and .proto file will look not too DRY in my particular use-case:

syntax = "proto3";

import "third_party_package/mypackage/annotations.proto"

// This is service
service Service {
  option (mypackage.description) = "This is service";
  // This is my method
  rpc Method (User) returns (User) {
      option (mypackage.comment) = "This is my method";
  }
}

// This is user
message User {
  option (mypackage.description) = "This is user";
}

I want to avoid that. I see huge potential of protoreflect as it opens new field of automation that could be done on top of gRPC services without any additional dependencies. If this could not be done in official library, I would rather strip this feature from my project because this is not the price I want my users to pay.

If you don't mind I could try to implement this in a pull request.

I appreciate your attention to this problem.

UPD: I need to apologize for my incompetence:

the only non-deprecated flag is --version

Just dug a tiny bit deeper to the protogen package and I see now where all options are sitting.

@AnatolyRugalev
Copy link
Author

I just submitted my first CL.

I appreciate the feedback

@dsnet
Copy link
Member

dsnet commented May 26, 2020

\cc @neild. this is a new API feature and needs more discussion.

Per the FAQ:

The Go implementation of protocol buffers strives to be consistent with the other language implementations. As such, we tend to shy away from feature that are overly specialized to just Go. Go-specific features hinder the goal of protocol buffers being a language-neutral data interchange format.

Something we should understand is whether the other language implementations (e.g., C++, Java, Python, etc) preserve comments. Last time I checked, I think the answer is no. And if so, do they provide an option to forcibly include them in the generated code?

The Go implementation of protobuf strive to avoid being a special snowflake in the feature set that it offers, especially when it comes to generated code.

@neild
Copy link
Contributor

neild commented May 26, 2020

I'm don't think we want to include source comments in generated packages.

We want generated code to be as uniform as possible. Options which customize the generated code work against that. This is particularly problematic when so many packages commit generated .pb.go files to source control, as in that case there's no good way for users to pick which options to use.

Including comments in generated packages is wasteful; it increases the size of the package with data that most users have no need for.

As @dsnet says, if other implementations do not provide this ability, that's a strong signal that we shouldn't either.

@puellanivis
Copy link
Collaborator

I remember when I was working with protos at Google, you barely ever opened up the generated code. The .proto file itself had the most complete information and documentation that you should need, and the generated file was basically treated like “machine-code”, or “object-code”. It was not intended for human consumption.

@AnatolyRugalev
Copy link
Author

Hi!

Thank you for your responses.

I don't want to be intrusive but I would like to provide more context for you and ask a final question.

I work in ~300 employee organization which specializes on software and hardware development. As you can imagine, developing hardware has a nasty drawback: backwards compatibility. You could throw any technology to this problem, but if user doesn't update his device's firmware - you can do absolutely nothing but to maintain BC by any means. Unfortunately for us that means that a lot of APIs should be integrated through REST API because this is how things worked before gRPC.

We decided to adopt gRPC and Protobuf in internal services about 3 years ago and since then I was pushing a lot of propaganda for this tech in my local communities. But I feel like there is a missing piece in gRPC puzzle: there is no way you can deploy zero-configuration easy-to-use and well-documented REST proxy. We always have to build them manually.

I see why Google keeps things as they are - every Google employee (my assumption) has access to .proto and is able to execute remote procedure calls through gRPC or it's proprietary counterpart. I got it. While this approach works great for internal APIs, when providing the external ones people heavily rely on REST API. Official Google API developers are aware of that and they provide REST alternatives to each RPC method. What I'm trying to say is that REST API is here with us forever because it's just easier for people to use. And I want to make gRPC easy to use either.

So I'm trying to build truly revolutionary piece of software (yeah, I know) to provide new way of gRPC adoption. The problem is that the only way to obtain documentation for gRPC is to have access to .proto source code in compile time. This is a huge bummer for anyone who wants to build something dynamic and user-friendly around gRPC. You have to choose one: documented api-gateway or dynamic one. I feel like this is unnecessary dependency for dynamic systems: each gateway should be redeployed if any underlying protocol was changed. And according to microservices best practices you could have a bunch of this api-gateways to support, for example, each for different kind of clients.

Developers (me including) really like to be able to change one thing to do the work. That's why automation exists. For me sometimes it's hard to promote gRPC when it introduces a lot of additional work each time you want to update something.

I think that we came a long way since first Protobuf release and I see that now we have officially supported way to call gRPC dynamically. I personally hope that it's time for Google to reconsider decision on dropping comments from generated descriptors. I mean, come on, no one cares about generated code size in 2020.

TL;DR:

I understand now that this topic should be discussed in google/protobuf. I don't have much experience pushing something into huge open source projects, so I hope that I can get an approval of this idea from Go Protobuf community. You can just like this message to let me know that I am on the right path.

With so much effort going into APIv2 from golang/protobuf maintainers I suppose that proto/reflect was designed as a package that could be used in production. Don't you think that production ready tools and services should provide usage instructions with them?

I would greatly appreciate the response and I would like to continue discussion in google/protobuf if you think that my proposal makes sense.

Thank you.

@AnatolyRugalev
Copy link
Author

I am closing this issue since there's nothing we could do without discussoin at google/protobuf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator-proto-option involves generators respecting a proto option to control generated source output
Projects
None yet
Development

No branches or pull requests

4 participants