Separate Logfile and directory for each client and date

Question

I have a windows TCP service, which has many devices connecting to it, and a client can have one or more devices.

Requirement:

Separate Folder per client with separate log file for each device.

so something like this:

/MyService/25-04-2016/
    Client 1/
        Device1.txt 
        Device2.txt 
        Device3.txt 

    Client 2/
        Device1.txt 
        Device2.txt 
        Device3.txt 

Now I have not used a 3rd Party library like log4net or NLog, I have a class which handles this.

public class xPTLogger : IDisposable
{
    private static object fileLocker = new object();

    private readonly string _logFileName;
    private readonly string _logFilesLocation;
    private readonly int _clientId;

    public xPTLogger() : this("General") { }

    public xPTLogger(string logFileName)
    {
        _clientId = -1;
        _logFileName = logFileName;
        _logFilesLocation = SharedConstants.LogFilesLocation; // D:/LogFiles/
    }

    public xPTLogger(string logFileName, int companyId)
    {
        _clientId = companyId;
        _logFileName = logFileName;
        _logFilesLocation = SharedConstants.LogFilesLocation;
    }

    public void LogMessage(MessageType messageType, string message)
    {
        LogMessage(messageType, message, _logFileName);
    }

    public void LogExceptionMessage(string message, Exception innerException, string stackTrace)
    {
        var exceptionMessage = innerException != null
                ? string.Format("Exception: [{0}], Inner: [{1}], Stack Trace: [{2}]", message, innerException.Message, stackTrace)
                : string.Format("Exception: [{0}], Stack Trace: [{1}]", message, stackTrace);

        LogMessage(MessageType.Error, exceptionMessage, "Exceptions");
    }

    public void LogMessage(MessageType messageType, string message, string logFileName)
    {
        var dateTime = DateTime.UtcNow.ToString("dd-MMM-yyyy");

        var logFilesLocation = string.Format("{0}{1}\\", _logFilesLocation, dateTime);

        if (_clientId > -1) { logFilesLocation = string.Format("{0}{1}\\{2}\\", _logFilesLocation, dateTime, _clientId); }


        var fullLogFile = string.IsNullOrEmpty(logFileName) ? "GeneralLog.txt" : string.Format("{0}.txt", logFileName);


        var msg = string.Format("{0} | {1} | {2}\r\n", DateTime.UtcNow.ToString("dd-MMM-yyyy HH:mm:ss"), messageType, message);

        fullLogFile = GenerateLogFilePath(logFilesLocation, fullLogFile);

        LogToFile(fullLogFile, msg);
    }

    private string GenerateLogFilePath(string objectLogDirectory, string objectLogFileName)
    {
        if (string.IsNullOrEmpty(objectLogDirectory))
            throw new ArgumentNullException(string.Format("{0} location cannot be null or empty", "objectLogDirectory"));
        if (string.IsNullOrEmpty(objectLogFileName))
            throw new ArgumentNullException(string.Format("{0} cannot be null or empty", "objectLogFileName"));

        if (!Directory.Exists(objectLogDirectory))
            Directory.CreateDirectory(objectLogDirectory);
        string logFilePath = string.Format("{0}\\{1}", objectLogDirectory, objectLogFileName);
        return logFilePath;
    }

    private void LogToFile(string logFilePath, string message)
    {
        if (!File.Exists(logFilePath))
        {
            File.WriteAllText(logFilePath, message);
        }
        else
        {
            lock (fileLocker)
            {
                File.AppendAllText(logFilePath, message);
            }
        }
    }

    public void Dispose()
    {
        fileLocker = new object();
    }
}

And then I can use it like this:

 var _logger = new xPTLogger("DeviceId", 12);

 _logger.LogMessage(MessageType.Info, string.Format("Information Message = [{0}]", 1));

The problem with the above class is that, because the service is multi-threaded, some threads try to access the same log file at the same time causing an Exception to Throw.

25-Apr-2016 13:07:00 | Error | Exception: The process cannot access the file 'D:\LogFiles\25-Apr-2016\0\LogFile.txt' because it is being used by another process.

Which sometimes causes my service to crash.

How do I make my Logger class to work in multi-threaded services?

EDIT

Changes to the Logger Class

public class xPTLogger : IDisposable
{
    private object fileLocker = new object();

    private readonly string _logFileName;
    private readonly string _logFilesLocation;
    private readonly int _companyId;

    public xPTLogger() : this("General") { }

    public xPTLogger(string logFileName)
    {
        _companyId = -1;
        _logFileName = logFileName;
        _logFilesLocation = SharedConstants.LogFilesLocation; // "D:\\MyLogs";
    }

    public xPTLogger(string logFileName, int companyId)
    {
        _companyId = companyId;
        _logFileName = logFileName;
        _logFilesLocation = SharedConstants.LogFilesLocation;
    }

    public void LogMessage(MessageType messageType, string message)
    {
        LogMessage(messageType, message, _logFileName);
    }

    public void LogExceptionMessage(string message, Exception innerException, string stackTrace)
    {
        var exceptionMessage = innerException != null
                ? string.Format("Exception: [{0}], Inner: [{1}], Stack Trace: [{2}]", message, innerException.Message, stackTrace)
                : string.Format("Exception: [{0}], Stack Trace: [{1}]", message, stackTrace);

        LogMessage(MessageType.Error, exceptionMessage, "Exceptions");
    }

    public void LogMessage(MessageType messageType, string message, string logFileName)
    {
        if (messageType == MessageType.Debug)
        {
            if (!SharedConstants.EnableDebugLog)
                return;
        }

        var dateTime = DateTime.UtcNow.ToString("dd-MMM-yyyy");

        var logFilesLocation = string.Format("{0}{1}\\", _logFilesLocation, dateTime);

        if (_companyId > -1) { logFilesLocation = string.Format("{0}{1}\\{2}\\", _logFilesLocation, dateTime, _companyId); }


        var fullLogFile = string.IsNullOrEmpty(logFileName) ? "GeneralLog.txt" : string.Format("{0}.txt", logFileName);


        var msg = string.Format("{0} | {1} | {2}\r\n", DateTime.UtcNow.ToString("dd-MMM-yyyy HH:mm:ss"), messageType, message);

        fullLogFile = GenerateLogFilePath(logFilesLocation, fullLogFile);

        LogToFile(fullLogFile, msg);
    }

    private string GenerateLogFilePath(string objectLogDirectory, string objectLogFileName)
    {
        if (string.IsNullOrEmpty(objectLogDirectory))
            throw new ArgumentNullException(string.Format("{0} location cannot be null or empty", "objectLogDirectory"));
        if (string.IsNullOrEmpty(objectLogFileName))
            throw new ArgumentNullException(string.Format("{0} cannot be null or empty", "objectLogFileName"));

        if (!Directory.Exists(objectLogDirectory))
            Directory.CreateDirectory(objectLogDirectory);
        string logFilePath = string.Format("{0}\\{1}", objectLogDirectory, objectLogFileName);
        return logFilePath;
    }

    private void LogToFile(string logFilePath, string message)
    {
        lock (fileLocker)
        {
            try
            {
                if (!File.Exists(logFilePath))
                {
                    File.WriteAllText(logFilePath, message);
                }
                else
                {
                    File.AppendAllText(logFilePath, message);
                }
            }
            catch (Exception ex)
            {
                var exceptionMessage = ex.InnerException != null
                                ? string.Format("Exception: [{0}], Inner: [{1}], Stack Trace: [{2}]", ex.Message, ex.InnerException.Message, ex.StackTrace)
                                : string.Format("Exception: [{0}], Stack Trace: [{1}]", ex.Message, ex.StackTrace);

                var logFilesLocation = string.Format("{0}{1}\\", _logFilesLocation, DateTime.UtcNow.ToString("dd-MMM-yyyy"));

                var logFile = GenerateLogFilePath(logFilesLocation, "FileAccessExceptions.txt");

                try
                {
                    if (!File.Exists(logFile))
                    {
                        File.WriteAllText(logFile, exceptionMessage);
                    }
                    else
                    {
                        File.AppendAllText(logFile, exceptionMessage);
                    }
                }
                catch (Exception) { }
            }

        }
    }

    public void Dispose()
    {
        //fileLocker = new object();
        //_logFileName = null;
        //_logFilesLocation = null;
        //_companyId = null;
    }
}

Answer 1

If you don't want to use existing solutions, the reasonable approach to handle multithreaded writes in your logger is to use queue. Here is a sketch:

public class LogQueue : IDisposable {
    private static readonly Lazy<LogQueue> _isntance = new Lazy<LogQueue>(CreateInstance, true);
    private Thread _thread;
    private readonly BlockingCollection<LogItem> _queue = new BlockingCollection<LogItem>(new ConcurrentQueue<LogItem>());

    private static LogQueue CreateInstance() {
        var queue = new LogQueue();
        queue.Start();
        return queue;
    }

    public static LogQueue Instance => _isntance.Value;

    public void QueueItem(LogItem item) {
        _queue.Add(item);
    }

    public void Dispose() {
        _queue.CompleteAdding();
        // wait here until all pending messages are written
        _thread.Join();
    }

    private void Start() {
        _thread = new Thread(ConsumeQueue) {
            IsBackground = true
        };
        _thread.Start();
    }

    private void ConsumeQueue() {
        foreach (var item in _queue.GetConsumingEnumerable()) {
            try {
                // append to your item.TargetFile here                    
            }
            catch (Exception ex) {
                // do something or ignore
            }
        }
    }
}

public class LogItem {
    public string TargetFile { get; set; }
    public string Message { get; set; }
    public MessageType MessageType { get; set; }
}

Then in your logger class:

private void LogToFile(string logFilePath, string message) {
    LogQueue.Instance.QueueItem(new LogItem() {
        TargetFile = logFilePath,
        Message = message
    });
}

Here we delegate actual logging to separate class which writes log messages one by one, so cannot have any multithreading issues. Additional benefit of such approach is that logging happens asynchronously and as such does not slow down the real work.

Drawback is you can lose some messages in case of process crash (don't think that is really a problem but still mention it) and you consume separate thread to log asynchronously. When there is one thread it's not a problem but if you create one thread per device, that might be (though there is no need to - just use single queue, unless you really write A LOT of messages per second).