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 Versioning details #1648

Merged
merged 22 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions OpenTelemetry.sln
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
README.md = README.md
RELEASING.md = RELEASING.md
tool_FinalizePublicApi.ps1 = tool_FinalizePublicApi.ps1
VERSIONING.md = VERSIONING.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{7CB2F02E-03FA-4FFF-89A5-C51F107623FD}"
Expand Down
103 changes: 103 additions & 0 deletions VERSIONING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# Versioning details
cijothomas marked this conversation as resolved.
Show resolved Hide resolved

This follows the [OpenTelemetry versioning proposal](https://github.com/open-telemetry/oteps/pull/143/files)

OpenTelemetry .NET follows [SemVer V2](https://semver.org/spec/v2.0.0.html)
guidelines. This means that, for any packages released from this repo, all
public APIs will remain backward compatible, unless a major version bump occurs.
This applies to the API, SDK, as well as Exporters, Instrumentation etc. shipped
from this repo.

For example, users can take a dependency on 1.0.0 version of any package, with
the assurance that all future releases until 2.0.0 will be backward compatible.
Any method/function which are planned to be removed in 2.0, will be marked
[Obsolete](https://docs.microsoft.com/dotnet/api/system.obsoleteattribute)
first.

## Pre-releases

Pre-release packages are identified by identifies such as -Alpha, -Beta, -RC
etc. There is no API guarantees in pre-releases. In general, an RC pre-release
is more stable than a Beta release, which is more stable than an Alpha release.
cijothomas marked this conversation as resolved.
Show resolved Hide resolved

### Public API change detection

For convenience, every project in this repo uses
[PublicApiAnalyzers](https://github.com/dotnet/roslyn-analyzers/tree/master/src/PublicApiAnalyzers)
and lists public API in the directory "publicAPI". Any changes to public API,
without corresponding changes here will result in build breaks - this helps
catch any unintended changes to public API from being shipped accidentally. This
also helps reviewers quickly understand if a given PR is making public API
changes. For example,
[this](https://github.com/open-telemetry/opentelemetry-dotnet/tree/master/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi)
shows the public per target framework for the
`OpenTelemetry.Instrumentation.AspNetCore` package.

Since no stable version has been released so far, every API is listed in the
"Unshipped.txt" file. Once 1.0.0 is shipped, it'll be moved to "Shipped.txt"
file.

## Packaging

OpenTelemetry is structured around signals like Traces, Metrics, Logs, Baggage
etc. OpenTelemetry .NET does not ship separate package per signal. There is a
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
single package which includes all the signals. i.e `OpenTelemetry.API` package
will consist of API components from *all* the signals. `OpenTelemetry` package
will consist of SDK components from *all* the signals. Instrumentations also

Choose a reason for hiding this comment

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

Something interesting that was requested in Java, was the ability to offer a MicroMeter-based Metrics implementation, which would mean that each SDK signal portion would exist in its own package.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually realized that there is some more clarifications required here. For SDK, its true that we have single package for traces and metrics. For API, its somewhat complex, as most of Tracing API is provided by System.Diagnostics.DiagnosticSource package, Context/Propagation is provided by OpenTelemetry.API. We do not know yet whether .NET team will use the System.Diagnostics.DiagnosticSource package itself for metrics, or a brand new package...

follow the same model - for example, There will be a single package
`OpenTelemetry.Instrumentation.AspNetCore` for ASP.NET Core instrumentation,
which produces Traces, Metrics and handles propagation, instead of separate
packages for traces and metrics.

## Experimental Signals

### Current approach

Given the fact that all signals are shipped as a single package, the following
approach is currently used to deal with experimental signals.

Any experimental signal which is shipped as part of a normal package will be
marked
[Obsolete](https://docs.microsoft.com/dotnet/api/system.obsoleteattribute).
(Ideally we need an "Experimental" attribute, but .NET has only built-in support
for "Obsolete", so we are leveraging it. Using "Obsolete" code results in
compile time warnings to caution users.) This allows an experimental signal to
co-exist in the same package as other, non-experimental signals. For example,
the OpenTelemetry 1.0.0 package will consist of Traces and Metrics. As Metrics
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved
signal is not ready for stable release, every method dealing with Metrics will
be marked "Obsolete". Once Metrics signal gets stable, it'll be added to a
release with minor version bump, say 1.2.0. The "Obsolete" Metric methods can be

Choose a reason for hiding this comment

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

One tricky thing is that you may need two policies for Deprecated elements;

  1. Ones using it as Experimental will have this attribute removed in the (near) future.
  2. Ones that are effectively Deprecated and will be removed at some point in the (distant) future.

Copy link
Member

Choose a reason for hiding this comment

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

@cijothomas Thanks for taking the time to write this up! I think I'm leaning towards approach 2 for this reason @carlosalberto brings up above. I'm thinking it will be hard for users to know from [Obsolete] what is being removed and what is experimental. We could help with the text in the attribute, but I think having the experimental stuff in a pre-release package is more obvious and a better experience for users than combing through warnings. Also, NuGet does support deprecated packages (example Microsoft.CodeAnalysis.FxCopAnalyzers - notice the yellow warning, click for details) which we could possibly use when experimental stuff becomes official and is moved into main libraries?

Copy link
Member

Choose a reason for hiding this comment

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

I think using [Obsolete] attribute for experimental features could be confusing, and is probably against the common practice?

I would vote for using [Obsolete] only on things that we plan to remove in the next major release, not on things that we plan to introduce in the future.

removed with a major version bump.

### Alternate option
Copy link
Member

@alanwest alanwest Dec 9, 2020

Choose a reason for hiding this comment

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

I'm not settled on this yet, but I'm beginning to warm up to this approach. My main concern was that early adopters of metrics will likely strive to stay current as metrics functionality evolves. As metrics becomes more and more stable, so will the application code they have written. I think it would be ideal that when metrics goes GA, early adopters would not then be required to switch their code to use a different package/namespace.

Two questions:

  • If we initially shipped metrics in a OpenTelemetry.Metrics.Experimental package, could we simply use the OpenTelemetry.Metrics namespace for all public API? This way users would not then be required to change a bunch of usings when metrics goes GA.
  • Perhaps the separate package approach + the use of the obsolete attribute would be beneficial? That is, the build warnings are a nice signal that you're using something at your own risk.

Copy link
Member Author

Choose a reason for hiding this comment

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

Assuming we get metrics into own separate package, yes agree we should use same namespace. The goal would be - early adopters who were using the experimental, will find it easy to migrate. The migration involves just removing the metric-experimental package, and upgrading the opentelemetry package to the one containing metrics, no code change. (Assuming they kept up-to-date with the experimental).

Copy link
Member Author

Choose a reason for hiding this comment

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

If we are using separate package approach, then we can name it simply OpenTelemetry.API/SDK version 1.0.0-beta.1. The pre-release version already indicate its a unstable package, so additionally marking obsolete won't be necessary.

Choose a reason for hiding this comment

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

If we initially shipped metrics in a OpenTelemetry.Metrics.Experimental package, could we simply use the OpenTelemetry.Metrics namespace for all public API?

If we go with separate packages, I'm all up for this (definitely making the users' lives easier).

Copy link
Member

Choose a reason for hiding this comment

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

then we can name it simply OpenTelemetry.API/SDK version 1.0.0-beta.1.

You mean the same package name as we have today? That is OpenTelemetry and OpenTelemetry.Api?

Since we have already released version 1.0.0-rc1.1 this won't work because 1.0.0-rc1.1 > 1.0.0-beta.1 based on how semver and nuget handles things.

I envisioned that separate package would mean difference package name, so something like OpenTelemetry.Metrics.Experimental as you suggested.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea you are right. I was trying to say - we can drop the word "Experimental" from the package name. Like OpenTelemetry.Metrics .9.0-alpha1 for instance? the pre-release alone is sufficient to indicate its not stable. right?


An alternate option to deal with experimental signal is to remove the signal
from the common packages, and ship as separate package. For example,
OpenTelemetry 1.0.0 will only contain Traces, and there will be a separate
package OpenTelemetry.Metrics.Experimental, which contains the metric signal.
Once the signal achieve stable quality, it'll be made part of the main package
and released as a minor version update. i.e OpenTelemetry 1.2.0 will contain
Traces and Metrics.

## Examples
cijothomas marked this conversation as resolved.
Show resolved Hide resolved

Following shows an example on how the `OpenTelemetry` package versioning works:

`OpenTelemetry` 0.7.0-beta1 release : Pre-release, no API guarantees.

`OpenTelemetry` 1.0.0-RC1 release : Pre-release, no API guarantees, but more
stable than beta.

`OpenTelemetry` 1.0.0 release : Stable release consisting of Traces, Propagators
and Metrics. Metrics will be marked "Obsolete", to warn users that Metrics is
not Stable.

`OpenTelemetry` 1.0.1 release : Bug fixes.

`OpenTelemetry` 1.1.0 release : New features added.

`OpenTelemetry` 1.2.0 release : Add metric support. This will be additive changes.
Copy link
Member

Choose a reason for hiding this comment

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

Would all the methods for metrics previously marked obsolete be removed? Or would that not happen until 2.0.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't we keep it until 2.0 ? So that, if someone took a dependency on it, they can still compile their code without issues when upgrading to 1.2.0.

Choose a reason for hiding this comment

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

Or would that not happen until 2.0.0?

That's (more or less) the idea. In OpenTracing we used to remove Deprecated members right in the next release and, well, I remember quite a few complains :)

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, yea I see how this may become tricky. If we see a desire to change the name or signatures of methods as we evolve things, then I think that would be mean we'd have to maintain all variations of these methods if we require that an upgrade to 1.2.0 must still compile fine.

Makes me more strongly favor the separate package approach.


`OpenTelemetry` 1.3.0 release : More features.

`OpenTelemetry` 2.0.0 release : Remove all Obsolete methods from 1.* releases.