search

microsoft / ApplicationInsights-Profiler-AspNetCore

Application Insights Profiler sample and documentation
MIT License
66 stars 24 forks source link

InvalidOperationException "System.Diagnostics.Tracing.EventPipe type == null" after .NET 6.0 migration #151

Closed semornas closed 2 years ago

semornas commented 2 years ago

Hello,

We are facing some issues with the ApplicationInsights Profiler while upgrading our ASP.NET Core 3.1 Apps to .NET 6.0. The apps are hosted on App Service on Linux Containers and profiling was working fine with .NET Core 3.1.

Repro:

Now that we have .NET 6.0 instead, the Profiler is throwing a System.InvalidOperationException:

Looking at the disassembled code of Microsoft.ApplicationInsights.Profiler.Core I saw the following lines in the TraceControl class where the exception comes from:

Type type = !(assembly == (Assembly) null) ? assembly.GetType("System.Diagnostics.Tracing.EventPipe") : throw new InvalidOperationException("System.Private.CoreLib assembly == null");
this.m_enableMethod = !(type == (Type) null) ? type.GetMethod("Enable", BindingFlags.Static | BindingFlags.NonPublic) : throw new InvalidOperationException("System.Diagnostics.Tracing.EventPipe type == null");

The exception is therefore thrown when assembly.GetType("System.Diagnostics.Tracing.EventPipe") returns null, which is the case now in .NET 6.0 (I believe this relates to this PR: https://github.com/dotnet/runtime/pull/48053)

Package versions:

Workaround

No workaround identified

Is the use of the Profiler broken in .NET 6.0 or do we need to do something to make it work?

Thanks!

xiaomi7732 commented 2 years ago

Hi @semornas thanks for your report. Let me take a look.

xiaomi7732 commented 2 years ago

Hey @semornas I did some investigation. This code path is there for falling back on old .NET Core runtime (like 2.1 or so). It doesn't feel right for an application on .NET 6 to run into it. I test it quickly on my box and I couldn't repro it.

Maybe there's something going on with the runtime image for the container? Do you have a reproduce that I can use for investigation, or could you please let me know which base image did you use that we can try to reproduce it ourselves? Thanks!

xiaomi7732 commented 2 years ago

Hey @semornas Thanks for sending the info offline. Just to put it on-record, here's some info for debugging:

the runtime of the apps is Microsoft.AspNetCore.App 6.0.1 (base image: mcr.microsoft.com/dotnet/aspnet:6.0.1-alpine3.14) and the apps are built using .NET SDK 6.0.101 (base image: mcr.microsoft.com/dotnet/sdk:6.0.101-alpine3.14) Based on that, I created a repo trying to reproduce the issue - it uses the exact same image while I am not seeing the error.

Could you please help check to see what is that that we are missing there? Click to open the repo.

semornas commented 2 years ago

Thanks for your help @xiaomi7732; I pushed some additional code in your repo that should expose the issue in local debugging. Hope it will help troubleshooting :)

xiaomi7732 commented 2 years ago

Thanks for that! Let me take a look.

xiaomi7732 commented 2 years ago

Thanks for the information. I see what is going on. I have put a workaround in the repo, please check it out.

Do I understand the intention to pre-warm up singletons is to gain better perf or fail faster?

Here's, I think, what happened: In Profiler implementation, we have 2 types do the same thing: TraceControl - for backward compatibility works up to .NET 5.0 until this change https://github.com/dotnet/runtime/pull/48053. DiagnosticsClientTraceControl - newer implementation relies on newer version of .NET (Core 3.1, 5.x, 6.x ..) runtime. They both are registered as singleton.

Now, normally, TraceControl, although registered, will only be injected lazily, and dynamically when DiagnosticsClientTraceControl fails, that's a strong indication that the app is running on an older version of runtime, and thus the fallback.

The way the lazy load works rely on that TraceControl won't be created when DiagnosticsClientTraceControl works.

However, in this case, the warmup service appended to the project creates instances for every singleton ahead of time, and it includes TraceControl; Now, TraceControl rely on something that is removed from the runtime and thus the exception.

For .NET 6 applications, we don't really need TraceControl, so the work around we put there is to skip warm up that one.

We will need think through different scenarios to make a proper fix, and this issue will be kept open until a fix is available.

What do you think @semornas? And please let us know if the work-around is good enough for a short-term mitigation. Thanks!

semornas commented 2 years ago

Hi @xiaomi7732,

Again, thank you for your help! The explanation makes complete sense; this is only because we try to activate all registered singletons (indeed we do that in order to optimize the performances of the application) at startup, that we run into this issue.

The proposed workaround, which is to not activate the Microsoft.ApplicationInsights.Profiler.Core.TraceControls.TraceControl type, is perfectly acceptable to mitigate our problem. Thanks !

xiaomi7732 commented 2 years ago

Hi @semornas I am glad to hear that. Just so that you know, we landed on a fix to remove the need for the workaround. Please pay attention to the next release. Again, I am keeping this issue open until the fix is released.

xiaomi7732 commented 2 years ago

Hi @semornas The fix is released in 2.3.1-beta4 here: https://www.nuget.org/packages/Microsoft.ApplicationInsights.Profiler.AspNetCore/2.3.1-beta4