rocklib / rocklib.logging Goto Github PK

Add missing documentation:

• LogProviderErrors.md
• Tracing.md

https://github.com/RockLib/Rock.Logging/blob/2f620f511bd8755c5d78bb4e16e125211e863605/Rock.Logging/LogProviders/HttpEndpointLogProvider.cs#L118

Expand on documentation in the following files:

• LogProvider.md
• Formatting.md - Adding the template formatter

Change format to HowTo instead of Tutorial style documentation for

• Console.md
• File.md
• RollingFile.md

In the open source rock framework the logger is not fully functional. It's working for windows apps, console apps but it seems to be not logging anything for web apps. More particularly the log provider that's failing is HttpEndPointLogProvider.

With 4.6.1 already EOL and unsupported is there a plan to upgrade this library to NET6.0 or create a new one leveraging the new framework?

If so, is there an ETA?

Review documentation that is currently in process:

• ASPNETCore.md
  • Make edits based on the review
• ContextProviders.md
  • Make edits based on the review
• LogProcessor.md
  • Make edits based on the review

From what I can see in your implementation, when I call for an instance of a logger for the first time, it creates that instance but subsequent calls are pulled from the dictionary. I do not see any checks that the instance is still valid and in an application with per scope/per instance dependency injection, that instance will get disposed of at the end of the call to it.

This specific request is to include ThreadID as a default extended property, but it could expand to other default data points as well. I'm not sure what those would be, but i'm sure a list could be put together including potentially beneficial inclusions.

ThreadID, specifically, could be very helpful when logging using multi-threaded applications. The only potential downside I could see is that you could argue it's inclusion could bloat log messages, but I also think that ThreadID is one of those datapoints that's not helpful if you don't need it, but it's absence can cause some big confusion if you do need it.

isSingletonLoggers is assigned at the time of calling "AddLogger". This fails when the count of ILogger instances does not match on execution of the LoggerLookupRegistration due to access of the index here: !isSingletonLoggers[i].

It is common using the WebApplicationFactory from the ASP.NET Testing Framework to inject your own mock instance into the service container. This leads to 2 ILoggers existing in the service container and this hard to debug exception being thrown:

System.IndexOutOfRangeException : Index was outside the bounds of the array.
   at RockLib.Logging.DependencyInjection.ServiceCollectionExtensions.<>c__DisplayClass7_1.<SetLoggerLookupDescriptor>b__3(String name)
   at RockLib.Logging.RockLibLoggerProviderExtensions.<>c__DisplayClass3_0.<AddRockLibLoggerProvider>b__0(IServiceProvider serviceProvider) in /_/RockLib.Logging.Microsoft.Extensions/RockLibLoggerProviderExtensions.cs:line 82

https://github.com/RockLib/RockLib.Logging/blob/main/RockLib.Logging/DependencyInjection/ServiceCollectionExtensions.cs#L207

        // Capture which loggers are singleton according to index.
        var isSingletonLoggers = services.Where(service => service.ServiceType == typeof(ILogger))
            .Select(service => service.Lifetime == ServiceLifetime.Singleton)
            .ToArray();

        LoggerLookup LoggerLookupRegistration(IServiceProvider serviceProvider) => name =>
        {
            // Select the first logger that has a matching name.
            var loggers = serviceProvider.GetServices<ILogger>().ToArray();
            var selectedLogger = loggers.First(logger => NamesEqual(logger.Name, name));

            // Immediately dispose any non-singleton loggers that weren't selected.
            for (var i = 0; i < loggers.Length; i++)
            {
                if (!isSingletonLoggers[i] && !ReferenceEquals(loggers[i], selectedLogger))
                {
                    loggers[i].Dispose();
                }
            }

            return selectedLogger;
        };