serilog / serilog-extensions-logging

Serilog provider for Microsoft.Extensions.Logging
Apache License 2.0
313 stars 100 forks source link

Observe Microsoft.Extensions.Logging.LogLevel.None #203

Closed 0xced closed 2 years ago

0xced commented 2 years ago

Before this commit: LogLevel.None is converted to LogEventLevel.Fatal and events with a None level are logged as fatal.

After this commit: LogLevel.None is observed and logs and events with a None level are ignored.

The None level is documented as such:

Not used for writing log messages. Specifies that a logging category should not write any messages.

Note: this erroneous behaviour was seen in a real-world scenario:

  1. BuildOrgConnectUri CoreClass () is logged as TraceEventType.Start
  2. TraceEventType.Start is converted to LogLevel.None
  3. LogLevel.None is converted to LogEventLevel.Fatal

As a result, BuildOrgConnectUri CoreClass () is logged as fatal whereas it should have been ignored.

nblumhardt commented 2 years ago

Thanks for the PR!

To me, it seems like the behavior in linked snippet (2) is a bug (None shouldn't be used for log events, as you've quoted), but since this gets us closer to the behavior of the built-in logger I think it's a plus and we should merge 👍