-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Tracking issue: Opaque API #1657
Comments
For golang/protobuf#1657 Change-Id: I7b2b0c30506706015ce278e6054439c9ad9ef727 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/634815 TryBot-Bypass: Michael Stapelberg <[email protected]> Reviewed-by: Joseph Tsai <[email protected]> Reviewed-by: Damien Neil <[email protected]>
For golang/protobuf#1657 Change-Id: I8081e04050a38b0cb1b48ad674b683de61d36770 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/634816 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Damien Neil <[email protected]>
For golang/protobuf#1657 Change-Id: I3c35fceeddedcd159a5adacec3113e12497ebd27 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/634817 Reviewed-by: Damien Neil <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
For golang/protobuf#1657 In order to change the default API level, one can now specify: protoc […] --go_opt=default_api_level=API_HYBRID To override the default API level for a specific file, use the apilevelM mapping flag (similar to the M flag for import paths): protoc […] --go_opt=apilevelMhello.proto=API_HYBRID (Similar to the M option.) Change-Id: I44590e9aa4c034a5bb9c93ae32f4b11188e684a0 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/634818 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Damien Neil <[email protected]>
For golang/protobuf#1657 Change-Id: I01007ed586d1833517735cf5f38cca302efb9801 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/635137 Reviewed-by: Cassondra Foesch <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Damien Neil <[email protected]>
For golang/protobuf#1657 Change-Id: Icdf7254bced1c0987ff2e969fd096d6eef3918f7 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/635139 Reviewed-by: Damien Neil <[email protected]> Reviewed-by: Cassondra Foesch <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
For golang/protobuf#1657 Change-Id: I52d0f83a61964eb15ac9c740a06e7b76d61edc86 Reviewed-on: https://go-review.googlesource.com/c/website/+/634835 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Damien Neil <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
Is there a proposal somewhere that took into account community feedback? I can't seem to find anything on the mailing lists, github or code review older than a week ago. It seems like we just went through a major API break in Go protobuf and we have yet another in just 4 years. There are still projects that are trying to migrate to the last set of big changes. The departure of the gogo project was a net loss for the protobuf community after the 2020 change. It required a massive amount of work and testing to move to the new API with a lot of risk and very little benefit. Looking at the new motivations enumerated in the blog post, I wouldn't really put any of those concerns on even my top 10 problems when using protobuf and grpc. Even when I have encountered these problems, I was able to workaround the problem with an existing plugin, bespoke code generation or static analysis (rulesguard for the pointer compare is trivial). Looking at the changes, its unclear why this can't be an alternative plugin, rather than a core breaking feature. At a minimum, it would seem prudent to release the Opaque API as an experimental plugin to get some feedback before considering it as a "default" in the 2024 edition. Getting some outside usage will make sure we won't have the same issues that occurred with the last API change. I've been a long term advocate of Protobuf and GRPC for a number of projects. I've taken some criticism for that backing over the years due to the problems that has brought to projects, so it's going to be hard to continue that advocacy with yet another major change to such a foundational project. While I don't want to take away the engineering accomplishment made here, I would just hope to see some time for feedback to ensure this is the right direction. |
Hey Stephen Thank you for your message. I will try and address some of your points as best as I can, but note that I joined the project in 2022, and the Opaque API project was going on for much longer.
There is no public proposal for this change, but we did involve the community (privately) as early as 2023 and again, in multiple feedback rounds, over the recent few weeks leading up to yesterday’s release. Your name did not pop up when I compiled the list of folks to reach out to. If you would like to be included in the future, please let me know which company or organization you are speaking for and provide an e-mail address so we can reach out to you.
No, this change is not a breaking API change. Your existing programs continue to work as before (nothing breaks), and the announcement clearly states that there is no obligation or expectation to migrate. Even when the default API level changes with edition 2024, it is easy to stick to the Open Struct API by invoking
Just to be clear: the gogo/protobuf project was a community-driven project which was not coordinated with the Go Protobuf project.
We have already incorporated a lot of feedback over the last few years as we migrated Google’s Go codebase to the Opaque API. While the implementation can always be improved, the generated API itself seems pretty stable at this point. If you have suggestions for specific changes, feel free to file them as a feature request in this issue tracker. Thanks |
I'd love to give feedback early if given the opportunity (and do reach out to me, I'm at Docker now) but I would feel better if the project could commit to public proposals in the future. Involving the community privately here is the problem. That is a very unwelcoming and un-inclusive model for change and its easy to cherry pick positive responses in such an environment. I understand why its easier to do some things in private but this project is used widely. While its stated this is not a breaking change, if one does nothing and upgrades A few problems I foresee:
I'll give the new Opaque API a try and get back to you if I discover more issues or if these aren't that big of a problem.
No, it wasn't but the decisions of the Go Protobuf project effectively killed the vibrant community around that project. The Gogo project solved a number of problems that the main implementation ignored or didn't see as an issue. I hope that the attitude isn't that it is lesser because it was community-driven. Either way, it's important that this project take care not to create unintended consequences. The specific changes I'm asking for are the following:
I think these are reasonable actions that can be taken to create trust across the community and show folks that this can continue to be a strong, foundational dependency in their products. Appreciate you hearing me out. |
As a former maintainer, I can give some historical context (also as someone who's outside of Google now). The protobuf reflection and opaque API both began around the same time in 2018. The opaque API was motivated by at least two reasons 1) better performance not using pointers to represent presence, and 2) better compliance with the protobuf language itself. Protobuf reflection was a prerequisite in order to pursue the opaque API because there was a lot of code that used Go reflection on protobuf messages and assumed that it was of a particular representation. Another motivation for the protobuf reflection API was because Google consistently ran into panics due to Go reflection assuming that the protobuf message was produced by the golang/protobuf generator when it was actually produced by the gogo/protobuf generator, and vice-versa. Protobuf reflection is supposed to be agnostic to the underlying generated representation. A proposal for protobuf reflection was announced to the Go community back in 2018 and feedback was sought. Back then people had already raised concerns about gogo/protobuf and the intention was for gogo/protobuf to support protobuf reflection (in which case there would be no compatibility issues. Unfortunately, the maintainer of gogo/protobuf was unable to commit time to implement protobuf reflection. I offered to help, but when I started diving into the gogo/protobuf codebase, I realized that the great many features it supported allowed users to specify behavior that fundamentally went against the protobuf language and was therefore impossible to properly represent through protobuf reflection. Technically, it probably would have been possible to implement reflection for most features, but the large feature space of gogo/protobuf (while enjoyed by many users) was also its fatal Achilles' heel as it led to a very high maintenance burden. (As an aside: Personally I think some of the gogo/protobuf features are reasonable. Philosophically, gogo/protobuf takes a stance of making generated Go protobufs great in Go, which is stance that many Go enthusiasts are happy with. However, the golang/protobuf project is a sub-project is part of the wider protobuf project, which has the goal of making protobufs decent in all languages, but not become overly-specialized to any particular language. Many requested features of golang/protobuf were rejected because of this philosophical difference.) Going back to the opaque API, the intention was to open source the opaque API after the release of protobuf reflection in 2020, which would have included a more thorough discussion and feedback about what it should eventually entail. However, due to internal company turmoil, I left Google not long after, which resulted in golang/protobuf staying somewhat stagnant and the opaque API (which was already being massively used inside Google) to functionally become frozen as stable over the years. Thus, the fault of the opaque API not being more widely discussed ahead-of-time is mine and not the present maintainers. In fact, they are the ones who have to deal with my sins, so I thank them for that. |
I'm not sure I see this problem. The option is scoped to the Go language: option features.(pb.go).api_level = API_OPAQUE; |
@dsnet I'll respond in more detail but want to give you a quick answer. I think the point about rust was confusing but the classic problem with this was having generation ignore language-specific directives. Let's assume the entire projects are in Go and is set to use the Open API:
But, then, I have a dependency that has this:
Now, one is required to adopt the opaque api if they want to use the dependency or write some dependency post-processing to remove or ignore the line. |
I agree that working in the public as much as possible is best for Open Source projects in general, and that’s also what I aim for in general. But I would also like to remark that in this particular case, it would not have made sense for me to create a public proposal. As Joe has already outlined, the Opaque API was already rolled out within Google, and rolling it back was concluded to be prohibitively expensive. The only options I saw were “forever fork Go Protobuf inside/outside Google” (clearly not great for anyone) or “release the Opaque API as-is and improve from there”. We went with the latter. In other words: the community could (and did!) affect the “how”, but not the “if”.
Your description glosses over this (“with the 2024 edition”), so I want to stress again: If you do nothing and upgrade If you migrate your .proto definitions to edition 2024 with prototiller, the file will be changed such that the old behavior remains. Only if you actively signal that you want to opt into the new behavior (by selecting edition 2024 without any options that keep the old behavior), behavior will change. I think that’s reasonable.
What sort of “problems” are you referring to here? Programs are free to work with .proto files on different API levels.
For performance-sensitive code such as hot loop bodies, we recommend you prefer Setters over Builders: https://protobuf.dev/reference/go/opaque-faq/#builders-faster and https://protobuf.dev/reference/go/opaque-faq/#builders-vs-setters For the majority of the code, builders are fine (at least in our experience).
We tried to refer to the old API as “Open Struct API” throughout, but that realization came late in the process (based on external user feedback, actually). Can you let me know where you saw a reference (in prose, not code) to “Open API” please? |
@dsnet Thanks for the context. As you know, I was there and you and I had interactions with you over this matter and other related matters. While I concede that gogo did have its share of design problems, it was stable and it gave a lot of protobuf users agency over generation, supporting many more plugins and capabilities. I am not sure the reflection API was worth the loss of that project nor do I think they had to be mutually exclusive. I hope we can look back on that situation and avoid repeating Go protobuf's part in that outcome.
If I have a package that exports Open Struct API today, one cannot remove that and move to Opaque API without breaking downstream users of the package. One must stay on the "Hybrid" model. While protobuf itself may not break compatibility, it may introduce breaks in other packages and users that incidentally make these changes. Packages won't be "free to work with .proto files on different API levels". Project maintainers will be forced into enabling these APIs to satisfy downstream users.
I suggested another option: release the Opaque API as a separate generator. The community has been doing this for a long time it is a tried and true technique in introducing alternatives. Introducing this as a separate generator will allow the community to evolve naturally and avoid unintended outcomes. It might be prudent to offer it as an option while leaving the rest of the changes in place. Thank you for taking the time to hear me out. I hope you'll consider these suggestions carefully and thoughtfully. Like I said, I'll give this a try and get back to you. |
Yes, that’s correct.
I don’t understand what you mean by that. Can you rephrase please, or perhaps spell out an example?
I don’t understand what advantages you see in requiring users to use a separate generator name, especially at this point when the Opaque API has already been released. Are you suggesting we un-release the Opaque API? That’s not going to happen. Are you suggesting we change the configuration surface post-release? That’s not going to happen either. The only kind of changes we can make at this point are additions, and adding a new generator name does not seem useful to me. If we had any doubts regarding the stability of protoc-gen-go and wanted to keep the code separate, I think a separate generator binary would have been a good option. But that’s not the case: this code has been stable for years and has been released by now, thus far without causing any issues. I will be out of office soon and will return to this conversation in January. Thank you for your feedback and happy holidays! |
Transitioning a project outside the google ecosystem can uncover significant challenges. Engaging maintainers with large protobuf codebases can provide insights into potential issues during a controlled migration to new types. While the migration tool is valuable, it doesn't address dependencies outside a maintainer's control. Comprehensive documentation on the transition process can assist maintainers in overcoming these hurdles, though uncertainty persists. A major concern is shifting from Open Struct to Opaque in widely used packages. For instance, the containerd API depends on Open Struct-generated protobufs. Switching entirely to Opaque could disrupt users and may necessitate maintaining a Hybrid mode indefinitely. Incrementing major versions for minor type changes may not be well-received. Additionally, maintainers adopting the Opaque API without Hybrid mode risk compatibility issues, placing significant responsibility on them. A separate generation plugin may be a good tool in easing this transition. Since the Opaque API has been available for only about a week with no major migrations, it's premature to declare success. We've already encountered issues like golang/protobuf issue #1661. I don't advocate reverting to the previous API; the Opaque API has clear advantages. Instead, I suggest just slowing down the suggested adoption of Opaque generation configuration until its been vetted outside Google. Currently, these usability challenges are the most significant barrier to protobuf adoption. When recommending protobufs, colleagues face various issues with Enjoy your time off! Merry christmas! 🎄 |
This issue provides a central place for questions and discussion around the Opaque API.
Please read the Opaque API announcement blog post first!
Before posting questions, please check if the Opaque API FAQ answers your question.
Release Plan / Status
The text was updated successfully, but these errors were encountered: