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;
    }
}

`NLog` is thread safe. You can use it for multithreaded services. See [this answer](http://stackoverflow.com/a/5706633/579895) — Pikoh, Apr 25 '16 at 14:10
Have you tried moving the 'lock (fileLocker)' lock to wrap the whole contents of the LogToFile method? I.e. to prevent WriteAllText and AppendAllText being called on the same file from two different threads. — user469104, Apr 25 '16 at 14:18
@Pikoh how do I create the above mentioned Date/Folder/LogFile structure in NLog? — Dawood Awan, Apr 25 '16 at 14:23
Well, as an example,i've got this config for my logs:`´`. It creates something like `c:/Logs/app/machinename20160203.txt` — Pikoh, Apr 25 '16 at 14:26
You've got sample configurations [here](https://github.com/nlog/NLog/wiki/File%20Target). The NLog wiki is pretty good btw. — Pikoh, Apr 25 '16 at 14:30

score 5 · Accepted Answer · answered May 04 '16 at 09:40

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).

I implemented this method a couple of days ago - and it is in production now. Regarding performance it is good - Having one Queue/Thread to do all the logging is working fine, there are no longer any File Access Exceptions - Thanks — Dawood Awan, May 11 '16 at 05:49

score 3 · Answer 2 · answered Apr 25 '16 at 14:10

While it probably isn't the most elegant solution, you could have retry logic built in. For example:

int retries = 0;
while(retries <= 3){
    try{
        var _logger = new xPTLogger("DeviceId", 12);

        _logger.LogMessage(MessageType.Info, string.Format("Information Message = [{0}]", 1));
        break;
    }
    catch (Exception ex){
        //Console.WriteLine(ex.Message);
        retries++;
    }
}

Also, I wrote that code just now without actually testing it so if there's some stupid error in it forgive me. But quite simply it'll try to write to the log as many times as you set in the "while" line. You can even add a sleep statement in the catch block if you think it'd be worth it.

I have no experience with Log4Net or NLog so no comment there. Maybe there's a sweet solution via one of those packages. Good luck!

Oh also, it'd probably be a better idea to put that retry block in the LogMessage method to minimize code. — briansyph, Apr 25 '16 at 14:11
I forgot to mention in my question that sometimes the service crashes using my xPTLogger, so that is why I am looking for alternative — Dawood Awan, Apr 25 '16 at 14:19

Separate Logfile and directory for each client and date

2 Answers2

Linked