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.

Sign up for GitHub

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

Jump to bottom

Add rationale and description of the release and versioning strategy. #2212

Closed
wants to merge 2 commits into from
Closed

Add rationale and description of the release and versioning strategy. #2212

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bd9cc8
Add rationale and description of the release and versioning strategy.
jkwatson Dec 7, 2020
4ab81fe
typo fix
jkwatson Dec 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion docs/rationale.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,69 @@ try (Scope scope = TracingContextUtils.currentContextWith(span)) {
```

It would not be possible to call `recordException` if `span` was also using try-with-resources.
Because this is a common usage for spans, we do not support try-with-resources.
Because this is a common usage for spans, we do not support try-with-resources.


## Versioning and Releases

### Assumptions

- This project uses semver v2, as does the rest of OpenTelemetry.

### Goals

- API Stability:
- 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
Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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!

Copy link
Contributor

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?

Copy link
Contributor Author

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?

SDK library `opentelemetry-sdk-trace:1.11.33`.
- 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.
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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 :)

Copy link
Contributor Author

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.

- Internal interfaces are allowed to break.
- Strict ABI compatibility is not required.
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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).

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

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 :)

Copy link
Contributor Author

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!

Copy link
Member

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).


### Methods

- Mature signals
- API modules for mature (i.e. released) signals will be transitive dependencies of the `opentelemetry-api` module.
- Methods for accessing mature APIs will be added, as appropriate to the `OpenTelemetry` interface.
- SDK modules for mature (i.e. released) signals will be transitive dependencies of the `opentelemetry-sdk` module.
- Configuration options for the SDK modules for mature signals will be exposed, as appropriate, on the `OpenTelemetrySdk` class.

- Immature or experimental signals
- API modules for immature signals will not be transitive dependencies of the `opentelemetry-api` module.
- API modules will be named with an "-experimental" suffix to make it abundantly clear that depending on them is at your own risk.
- API modules for immature signals will be co-versioned along with mature API modules.
- The java packages for immature APIs will be used as if they were mature signals. This will enable users to easily transition from immature to
mature usage, without having to change imports.
- SDK modules for immature signals will also be named with an "-experimental" suffix, in parallel to their API modules.

### Examples

Purely for illustration purposes, not intended to represent actual releases:

- `v1.0.0` release:
- `io.opentelemetry:opentelemetry-api:1.0.0`
- Contains APIs for tracing, baggage, propagators (via the context dependency)
- `io.opentelemetry:opentelemetry-api-metrics-experimental:1.0.0`
- Note: packages here are the final package structure: `io.opentelemetry.api.metrics.*`
- `io.opentelemetry:opentelemetry-sdk-trace:1.0.0`
- `io.opentelemetry:opentelemetry-sdk-common:1.0.0`
- Shared code for metrics/trace implementations (clocks, etc)
- `io.opentelemetry:opentelemetry-sdk-metrics-experimental:1.0.0`
- Note: packages here are the final package structure: `io.opentelemetry.sdk.metrics.*`
- `io.opentelemetry:opentelemetry-sdk-all:1.0.0`
- The SDK side of `io.opentelemetry:opentelemetry-api:1.0.0`
- No mention of metrics in here!
- `v1.15.0` release (with metrics)
- `io.opentelemetry:opentelemetry-api:1.15.0`
- Contains APIs for tracing, baggage, propagators (via the context dependency), metrics
- `io.opentelemetry:opentelemetry-sdk-trace:1.15.0`
- `io.opentelemetry:opentelemetry-sdk-common:1.15.0`
- Shared code for metrics/trace implementations (clocks, etc)
- `io.opentelemetry:opentelemetry-sdk-metrics:1.15.0`
- Note: packages here have not changed from the experimental jar...just a jar rename happened.
- `io.opentelemetry:opentelemetry-sdk-all:1.15.0`
- The SDK side of io.opentelemetry:opentelemetry-api:1.15.0