Log Level of Critical for "StartProfiler triggered.", "StopProfiler succeeded.", etc. ??

[dstj]

Describe the bug I'm logging to Application Insights and I see a lot of entries in AppTraces with a Severity Level of 4 (i.e. Critical). This really seems bizarre as the messages are "triggered" and "succeeded". I see in order

• StartProfiler triggered.
• StartProfiler succeeded.
• StopProfiler triggered.
• StopProfiler succeeded.

No other messages in between.

This creates a lot of noise in the logs.

To Reproduce I don't think my setup is special or anything, here's my registration:

private static void RegisterApplicationInsights(IServiceCollection services)
{
    services.AddApplicationInsightsTelemetry();
    services.AddSingleton<ITelemetryProcessorFactory>(_ => new GenericAspnetCoreTelemetryFilterFactory<WebSocketTelemetryFilter>());
    services.AddSingleton<ITelemetryInitializer, XRequestIdHttpHeaderTelemetryInitializer>();

    var containerAppName = Environment.GetEnvironmentVariable("CONTAINER_APP_NAME");
    services.AddSingleton<ITelemetryInitializer>(new RoleNameTelemetryInitializer(containerAppName));

    // To replace ambiguous: services.ConfigureTelemetryModule<NpgsqlDependencyTrackingTelemetryModule>((module, o) => { });
    services.AddSingleton<ITelemetryModule, NpgsqlDependencyTrackingTelemetryModule>();
    services.AddSingleton(typeof(ITelemetryModuleConfigurator), new TelemetryModuleConfiguratorAspNetCore((config, options) => { }, typeof(NpgsqlDependencyTrackingTelemetryModule)));

    services.AddServiceProfiler();
}

Expected behavior These message should be Debug or Information, definitely not Critical.

Screenshots

Desktop (please complete the following information):

• OS: Alpine Linux 3.18 (via Docker)
• Package version (Microsoft.ApplicationInsights.Profiler.AspNetCore): 2.7.1
• Application Insights: 2.22.0

Additional context

[xiaomi7732]

@dstj Thanks for the report. There was a reason for those to be critical. Let me check to see if those reasons still valid and see there can be a fix for that.

I want to make them Informational by default, but if I can't, is it acceptable if we make it Critical by default and allow you to downgrade those lower level by configuration?

Thanks!

[dstj]

@xiaomi7732 Thanks for looking into this. There was a reason from those to be "critical" messages? Do they indicate a serious problem? The wording of the messages does not seem to indicate they do...

| I want to make them Informational by default, but if I can't, is it acceptable if we make it Critical by default and allow you to downgrade those lower level by configuration?

Well, if this needs to be critical and an action should be taken to avoid those messages altogether, I'd rather have a clearer error message and documentation to be honest. I will then fix my system accordingly.

I'm wondering, is is because the profiler is not meant to be on for long periods of time?

If there is no cause for alarm, then I see no reason for the severity level to be critical, so any way to make them informational works for me (even though having something critical by default that is not a critical issue seems bizarre).

I'll await the result of your analysis. Thanks.

[pharring]

@xiaomi7732 I think we copied Critical from the Windows profiler codebase and, honestly, it's bothered me for years. I think the original authors (before my time) wanted a logging level that was "on by default" to show that the profiler was actually doing something. They weren't warnings. Neither were they errors. They are Information-level messages, but that was often below the default logging level. So, they rose all the way up to Critical.

I'd be OK making them Informational.

@dstj No cause for alarm. This is historical baggage. We'll figure out how to make the change safely.

[xiaomi7732]

@pharring Thanks. That's what I think what it is as well.

And just for the record, the other reason, was attempting to avoid these entries being sampled in the agent telemetries for troubleshooting. And I think that's not reasonable either.

And as @dstj pointed out, these aren't actually critical and causes confusion.

All those reasons, I am more & more leaning toward downgrade the levels.

[xiaomi7732]

A PR has been approved for this change. I'll post the package info when it's available.

[xiaomi7732]

Please checkout the new package: https://www.nuget.org/packages/Microsoft.ApplicationInsights.Profiler.AspNetCore/2.7.2-beta1. I'll close this issue. Feel free to reopen if there's any thing.