Consume opamp-spec as a submodule and use protos from it

[tigrannajaryan]

Depends on open-telemetry/opamp-go#133

This is similar to what we do with OTLP protos in https://github.com/open-telemetry/opentelemetry-proto-go

We need a makefile to re-generate the files every time protos are updated. Generated files should be stored in this repo.

Ideally also a github action that verifies that protos match the generated file.

[tigrannajaryan]

@jlegoff can you help with this?

[jlegoff]

@tigrannajaryan yes, I can take it when the protos are moved to opamp-spec

[tigrannajaryan]

Sounds good, assigned to you.

In the meantime if you want to start with #3 it would be great.

[tigrannajaryan]

Protos are in the spec now: open-telemetry/opamp-spec#126

If you need anything to be changed in proto files for Java (e.g. java_ options) please create a PR against the spec repo.

[jlegoff]

I have started to work on this issue here - I'm struggling with a few build issues but I'm getting help from @jack-berg

[tigrannajaryan]

I have started to work on this issue here - I'm struggling with a few build issues but I'm getting help from @jack-berg

Sounds good. Please move incrementally, keep the PRs small so that they are easy to review.

[jlegoff]

Please move incrementally, keep the PRs small so that they are easy to review

I've made a first PR and kept is as small as I could, even though there's a lot of boiler plate code with the gradle scripts 😅

[jack-berg]

Generated files should be stored in this repo.

@tigrannajaryan why store the files in the repo?

We need a makefile to re-generate the files every time protos are updated.

Why use a makefile instead of standard java tooling with gradle?

I think the opentelemetry-proto-java and opentelemetry-java gets this right:

• Its published artifacts contain the classes generated from the protos, but the classes are not checked into source code. If desired, test code could be written that asserts their presence along with any other behavioral invariants we want to guarantee.
• The current version of the referenced protos can live in source code. Updating to the latest only requires bumping one number in source code, like we do with the otlp protos in opentelemetry-java.

[tigrannajaryan]

@tigrannajaryan why store the files in the repo?

I find it helpful to look at diffs in generated file in PRs. For example if we change generation options diffs are very important to see the effect of the change. If the files are not stored in the repo there is no way to do it. Also if generated files are not stored in the repo ensuring reproducing build is more difficult - you have to ensure you generation process is also reproducible, which is doable but adds one more thing to worry about.

Why use a makefile instead of standard java tooling with gradle?

Ignore that. Use whatever tool makes sense for java.

[jack-berg]

I find it helpful to look at diffs in generated file in PRs. For example if we change generation options diffs are very important to see the effect of the change. If the files are not stored in the repo there is no way to do it.

I don't disagree with that. However, it does break convention with opentelemetry-java. In my experience, the tooling we use to generate proto classes is very reliable. Having modules that depend on the generated classes like the eventual opamp client / server and corresponding test files improves confidence.

I'm neutral overall, given that there will be no change in experience for someone consuming the artifact whether or not the generated classes are checked into source code.

[tigrannajaryan]

If you don't see the need to have the diffs of generated files in PRs then go with whatever you prefer. It should be long-term maintainers' decision and I am only a temporary maintainer here. It is also possible to change this decision later and start storing generated files in the repo, so no big deal, start with what you prefer now.