2
\$\begingroup\$

Use Case: I have a form application in which I log events to a file/database and have a need to also raise those events to the application itself (for display purposes). Think old school updating status bar with the same content that you will log to a file.

The solution I put in place is a custom log4net class (since we're already using that for the actual file and databases logging). This simple class just raises an event when the message payload of the caller. This way I can use a single call to the log instead of multiple calls to different loggers.

This is a working class, I'm just looking for input on pitfalls on the code or potential performance issues that might be found (and to also share the idea for others that need it). I'm also wondering missing some best practices or something simple that I've overlooked.

I the case of my requirements, this event may be called 30 times/sec across multiple different threads.

MemoryQueueEventArgs isn't the actual final class name, it was another direction I was going to go, I just haven't renamed it yet.

public class MemoryQueueEventArgs : EventArgs
{
  // Customize to your business requirements
  public DateTime CreateDate { get; set; }
  public string Message { get; set; }
  public string Level { get; set; }
  public string LoggerName { get; set; }
  internal int Count { get; set; }
}

public class MemoryQueueAppender : AppenderSkeleton
{
  private object objectLock = new Object();

  #region Public Methods
  public static MemoryQueueAppender GetActiveAppender(Type type)
  {
    IAppender[] appenders = log4net.LogManager.GetLogger(type).Logger.Repository.GetAppenders();
    return (appenders
     .FirstOrDefault(a => a.GetType().Name == "MemoryQueueAppender") as MemoryQueueAppender)
    ;
  }

  protected event EventHandler<MemoryQueueEventArgs> _LogUpdate;
  public event EventHandler<MemoryQueueEventArgs> LogUpdate
  {
    add
    {
      lock (objectLock)
      {
        _LogUpdate += value;
      }
    }
    remove
    {
      lock (objectLock)
      {
        _LogUpdate -= value;
      }
    }

  }
  #endregion Public Methods

  protected override void Append(log4net.Core.LoggingEvent loggingEvent)
  {
    MemoryQueueEventArgs mqea = _CreateMemoryQueueEventArgs(loggingEvent);
    this.OnLogUpdate(mqea);
  }

  protected override void Append(log4net.Core.LoggingEvent[] loggingEvents)
  {
    base.Append(loggingEvents);
  }


  #region Private Methods

  private MemoryQueueEventArgs _CreateMemoryQueueEventArgs(log4net.Core.LoggingEvent loggingEvent)
  {
    return new MemoryQueueEventArgs
    {
      CreateDate = DateTime.Now,
      Level = loggingEvent.Level.Name,
      LoggerName = loggingEvent.LoggerName,
      Message = RenderLoggingEvent(loggingEvent),
    };
  }

  protected virtual void OnLogUpdate(MemoryQueueEventArgs e)
  {
    EventHandler<MemoryQueueEventArgs> handler = _LogUpdate;
    if (handler != null)
    {
      handler(this, e);
    }
  }

  #endregion Private Methods

}

Calling Code:

public TestDevelopment()
{
  MemoryQueueAppender appender = MemoryQueueAppender.GetActiveAppender(this.GetType());
  appender.LogUpdate += appender_LogUpdateEvent;
}

void appender_LogUpdateEvent(object sender, MemoryQueueEventArgs e)
{
  System.Diagnostics.Debug.WriteLine(string.Format("[{0}] {1}", e.Level, e.Message));
}
\$\endgroup\$
1
\$\begingroup\$

I log events to a file/database and have a need to also raise those events to the application itself (for display purposes).

Generally this is not a bad idea but...

I the case of my requirements, this event may be called 30 times/sec across multiple different threads.

with this many log events per second it is. No human can read it anymore unless you are writing applications for Jedis.

Use a custom named logger and send to the UI only those messages that have some value to the user not just everything that gets logged.

Or create some kind of a filter that reduces the number of messages to a reasonable count.

\$\endgroup\$
2
  • \$\begingroup\$ As for the rate limiting, I was thinking the same thing but in the next iteration of the logger, but I didn't want the lack of that to limit the first implantation of this in the code. \$\endgroup\$
    – Gary Smith
    Jan 24 '17 at 15:03
  • \$\begingroup\$ @GarySmith 1.800 events per minute... who is going to read it? Are you sure you need all this usless data? \$\endgroup\$
    – t3chb0t
    Jan 24 '17 at 15:45
1
\$\begingroup\$
  1. What are you trying to achieve by locking _LogUpdate += value statement? It feels like you are misusing the lock, because default event implementation (when you declare event in a field-like fashion) is already thread-safe. You can just declare:

    public event EventHandler<MemoryQueueEventArgs> LogUpdate;
    
  2. You shouldn't override methods, if all you do is call base implementation.
  3. I don't like the idea of having static method that returns appender. It adds a static dependency to your code with all the downsides normally associated with it. This approach is also hard to extend, if later you want to display something not related to logging inside your status bar. A better approach in my opinion would be to inject event aggregator to your appender and fire some StatusInfo message. Then your status bar can listen to those messages and update itself accordingly, without static dependencies and without any knowledge of your logging framework. Furthermore, it would allow you to send those messages not only from your custom log appender but from other places too without any modifications to status bar code.
\$\endgroup\$
1
  • \$\begingroup\$ For the lock within the add/remove I've just followed the Microsoft examples on using it (msdn.microsoft.com/en-us/library/cc713648.aspx). The override was just left over when I had a system.diag's in there. I'll have to put some thought into the static issue, that was more of simplification for the setup in the app to get to the core appender to subscribe to the event. \$\endgroup\$
    – Gary Smith
    Jan 25 '17 at 16:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.