Log Level of Critical for "StartProfiler triggered.", "StopProfiler succeeded.", etc. ?? about applicationinsights-profiler-aspnetcore HOT 4 OPEN

@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.

from applicationinsights-profiler-aspnetcore.

@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!

from applicationinsights-profiler-aspnetcore.

@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.

from applicationinsights-profiler-aspnetcore.

@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.

from applicationinsights-profiler-aspnetcore.