3

Is AsyncForwardingAppender of the Log4Net.Async package safe to use in an ASP.NET MVC Web Application? I'm worried that it will clog up the available threads.

It comes down to me wanting to make a call to an async method to send the logs to an HTTP API. I could use an async void method like the way this guy did it:

protected override async void Append(log4net.Core.LoggingEvent loggingEvent)
{
    var message = new SplunkMessage(loggingEvent.Level, loggingEvent.ExceptionObject);
    var success = await _splunkLogger.Report(message);
    //do something with the success status, not really relevant
}

He later updated his code:

public void DoAppend(log4net.Core.LoggingEvent loggingEvent)
{
    var clientIp = _clientIpRetriever.GetClientIp();
    var message = new SplunkMessage(loggingEvent.Level, loggingEvent.ExceptionObject, clientIp);
    SendMessageToSplunk(message);
}

private async void SendMessageToSplunk(SplunkMessage message)
{
    try
    {
        var success = await _splunkLogger.Report(message);
        //do something unimportant
    }
    catch(Exception x)
    {
        LogLog.Error(GetType(), "Error in SplunkAppender.", x);
    }            
}

But I'm too scared to actually try it because of the dangers involved: "First off, let me point out that "fire and forget" is almost always a mistake in ASP.NET applications".

Any suggestions?

Community
  • 1
  • 1
Korijn
  • 1,383
  • 9
  • 27
  • 2
    You can use the producer/consumer pattern which will use exactly 2 threads. – kha Mar 06 '15 at 19:07
  • Updated my question to better reflect the type of application I'm building (ASP.NET MVC Web Application). Is it really a good idea to have a single background thread reserved permanently for logging in such an environment? – Korijn Mar 06 '15 at 19:10
  • Really depends on your setup. As long as you don't spawn a separate thread per request (ie inject them as singletons) I don't see why not. On many web servers I've seen, they only subscribe to external queues once for instance which would be subscribed in their own thread. At the very least, it's better than spawning new threads every time you need to log. – kha Mar 06 '15 at 19:14
  • Alright, updated my question again... I found `AsyncForwardingAppender` in this package called `Log4Net.Async` which appears to do what you describe. I'm not sure if it's intended for web applications though. What do you think? https://github.com/cjbhaines/Log4Net.Async – Korijn Mar 06 '15 at 19:27
  • I haven't used it to be perfectly honest. What we used to do in one project was to use producer/consumer and dispatch logs using UDP since we didn't particularly care if some of them got lost. Based on the link you sent, the library seems to be queuing up the messages and dispatching them when a thread is available which is more or less producer/consumer though it doesn't promise it will use maximum one thread (I would imagine it does though otherwise handling the logs in the correct order would be a nightmare). Be careful that it will block after 1000 log messages are queued so if [cont]... – kha Mar 06 '15 at 19:38
  • 1
    [cont]... you're spamming the logs (which you should never do in a web environment) you'll get into trouble but then again, you'd be in trouble regardless of your solution if you're producing logs faster than they can be consumed. My personal opinion would be to use this library, test it under stress and if it doesn't cause issues, keep it. If it does, use your own queue instead. I appreciate you're really worried about using background threads in a web env (I would be too!) but this is the standard pattern for addressing the problem you're describing. – kha Mar 06 '15 at 19:44
  • Thanks for your helpful comments. I'm actually sending my log entries to an Azure Queue. Another application pulls them from there into ElasticSearch. The call to the Azure Queue is available in both Async and Sync flavors, so I think for now I'll resort to logging only on ERROR and FATAL levels and just taking the performance hit of sending in Sync style. Other logging levels will remain disabled until I have the time to actually perform all that testing you've suggested. Thanks again! – Korijn Mar 06 '15 at 19:48
  • 1
    No problem at all. Good luck! – kha Mar 06 '15 at 19:52
  • @kha: If you don't care that some of them are lost, then you can just call the method and not `await` the task: `var _ = LogAsync(...);` – Stephen Cleary Mar 06 '15 at 20:13
  • @StephenCleary We implemented it a long time ago. We were using .NET 2.0 (or maybe 3.5. Not sure anymore) so we didn't have the luxury of the new amazing async/await. Log4Net provided the UDP functionality back then (http://logging.apache.org/log4net/release/sdk/log4net.Appender.UdpAppender.html) and we just used producer/consumer to dispatch stuff off-thread from client machines which were all over the world but you're correct in that you can use async without await these days. Having said that, the question wanted to avoid spawning too many threads which producer/consumer would address. – kha Mar 06 '15 at 20:18

1 Answers1

2

Looking at the source you can see that the AsyncForwardingAppender uses only one thread to dequeue the events. Using it won't kill your MVC app since only one thread will be used.

Regarding "Fire and forget" as a bad pattern in web apps, you have to add a grain of salt to the statement since the answer talks about the danger of letting a functional operation go unsupervised, not a logging one. Logging should be able to fail without your application ceasing working (which is why log4net never says anything when configuration or logging fails)

samy
  • 14,832
  • 2
  • 54
  • 82