Implementing Non blocking thread safe singleton logging class

Question

I want to write a logger class which writes output to a log file based on user actions. I have written a logmessage class for this purpose. Following is what I have.

public class LogMessage
{
    private  static Object padlock = new Object();
    private static string serviceDirectory ="C:\\MySite\\";
    private static string filePath = "\\Logs\\Combined\\ActivityMonitorLog-" + DateTime.Now.ToString("dd'-'MM'-'yyyy") + ".log";

    private static volatile LogMessage instance = new LogMessage();
    public static LogMessage Instance
    {
        get
        {
            if (instance == null)
            {
                lock (padlock)
                {
                    if (instance == null)
                        instance = new LogMessage();
                }
            }

            return instance;
        }
    }


    public void SaveLogMessage(string userName, string message, string stackTrace)
    {
        lock (padlock)
        {
            using (StreamWriter logWriter =
                new StreamWriter(serviceDirectory + filePath, true))
            {
                string logMessage = "";
                if (!string.IsNullOrEmpty(stackTrace))
                    logMessage = string.Format("{0} >> user : {1} ::: {2} ::: stack trace ::: {3}", DateTime.Now.ToString(), userName, message,stackTrace);
                else
                    logMessage = string.Format("{0} >> user : {1} ::: {2}", DateTime.Now.ToString(), userName, message);

                logWriter.WriteLine(logMessage);

            }
        }
    }
}

And when i want do a log entry I am calling the about method like below

LogMessage.Instance.SaveLogMessage("My User", "Enter Logout PageLoad : Current Method :" + "Page_Load @ Logout.aspx.cs", "");

I believe I have got the singleton part and thread safety achieved. But I don't think this is non blocking as multiple users are logged in at the same time each one will have to wait until the file is available to be written. I am pondering about using a backgroundworker to call this SaveLogMessage() from the LogMessage singleton object. Is that the correct way of doing it?

UPDATE

I have implemented this using backgroundworker and the complete class and how to call is mentioned in the answers. Thanks for inputs @bret and @huan

It is not possible to answer if code that you have not written yet is correct or not. Obviously from educational point of view trying to use background worker is "correct way" as you'll learn new things. From practical point of view you'd just use existing proved logging library and use it... So here is my *opinion* - yes, this is correct way. — Alexei Levenkov, Jul 01 '17 at 02:46

score 1 · Answer 1 · answered Jul 01 '17 at 02:55

For your "instance" field, use "readonly" instead of "volatile" because the value of the field never changes.

You don't really even need the public Instance property--you could make SaveLogMessage into a static method and use it directly.

In the SaveLogMessage body you could use Task.Run(() => { ... }) to do the logging action on a background thread to make it non-blocking. This might not be super efficient if you're logging very frequently.

Consider using System.Diagnostics.Trace instead, which gives you more flexibility.

score 1 · Answer 2 · answered Jul 01 '17 at 03:05

It is better to use ReaderWriterLockslim to add only write lock when you are trying to write.
I think a static method with ReaderWriterLockslim is good enough. there is a good post here
If you do want to use singleton here, you may use Lazy refer to Implementing the Singleton Pattern in C#

btw why not use log4net or any other existing library.

score 1 · Accepted Answer · answered Jul 05 '17 at 04:10

In the end I have managed to achieve a non-blocking, thread safe,singleton logging class with the use of a background worker. I am submitting my solution in case someone find it useful someday.

using System;
using System.Collections.Generic;
using System.IO;
using System.Web;
using System.Threading;
using System.Threading.Tasks;
using System.ComponentModel;
using System.Diagnostics;
using System.Web.Configuration;
/// <summary>
/// Summary description for LogMessage
/// </summary>
public class LogMessage
{
    static ReaderWriterLock locker = new ReaderWriterLock();

    private static string serviceDirectory = HttpContext.Current != null ?
        AppDomain.CurrentDomain.BaseDirectory :
        Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().Location);
    private static string fullpath = serviceDirectory + "\\ActivityLog.log";
    private static readonly LogMessage instance = new LogMessage();

    public static LogMessage Instance
    {
        get { return instance; }
    }

    public void SaveLogMessage(string userName, string message, string stackTrace, bool inOut)
    {
        bool EnableActivityLogging = false;

        if (string.IsNullOrEmpty(WebConfigurationManager.AppSettings["EnableActivityLogging"]))
            return;

        EnableActivityLogging = Convert.ToBoolean(WebConfigurationManager.AppSettings["EnableActivityLogging"]);

        if (!EnableActivityLogging)
            return;

        BackgroundWorker logbw = new BackgroundWorker();
        logbw.DoWork += logbw_DoWork;
        logbw.RunWorkerCompleted += logbw_RunWorkerCompleted;

        List<string> paramList = new List<string>();
        paramList.Add(userName);
        paramList.Add(message);
        paramList.Add(stackTrace);
        paramList.Add(inOut.ToString());

        if (!logbw.IsBusy)
        {
            logbw.RunWorkerAsync(paramList);
        }
    }

    void logbw_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        Debug.Write("Log Message Background Worker is now free...");
    }

    void logbw_DoWork(object sender, DoWorkEventArgs e)
    {
        List<string> paramList = (List<string>)e.Argument;
        string userName = paramList[0].ToString();
        string message = paramList[1].ToString();
        string stackTrace = paramList[2].ToString();
        bool inOut = bool.Parse(paramList[3].ToString());

        try
        {
            locker.AcquireWriterLock(int.MaxValue);
            using (StreamWriter logWriter =
                new StreamWriter(fullpath, true))
            {
                string logMessage = "";
                if (!string.IsNullOrEmpty(stackTrace))
                {
                    if (inOut)//IN
                    {
                        logMessage = string.Format("{0} U:{1} IN:{2} E:{3}", DateTime.Now.ToString(), userName, message, stackTrace);
                    }
                    else//OUT
                    {
                        logMessage = string.Format("{0} U:{1} OUT:{2} E:{3}", DateTime.Now.ToString(), userName, message, stackTrace);
                    }
                }
                else
                {
                    if (inOut)//IN
                    {
                        logMessage = string.Format("{0} U:{1} IN:{2}", DateTime.Now.ToString(), userName, message);
                    }
                    else//OUT
                    {
                        logMessage = string.Format("{0} U:{1} OUT:{2}", DateTime.Now.ToString(), userName, message);
                    }
                }

                logWriter.WriteLine(logMessage);

            }
        }
        finally
        {
            locker.ReleaseWriterLock();
        }
    }

}

Now I can use this singleton object like below to save a log entry

    LogMessage.Instance.SaveLogMessage(Context.User.Identity.Name, "Custom Message" + " M:" + MethodInfo.GetCurrentMethod().Name + "@" + Path.GetFileName(Page.AppRelativeVirtualPath), "", true);

Implementing Non blocking thread safe singleton logging class

3 Answers3