opentelemetry-dotnet: Discussion: .NET 7 netstandard2.0 packages don't compile for out-of-support runtimes
If you look at an out-of-band NET 7 package like System.Diagnostics.DiagnosticSource it lists .NETStandard 2.0 as a target. The .NET Standard 2.0 doc lists a wide range of runtimes including ones that are no longer supported (or will be out of support by the time NET 7 ships).
Let’s say a user is on .NET 5 and upgrades OTel once it has a dependency on NET 7 DS. There will be no explicit .NET 5 target in either OTel or DS, but due to the presence of .NETStandard 2.0, the tooling will allow the upgrade.
What will happen in this case is an error will start showing up surfaced through MSBuild:
System.Diagnostics.DiagnosticSource doesn't support net5.0. Consider updating your TargetFramework to net6.0 or later.
The reason for this is the .NET 7 out-of-band packages include a sort of poison pill targets file:
This is not exclusive to System.Diagnostics.DiagnosticSource
. @alanwest did some digging we see it in System.Text.Encodings.Web
(referenced through System.Text.Json
) and Microsoft.Extensions.Logging.Abstractions
.
The main concern here is the confusion. We anticipate issues being opened in our repo because the intention was to upgrade OTel, not these other transitive packages where the errors are being thrown.
Some options we have discussed…
- Drop
netstandard2.0
&netstandard2.1
and just explicitly list out our supported runtimes (egnet462;net6.0;net7.0
). What this does is stop the upgrade in the first place. This will break many users consuming OTel fromnetstandard2.0
projects. But they probably would be broken anyway due to the poison pill? We would also lose mono/unity/xamarin but not sure if SDK works on those runtimes currently. The main thing this does is make the break explicit and better conveys the true supported runtimes. - Add our own poison pill targets file(s) in our packages. This would at least make the error less confusing and reference OTel instead of something transitive.
- Keep
netstandard2.0
on DS v6. Big maintenance headache to do this. Could also lead to different APIs being exposed which leads to issues like https://github.com/open-telemetry/opentelemetry-dotnet/issues/3434.
/cc @alanwest @cijothomas @utpilla @reyang @tarekgh @noahfalk
About this issue
- Original URL
- State: open
- Created 2 years ago
- Reactions: 6
- Comments: 24 (8 by maintainers)
Commits related to this issue
- Emit warning instead of an error in package compat Based on the discussed in https://github.com/open-telemetry/opentelemetry-dotnet/issues/3448, we are now emitting a warning instead of an error on u... — committed to dotnet/runtime by ViktorHofer 2 years ago
- Remove Diagnostics out of netstandard2.0 https://github.com/open-telemetry/opentelemetry-dotnet/issues/3448 — committed to ServiceStack/ServiceStack by mythz 2 years ago
- Emit warning instead of an error in package compat (#72518) Based on the discussed in https://github.com/open-telemetry/opentelemetry-dotnet/issues/3448, we are now emitting a warning instead of an e... — committed to dotnet/runtime by ViktorHofer 2 years ago
- Emit warning instead of an error in package compat (#72518) Based on the discussed in https://github.com/open-telemetry/opentelemetry-dotnet/issues/3448, we are now emitting a warning instead of an e... — committed to ovidiucosteanet/runtime by ViktorHofer 2 years ago
Want to thank @andrewlock for bringing this to our attention and reiterate that the behavior of not supporting something you say you do is a mine field that defies all expectations.
It’d be ok if this was an inert package with stub classes that did nothing on unsupported platforms, but to throw runtime exceptions and unintuitively break platforms it says it supports is a footgun that should never have existed. IMO its netstandard2.0 implementation should be changed so it has inert behavior that’s silently ignored or
netstandard2.0
should be removed as a supported platform.Thanks for the clarification @ViktorHofer, this has been frustrating me more and more recently, especially as the original explanation didn’t seem to justify the actual behaviour. The fact that you would silently switch from using netcoreapp assets to netstandard assets makes sense to me.
As for how to fix it… that’s harder.
I don’t see why they wouldn’t work? 🤔 If you have netstandard2.0 assets, and the runtime supports netstandard2.0, what’s the problem? And how are you testing those .NET standard assets?
This bothers me. You’re saying the .NET Standard libraries were never intended to work on platforms that support .NET Standard? I mean, that seems like the crux of this entire problem in that case, and it baffles me.
The whole point of .NET Standard as originally espoused was to allow targeting a common set of APIs. I get there was some nasty hacking around and shims etc to patch netstandard2.0 support into net461 (which caused no end of issues). And some “implementations” throw at runtime (a design which I similarly detest for obvious reasons). Is that the root cause?
Just thought I’d flag it here @noahfalk and @ViktorHofer - ServiceStack recently added a dependency on DiagnosticSource v6.0 NuGet pkg on the basis that it “supports” netstandard 2.0. As we’ve already discussed, that’s not really the case, so in reality ServiceStack accidentally dropped support for .NET Core 2.1 and .NET Core 3.0 (as well as any other .NET Standard platforms) in a minor release.
To me, this really highlights the danger of keeping the netstandard2.0 targets - @mythz is an experienced .NET developer, and still got caught by this.
From twitter:
@tarekgh Certainly doesn’t answer all the questions we have, but I found it interesting…
This is data representing modern .NET applications (i.e., not .NET Framework) reporting to New Relic. This data is sourced from metadata New Relic’s .NET agent sends when an application it’s monitoring starts up and connects.
These are all connections from the last 30 days. The majority of which are from LTS versions of .NET: 50% from .NET Core 3.1 and 30% from .NET 6.
I can’t say how representative this data is for users of OpenTelemetry .NET, but it does at least indicate .NET Core 3.1 is still going strong. It’ll be interesting to see how this changes between now and December when 3.1 reached EOL.
Feedback/Questions for Viktor
@ViktorHofer - Thanks! The example was very useful to understand where you are coming from. My perspective is a bit different though : ) I didn’t find this example about GetUtcNow a compelling justification of the policy. Partly it is because I anticipate the behavior change would be sufficiently small that 99.99% of customers wouldn’t notice it at all, and partly because I think most developers would begrudgingly accept that a major version update of a NuGet package running on an unsupported runtime might introduce behavior changes and bugs.
A couple suggestions given my thinking on the issue so far:
The claim is that developers won’t be able to update, but it doesn’t mention what is the enforcement mechanism. Also the sentence says “if you reference an impacted package”, the announcement contains a list of these packages, and OpenTelemetry is not a library in that list. I think people will interpret “reference” to mean a direct reference that appears in their project file whereas the actual policy seems to include any reference in the transitive closure of their NuGet package graph. In practice I think our current policy is asking developers to audit their entire transitive package graph any time they upgrade anything which is a much greater scope than what they are likely to assume reading the announcement.
The announcement repeatedly says that .NETStandard 2.0 is supported, which is likely to lead most library developers who author .NETStandard 2.0 libraries to believe they are unaffected by this change. It might also lead app developers who target netcoreapp2.1 or net46 to believe that they too are OK by referencing packages with .NETStandard 2.0 binaries. I think we need to clarify that none of these assumptions is accurate under the current policy.
Fwiw I’d be in favor of less aggresive blocking:
Feedback/Questions for the OTel crew
DiagnosticSource version 6.0 has had the poison pill in it for netcoreapp2.1 since it shipped, and OpenTelemetry 1.2.0 depends on it. Given that 1.2.0 has been downloaded 500K times and has been available for 3 months, if users were confused wouldn’t we already be seeing that signal? Have we gotten some complaints so far? Is there reason to believe that what is coming will be significantly worse than what we’ve already seen?
What is Open Telemetry’s stance on supporting runtime versions that the .NET team itself no longer supports? I think the easy policy would be to say when .NET team stops support, so does OTel. In that case when a hypothetical customer complains:
I’d imagine the OTel response would be:
PS A little shout out to Andrew
Off topic but I just wanted to say I’ve seen a lot of your blog posts on different topics and you consistently do an awesome job describing how .NET stuff works. I’ve written a number of the official .NET docs on docs.microsoft.com but I don’t think I achieve the same level of flow and clarity that your posts have. I’m both envious and thankful that your content is out there : )
@alanwest personally I think any data of this sort you’re comfortable sharing regularly could be valuable for the .NET ecosystem, for example it could help inform .NET platform decisions. cc @damianedwards
@tarekgh
I can’t tell you how much I want to be able to answer this question 😂, but the best I think I’ll be able to do is come up with are anecdotes. I intend to reach out to a few .NET customers to try to get a sense of the impact. Will update you with anything I learn.
Measuring OTel adoption faceted by interesting things like language and runtime version are hard problems at the moment. Any attributes on telemetry identifying this kind of metadata are optional per the spec. Things like user agent might be used to at least identify the language of the library used to send telemetry. However, many customers proxy their data through the OpenTelemetry Collector, so from the perspective of user agent it looks like it came from a Go service.
@tarekgh asked me to join this discussion. I work on dotnet/runtime’s infrastructure and drove many of the changes that you are observing and discussing here.
With .NET 6 we explicitly made the decision to remove out-of-support assets from our packages. This was communicated via https://github.com/dotnet/announcements/issues/190. The motivation for this is well explained in the announcement but TL;DR: “For .NET 6, we plan to no longer perform any form of harvesting to ensure that all assets we ship can be serviced”.
What the announcement didn’t cover is how we guarantee that customers don’t bind on a different asset - on an unsupported runtime - than what they previously relied on.
Imagine your library compiles against
netcoreapp3.1
and references the 6.0.x version of the DiagnosticSource package. Asnetcoreapp3.1
isn’t supported anymore in the 7.0.0 package, without an explicit msbuild error, the library would silently not bind against a .NETCoreApp asset anymore but against .NETStandard. The same applies if your library targetsnet461
(which is out-of-support) as the minimum supported .NET Framework asset in the 7.0.0 package isnet462
.In some cases this might just work, but many of our libraries work fundamentally differently on .NETStandard and were never designed to be invoked on .NETFramework or .NETCoreApp. Even though that goes against the basic principle of .NET Standard (a single implementation that works for many different runtimes), that’s the current situation in dotnet/runtime. As an example, [DiagnosticSource]https://github.com/dotnet/runtime/blob/main/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj) has completely different compile inputs on .NETStandard, .NETFramework and .NETCoreApp.
At the time we were updating our packages and dropping out-of-support assets, we felt that silently binding against a fundamental different implementation on an unsupported runtime will cause more issues for consumers, than blocking the consumption on out-of-support runtimes.
The feedback that we’ve received in the last two years on this was about the inconvenience around the incorrect support statement of netstandard2.0 in our packages. @andrewlock wrote this excellent blog post that provides even more insights into the change: https://andrewlock.net/stop-lying-about-netstandard-2-support/.
As you already identified, there are two escape hatches that I wouldn’t recommend to use as nothing guarantees that the .NETStandard asset works or will work on all supported .NETStandard runtimes:
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>
for consumers.ExcludeAssets=buildTransitive
metadata to the PackageReference.Please share your opinion and help us decide how to improve this scenario. Examples of some options:
cc @ericstj @danmoseley @terrajobst
Another big downside to this option:
One of the primary use cases of .NET Standard is for folks to put shared code in to their own netstandard2.0 targeted libraries. This enables them to share code between .NET Framework and modern .NET applications as they’re making the transition. We have end users of OpenTelemetry .NET doing this in order to ensure common practices and config are adopted uniformly across their teams.
Let’s say I currently have an environment with a mix of
net48
,net5
, andnet6
applications that are all referencing my ownnetstandard2.0
library. Now DiagnosticSource v7 is released. I should be able to make use of new features like theUpDownCounter
, for example, in mynet48
andnet6
applications. However, in order to useUpDownCounter
in my shared library, I’m now the burdened with multi-targeting it tonetstandard2.0;net48;net6
and conditionally compiling any usage ofUpDownCounter
.I think this begins to defeat sole purpose of .NET Standard. If the plan is to EOL .NET Standard then I think the right thing to do is to stop releasing libraries that target it.