open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.17k stars 751 forks source link

.NET Framework dependency issue #4126

Open jcfarsight opened 1 year ago

jcfarsight commented 1 year ago

I've been experimenting with including OpenTelemetry 1.3.2 in a .NET Framework solution and have run into a problem that others may encounter if they haven't already. It's worth noting that I'm hosting an ASP.NET Core web service (referencing the required Core DLLs) inside a .NET Framework solution.

The minimum version of dependency Microsoft.Extensions.Logging.Configuration is 3.1 (changed in PR 3196) however .NET Core 3.1 is no longer supported as of December 2022. Upgrading this to version 5.0+ is not an option in my case in .NET Framework as the InplaceStringBuilder class in dependency Microsoft.Extensions.Primitives has been removed, and this is required in some ASP.NET libraries (Microsoft.Net.Http.Headers as an example)

ASP.NET Core 2.1 libraries in .NET Framework follow the support period of the Framework itself, unlike 3.1 which does not. (https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core)

I appreciate that it made sense to bump the minimum version to 3.1 for the .NET Core target, but not for the .NET Framework target. Was this use case considered when the decision was made? Until I can migrate the solution to Core it looks like I either need to reference an unsupported version of the library or find an alternative solution.

cijothomas commented 1 year ago

Pinging other issues which are duplicate of this: #4301 #4335

As of today, asp.net core apps targeting .NET Framework are not supported. Two options (I think)

  1. Call this lack of support explicitly (just like .NET Framework 3.5 is explicitly listed as not supported)
  2. Bring back support for this. This might require conditionally adjusting dependencies to allow 2.1
dpk83 commented 1 year ago

Pinging other issues which are duplicate of this: #4301 #4335

As of today, asp.net core apps targeting .NET Framework are not supported. Two options (I think)

  1. Call this lack of support explicitly (just like .NET Framework 3.5 is explicitly listed as not supported)
  2. Bring back support for this. This might require conditionally adjusting dependencies to allow 2.1

I know option 2 is additional effort but it will ease the pain for so many customers and at the same time open up the path for broader adoption of OpenTelemetry, so I will vote for #2

cijothomas commented 1 year ago

Pasting a screenshot from the section from this doc which makes this an officially supported scenario : https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core#servicing image

rymeskar commented 1 year ago

+1 .NET Framework is absolutely a supported scenario on ASP.NET Core, albeit a legacy one.

The current approach of OTEL M.Extensions.* versioning is causing us issues internally. @spashaieva will reach out offline with some more details.

tarekgh commented 1 year ago

CC @danmoseley

julealgon commented 6 months ago

@cijothomas what's the latest on this?

We've been using OpenTelemetry.Instrumentation.AspNetCore v1.0.0-rc9.5 for a while now in a couple of our NET472 AspNetCore projects, as any versions after that seemed to completely break support for tracing and we'd get a ton of orphaned and "empty" spans.

Our team now noticed that the standard OTEL http.server.request.duration metric is also missing for those 2 projects.

We are in a position currently where upgrading to .NET Core 3.1+ is fairly challenging due to some dependencies we still have on full framework-only libraries. While we do have a plan for this migration, we'd like for the instrumentation to work in the meantime.

The way this problem is described in #5019 seems quite easy to overcome in the instrumentation by special-casing the way some arguments are treated, but it was closed as duplicate of this one which is still open.

Is the team going to make OTEL officially unsupported in NET Framework AspNetCore?

This AspNetCore instrumentation is the only one giving us trouble right now. We have latest Http, Sql, EFCore, Runtime, Wcf and AspNet (System.Web) ones working just fine ATM using their latest versions.

cijothomas commented 6 months ago

Is the team going to make OTEL officially unsupported in NET Framework AspNetCore?

This is already unsupported. By "officially" unsupported, did you mean to update https://github.com/open-telemetry/opentelemetry-dotnet?tab=readme-ov-file#supported-net-versions to call it out explicitly? Just like .NET Framework 3.5 is called out?

julealgon commented 6 months ago

Is the team going to make OTEL officially unsupported in NET Framework AspNetCore?

This is already unsupported. By "officially" unsupported, did you mean to update https://github.com/open-telemetry/opentelemetry-dotnet?tab=readme-ov-file#supported-net-versions to call it out explicitly? Just like .NET Framework 3.5 is called out?

Well considering its just the AspNetCore instrumentation that doesn't work properly (unless I'm unaware of other issues, if so, please let me know), maybe just make this explicit in the readme for that package.

cijothomas commented 6 months ago

Is the team going to make OTEL officially unsupported in NET Framework AspNetCore?

This is already unsupported. By "officially" unsupported, did you mean to update https://github.com/open-telemetry/opentelemetry-dotnet?tab=readme-ov-file#supported-net-versions to call it out explicitly? Just like .NET Framework 3.5 is called out?

Well considering its just the AspNetCore instrumentation that doesn't work properly (unless I'm unaware of other issues, if so, please let me know), maybe just make this explicit in the readme for that package.

We don't know if there are other issues. There is no CI tests for ASP.NET .NET Framework apps.