opentelemetry-dotnet-instrumentation icon indicating copy to clipboard operation
opentelemetry-dotnet-instrumentation copied to clipboard

Build a NuGet package that adjusts all of the dependency references

Open pellared opened this issue 3 years ago • 7 comments

Build an nuget package that adjusts all of the dependency references so that it can be used to mitigate the issue when the application uses a different version of OTel .NET SDK, System.Diagnostics.DiagnosticSource or any other dependency that can cause problems.

Originally posted by @RassK in https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/307#issuecomment-1012176089

pellared avatar Jan 13 '22 14:01 pellared

This seems inline with this proposal: https://github.com/dotnet/runtime/issues/66138#issuecomment-1071045008

pellared avatar Mar 17 '22 16:03 pellared

If application has higher version of OpenTelemetry SDK than the Auto-Instrumentation OpenTelemetry SDK. Higher version wins and if there are breaking changes between these versions, auto-instrumentation might affect an application. We might consider using this option with caution.

rajkumar-rangaraj avatar Mar 17 '22 17:03 rajkumar-rangaraj

If application has higher version of OpenTelemetry SDK than the Auto-Instrumentation OpenTelemetry SDK. Higher version wins and if there are breaking changes between these versions, auto-instrumentation might affect an application. We might consider using this option with caution.

I am not worried as much about it. We need to keep in sync with OpenTelemetry SDK versioning. We would need to think about how to tackle it. So far the SDK follows SemVer quite well. Maybe we will just have our major versions in sync with the SDK? We will see.

I am more worried about transitive dependencies such as Google.Protobuf.dll, Grpc.Core.Api.dll, Grpc.Net.Client.dll, Grpc.Net.Common.dll. System.Runtime.CompilerServices.Unsafe and potential breaking changes between the versions used by OpenTelemetry SDK or AutoInstrumentation and the application... And there could be more dependencies like that as the project evolves.

TBH I am not sure if there is an easy way to solve this problem as I think that the SDK library instrumentations need to be applied in the instrumented application's AppDomain. @open-telemetry/dotnet-maintainers - am I correct?

pellared avatar Mar 17 '22 17:03 pellared

Maybe instead of a nuget package to update the dependencies, would it be possible to have a nuget package that contains the auto instrumentation bits and updates the dependencies to match those bits so that everything will be consistent? Doing that might also open the door to allowing them to use both the SDK (that we packaged) directly while providing the flexibility for using byte-code instrumentation.

nrcventura avatar Apr 05 '22 21:04 nrcventura

My biggest worry about creating a NuGet package is that as the number of "source instrumentation" grows the more dependencies we will bring and it may lead to dependency hell.

However, I still think it is worth giving a try.

pellared avatar Apr 08 '22 09:04 pellared

We might need to prioritize this issue. In the cases where FileLoadException are thrown like #1027 we could ask to refer the auto-instrumentation Nuget instead of those specific Nuget that is causing a failure. This would be a simpler workaround for many of these file load issues.

rajkumar-rangaraj avatar Aug 16 '22 17:08 rajkumar-rangaraj

I like the idea of having a Nuget package to help with those dependencies, however I think that deciding what should go in it is a little less straightforward and will probably not handle all of the current FileLoadException issues. For example, the missing AspNetCore.Features library should probably not be in that Nuget package, so the Nuget package will not help resolve that specific issue. I also think that the unsafe compiler services dependency should be handled differently, so the Nuget issue would only help until we fix that direct dependency. As a result, I think we should have a better idea about which specific problems this type of Nuget package could help with resolving in order to determine if it is worth the maintenance cost.

nrcventura avatar Aug 16 '22 17:08 nrcventura