EventLogger for MVC application

Question

I have worked on one application related for booking space and required to store activity of user into table and access from admin side. For that I have created one database entity and EventLogger class.

public partial class UserEventLog
{
    public int ID { get; set; }
    public string UserID { get; set; }
    public Enum_SourceType SourceType { get; set; }
    public Enum_EventType EventType { get; set; }
    public string EventId { get; set; }
    public string EventName { get; set; }
    public Enum_LogType LogType { get; set; }
    public string LogMessage { get; set; }
    public string LogDescription { get; set; }
    public string IPAddress { get; set; }
    public string HostName { get; set; }
    public DateTimeOffset EventTriggeredOn { get; set; }
    public virtual AspNetUser User { get; set; }
}

related enum

public enum Enum_SourceType
{
    User,
    Order
}
public enum Enum_LogType
{
    Information,
    Warning,
    Error
}
public enum Enum_EventType
{
    Create,
    Update,
    Delete,
    Duplicate,
    DownLoad,
    Preview,
    Search,
    AppliedCoupon,
    Cancel,
    LoggedIn,
    Register,
    SendMail,
    ConfirmEmail,
    Account,
    View,
    SendMessage
}

and I have created EventLogger class for log event from whole application.

  public class EventLogger 
{

    public static IUserEventLogService UserEventLogService
    {
        get
        {
            return Core.ContainerManager.GetConfiguredContainer().Resolve<IUserEventLogService>();
        }
    }
    public static IUnitOfWorkAsync UnitOfWork
    {
        get
        {
            return Core.ContainerManager.GetConfiguredContainer().Resolve<IUnitOfWorkAsync>();
        }
    }
    public static async Task Log(DateTimeOffset eventTriggeredOn, Enum_EventType eventType, string eventName, Enum_LogType logType, string logMessage,string eventId = "", string logDescription = "")
    {

        var currentUserId = System.Web.HttpContext.Current.User.Identity.GetUserId();
        string ipAddress = GetLocalIPAddress();
        var userEventLog = new UserEventLog()
        {
            EventId = eventId,
            LogType = logType,
            EventName = eventName,
            EventTriggeredOn = eventTriggeredOn,
            EventType = eventType,
            LogMessage = logMessage,
            LogDescription = logDescription,
            IPAddress = ipAddress,
            UserID = currentUserId,
            ObjectState = Repository.Pattern.Infrastructure.ObjectState.Added,
            SourceType = Enum_SourceType.User
        };
        UserEventLogService.Insert(userEventLog);
       await UnitOfWork.SaveChangesAsync();
    }
    public static string GetLocalIPAddress()
    {
        string visitorIPAddress = GetVistorIp(HttpContext.Current.Request.ServerVariables);  
        return visitorIPAddress;
    }
    static string GetVistorIp(NameValueCollection headers)
    {
        if (!string.IsNullOrWhiteSpace(headers["HTTP_X_FORWARDED_FOR"]))
            return headers["HTTP_X_FORWARDED_FOR"];
        if (!string.IsNullOrWhiteSpace(headers["HTTP_CF_CONNECTING_IP"]))
            return headers["HTTP_CF_CONNECTING_IP"];
        return headers["REMOTE_ADDR"];
    }
}

Please suggest me if this solution is right or require any modification or any good solution for suggestion for my implementation.

Answer 1

Again enums should not have any prefix. Drop all those Enum_! You may like to take a look to .NET Naming Guidelines.

Inside the class UserEventLog you do not need to repeat Log prefix for its properties. For example just Message is enough. Think about a Window object: did you ever see properties named WindowTitle, WindowSize, WindowCanMaximize and so on?

Validate values, I might assign null to LogMessage (or an invalid enum for EventType) and it will go straight to the DB engine (which might reject it or not).

Do not hide resolved services in a property like UnitOfWork and UserEventLogService. If you use those objects more than once in the same call then it's a performance hit you do not want to pay (because it has to be resolved for each call and it might event be a new object, it depends on how it's configured in your DI framework). Resolve them once in ctor or better accept them as parameters.

In addition of a single method Log() with all those parameters you may want to introduce few extension methods (or overloads):

public static async Task LogUserWarning(this EventLogger logger,
                                        DateTimeOffset eventTriggeredOn,
                                        string eventName,
                                        string message)
{
    // ...
}

Think twice before using default values for parameters. If you own this code and the calling code then it's not an issue (probably) but if your code is released as a library, to be used by someone else, then you're introducing a strong contract (simply changing default value will not update callers unless they're recompiled, not to mention that they have to check if new values are what they expect). For a real-world example of this issue you may read Adding option parameter to a library in c# without creating a breaking change .

Using String.Empty instead of null for a string entry in a DB may be a waste but it depends on DB engine you're using (for example compare SQL Server and Oracle).

I see no reason for EventLogger to do not be sealed.

Do not use System.Web.HttpContext.Current directly. Accessing a static property makes impossible to test your code in unit testing outside an HTTP request. The same is true for GetLocalIPAddress(). If really you can't avoid it then move it to a derived class, at least you can test all the underlying logic in isolation. Better would be to introduce a service to retrieve those information:

interface IHttpRequestInspector
{
    string GetIpAddress();
    string GetAuthenticatedUserName();
}

Now you can create a default implementation using the code you already have but you can also have a MockedHttpRequestInspector class which can be used in unit testing:

sealed class MockedHttpRequestInspector : IHttpRequestInspector
{
    public MockedHttpRequestInspector(string ipAddress, authenticatedUserName)
    {
        _ipAddress = ipAddres;
        _authenticatedUserName = authenticatedUserName;
    }

    public string GetIpAddress() => _ipAddress;
    public string GetAuthenticatedUserName() => _authenticatedUserName;

    private readonly string _ipAddress;
    private readonly string _authenticatedUserName;
}

Some properties you declared in UserEventLog are not used in code you show (for example UserEventLog.User). Drop them?

Answer 2

Logger are very important for any project. In general logger design should be flexible enough to provide custom implementation along with that it should be able to work with existing log providers available in market such as Log4net etc..

I have an application in GitHub where you can use it.

visit https://github.com/tgvishnu/CSharp/tree/master/Vishnu.Logger

1. Log interface should expose simple method and add as extension methods

   public interface ILog
   {
      bool Log(LogLevel logLevel, Func<string> message, Exception exception = null);
   }
   

2. ILogProvider interface return the ILog implementer

   public interface ILogProvider
   {
       ILog GetLogger(string loggerName);
   }
   

3. Create an enum for different Log levels

   public enum LogLevel
   {
        Trace,
        Debug,
        Info,
        Warn,
        Error,
        Fatal
    }
   

4. Add extension methods to ILog inteface for each type of enum

   public static class LogExtensions
   {
       private static void LogFormat(this ILog logger, LogLevel logLevel, string message, params object[] args)
       {
           var result = string.Format(CultureInfo.InvariantCulture, message, args);
           logger.Log(logLevel, () => message);
        }
   
        public static bool IsDebugEnabled(this ILog logger)
        {
           if (logger.IsDebugEnabled())
           {
               logger.Log(LogLevel.Debug, () => message);
           }
        }
   
        public static void Debug(this ILog logger, string message)
        {
            if (logger.IsDebugEnabled())
            {
               logger.Log(LogLevel.Debug, message);
            }
        }
        public static void Debug(this ILog logger, Func<string> message)
        {
            if (logger.IsDebugEnabled())
            {
               logger.Log(LogLevel.Debug, () => message, exception);
            }
        }
   
        public static void DebugException(this ILog logger, string message, Exception exception)
        {
            if (logger.IsDebugEnabled())
            {
                 logger.LogFormat(LogLevel.Debug, message, args);
             }
        }
   
        public static void DebugFormat(this ILog logger, string message, params object[] args)
        public static bool IsErrorEnabled(this ILog logger)
        public static void Error(this ILog logger, string message)
        public static void Error(this ILog logger, Func<string> message)
        public static void ErrorException(this ILog logger, string message, Exception exception)
        public static void ErrorFormat(this ILog logger, string message, params object[] args)
        public static bool IsTraceEnabled(this ILog logger)
            .......
   
        public static string DefaultMessageFormat(this ILog logger, string component, LogLevel level, string message, Exception ex)
       {
           var stringBuilder = new StringBuilder();
           stringBuilder.Append(DateTime.Now.ToString("yyyy-MM-dd hh:mm:ss", CultureInfo.InvariantCulture));
           stringBuilder.Append(" ");
           stringBuilder.Append(component);
           stringBuilder.Append(" => ");
           stringBuilder.Append(("[" + level.ToString().ToUpper() + "]").PadRight(8));
           stringBuilder.Append(message);
           if (ex != null)
           {
            stringBuilder.Append(Environment.NewLine).Append(ex.GetType());
            stringBuilder.Append(Environment.NewLine).Append(ex.Message);
           stringBuilder.Append(Environment.NewLine).Append(ex.StackTrace);
            }
   
          return stringBuilder.ToString();
       }
   

5. Create a provider class in the project , where you can define list of available for the solution. Add the log provider available, the log providers should be added in the sequence based on their usage.

   public static class SampleLogProviders
   {
       private static ILogProvider _currentLogProvider = null;
       public delegate bool IsLoggerAvailable();
       public delegate ILogProvider CreateLogProvider();
       public static readonly List<Tuple<IsLoggerAvailable, CreateLogProvider>> LogProviderCollection = new List<Tuple<IsLoggerAvailable, CreateLogProvider>>()
       {
            new Tuple<IsLoggerAvailable, CreateLogProvider>(DebugLogProvider.IsLoggerAvailable, ()=> new DebugLogProvider()),
            new Tuple<IsLoggerAvailable, CreateLogProvider>(ConsoleLogProvider.IsLoggerAvailable, ()=> new ConsoleLogProvider()),
       };
   
       public static ILog GetLogger(string name)
       {
           ILogProvider provider = _currentLogProvider ?? ResolveLogProvider();
           ILog result = provider.GetLogger(name) ?? new NoLog();
           return result;
        }
   
        private static ILogProvider ResolveLogProvider()
        {
             ILogProvider result = null;
             foreach(var provider in LogProviderCollection)
             {
                 if(provider.Item1() == true)
                 {
                      result = provider.Item2();
                      break;
                  }
              }
   
              return result;
            }
    }
   

6. Create Log providers, in the above step we have used DebugProvider and ConsoleProvider, example of Console provider is shown below

   public class ConsoleLogProvider : ILogProvider
   {
       public ILog GetLogger(string loggerName)
       {
           return new ConsoleLog(loggerName);
       }
       public static bool IsLoggerAvailable()
       {
           return true;
       }
   }
   

7. Create LogBase class (this is optional)

   public abstract class LogBase : ILog
   {
       public virtual bool Log(LogLevel logLevel, Func<string> message, Exception exception = null)
       {
           if (message == null)
           {
               return this.IsLoggerEnabled(logLevel);
           }
   
           this.WriteMessage(logLevel, message(), exception);
           return true;
       }
   
       protected string GetDetaultMessage(string component, LogLevel level, string message, Exception ex)
       {
           return this.DefaultMessageFormat(component, level, message, ex);
       }
   
       protected abstract void WriteMessage(LogLevel level, string message, Exception ex = null);
   
       protected virtual bool IsLoggerEnabled(LogLevel logLevel)
       {
           return true;
       }
   }
   

8. Create Console Log class derived from LogBase or implement ILog interface directly.

   namespace Vishnu.Logger
   {
       public class ConsoleLog : LogBase
       {
           private readonly dynamic _logger;
   
           private Dictionary<LogLevel, ConsoleColor> _foregroundColors = new Dictionary<LogLevel, ConsoleColor>
                       {
                           { LogLevel.Fatal, ConsoleColor.Red },
                           { LogLevel.Error, ConsoleColor.Yellow },
                           { LogLevel.Warn, ConsoleColor.Magenta },
                           { LogLevel.Info, ConsoleColor.White },
                           { LogLevel.Debug, ConsoleColor.Gray },
                           { LogLevel.Trace, ConsoleColor.DarkGray },
                       };
   
           public ConsoleLog(dynamic logger)
           {
               _logger = logger;
           }
   
           protected override void WriteMessage(LogLevel level, string message, Exception ex = null)
           {
               ConsoleColor color = ConsoleColor.White;
               string msg = base.GetDetaultMessage((string)_logger, level, message, ex);
               if (_foregroundColors.TryGetValue(level, out color))
               {
                   var originalColor = Console.ForegroundColor;
                   try
                   {
                       Console.ForegroundColor = color;
                       Console.Out.WriteLine(msg);
                   }
                   finally
                   {
                       Console.ForegroundColor = originalColor;
                   }
               }
               else
               {
                   Console.Out.WriteLine(msg);
               }
           }
   
           protected override bool IsLoggerEnabled(LogLevel logLevel)
           {
               return base.IsLoggerEnabled(logLevel);
           }
       }
   }
   

9. Create a Logger class in the main application, and enable log for each features that you want to log. Logging for each feature can be set from App.config settings

   public class Logger
   {
       private static readonly ILog _feature1 = Create(typeof(Feature1));
       private static readonly ILog _feature2 = Create(typeof(Feature2));
       private static readonly ILog _feature3 = Create("Feature3");
   
       public static ILog Feature1
       {
           get
           {
               return _feature1;
           }
       }
   
       public static ILog Feature2
       {
           get
           {
               return _feature2;
           }
       }
   
       public static ILog Feature3
       {
           get
           {
               return _feature3;
           }
       }
   
       internal static ILog Create(string name)
       {
           return InitializeLogFacade(name);
       }
   
       internal static ILog Create(Type type)
       {
           return Create(type.FullName);
       }
   
       private static ILog InitializeLogFacade(string name)
       {
           return LogProviders.GetLogger(name);
       }
   } 
   

10. Use the logger as below

    public class Feature1
    {
        public static void GenerateLogs(string message)
        {
            Logger.Feature1.Debug(message);
            Logger.Feature1.Fatal(message);
            Logger.Feature1.Error(message);
            Logger.Feature1.Trace(message);
            Logger.Feature1.Info(message);
            Logger.Feature1.Warning(message);
            Logger.Feature1.DebugException(message, new Exception("Debug exception"));
            Logger.Feature1.FatalException(message, new Exception("Fatal exception"));
            Logger.Feature1.ErrorException(message, new Exception("Error exception"));
            Logger.Feature1.TraceException(message, new Exception("Trace exception"));
            Logger.Feature1.InfoException(message, new Exception("Info exception"));
            Logger.Feature1.WarningException(message, new Exception("Warning exception"));
    
        }
    }