-
Notifications
You must be signed in to change notification settings - Fork 833
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 rationale and description of the release and versioning strategy. #2212
Add rationale and description of the release and versioning strategy. #2212
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2212 +/- ##
============================================
+ Coverage 85.51% 85.72% +0.21%
Complexity 1831 1831
============================================
Files 210 210
Lines 7207 7189 -18
Branches 812 811 -1
============================================
Hits 6163 6163
+ Misses 730 712 -18
Partials 314 314
Continue to review full report at Codecov.
|
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.
Seems ok to me, if not overspecified. 😁 . It could be nice to have an assumption about everything else being intended to improve compatibility and foster collaboration and explicitly not to stifle/slow development.
No other real feedback/concerns.
- Once the API for a given signal (spans, logs, metrics, baggage) has been officially released, that API module will | ||
function, *with no recompilation required*, with any SDK that has the same major version, and equal or greater minor or patch version. | ||
- For example, libraries that are instrumented with `opentelemetry-api-trace:1.0.1` will function with |
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.
Looking through https://github.com/open-telemetry/oteps/pull/143/files I couldn't find a point on this so I guess it's just #2203 :)
Given using a bom is easy and we can strongly encourage it, I don't know if we need to require this
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.
What are you saying we don't need to require? Shouldn't an 1.x API work with any 1.y SDK as long as y >= x?
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.
That's my suggestion - it's not unreasonable to require x and y to match up, it's one of the reasons to publish BOMs. Brave, Armeria, gRPC are all projects I know that expect using the BOM to prevent dependency conflicts (just had to work through an issue yesterday because of not using gRPC bom :) ), I don't think we have a reason we need to be stricter here either. It would allow sharing internal APIs between the two, and have less worry of e.g., adding methods to interfaces.
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.
But, if there is a library that is instrumented with API v. 1.0.0, that would mean you can't use SDK v. 1.1.0 with it? Doesn't that defeat the entire purpose of ABI compatibility, let alone API compatibility? This seems to directly contradict what you're saying about SDK ABI requirements!
Or else, we're talking at cross purposes about something!
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.
@anuraaga this statement, as written here, MUST be true. If my app has a dependency on a library, which is instrumented (meaning was compiled against) API v1.0.1, then it MUST work if offered SDK v1.11.33 during runtime.
I think what you want to fix in writing is "should we allow having different versions of API and SDK during runtime". E.g. is it OK for application developer to specify in their build.gradle SDK v1.11.33 and API v1.0.1? I agree with Anuraag here that we should discourage that.
But! What if an application developer has SDK v1.11.33 and some library depends on API v1.20. What will SDK do?
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.
What will it do will obviously depend on the nature of the changes. If they happen to be backward compatible, then things should work. And, if they aren't, then probably all hell will break loose and NoClassDefFoundErrors will abound at runtime, unfortunately.
Can we maybe detect versions at startup and fail fast if there are likely to be compatibility issues?
- We call this requirement the "ABI" compatibility requirement for "Application Binary Interface" compatibility. | ||
- SDK Stability: | ||
- Public portions of the SDK (constructors, configuration, und-user interfaces) must remain backwards compatible. |
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 think these first two points aren't unique to SDK
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 was thinking this is a lesser requirement than the API requirements.
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.
Don't quite follow - how are they lesser? They read the same as API requirements to me :)
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 probably need to update this to be more clear. I think rather than "public", we need to provide a way to specify what pieces of the SDK are under this backwards-compatibility requirement and what isn't. Unless we put the whole SDK into one package, I think it will be hard to make sure that all public classes/interfaces are fully supported.
- Public portions of the SDK (constructors, configuration, und-user interfaces) must remain backwards compatible. | ||
- Internal interfaces are allowed to break. | ||
- Strict ABI compatibility is not required. |
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.
This can't be allowed right? I think many SDK classes are still too important (OpenTelemetrySdk.Builder
for example, span processors, sampler) to risk dependency hell by breaking ABI. Imagine a custom sampler someone published, we can't break it.
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 think this is worth having a discussion about. I'm pretty hesitant to say we're going to maintain ABI compatibility across the entire swath of the SDK at this point, given how youthful it is. A custom sampler should be something that it is easy for an SDK user to update, correct? It's not going to be backed into a library or instrumentation, is it?
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.
It requires the sampler to also update to the newer SDK first, something we shouldn't force post-1.0 for SDK either. For whatever reason, it might be slow to update, or may be abandoned but still useful - the moment we create a situation where a user can't upgrade OTel because they have a dependency linked to the SDK, which IMO is quite valid), then we lose most of the value of semver in the first place. If we didn't, I'd say we'd need to list even trace as sdk-trace-experimental
, we need the experimental flag to reflect whether we are going to introduce dependency hell on users or not.
Another example is Spring, which will be using the SDK classes. If our SDK is 1.0, we should be in a state where we don't cause version conflicts between Spring and OTel (Spring 4.2.3 requires OTel SDK 1.3.4, yikes).
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.
So... what SDK APIs are going to be locked in stone forever? How do we distinguish one public SDK API from another? I feel like if we're going to require strict ABI compatibility with some subset of the SDK, we need a way to publish that module separately, as something like opentelemetry-sdk-api
or something. Any ideas on how to achieve this goal?
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.
All of it I think - the reason we have any public methods at all is because we expect code outside our control to use them. As soon as that happens, we need to commit to those public methods, or we'll cause conflicts - any library that causes conflicts becomes unpopular fast. I don't see anything special about the SDK vs the API in this regard.
We should be as diligent as we can by 1.0 to get things into a good place - we probably won't get everything and we'll have methods deprecated until the next major, etc. It is what it is, but it's safer than introducing ABI breaks that could cause dependency locks for users.
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.
First, I agree with Anuraag, that SDK should have stable public interface. With the same API/ABI compatibility guarantees as API.
Second, as I read it, OTEP 143 allows as to stabilise SDK separate from API. Thus we don't have to claim SDK stability yet. We can just release stable API and continue working on SDK.
Third, if we are going to provide stability guarantees for SDK, we have to distinguish public and internal parts of it. And guarantee only public parts. How to achieve that: separate packages, separate artifacts, JPMS, OSGI - is a separate question :)
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.
yes, after consideration and discussion, I also agree with this. I don't know how we're going to achieve it, but I also think it's important.
Ideas on how to effectively version the various parts of the project independently are very welcome!
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.
Ya, being able to release breaking SDK 2.0 without having to bump API to 2.0 is nice
I think we have similar issue in instrumentation repo, e.g. being able to release grpc library instrumentation 2.0 without having to bump opentelemetry-instrumentation-api
.
This will require a bunch more release infra to manage versioning/releasing modules independently (and using prefixed tags, e.g. api-1.1
, sdk-2.0
).
closing in favor of #2250 |
To fulfill the ask from open-telemetry/opentelemetry-specification#1267