Can OpenTelemetry .NET SDK require 6.0 version of Microsoft.Extensions.Logging?

[reyang]

Previously we cannot because we have to support version 2.1.

Now we don't need to support 2.1 anymore, can we follow what's been done for System.Diagnostics.DiagnosticSource, considering the general guidance from the .NET team:

• You SHOULD use the latest stable version whenever possible, regardless of which version of runtime you are using.
• If you are using version 3.1.x ILogger, upgrade to the latest stable version. There shouldn't be breaking changes.
• If you are using version 1.x, 2.x or 3.0.x ILogger, upgrade to the latest stable version since these versions are no longer supported, except for the special case below:
  • If you have existing dependency on version 2.1.x ILogger, and you are running ASP.NET Core 2.1 application on .NET Framework, use the latest patch version in the 2.1.x line until you're ready to move away from ASP.NET Core 2.1.

opentelemetry-dotnet/build/Common.props

Line 36 in eaa9370

<MicrosoftExtensionsLoggingPkgVer>[3.1.0,)</MicrosoftExtensionsLoggingPkgVer>

opentelemetry-dotnet/build/Common.props

Line 46 in eaa9370

<SystemDiagnosticSourcePkgVer>6.0.0</SystemDiagnosticSourcePkgVer>

[reyang]

This would potentially help #1258.

[CodeBlanch]

@reyang Where did that .NET guidance come from? Any link you can share? I don't necessarily have an issue with this, but was curious if the guidance was geared more towards application maintainers rather than library authors. I've always targeted lowest common denominator in my libraries but possible my thinking needs an adjustment 🤣

[reyang]

@reyang Where did that .NET guidance come from? Any link you can share?

I don't have any public link. IIRC this was based on the discussion with the .NET runtime team, in general Microsoft.Extensions.Logging is trying to maintain the backward compatibility since it got moved from ASP.NET to .NET runtime repo, in a very similar fashion as System.Diagnostic.DiagnosticSource package. We can confirm this with the owner of Microsoft.Extensions.Logging.

[pellared]

You SHOULD use the latest stable version whenever possible, regardless of which version of runtime you are using.

I am not sure if the instrumentation should force bumping dependencies (unless e.g. there are security concerns involved). What if someone has a reason to use an older version (e.g. it has better performance for some use cases)?

On the other hand, working against the latest version would make this repository easier to maintain.

Maybe we should document the approach regarding dependencies versioning here?

[jviau]

As a rule of thumb, libraries should keep the most minimal and least strict dependencies as possible. Not just to give control to the customer to selectively upgrade, but to avoid unresolvable package conflicts. What if another library author has set a requirement for Microsoft.Extensions.Logging < 6.0? Even if by mistake. That library is now incompatible with OTel.

So unless OTel absolutely needs something from 6.0, it should not upgrade.

[rwkarg]

So unless OTel absolutely needs something from 6.0, it should not upgrade.

This is the way to go for a library for the reasons @jviau mentions.

It could have explicit targets for ‘netcoreapp3.1’ that have 3.1.0 dependencies and also target ‘net6.0’ with 6.0.0 dependencies, etc. but that additional complexity is mostly of value if there is some new api in the newer version to conditionally utilize in the newer target frameworks.

[reyang]

Currently the only benefit I can see is purely engineering cost as @pellared mentioned in #3205 (comment).

Moving forward, if Microsoft.Extensions.Loggging added new functionality that OpenTelemetry .NET SDK needs, we can either do the same thing as System.Diagnostic.Source or using conditional logic (either compile time or run time, which ever makes sense).

On the downside, auto-instrumentation might see problems.

[cijothomas]

@pellared @jviau @rwkarg #4933 Please review the PR which is addressing this. I do not think you'll get notified on the PR automatically, but given you have commented in this issue, I think it'll be good to get your thoughts as well on the PR.

[jviau]

I still believe you will reach the broadest set of customers by keeping your dependencies at the lowest possible version. But if you have a good reason for upgrading these dependencies then that is a tradeoff you will need to evaluate. It is true Microsoft.Extensions.* are low risk here as they keep back-compat across major versions, but that risk isn't non-zero.

The plug-in scenario could have issues as well - such as Azure Functions. The app is already compiled against Microsoft.Extensions.*/6.0.0, but extensions can be load dynamically at runtime. If these extensions try to bring in OTel and thus Microsoft.Extensions.*/8.0.0, there would be issues as we 6.0.0 is already loaded.

With that said, OTel is already incompatible with Azure Functions due to System.Diagnostics.DiagnosticSource 7.0.0 being required (Azure Functions loads 6.0.0). So not the best example but demonstrating the plugin pattern is what's important here. It would be up to OTel to decide if minimizing risk to those scenarios is important or not.