Nlog: Custom NLog target with async writing

[grokky1]

Type: Question
NLog version: 5.0.0-beta05
Platform: .NET Core

I want to write a custom target that logs to my database using Entity Framework Core.

In NLog.Targets.Target there is this:

protected virtual void Write(LogEventInfo logEvent);

However my code is async, so I must do:

protected override /*async Task*/void Write(LogEventInfo logEvent)   // problem is here
{
    await WriteToEfContext(logEvent);               // problem is here
    // and so on...
}

But there is no Task WriteAsync version of that method.

How do I write a custom target, with async support?

[snakefoot]

How do I write a custom target, with async support?

I guess you override the following methods, like this (Not tested with compiler):

private Task _previousTask;

protected override void Write(AsyncLogEventInfo logEvent)
{
    AsyncContinuation continuation = logEvent.Continuation;
    if (_previousTask == null)
        _previousTask = WriteTask(logEvent.LogEvent).ConfigureAwait(false).Start();
    else
        _previousTask = _previousTask.ContinueWith(WriteTask(logEvent.LogEvent));
    _previousTask = _previousTask.ContinueWith(prevTask => continuation(prevTask.Exception));
}

protected override void FlushAsync(AsyncContinuation asyncContinuation)
{
    if (_previousTask == null)
         asyncContinuation(null);
    else
        _previousTask = _previousTask.ContinueWith(prevTask => asyncContinuation(null));
}

private Task WriteTask(LogEventInfo logEvent)
{
    // TODO Create custom write Task
    return WriteToEfContext(logEvent);
}

[grokky1]

@snakefoot Thanks, this looks really cool!

@304NotModified What do you think of it?

Maybe it's a good idea to wrap this behavior in a new method:

protected virtual Task WriteAsync(LogEventInfo logEvent);

It's such a common use case.

---

PS: I would love to make a PR for this, but I have to admit that this sort of stuff isn't one of my strengths. I am sure I am not alone, that is why for such a common use case it is maybe a good idea to have the traditional .NET void Foo() and Task FooAsync() pattern?

[304NotModified]

I think a new (abstract) base class (AsyncTarget) would be nice.

[grokky1]

@304NotModified Interesting... However, why would that be "better" than an async overload?

(It may be more effort that it's worth, maybe would require re-implementation of the useful subclasses, e.g. AsyncTarget, AsyncTargetWithLayout, AsyncTargetWithLayoutHeaderAndFooter, etc.)

[304NotModified]

Well then it's less clear which method to implement in each case.

The targetWithLayout and others are only small additions to Target.

[304NotModified]

Thanks @snakefoot. You could also copy it to here http://stackoverflow.com/questions/42605275/custom-nlog-target-with-async-writing

Will upvote it :)

[snakefoot]

@304NotModified Think I will have to do some testing first, my pseudo-code didn't pass the compiler test. The ContinueWith-logic had to be changed to call the WriteAsyncTask using a delegate, and this required context-capture of the LogEvent-struct. This I didn't like, but now the logic has become complicated.

Now it has the following properties:

• Only uses a single Task to protect against out-of-order writing of LogEvents (And avoids using the entire thread pool).
• WriteAsyncTask is called without holding Target-Lock-object to improve concurrency (loggers can schedule more LogEvents while the initial LogEvent call is being started).
• Having a requestQueue to make it easier to monitor pending requests (instead of navigating in a ContinueWith-chain).

Following features could probably be added:

• Support batching of multiple LogEvents in a single write operation.
• Support parallel writing of LogEvents using a limited number of concurrent Tasks.

See PR #2006 (Just added timeout support)

[grokky1]

I was just going to report it doesn't compile, then I see that post. Way way above my head! 😃

This is an excellent piece of code to include into the library itself.

I volunteer myself as a guinea pig for using it.

[snakefoot]

You could also copy it to here

I have now also posted the code on stackoverflow. Maybe more eyes will reveal stuff, that my testing didn't show.

[snakefoot]

@grokky1 I have updated the AsyncTaskTarget to also have timeout monitoring (Default 150 secs). The updated code can be found in #2006

[grokky1]

@snakefoot That's really nice to have. I am playing around with it now in my dev branch, and I like it so far. I hope we can get it into an alpha release.

[304NotModified]

@grokky1 do you have some experience to share about the AsyncTaskTarget?

[grokky1]

@304NotModified @snakefoot Yeah I was using it and it seems to work fine so far. I was using the target as part of .NET Core's built in container, as well as Autofac. This new async version (I'm using it to log to my EF context) allows me to use it as transient or per-request, which is what I needed, because my context is registered as per-request.

My team though decided to go another route, because the it's still not "in the wild" for long enough. But I'm still using it on some personal projects.

Thanks to both of you!

[snakefoot]

@grokky1 Any input for this wiki-page about AsyncTaskTarget: https://github.com/NLog/NLog/wiki/How-to-write-a-custom-async-target