Implementing a custom logger in .NET

Question

One of my latest projects has been implementing a custom .NET logger, as a lot of our products use their own logging solution and we're trying to standardize, but that means accounting for a lot of different configurations as well as multi-targeting across .NET Framework 4.6.2 and .NET 8.

What I have works, but there are some things I'm unsure about whether or not I've done right or the best way, as this is the first time I've made a custom logger.

Code will have business references removed, but will not impact functionality.

---

Logger Factory

/// <summary>
/// Create an instance of logger with the specified name and default settings.
/// </summary>
/// <param name="loggerName"></param>
/// <returns></returns>
public override Logger CreateInstance(string loggerName)
{
 var defaultConfig = new LoggerConfiguration
 {
 MinimumSeverity = (LogLevel)MinimumSeverity,
 EventLogName = "EventLogName",
 EventLogSource = "EventLogSource",
 LogName = "LogName",
 LogPath = "<path>",
 MaxLogFileCount = LogMaxCount,
 MaxLogFileSize = LogMaxSize,
 SplitLogByService = true
 };
 // Try to get config from AppSettings.Json for the specified loggerName, fallback to Logging:Logger, then to Core Settings. 
 var section = _configuration.GetSection($"Logging:{loggerName}");
 var loggerConfig = section.Exists()
 ? section.Get<LoggerConfiguration>()
 : _configuration.GetSection("Logging:Logger").Get<LoggerConfiguration>() ?? defaultConfig;
 return new Logger(loggerName, () => loggerConfig);
}

This inherits a Factory base class:

public abstract class Factory<T> : IFactory<T> where T : class
{
 protected readonly IServiceProvider _serviceProvider;
 protected Factory(IServiceProvider serviceProvider)
 {
 _serviceProvider = serviceProvider;
 }
 public virtual T CreateInstance()
 {
 return (T)ActivatorUtilities.CreateInstance(_serviceProvider, typeof(T));
 }
 public virtual T CreateInstance(string property)
 {
 return (T)ActivatorUtilities.CreateInstance(_serviceProvider, typeof(T), property);
 }
}

---

Dependency Injection

Injected through DI like this:

public static IServiceCollection AddLoggerProviders(this IServiceCollection services)
{
 services.AddLogging(builder =>
 {
 // remove default providers
 builder.ClearProviders();
 // Add your custom logger
 builder.AddLogger();
 });
 services.AddSingleton<IFactory<Logger>, LoggerFactory>();
 return services;
}

Usage:

_logger = loggerFactory.CreateInstance("ServiceName");
_logger.LogInformation("blah");

---

Logger Extensions

In DI, AddLogger() comes from my logger package, inside LoggerExtensions:

/// <summary>
/// Adds the custom logger to the logging builder.
/// </summary>
/// <param name="builder">The logging builder.</param>
/// <returns>The logging builder with the custom logger added.</returns>
public static ILoggingBuilder AddLogger(this ILoggingBuilder builder)
{
 builder.AddConfiguration();
 builder.Services.AddOptions<LoggerConfiguration>();
 builder.Services.TryAddEnumerable(
 ServiceDescriptor.Singleton<ILoggerProvider, LoggerProvider>());
 LoggerProviderOptions.RegisterProviderOptions
 <LoggerConfiguration, LoggerProvider>(builder.Services);
 return builder;
}

---

LoggerProvider

[ProviderAlias("Logger")]
public sealed class LoggerProvider : ILoggerProvider
{
 #region Private Fields
 private readonly ConcurrentDictionary<string, Logger> _loggers = new(StringComparer.OrdinalIgnoreCase);
 private readonly IDisposable? _onChangeToken;
 private LoggerConfiguration _currentConfig;
 #endregion Private Fields
 #region Public Constructors
 /// <summary>
 /// Initializes a new instance of the <see cref="LoggerProvider"/> class with the
 /// specified configuration.
 /// </summary>
 /// <param name="config">The configuration options monitor.</param>
 public LoggerProvider(IOptionsMonitor<LoggerConfiguration> config)
 {
 _currentConfig = config.CurrentValue;
 _onChangeToken = config.OnChange(UpdateConfig => _currentConfig = UpdateConfig);
 }
 #endregion Public Constructors
 #region Public Methods
 /// <summary>
 /// Creates a logger with the specified category name.
 /// </summary>
 /// <param name="serviceName">The category name for the logger.</param>
 /// <returns>An instance of <see cref="ILogger"/>.</returns>
 public ILogger CreateLogger(string serviceName) =>
 _loggers.GetOrAdd(serviceName, name => new Logger(name, () => _currentConfig));
 /// <summary>
 /// Releases all resources used by the <see cref="LoggerProvider"/>.
 /// </summary>
 public void Dispose()
 {
 _loggers.Clear();
 _onChangeToken?.Dispose();
 }
 public void UpdateConfiguration(LoggerConfiguration newConfig)
 {
 _currentConfig = newConfig;
 }
 #endregion Public Methods
 #region Private Methods
 private LoggerConfiguration GetCurrentConfig() => _currentConfig;
 #endregion Private Methods
}

---

Logger Constructor

public Logger(string name, Func<LoggerConfiguration> getCurrentConfig)
{
 LoggerConfiguration loggerConfiguration = getCurrentConfig();
 MinimumSeverity = loggerConfiguration.MinimumSeverity;
 EventLogName = loggerConfiguration.EventLogName;
 MaximumFileSize = loggerConfiguration.MaxLogFileSize;
 MaximumLogCount = loggerConfiguration.MaxLogFileCount;
 var basePath = string.IsNullOrWhiteSpace(loggerConfiguration.LogPath)
 ? Directory.GetCurrentDirectory()
 : loggerConfiguration.LogPath;
 // Ensure LogPath ends with "Logs"
 if (!basePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
 .EndsWith("Logs", StringComparison.OrdinalIgnoreCase))
 {
 LogPath = Path.Combine(basePath, "Logs");
 }
 else
 {
 LogPath = basePath;
 }
 ServiceName = name;
 if (loggerConfiguration.SplitLogByService)
 {
 LogName = loggerConfiguration.LogName + "-" + ServiceName;
 EventLogSource = loggerConfiguration.EventLogSource + "-" + ServiceName;
 }
 else
 {
 LogName = loggerConfiguration.LogName;
 EventLogSource = loggerConfiguration.EventLogSource;
 }
 FullPath = Path.Combine(LogPath, $"{LogName}.log");
}

---

Configuration Binding

But, I previously had the AddLogger use .BindConfig() on the .AddOptions() line:

builder.Services.AddOptions<LoggerConfiguration>().BindConfig("Logging:Logger");

But that meant that if it couldn't find an appsettings.json to bind to, it would bomb out.

---

Logger Structure

The logger itself is made up of:

• An abstractions class
• An extensions class
• The Logger itself
• A LoggerConfiguration class
• A formatter class
• The provider

---

Questions

I guess my questions are a little subjective, but purely because this is something totally new to me, I could use some guidance more than anything?

• Is my implementation reasonable or could it be better?
• Should I really be using BindConfig() or is that not totally necessary?
• Any other comments or suggestions greatly appreciated.

---

• STerliakov: I'm not familiar with C# in enterprise systems, but are the documentation comments really supposed to occupy 5 lines and just repeat the information already encoded in function name and signature? At least it's hardly useful on the Dispose method that isn't normally called by people, but constructor and CreateLogger suffer from this verbosity to greater extent. What information does that comment add to what we can already see in the signature?
• Short Int: @STerliakov Comments are meant to explain why you do something a certain way(not what the code is doing),so that others building on the code understand the why. From what I read there's only 1 comment in the code, what you see between <summary></summary> etc, etc is what is displayed as a tooltip when you're calling these methods.
• Short Int: @STerliakov, on the matter of Dispose I think it would depend on the person using a IDisposable. When using a stream, I rarely ever use using Stream stream = ... I prefer stream.Flush and stream.Dispose but then again it would depend on ones programming style and what feels comftorable.
• STerliakov: @ShortInt yep, I mean those documentation comments with xml-like fragments. Are they really supposed to repeat method name and parameters? The doc for AddLogger is really just that, can trivial docs that add no information be just omitted?
• Short Int: @STerliakov I can understand how it may appear that it doesn't add anything useful; however, you have to keep in mind that you see the source code, the person that is relying on intellisense to tell you what something is doing, can't see that source code only the method name and parameters. If you read here learn.microsoft.com/en-us/dotnet/csharp/language-reference/… you will see they use some of the same techniques

Answer 1

Unless I'm missing something, there is a major issue: you say that you are "trying to standardize," but in practice you're doing exactly the opposite.

In .NET, there is already a standard way to do logging: Microsoft.Extensions.Logging. It is, by the way, compatible with both .NET and .NET Framework. In general, in addition to this standard logging, one could also rely on one of the numerous logging libraries, most popular being, as far as I know, NLog, log4net, and Serilog. The benefit of those libraries is in particular the extensive configuration possibilities, such as when you use ELK stack to consolidate your logs.

Your approach is problematic in several ways:

• Don't reinvent the wheel. There is a lot of code in your question, and you haven't even included the tests. All this code has to be maintained over time, and will grow as the new requirements will arrive. By comparison, Serilog's basic logging example has five lines of code. That includes the using Serilog.
• New developers joining your company are probably familiar with one or more of the popular logging libraries. Not a single one has prior experience with yours.
• Third-party libraries rely on ILogger. Adding their logs to the logs of your application is therefore straightforward when you use the standard interface. With your approach, it becomes much less straightforward—for a closed-source library, you'll have to actually decompile it, tamper its IL, and hope you didn't break the terms of use.
• Every single project (.csproj) needs to reference your assembly in order to do logging. This violates the fifth of the SOLID principles: the dependency inversion principle.

Answer 2

I don't see any fundamental issue with your implementation, it is easy to follow, and it seems reasonable to me. So, good job. Here are just some minor observations.

Logger Factory

The CreateInstance method name sounds too generic for me. Most of the built-in factories in .NET uses a more explicit name like ILoggerFactory's CreateLogger or IHttpClientFactory's CreateClient, etc. I think following the same naming convention as the built-in one might ease adaption as well.

IMHO if you would allow to pass an optional LoggerConfiguration to the create method that would allow more flexiblity.

Also moving the defaultConfig to class level static member would embrace reusability.

The comment about the fallback chain is just echoing your code. I would suggest trying to capture the context instead, like why do you need more than one fallback.

Dependency Injection

AddLoggerProviders name is a bit misleading since it registers only a single provider. Also, it removes previously registered providers. AddCommonLogging or something like this would be a better name IMHO.

Logger Extensions

Quite frankly I don't see the point why you need the AddLogger method separately. It would nicely fit into AddLoggerProviders.

Reusability could a reason, but the factory registration into the DI is not part of this method. So, I'm not sure what's the added value of this separation.

Logger Provider

In general, I'm not against the usage of the region blocks, but here it seems to me that you have abused the concept. If you want to separate the public stuff from the private ones then I would suggest using partial class definitions to split the class into two files.

This _onChangeToken name is really bumping out from the context. The rest of field and variable names tries to follow a consistent naming, but this one does not seem to fit into that. Similarly, the UpdateConfig is also not convenient here.

Inside the UpdateConfiguration method you should perform at least a null check before you update the _currentConfig. I would suggest implementing some basic sanity check against the LoggerConfiguration to avoid bad or partial configurations.

The GetCurrentConfig method is not used anywhere.

Logger Constructor

The getCurrentConfig method could return null or bad/partially populated configuration. Yet again I would suggest implementing some basic sanity check.

EventLogName = loggerConfiguration.EventLogName;
MaximumFileSize = loggerConfiguration.MaxLogFileSize;
MaximumLogCount = loggerConfiguration.MaxLogFileCount;

Here the MaximumFileSize should be renamed to MaximumLogFileSize. The MaximumLogCount name is a bit misleading because it could be interpreted as the maximum number of log entries inside a log file. But that's not the intent here based on the right-hand size field name.

I think if you would extract the path handling logic into a dedicated method that would help the testability of the class.

I would suggest using either the + operator or string interpolation for string concatenation everywhere (at least in a single method).