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

Add tooling to build Java stubs and add a workflow for it. #231

Closed

Conversation

anuraaga
Copy link

@anuraaga anuraaga commented Nov 6, 2020

Building and publishing stubs from this repo would make things a lot simpler to consume, removing the need for git submodules, which hurt contributor experience, and allowing publishing versions to be tied directly to the commits in this repo.

This is a first step that adds the ability to build Java stubs. If this approach seems ok, it would be trivial to add publishing + register bintray key to get these published automatically.

Sample workflow run https://github.com/anuraaga/opentelemetry-proto/runs/1362112075?check_suite_focus=true

Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did have this for Go, but decided to do the opposite, removing the generated Go code: #79

Now Go might be a special case here and I don't see any disadvantage to the Java stub generation in particular, so I don't necessarily oppose this PR.

@@ -0,0 +1 @@
rootProject.name = "opentelemetry-proto"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need a settings.gradle for a single-project build?

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use the docker build tool if you want, we should not host in this repo language specific build tools (hard to maintain all of them for the maintainers which will not know all the possible tools).

@anuraaga
Copy link
Author

anuraaga commented Nov 7, 2020

@bogdandrutu Do you mean packaging the build.gradle / gradlew into the current docker builder image instead of in this repo?

@bogdandrutu
Copy link
Member

You can use directly that container, it has protoc and everything needed to compile the proto files. No need to use gradle.

@anuraaga
Copy link
Author

anuraaga commented Nov 12, 2020

@bogdandrutu It can generate proto files but it can't compile them without gradle or maven (or a lot of tedious java commands / dependency downloading / xml writing) right?

@yurishkuro
Copy link
Member

@anuraaga where do you need those files to be used? Our standing recommendation is to check them in elsewhere. For example, if OTEL Java SDK needs these types in order to export the data, it can have a module with the generated files, using whichever build system is employed by the SDK repo. If Java SIG wishes to make those classes reusable, they can publish them as an independent artifact.

@anuraaga
Copy link
Author

@yurishkuro Yeah we're currently publishing code for Java, but we have a problem with versioning vs this repo. They have different versioning and release cadence, so it seems like publishing the artifacts here and tying to the commits, release versions here, makes things much easier to reason about.

open-telemetry/opentelemetry-java#2021

For example, snapshots can just be published from here on commits, and when someone needs to try out a new protocol they can. We don't want to sync unreleased versions across the repos due to the unrelated release cycle.

Building in each language is one approach, but building languages here is another, and maybe it could make sense for these reasons? Shared ownership of the build infra in this repo with the languages seems reasonably simple.

@yurishkuro
Copy link
Member

I don't think independent versioning is a problem, it's a feature (of protobuf, in large part).

Assume there is a new field added to the proto IDL. Even if a new Java artifact is published synchronously with the IDL change, your Java SDK will not pick up that new version automatically, or even if it does it would still be missing the code to actually populate the new field. A better workflow is:

  • new IDL version is published
  • Java SDK repo picks up the new version in a PR that makes use of the new fields

Anyway, this has been discussed a lot in #79. I suggest writing an abstract of those discussions as a rationale somewhere into the README of this repo.

@anuraaga
Copy link
Author

Skimmed through #79 - I don't see a lot of discussion in there. It sounds like you are considering OTLP an implementation detail of the SDKs, but this isn't the case. There are and will be backends, not just the collector using it, written in many languages, e.g. we'll have some OTLP Java server code coming down the line at some point. Or other use cases like integration tests. These don't use the SDK at all, and it's the proto that should be owning making itself consumable for these use cases right? Easy consumption from many languages is a feature of proto - otherwise an IDL could just be written in markdown really - so it seems to make sense for the proto to publish itself in a way that can be consumed by languages. Take the proto and build it yourself seems like it prioritizes maintainers over end users, and wastes some trees too having many different projects running the exact same proto build.

I feel like part of #79 was conflation of how to publish Go stubs - Go stubs can be published to a different repo from here and is definitely a good idea. I've had good experiences publishing generated code to all the package repos, and go stubs to a separate GitHub repo in this project

https://github.com/infostellarinc/stellarstation-api

Make a release, and the proto is consumable by all end users since it's available in all the supported languages automatically.

@yurishkuro
Copy link
Member

Easy consumption from many languages is a feature of proto.

Precisely, that's why proto comes with protoc that can generate stubs for a target language. To make it even easier, OTEL provides a Docker image with protoc and dependencies already included. However, there are also numerous plugins to protoc that affect how the stubs are generated, to meet specific requirements of the projects, there's no "one way" to generate the stubs. The IDL files are the shared interface.

This in no way prevents people from having another repository that generates and publishes generated stubs as artifacts, if there is a demand for that and there are people who're willing to maintain such distribution. GitHub supports cross-repo workflows where a release of new proto version can trigger a build and release from the artifact repo.

In the end, this is a debate about monorepo vs. micro-repos. Neither is more "right" than the other. The open source usually favors micro-repos, for better or worse, which is what OTEL is using as well. Monorepos, especially as multi-lingual as the OTEL project, typically require a dedicated tooling team.

@anuraaga
Copy link
Author

I don't know if it's so much about monorepos - this is still just one item, the proto, but published in the many different ways it can be consumed - versioning is consistent among them all, tied to the IDL / release tag in this repo which conceptually seems to make a lot of sense. Monorepos are combining lots of different business logic projects, which may be tied by a proto but are otherwise completely independent codebases, probably with different push processes / release schedules. Oh well, sounds like there isn't any appetite for this so will go ahead and close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants