NLog AsyncTaskTarget and aspnet-items

Created on 7 Apr 2019  路  4Comments  路  Source: NLog/NLog

First - Thanks for the amazing work on this library!

This is more of a question than a bug.

I have an AsyncTaskTarget that posts log events to Azure and will batch events and could delay and up to 10s. In the layout, I'd like to include items for the ASP.NET HttpContext.Items collection because MDLC is not available. My fear is that by the time the events are rendered in the task, the HttpContext and it's items may no longer be valid, or may be handling a different web request. Any validity to this case or is this addressed via another mechanism? Anything I can do to mitigate this risk?

For reference, here's my Azure Blob Async Target Task method.

protected override Task WriteAsyncTask(IList<LogEventInfo> logEvents, CancellationToken cancellationToken)
{
    if (!logEvents.Any())
        return Task.CompletedTask;

    var task = Task.Run(async () =>
    {
        var body = string.Join(Environment.NewLine, logEvents.Select(x => Layout.Render(x))) + Environment.NewLine;
        using (var memoryStream = new System.IO.MemoryStream(Encoding.UTF8.GetBytes(body)))
        {
            await _currentLogBlob.AppendBlockAsync(memoryStream, null, null, Options, null, cancellationToken);
        }

    }, cancellationToken);

    return task;
}
question

All 4 comments

You have indeed identified a very important issue when doing async-logging. It is not safe to access objects where the lifetime completed a long time ago (This can also be simple stuff like the threadid of the logging thread).

This issue can also be seen with the old horse AsyncTargetWrapper, that shares the same challenges as
AsyncTaskTarget.

NLog uses its Layout-engine to help solve this issue. When the NLog Logger writes to an async target then it performs PrecalculateVolatileLayouts that captures the output of of NLog Layouts that are not marked to have unlimited lifetime (Without ThreadAgnostic-attribute).

AsyncTaskTarget automatically performs PrecalculateVolatileLayouts and will force the logging-thread to capture all NLog Layout details, so they can be accessed safely later in time

P.S. You should consider changing your logic to this:

```c#
protected override async Task WriteAsyncTask(IList logEvents, CancellationToken cancellationToken)
{
if (!logEvents.Any())
return Task.CompletedTask;

using (var memoryStream = new System.IO.MemoryStream(1024 * logEvents.Count))
using (var streamWriter = new System.IO.StreamWriter(memoryStream, Encoding.UTF8, 1024, true))
{
   foreach (var logEvent in logEvents)
   {
     streamWriter.Write(RenderLogEvent(this.Layout, logEvent));
     streamWriter.WriteLine();  // Performs unnecessary allocation when getting string as parameter
   }
   streamWriter.Flush();
   memoryStream.Position = 0;
   await _currentLogBlob.AppendBlockAsync(memoryStream, null, null, Options, null, cancellationToken);
}

}
```

If you are really good at making a prediction of the initial required capacity of the MemoryStream (Better than 1024 * logEvents.Count). Then you will have minimal allocations and better performance.

Perfect. Thanks for the detailed explanation. Glad to know it's already handled by the target. I'll update the target with your recommendations.

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

Was this page helpful?
0 / 5 - 0 ratings