-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Generate Java classes from OTLP proto files #133451
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
Conversation
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
Pinging @elastic/es-delivery (Team:Delivery) |
Hm, so one of the issues with this is that the
Is that something we'd be willing to make an exception for protobuf or should we look at alternatives? |
There's precent for this in plugins/repository-hdfs/build.gradle
Looks like there's precedent for ignoring these classes, so I did the same. |
x-pack/plugin/otel-data/build.gradle
Outdated
} | ||
} | ||
|
||
afterEvaluate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want avoid afterEvaluate as its kind of deprecated gradle api. I understand the external plugin relies on it.
x-pack/plugin/otel-data/build.gradle
Outdated
// We always need the ByteString (UTF-8) representation of string attributes | ||
def adjustAnyValueBuilder = tasks.register("adjustAnyValueBuilder") { | ||
doLast { | ||
ant.replace( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are in the process of removing all usages of antBuilder in our build scripts as it has a couple of flaws and breaks new features in gradle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will have a go on looking into simplifing the gradle logic here to match our generall gradle api usages and best practices.
@felixbarny I approved the PR. We can address the rework of the build later on as this should not block you from merging imo |
@breskeby there's one more challenge related to the
Do you have a suggestion on how to fix this? Some of the options I'm seeing are to disable the task or to change the implementation to ignore |
Ah, nvm, I think I fixed it by using the |
Downloads the latest OTLP protocol buffer release and generates Java classes from them.
Some noteworthy aspects of this:
protoc
version that's specific to the current platform. That makes it hard to generate all the verification metadata that's used on other platforms, like on CI or for other contributors. I've added a script to download and create the SHA-256 hash for allprotoc
platform variants.Part of #133057