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:

image

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 (eg net462;net6.0;net7.0). What this does is stop the upgrade in the first place. This will break many users consuming OTel from netstandard2.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

Most upvoted comments

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.

  1. Drop netstandard2.0 assets in our packages entirely: My gut reaction initially was that this was the correct option, because the error implies that you don’t really support netstandard2.0. But based on your previous message, the reality is, you do support netstandard2.0, and the error is about the fact the new package would bind the netstandard assets instead of netcoreapp assets. So yeah, you can’t, and shouldn’t, do this.
  2. Drop netstandard2.0 implementations from packages entirely, by making them throw on initialization: This is the worst of all worlds IMO - it restores, builds, and throws at runtime? Please no.
  3. Remove the msbuild error and allow out-of-support runtimes to rely on the .NETStandard asset.: I understand wanting to warn people that they’re binding against different assets, but if you ship netstandard2.0 assets you support netstandard2.0. It’s up to the runtime to support netstandard2.0 right? So if you test those netstandard2.0 assets, then your job is done. I guess the concerning part is “We don’t test today on anything except supported .NETCoreApp and .NETFramework versions” implies to me that the netstandard2.0 assets aren’t actually tested 🤔 And RE communication - I think that can be done via the TFMs you target - if you target netstandard2.0 and netcoreapp3.1, then clearly you’re using netstandard2.0 for .NET Core 2.1. I don’t see why additional warnings are required, that’s just how NuGet works? 🤷
  4. Downgrade the msbuild error to an msbuild warning If you insist on a warning/error, this seems like the minimum.

This would still defeat the purpose of .NETStandard as many packages will most likely not work on older runtimes

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?

many of our libraries work fundamentally differently on .NETStandard and were never designed to be invoked on .NETFramework or .NETCoreApp

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

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

runtimes

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] These assumptions are a good example of why we decided to explicitly block the invocation of the .NETStandard asset on the .NETFramework and .NETCoreApp runtime

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

  1. Whatever we do, we should make sure developers understand what to expect and I don’t think our previous announcement is sufficient to have people understand all the implications of the new policy. Perhaps we should chat with Rich or Immo about how to best make clarifications? I’d be happy to help. Reading through the announcement you linked to, nothing in that announcement says .NET developers will get build errors now. I noticed one sentence that maybe was intended to imply this, but I never would have guessed that conclusion myself without significant additional deliberation:

If you’re currently referencing an impacted package from an earlier framework, you’ll no longer be able to update the referenced package to a later version

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.

  1. How much feedback have we been hearing from customers around the presence of these poison pills either good or bad? Now that these poison pills have been present in our released packages for 6 months we should be able to evaluate how that decision is working out. Are we seeing a variety of people unhappy with the result or is confusion/aggravation relatively sparse?

Fwiw I’d be in favor of less aggresive blocking:

  • Instead of making it an error to restore on unsupported runtimes we could make it a warning.
  • We could change the error message so that people better understand the issue and their options. For example it could say:
netcoreapp2.1 is no longer under support and has not been tested with System.Runtime.CompilerServices.Unsafe 7.0.
Consider upgrading your TargetFramework to netcoreapp3.1 or later. You may also set
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> in the project file
to ignore these errors and attempt to run in this unsupported configuration at your own risk.
  • I think it would probably be fine not to warn or block at all and just let people on unsupported runtimes see behavior changes, but I’m willing to accept that it wasn’t what we decided to do in .NET 6 and without some significant negative feedback there is probably no reason to risk changing our .NET 6 decision that drastically.

Feedback/Questions for the OTel crew

[@cijothomas] 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.

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?

[@cijothomas] The case we see in OTel is users have some mix of hosting applications (let’s say .NET Framework 4.6.1 & .NET 5)

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 just updated to OTel 1.4 and it gives me an error when I try to use it on net5.0. What gives?

I’d imagine the OTel response would be:

Microsoft support for .net5.0 ended May 10th 2022 and OTel stopped support at the same time. OTel 1.4 shipped in 2023
and did not include any .net 5.0 support. If you need to stay on .net5.0 despite it no longer being supported then we'd
recommend you use OTel 1.3 which was the last shipped version of OTel designed and tested to work with .net5.0.

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

do you have more data about that?

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. As netcoreapp3.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 targets net461 (which is out-of-support) as the minimum supported .NET Framework asset in the 7.0.0 package is net462.

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:

  1. Set <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> for consumers.
  2. Add ExcludeAssets=buildTransitive metadata to the PackageReference.

Please share your opinion and help us decide how to improve this scenario. Examples of some options:

  1. Drop netstandard2.0 assets in our packages entirely (this will cause app models like source generators and UWP which don’t run on .NETFramework or .NETCoreApp to be shut off). This would be way too disruptive as the current ecosystem builds and relies on .NETStandard, especially class libraries.
  2. Drop netstandard2.0 implementations from packages entirely, by making them throw on initialization. This would allow class libraries to continue consuming our .NETStandard packages but there wouldn’t be an msbuild error anymore and instead a runtime load error for any runtime. This might make it clearer that our packages don’t provide an implementation but only a contract that libraries can bind on. We already do this for some packages which don’t work cross-platform: https://github.com/dotnet/runtime/blob/87aa786b3997a23501912abd4654e4fd9958230c/src/libraries/Microsoft.Win32.SystemEvents/src/Microsoft.Win32.SystemEvents.csproj#L18.
  3. Remove the msbuild error and allow out-of-support runtimes to rely on the .NETStandard asset. If we would do this, how would we communicate the “real” minimum supported .NETCoreApp and .NETFramework versions? We can’t provide any guarantee on if the .NETStandard implementation is runnable on a given runtime. (We don’t test today on anything except supported .NETCoreApp and .NETFramework versions).
  4. Downgrade the msbuild error to an msbuild warning so that customers can still use package in an unsupported state on an out-of-support runtime without any guarantee. This would still defeat the purpose of .NETStandard as many packages will most likely not work on older runtimes.

cc @ericstj @danmoseley @terrajobst

Keep netstandard2.0 on DS v6.

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, and net6 applications that are all referencing my own netstandard2.0 library. Now DiagnosticSource v7 is released. I should be able to make use of new features like the UpDownCounter, for example, in my net48 and net6 applications. However, in order to use UpDownCounter in my shared library, I’m now the burdened with multi-targeting it to netstandard2.0;net48;net6 and conditionally compiling any usage of UpDownCounter.

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.