nreco / logging

Generic file logger for .NET Core (FileLoggerProvider) with minimal dependencies
MIT License
296 stars 60 forks source link

Trace logging isn't working correctly for in code config #26

Closed zach-delong closed 3 years ago

zach-delong commented 3 years ago

I am fiddling around with trace logging for debugging some of my code and I haven't been able to write the trace level to file no matter what I do. I have this configuration:

            {
                loggingBuilder
                    .SetMinimumLevel(LogLevel.Trace)
                    .AddConsole()
                    .AddFile($"log.txt", append: true);
            });

and my file output looks like this:

2021-07-21T15:13:37.7811744-04:00   INFO    [StaticSiteGenerator.Markdown.Parser.MarkdownFileParser]    [0] Starting to convert string contents to Markdown

but my console log looks like this:

trce: StaticSiteGenerator.StaticSiteGenerator[0]
      Starting conversion of static site.
info: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Starting to convert string contents to Markdown
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Starting to convert file markdownInput/markdownFile.md
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Reading file: markdownInput/markdownFile.md
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Read file contents: # Test Header

      Test body

trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Succesfully converted markdownInput/markdownFile.md into Block Elements
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Converted file: markdownInput/markdownFile.md
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Starting to convert file markdownInput/headerOnlyFile.md
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Reading file: markdownInput/headerOnlyFile.md
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Read file contents: # This is one header
      # This is another header

trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Succesfully converted markdownInput/headerOnlyFile.md into Block Elements
trce: StaticSiteGenerator.Markdown.Parser.MarkdownFileParser[0]
      Converted file: markdownInput/headerOnlyFile.md
trce: StaticSiteGenerator.StaticSiteGenerator[0]
      Finished conversion of static site.

which is obviously more detailed. Is this style of configuration simply unsupported?

zach-delong commented 3 years ago

For further information, here is the actual code setting up the logging in context. https://github.com/ZacheryPD/S3G/blob/feature/logging/StaticSiteGenerator/ServicesConfiguration.cs

VitaliyMF commented 3 years ago

Most likely this happens because default 'internal' FileLogger's MinLevel is LogLevel.Debug (which filters all 'Trace' level entries): https://github.com/nreco/logging/blob/master/src/NReco.Logging.File/FileLoggerProvider.cs

When you use "SetMinimumLevel" method this configures minimal logging level for the log entries filtering that is common for all logging providers, however each logging provider can have its own 'MinLevel'. To override default FileLoggerProvider.MinLevel use this code:

                loggingBuilder
                    .SetMinimumLevel(LogLevel.Trace)
                    .AddConsole()
                    .Services.Add(ServiceDescriptor.Singleton<ILoggerProvider, FileLoggerProvider>( 
                (srvPrv) => {
                    return new FileLoggerProvider($"log.txt", append: true) { MinLevel = LogLevel.Trace };
                }
            ));

To avoid that I guess it would be enough to change a default value for MinLevel from LogLevel.Debug to LogLevel.Trace.

VitaliyMF commented 3 years ago

Default MinLevel was changed from Debug to Trace + added FileLoggerOptions.MinLevel property (these changes are shipped in 1.1.2).

zach-delong commented 3 years ago

Okay! Thank you for your help!

One question, and I'm happy to open a PR if it would make my question more clear, would it be better to maintain backwards compatibility by adding a new optional parameter for minimum log level? Or perhaps to add a new extension method to support it? That way it maintains backwards compatibility but it doesn't require manually creating the logger.

VitaliyMF commented 3 years ago

@ZacheryPD I'm sure that LogLevel.Debug was used instead of LogLevel.Trace (which is real minimal possible level) by mistake. In any way, filtering by log level in the logger provider is rarely used, usually rules are configured for all logging providers in the "Logging" config section. Users that need exactly LogLevel.Debug (which was by default) for the FileLoggingProvider now can set this level explicitely in FileLoggerOptions (or in appsettings.json config section).

zach-delong commented 3 years ago

Makes perfect sense to me. Thank you for explaining!