Upgrade from Enterprise Library Logging 3.1 to 6.0 Writer.Dispose() no longer works for closing flat file log

Question

Our application uses the Enterprise Library logging version 3.1 to write to a flat file log. We called Logger.Writer.Dispose() after doing a Logger.Write every time which allows for us to delete or access the flat file while the application is still running. Although this is not recommended for performance and is not thread safe, we needed to be able to write to the file from multiple threads. We didn't want to spend too much time setting up msmq and writing something custom for logging, so the dispose method worked best.

After upgrading to version 6.0, we can no longer dispose of the Logger.Writer without having a way to create it again.

What do we need to do in order to determine if the Logger.Writer has been dispose and re-create it?

This is how I'm creating the Logger.Writer:`

        IConfigurationSource configurationSource = ConfigurationSourceFactory.Create();
        LogWriterFactory logWriterFactory = new LogWriterFactory(configurationSource);
        Logger.SetLogWriter(logWriterFactory.Create(), false);`

Is there a way to see if the LogWriter is already set/created or check if it's been disposed?

Answer 1

I can think of 4 ways to do what you want:

1. Catch the InvalidOperationException thrown by Logger.Writer when the LogWriter instance is null.
2. Create a custom NullLogWriter class, set that value as the LogWriter and check for that type to determine if the LogWriter is null.
3. Copy the EntLib Logger facade code and add the exact functionality you require.
4. Use reflection to access the private writer member variable

The main stumbling block that these approaches try to work around is that the Logger facade does not give access to the LogWriter instance if the value is null; instead it will throw an InvalidOperationException.

Option 1 - Catch InvalidOperationException

The downside for option 1 is that catching an exception for every logging call is a performance hit (although you already have a big hit due to your existing design) and considered a code smell.

The code might look something like this:

Log("Test 1", "General");            
Log("Test 2", "General");            

static void Log(string message, string category)
{
    bool isLogWriterDisposed = false;
    try
    {
        // If Writer is null InvalidOperation will be thrown
        var logWriter = Logger.Writer;
    }
    catch (InvalidOperationException e)
    {
        isLogWriterDisposed = true;
    }

    if (isLogWriterDisposed)
    {
        InitializeLogger();
    }

    // Write message
    Logger.Write(message, category);

    // Dispose the existing LogWriter and set it to null
    Logger.Reset();
}

Option 2 - Custom NullLogWriter

Option 2 might look like this:

public class NullLogWriter : LogWriter
{
    /// <inheritdoc />
    public NullLogWriter(LoggingConfiguration config) : base(config)
    {
    }

    /// <inheritdoc />
    public NullLogWriter(IEnumerable<ILogFilter> filters, IDictionary<string, LogSource> traceSources, LogSource errorsTraceSource, string defaultCategory) : base(filters, traceSources, errorsTraceSource, defaultCategory)
    {
    }

    /// <inheritdoc />
    public NullLogWriter(IEnumerable<ILogFilter> filters, IDictionary<string, LogSource> traceSources, LogSource allEventsTraceSource, LogSource notProcessedTraceSource, LogSource errorsTraceSource, string defaultCategory, bool tracingEnabled, bool logWarningsWhenNoCategoriesMatch) : base(filters, traceSources, allEventsTraceSource, notProcessedTraceSource, errorsTraceSource, defaultCategory, tracingEnabled, logWarningsWhenNoCategoriesMatch)
    {
    }

    /// <inheritdoc />
    public NullLogWriter(IEnumerable<ILogFilter> filters, IDictionary<string, LogSource> traceSources, LogSource allEventsTraceSource, LogSource notProcessedTraceSource, LogSource errorsTraceSource, string defaultCategory, bool tracingEnabled, bool logWarningsWhenNoCategoriesMatch, bool revertImpersonation) : base(filters, traceSources, allEventsTraceSource, notProcessedTraceSource, errorsTraceSource, defaultCategory, tracingEnabled, logWarningsWhenNoCategoriesMatch, revertImpersonation)
    {
    }

    /// <inheritdoc />
    public NullLogWriter(IEnumerable<ILogFilter> filters, IEnumerable<LogSource> traceSources, LogSource errorsTraceSource, string defaultCategory) : base(filters, traceSources, errorsTraceSource, defaultCategory)
    {
    }

    /// <inheritdoc />
    public NullLogWriter(IEnumerable<ILogFilter> filters, IEnumerable<LogSource> traceSources, LogSource allEventsTraceSource, LogSource notProcessedTraceSource, LogSource errorsTraceSource, string defaultCategory, bool tracingEnabled, bool logWarningsWhenNoCategoriesMatch) : base(filters, traceSources, allEventsTraceSource, notProcessedTraceSource, errorsTraceSource, defaultCategory, tracingEnabled, logWarningsWhenNoCategoriesMatch)
    {
    }

    /// <inheritdoc />
    public NullLogWriter(LogWriterStructureHolder structureHolder) : base(structureHolder)
    {
    }
}

class Program
{
    private static readonly LogWriter NullLogWriter = new NullLogWriter(new LoggingConfiguration());

    static void Main(string[] args)
    {
        Log("Test 1", "General");            
        Log("Test 2", "General");            
    }

    static void Log(string message, string category)
    {
        if (HasLogWriterBeenDisposed())
        {
            InitializeLogger();
        }

        // Write message
        Logger.Write(message, category);

        // Set the logger to the null logger this disposes the current LogWriter
        // Note that this call is not thread safe so would need to be synchronized in multi-threaded environment
        Logger.SetLogWriter(NullLogWriter, false);
    }

    static bool HasLogWriterBeenDisposed()
    {
        try
        {
            // If logger is the NullLogWriter then it needs to be set
            return Logger.Writer is NullLogWriter;
        }
        catch (InvalidOperationException e)
        {
            // If InvalidOperationException is thrown then logger is not set -- consider this to be disposed
            return true;
        }
    }

    static void InitializeLogger()
    {
        IConfigurationSource configurationSource = ConfigurationSourceFactory.Create();
        LogWriterFactory logWriterFactory = new LogWriterFactory(configurationSource);
        Logger.SetLogWriter(logWriterFactory.Create(), false);            
    }
}

It has an additional class but doesn't throw an exception on every logging call.

Option 3 - Duplicate Logger

Option 3 will have direct access to the private writer member variable so whether it is null could be checked and true/false returned.

Option 4 - Reflection

This approach might look similar to Option 2 but with a reflection check instead of a check against NullLogWriter. Note that the reflection call will fail if running under medium trust.

Log("Test 1", "General");
Log("Test 2", "General");

static bool HasLogWriterBeenDisposed()
{
    var fieldInfo = typeof(Logger).GetField("writer", BindingFlags.NonPublic | BindingFlags.Static);       
    return fieldInfo?.GetValue(null) == null;
}

static void Log(string message, string category)
{
    if (HasLogWriterBeenDisposed())
    {
        InitializeLogger();
    }

    // Write message
    Logger.Write(message, category);

    // Dispose and set the logger to the null
    Logger.Reset();
}

static void InitializeLogger()
{
    IConfigurationSource configurationSource = ConfigurationSourceFactory.Create();
    LogWriterFactory logWriterFactory = new LogWriterFactory(configurationSource);
    Logger.SetLogWriter(logWriterFactory.Create(), false);
}

Conclusion

IMO, none of these approaches is ideal. The real issue is the original (non-recommended) design forcing us down one of these roads. Also these approaches are not thread-safe. If I had to choose an approach I would probably go with Option 2 (NullLogWriter) because it doesn't rely on code duplication, catching an exception on every call, or using reflection to access private member variables all of which are a code smells to one degree or another.

Addendum

If what you want is a thread-safe implementation that does not keep the file open then you can stick to the open/dispose approach you were doing (despite the inefficiencies), avoid using the Enterprise Library Logger static facade and instead use your own static class/singleton that handles the logic exactly how you want. Something like this should work:

public static class CustomLogger
{
    private static readonly LogWriterFactory LogWriterFactory = new LogWriterFactory(ConfigurationSourceFactory.Create());
    private static readonly object Locker = new object();

    public static void Write(string message, string category)
    {
        lock (Locker)
        {
            using (var logWriter = LogWriterFactory.Create())
            {
                logWriter.Write(message, category);
            }
        }
    }
}


CustomLogger.Write("Test 1", "General");
CustomLogger.Write("Test 2", "General");