-
Notifications
You must be signed in to change notification settings - Fork 773
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
Proposal for dropping .NET Standard #3701
Closed
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # Dropping support for .NET Standard | ||
|
|
||
| ## Background | ||
|
|
||
| The OpenTelemetry .NET project depends heavily on APIs provided by [.NET Runtime](https://github.com/dotnet/runtime). | ||
| Specifically, the APIs provided under `System.Diagnostics.DiagnosticSource`. | ||
|
|
||
| With the release of .NET 6, [it was announced](https://github.com/dotnet/announcements/issues/190) | ||
| that the .NET team will drop out-of-support frameworks for a number of packages | ||
| including `System.Diagnostics.DiagnosticSource`. This practice will continue | ||
| with the release of .NET 7. Frameworks no longer supported will include .NET | ||
| Core 3.1 and .NET 5. Refer to the [.NET download](https://dotnet.microsoft.com/download/dotnet) | ||
| page to view the end of support dates for each version of .NET. | ||
|
|
||
| The core packages offered by OpenTelemetry .NET currently ship a .NET | ||
| Standard build (i.e., `netstandard2.0` and/or `netstandard2.1`). | ||
| Therefore, OpenTelemetry .NET can technically be consumed by projects targeting | ||
| out-of-support frameworks - even as far back as .NET Core 2.0. | ||
|
|
||
| However, referencing the latest version of OpenTelemetry .NET by an application | ||
| targeting .NET Core 3.1 or .NET 5, for example, will generate the following | ||
| build warnings: | ||
|
|
||
| > System.Diagnostics.DiagnosticSource doesn't support netcoreapp3.1. Consider updating your TargetFramework to net6.0 or later. | ||
|
|
||
| > System.Diagnostics.DiagnosticSource doesn't support net5.0. Consider updating your TargetFramework to net6.0 or later. | ||
|
|
||
| These warnings can be suppressed by setting the | ||
| `SuppressTfmSupportBuildWarnings` MSBuild property. However, | ||
| OpenTelemetry .NET is not tested against out-of-support frameworks. Therefore | ||
| there is no guarantee that it will continue to function. | ||
|
|
||
| ## The plan to remove .NET Standard targets | ||
|
|
||
| The plan moving forward is to align the support cycle of OpenTelemetry .NET | ||
| with the support cycle of .NET. To achieve this, OpenTelemetry .NET plans to | ||
| drop .NET Standard builds of its artifacts. OpenTelemetry packages will only | ||
| offer implementation specific target frameworks (e.g., `net462` and `net6.0`). | ||
| This will enable us to easily test and validate that OpenTelemetry .NET works | ||
| as expected for all the frameworks we target. | ||
|
|
||
| At this time, the frameworks targeted will include at least `net462` and | ||
| `net6.0`, but may include others like `net7.0`, when appropriate. As target | ||
| frameworks reach end of life, we will remove those targets during a major | ||
| version release roughly aligned with the corresponding major version release | ||
| of .NET. | ||
|
|
||
| For example: | ||
|
|
||
| Currently the [OpenTelemetry SDK project](https://github.com/open-telemetry/opentelemetry-dotnet/blob/ee11de90a37915c68d9d44cdd283ba6047b394a3/src/OpenTelemetry/OpenTelemetry.csproj#L4) | ||
| contains the following targets: | ||
|
|
||
| ```xml | ||
| <TargetFrameworks>net6.0;netstandard2.1;netstandard2.0;net462</TargetFrameworks> | ||
| ``` | ||
|
|
||
| When .NET 6 reaches end of life November 2024, the `net6.0` and .NET Standard targets | ||
| will be removed and we will perform a major version release of the OpenTelemetry | ||
| .NET SDK. Presumably, at that time .NET 8 will have been released, and the | ||
| the project file may look like this: | ||
|
|
||
| ```xml | ||
| <TargetFrameworks>net8.0;net462</TargetFrameworks> | ||
| ``` | ||
|
|
||
| Since dropping a framework from a package is a source breaking change, the above | ||
| example describes our process for all of our stable packages. | ||
|
|
||
| That said, the OpenTelemetry .NET project offers a number of packages that have | ||
| not yet had a stable release. We will be removing .NET Standard builds from the | ||
| following packages in the next minor release: | ||
|
|
||
| * `OpenTelemetry.Exporter.Prometheus.AspNetCore` | ||
| * `OpenTelemetry.Exporter.Prometheus.HttpListener` | ||
| * `OpenTelemetry.Exporter.ZPages` | ||
| * `OpenTelemetry.Extensions.Hosting` | ||
| * `OpenTelemetry.Instrumentation.AspNetCore` | ||
| * `OpenTelemetry.Instrumentation.GrpcNetClient` | ||
| * `OpenTelemetry.Instrumentation.Http` | ||
| * `OpenTelemetry.Instrumentation.SqlClient` | ||
| * `OpenTelemetry.Shims.OpenTracing` | ||
|
|
||
| ## How users will be impacted by removing .NET Standard builds | ||
|
|
||
| ### Sharing code between .NET Framework and .NET 6+ applications | ||
|
|
||
| It has been a common practice for users to place shared code in a class library | ||
| targeting .NET Standard. For example, the following project might be used to | ||
| centralize the configuration of OpenTelemetry across both your .NET Framework | ||
| and .NET 6+ applications. | ||
|
|
||
| ```xml | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.0</TargetFrameworks> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="OpenTelemetry" Version="xxx" /> | ||
| </ItemGroup> | ||
| </Project> | ||
| ``` | ||
|
|
||
| With the removal of .NET Standard targets from OpenTelemetry packages, this | ||
| example project must now be multi-targeted as follows: | ||
|
|
||
| ```xml | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFrameworks>net6.0;net462</TargetFrameworks> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="OpenTelemetry" Version="xxx" /> | ||
| </ItemGroup> | ||
| </Project> | ||
| ``` | ||
|
|
||
| ### Referencing OpenTelemetry packages from Xamarin and Mono projects | ||
|
|
||
| Technically, both Xamarin and Mono implement `netstandard2.0`. However, | ||
| OpenTelemetry does not currently support either despite currently offering | ||
| a `netstandard2.0` build. | ||
|
|
||
| If in the future Xamarin or Mono are supported by OpenTelemetry .NET, we | ||
| will add this support via framework specific targets (i.e., `xamarin.android` | ||
| or `net6.0-android`). | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
major version is something like 2.0 / 3.0?
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, that's what I'm proposing 😄.
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.
#2138 historically we only did minor version bump when we dropped framework support (when the fw itself went out-of-support)
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 don't have strong opinion on this. I guess either way should work.
If we go down this path, there is a need to be able to communicate/distinguish whether a major version bump is purely because of certain version drop, or because of some API breaks, or a combination of both. For example, a user might be worried about upgrading from 2.0 to 3.0 due to the fear of breaking changes, although what actually happened is just a drop of deprecated runtime version.
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, this differs from our prior pattern of practice, and I'm suggesting we reconsider our practice for the future. As mentioned in dotnet/announcements#190
I think a major version number is the correct tool to communicate this to users clearly. It aligns with the .NET Runtime team's versioning of DiagnosticSource in regards to dropping supported frameworks.
That said, there certainly are other examples of library authors dropping frameworks with minor version releases of their libraries. Though, I do not think a minor version bump communicates very clearly whether the drop in support was intentional or not.
I'd be very interested in some official guidance to library authors in this respect. Maybe here https://learn.microsoft.com/en-us/dotnet/standard/library-guidance/versioning. Something like:
Maybe .NET would consider this overstepping their bound with their guidance, but I'd like to hear their thoughts.
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, fair. Personally, I think this would be a worthy goal in the long run. So basically adopting option number 2.
In the short run though, I was more considering option number 3. Version 1.x of OTel .NET has served as a learning grounds as OpenTelemetry has matured. We already have a number of things marked
Obsoleteand we're anticipating some more. I think cleaning these things up in a breaking major release will end up benefitting users in the long run.Though let me ask a different question. Do we all agree we want to drop .NET Standard targets from our core packages in the future? If so, my primary intent with this announcement is to:
Whether this be in a minor or major release is more of a technical detail.
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.
In other words, I'm proposing this announcement as our resolution to #3448.
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.
#3703 as an alternative resolution to #3448.
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.
IMO, option (3) is the best one. I think many people assume minor version number bump = "not a breaking change, may add features, no need to even look at release notes if I don't feel like it." Doing a minor version bump that broke people because it dropped support for the runtime they were using would be unexpected.
For option (2) I'd be careful about promises that you will never make a breaking change, it could become a real straight jacket. I'd suggest something more like you strongly value compatibility and breaking changes should be rare.
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.
Thank you for your guidance @noahfalk!
This is the primary reason I'm suggesting changing our practice to a major version bump under this proposal because with the absence of .NET Standard targets, dropping support for a runtime version means users on old frameworks can not upgrade.
Previously, when we dropped
net461,netcoreapp3.1, andnet5.0targets we have done it with a minor version bump. Though, in all of these instances, users targeting these frameworks can currently still upgrade as they will fall back to ournetstandard2.xtargets. It is for this reason I was not as inclined to recommend a major version bump.@cijothomas @CodeBlanch @utpilla Maybe we chat once more in next week's SIG meeting before choosing between this proposal or #3703? If we're set on dropping .NET Standard, we can also further discuss the topic of major vs. minor version bump.