[sdk-logs] Improve OpenTelemetryLogger.IsEnabled

[reyang]

This change should be almost transparent to the user, unless the user is relying on the side effect:

if (logger.IsEnabled(level))
{
    // code with side effect
}

With this change, it will be possible to save some CPU time if it is expensive to prepare the logging arguments:

if (logger.IsEnabled(level))
{
    var expensive = GetSomethingExpensive();
    logger.LogSomething(expensive);
}

The logic was originally written by me in #1329.

It was later refactored (with no change to the actual logic) https://github.com/open-telemetry/opentelemetry-dotnet/pull/1869/files#r588851416.

[cijothomas]

it was never clear to me why the LogLevel.None check was needed.. If the LogLevel.None is used, the ILogger infra should not be even sending the log to this provider at all (maybe thats just my interpretation!)

[CodeBlanch]

@cijothomas

It does appear somewhat standard: https://github.com/dotnet/runtime/blob/703f6e846027681a052970929989bd36e81636cd/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLogger.cs#L73-L76

But you are correct the LoggerFactory also does a fast-check up front: https://github.com/dotnet/runtime/blob/703f6e846027681a052970929989bd36e81636cd/src/libraries/Microsoft.Extensions.Logging/src/LoggerInformation.cs#L32-L35

My guess would be it is because someone might swap out the default ILoggerFactory with their own implementation which might not have the logic?

[cijothomas]

Yes it appears standard in the sense - every logging provider I have seen has that logic! I wasn't expecting that providers have to do that job, instead handled by ILogger itself.!

[CodeBlanch]

LGTM