search

open-telemetry / opentelemetry-java-instrumentation

OpenTelemetry auto-instrumentation and instrumentation libraries for Java
https://opentelemetry.io
Apache License 2.0
1.8k stars 788 forks source link

Have master branch depend on OTel time-stamped snapshots? #728

Closed anuraaga closed 3 years ago

anuraaga commented 3 years ago

Given there's a lot of work being put into the spec, e.g., #723, I wonder whether instead of having the otel-snapshot branch just make master depend on the snapshots until GA? I think it's fairly common to do SNAPSHOTs for dev, non for release, and is codified in maven-release-plugin and gradle-release-plugin. This would allow us to keep up faster which changes in the spec and probably speed up towards GA. The downside is from what I understand it can be a lot of work to maintain our API shim, so targeting snapshots might increase that work so much that it's not worth it. Wonder what people's thoughts are.

iNikem commented 3 years ago

So far we have the majority of development happening on master branch. Meaning that it does not depend on latest and greatest of otel-java. Yes, we implement latest changes with some delay, but I don't think this is a big problem yet.

It may be a good idea when GA nears, but if we break our main branch every day now, it will slow us down.

anuraaga commented 3 years ago

Ah I meant depending on the specific snapshot version, and we could update that whenever there's a new API we need, so it shouldn't result in automatic breakage. For example, looks like this

https://github.com/aws-samples/aws-xray-sdk-with-opentelemetry-sample/blob/both-otel-and-xray/dependencyManagement/build.gradle.kts#L41

Would that address the concern? I think it could in general speed things up even at this stage (the bigger concern is the maintenance of the API shim for me which I'm not sure how much work it actually is)

iNikem commented 3 years ago

But it is still a snapshot version? It changes every time otel-java merges to main branch. E.g. today we have broken snapshot branch due to upstream changes.

anuraaga commented 3 years ago

Nah the version points at the specific published artifact which has a timestamp-based version, so it wouldn't change even when -java has commits added. Sorry that wasn't clear from my proposal, I don't think we should depend on dynamic snapshot :)

iNikem commented 3 years ago

A, now I understand :) Who and when updates that?

anuraaga commented 3 years ago

I would say whoever wants to implement usage of a new SDK API would update when they want to start on it in the instrumentation. Doesn't have to be a formal schedule but just to unblock work on new features. I would generally update when implementing a new semantic attribute for example.

trask commented 3 years ago

I like it. It will make it much easier to do work that crosses both repos (cc: @jkwatson).

If we can have a nightly build against latest SNAPSHOT, that would help us to be proactive about breakages (e.g. in the API shim), so when someone wants to update the SNAPSHOT version to use a new feature they don't have to struggle with those breakages.

My only concern is reproducibility of the builds (e.g. for bisecting a regression), but it looks like those snapshots are kept around for a long time, e.g. https://oss.jfrog.org/artifactory/libs-snapshot/io/opentelemetry/opentelemetry-sdk/0.1.0-SNAPSHOT/.

So, yeah, 👍