Fixes #50 - include named scopes

[nblumhardt]

When includeNamedScopes is specified as true in the AddSerilog() call, string-valued scope data will be included in a new property called Scope on the resulting log events:

using (logger.BeginScope("Outer"))
{
    logger.LogInformation("Outer");

    using (logger.BeginScope("Inner"))
    {
        logger.LogInformation("Inner");
    }

    logger.LogInformation("Outer, again");
}

logger.LogInformation("Done");

No breaking changes, though a few constructors were added to SerilogLoggerProvider to facilitate this.

Alternatives

The opt-in nature of the change is following the console logger provider, which requires includeScopes to be passed for scope information to be recorded.

It could make sense to enable this by default, since developers are essentially opting in with BeginScope(), and object-valued (i.e. key-value-pair) scope state is already included.

Thoughts?

[nblumhardt]

Switched to an opt-out. The parameter is now ignoreNamedScopes = false.

[nblumhardt]

One other consideration; properties added with BeginScope(new Dictionary<string,object>{ ["A"] = 1 }) will be pushed onto the LogContext; named scope properties as implemented in this PR won't.

The original reason for pushing named properties onto LogContext was to carry though the RequestId and other properties added by ASP.NET middleware during the request handling lifecycle. I think it would be consistent to handle named scopes in the same way, but not necessarily advantageous to do so.

[nblumhardt]

@serilog/reviewers-core any thoughts on this? Will merge to dev later today otherwise. Cheers!

[nblumhardt]

Some changes:

• Most values are now added to Scope, whether string, numeric, or formatted.
• Those added via an IEnumerable<KeyValuePair<string,object>> are treated purely as properties.
• There's no longer an opt-out; this can be reinstated if it's requested, but the benefits of opting out are unclear.
• Everything always ends up on LogContext - including Scope.
• An explicit .NET 4.6 TFM added, so that the more reliable AsyncLocal is used when available.

Test method:

using (_logger.BeginScope("Some name"))
using (_logger.BeginScope(42))
using (_logger.BeginScope("Formatted {WithValue}", 12345))
using (_logger.BeginScope(new Dictionary<string, object> { ["ViaDictionary"] = 100 }))
{
    _logger.LogInformation("Hello from the Index!");
}

Produces:

[nblumhardt]

This will occur when _state is a FormattedLogValues object.

[nblumhardt]

It's a shame we don't have CreatePropertyValue() here, to skip the extra allocation. We could special-case string to improve it a little.

[cilerler]

Do you think, you may also inject values in Dictionary into the Scope? That would be awesome.

I mean following line didn't appear in Scope which I believe it should

using (_logger.BeginScope(new Dictionary<string, object> { ["ViaDictionary"] = 100 }))

I know that the second parameter is object but still serialization to JSON would be 😎

[nblumhardt]

Thanks for the feedback.

I'm hesitant to add the extra weight to each log event that this would entail. The Dictionary case is how correlation ids like OrderId, MessageId, CustomerId and so-on can be added to a sequence of events.

Adding a whole additional structured object to Scope in this scenario (as well as adding the scope property itself, for a start) will start to get expensive quickly.

[cilerler]

I hear you, and you are right. Thanks again! Is there a build that I can test?

[nblumhardt]

Feeling pretty good about the direction here - let's get it through to dev so everyone can kick the tyres :-)