-
Notifications
You must be signed in to change notification settings - Fork 129
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
Removing protobuf dependency #1008
Removing protobuf dependency #1008
Conversation
are all of those serializer classes hand-written? |
Yes. They used to be auto-generated using MapStruct, however, that caused the PR to be bigger since MapStruct couldn't reuse the existing builders/creators for all the implementations of the OTel API in the Java SDK. The changes made to all of them here are minimal though, it's mostly accessing fields directly instead of through getters (because Wire doesn't create those to reduce the number of methods), and also renaming some builder setters to the style added by Wire. |
@LikeTheSalad no need to add change log in this repo, just update the PR title to what you would like displayed and we'll pull it in semi-automatically before making the next release |
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Description:
Bug fix - Since this module has a dependency on OTel Java proto it means that it also gets Protobuf's java lib as a transitive dependency. The protobuf-java library is not ideal for Android projects due to its size which is why there's another version of it, called protobuf-javalite, that aims to be more suitable for Android use cases due to its smaller size.
Given that the
protobuf-javalite
dependency is better for Android projects, some projects might have it included already which causes a problem if they later add thedisk-buffering
dependency as well, since it comes with the "regular protobuf dependency". This is an issue because both, the regular and lite versions of the protobuf libs, contain classes that have the exact same fully qualified names, which makes Gradle raise a compilation error due to the "duplicated classes" it finds across different dependencies.This was discussed in the latest Java SIG meeting where a couple of interesting ideas were proposed to avoid this issue such as:
disk-buffering
dependency into a project that already has the protobuf-javalite dependency. Ideally, I'd like this lib to be easy to use.The approach I'm proposing to go with in this PR is to not use any of the protobuf dependencies, neither the "regular" nor the "lite" one, in
disk-buffering
. Instead, we can use Wire, which is a third-party tool (already being used in other OTel Java projects) that takes care of generating the Java sources from the proto files without having any dependency on Google's protobuf libraries. As a bonus side of using this tool, we get smaller generated sources since Wire makes them a bit more android friendly by reducing the number of methods for each generated class.Existing Issue(s):
#1009
Testing:
Unit, integration, manual tests.
Documentation:
No documentation changes are needed as part of this PR since the changes are only internal.
Outstanding items: