elastic / apm-agent-dotnet

https://www.elastic.co/guide/en/apm/agent/dotnet/current/index.html
Apache License 2.0
575 stars 207 forks source link

Hook: changing version resolution to be more tolerant of versions upgrades #1619

Open Matiszak opened 2 years ago

Matiszak commented 2 years ago

Currently .net hook is diciding which version of the assemblies to load depending on version of System.Diagnostics.DiagnosticSource package. But it seems sometimes this version might be bumped up by using some of the nuget packages (e.g. https://github.com/open-telemetry/opentelemetry-dotnet/issues/2824). Since for me it seems more of an issue with OpenTelemetry nuget it also poses a question if the version resolution shouldn't be more based on environment version than on single library version or some other way ?

Is your feature request related to a problem? Please describe. This problem happened because our solution running in net5.0 had reference to nuget which bumped System.Diagnostics.DiagnosticSource to 6.0.0.0. For us the whole process failed fast on startup (luckily). But I imagine if support for .net 6 was already added to ElasticApm then AspNetCore libraries version 6.x.x.x would be loaded by ElasticApm which would be risky. Also if I just referenced all of the libraries in source code (instead of using hook) correct versions of apm libraries would be choosen based on 'net5.0' runtime and not version of some library (https://github.com/elastic/apm-agent-dotnet/blob/main/src/Elastic.Apm.AspNetCore/Elastic.Apm.AspNetCore.csproj#L33-L38)

[2022-01-26 20:34:19Z] Check if System.Diagnostics.DiagnosticSource is loaded
[2022-01-26 20:34:19Z] Assemblies loaded:
System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
<<< REDACTED, our service >>>
System.Runtime, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
ElasticApmAgentStartupHook, Version=1.12.1.0, Culture=neutral, PublicKeyToken=ae7400d2c189cf22
System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Runtime.Extensions, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.IO.FileSystem, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Linq, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Runtime.Loader, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Memory, Version=5.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
System.Text.RegularExpressions, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Console, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Threading, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Text.Encoding.Extensions, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
Microsoft.Win32.Primitives, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
[2022-01-26 20:34:19Z] No System.Diagnostics.DiagnosticSource loaded. Load using AssemblyLoadContext.Default
[2022-01-26 20:34:19Z] System.Diagnostics.DiagnosticSource 6.0.0.0 loaded
[2022-01-26 20:34:19Z] No compatible agent for System.Diagnostics.DiagnosticSource 6.0.0.0. Agent not loaded
[2022-01-26 20:34:19Z] No Elastic.Apm.StartupHook.Loader.dll assembly loaded. Agent not loaded
[2022-01-26 20:34:19Z] Get Elastic.Apm.StartupHook.Loader.Loader type
Unhandled exception. System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at StartupHook.InvokerLoaderMethod(Assembly loaderAssembly) in C:\Users\russc\source\apm-agent-dotnet\src\ElasticApmAgentStartupHook\StartupHook.cs:line 179
   at StartupHook.Initialize() in C:\Users\russc\source\apm-agent-dotnet\src\ElasticApmAgentStartupHook\StartupHook.cs:line 118
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.StartupHookProvider.CallStartupHook(StartupHookNameOrPath startupHook)
   at System.StartupHookProvider.ProcessStartupHooks()

Describe the solution you'd like Different resolution mechanism - based on runtime or other libraries which are less likely to be bumped up.

Describe alternatives you've considered For .net 5 solutions on docker (since we know 100% it's .net 5) copy folder 5.0.0 to 6.0.0. Also separately hunt down libraries that are causing bump of System.Diagnostics.DiagnosticSource.

m-standfuss commented 1 year ago

We also just ran into this same issue after taking an upgrade to from 6.x -> 7.x on System.Diagnostics.DiagnosticSource