Skip to content

Logging overhaul

Mateusz Brawański requested to merge feature/ms-logging into master

Summary

This pull request overhauls the logging components of the library by switching them to Microsoft Logging Abstractions. This adds compatibility with 3rd party logging libraries, and makes plugging in external logging implementations easier. Additionally, it makes D#+ logging API more consistent with other implementations.

Details

The library comes with a default implementation tree for ILogger<BaseDiscordClient> - that is an ILoggerFactory, which holds ILoggerProviders, which creates ILogger<BaseDiscordClient> instances. This is implemented via the following types:

  • DefaultLogger - the logger implementation. Implements ILogger<BaseDiscordClient>. Outputs to the console. The output format is almost the same as the prior DebugLogger implementation, with a few improvements to the format (mostly to make it column-consistent).
  • DefaultLoggerProvider - internal, logger provider. Creates and configures instances of DefaultLogger. Implements ILoggerProvider.
  • DefaultLoggerFactory - internal, logger factory. Uses registered ILoggerProvider (currently only DefaultLoggerProvider) instances to create ILogger<BaseDiscordClient> instances. Creates composite loggers, which wrap all created instances.
  • CompositeDefaultLogger - internal, composite logger. Created by DefaultLoggerFactory. Uses ILoggerProvider instances from the factory to log to multiple ILogger<BaseDiscordClient> instances.
  • ShardedLoggerFactory - internal, passthrough singleton container. Used by DiscordShardedClient to pass composite logger instances to child DiscordClient instances.

Additionally, the existing DebugLogger implementation was completely removed, along with LogLevel enum. The latter has been replaced with LogLevel enum from the logging abstractions package.

Configuration options changed:

  • LogLevel -> MinimumLogLevel - name changed for clarity. Additionally, now uses the new LogLevel type.
  • DateTimeFormat -> LogTimestampFormat - name changed for clarity.
  • UseInternalLogHandler - removed, as it's no longer in use.
  • LoggerFactory - allows changing logger implementations. Currently, this defaults to null. When a null value is specified when a client implementation is instantiated, this value is replaced by an instance of DefaultLoggerFactory, which then has an instance of DefaultLoggerProvider with above configuration options speicified registered to it. Specifying any valid ILoggerFactory instance here will skip that step. The factory is used to create new ILogger<BaseDiscordClient> instances for client implementations. Currently, the instance creation happens exactly once - when the client is instantiated.

Currently, event IDs of <1000 are considered to be internal to D#+, however this is not a hard limit/definition. This is also open to discussion.

Changes proposed

  • Replace existing DebugLogger implementation with new DefaultLogger implementation, compatible with ILogger interface.
  • Replace D#+'s LogLevel enum with MS Logging Abstractions LogLevel version of it.
  • Allocate event IDs to prevent conflicts.
  • Update logging-related configuration options to better reflect their purpose.
  • LogTaskFault has been made a standalone extension method on Task.

Notes

This pull request partly supersedes #464. Paging @WamWooWam for input.

Pull request currently marked as WIP as the work is not completed yet. Some discussion needs to happen before it's finalized.

Status of this PR

  • Initial changes specced out and staged for testing
  • Ready for review
    • Design discussion
      • Composite logger
      • Splitting the logger code across multiple files
      • Grammar
      • Ratelimit logging mismatches
      • Log level format
      • Settle on colours
    • Bring in changes from #464
    • Up for discussion: provide a semi-compatibility module (rejected)
  • Ready for merge

Merge request reports

Loading