WIP LogFiltering integration

[Aaronontheweb]

Changes

Attempts to add https://getakka.net/articles/utilities/logging.html#filtering-log-messages support to Serilog

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

• This change follows the Akka.NET API Compatibility Guidelines.
• This change follows the Akka.NET Wire Compatibility Guidelines.
• I have reviewed my own pull request.
• Design discussion issue #
• Changes in public API reviewed, if any.
• I have added website documentation for this feature.

[Aaronontheweb]

Detailed my changes - haven't written any tests that actually use the formatter with Serilog yet; will need to add those before this can be merged.

[Aaronontheweb]

So the out _ we are discarding here is a pre-computed version of the entire formatted output string from the ILogFormatter. If the user has specified log filtering rules that look at the content of the string payload, it's necessary for us to expand it ahead of time.

If the user only looks at the LogSource then this may not be necessary - we can exclude a log without expanding it first (good for performance.)

However, the worst case scenario is what we're doing here: we expand the log once using the SerilogLogFormatter, check the output, discard it, and then format it again using a totally different algorithm.

My questions are:

1. Do we still need all of the custom formatting done by this class? Is the SerilogLogFormatterr enough?
2. If we still need to do all of this formatting, are we absolutely killing the perf of this logger if we allow filtering by log message? We warn in the docs that this setting is for reducing the amount of noise in the logs and will likely have a performance impact, but still - double-formatting is something I would like to avoid if at all possible.

[Danthar]

Ah the LogFilter expands a message simply by doing a Message.ToString(). This will not contain all the extra details that the SerilogLogger allows for. At least not in its current implementation.
This also means that any content filters will never be able to work on any of the metadata that serilog provides. So the extra LogContext enrichers and any custom registered enrichers will not be available for evaluation by the LogFilter. This seems like an important aspect to note in the documentation.

Considering that Serilog does its own formatting with those enrichers internally. And the api for that (getting a formatted message) is as far as i know (haven't looked very far admittedly) not public. We will probably not be able to provide this feature in such a way.

I do see some other issues in the SerilogLogger implementation. In the past years LogContext support and enrichters have been added. See the GetLogger function at line 54.

I cant add any comments there directly in github for reasons unknown. But the Log.Logger.ForContext api causes an issue that any Log context thats added is persisted on the logger. Instead we should be using the disposable LogContext api there.

using (LogContext.PushProperty("contextname", contextvalue))
{
 using (LogContext.PushProperty() ..//more properties if needed
} 

See: https://github.com/serilog/serilog/wiki/Enrichment#the-logcontext

Also this should then probably not be done in the GetLogger function as these LogContext calls should wrap the actual logging call to serilog.

Also the https://github.com/serilog/serilog/wiki/Enrichment#the-logcontext does not seem to allow to set multiple Context Properties. As you seem to get a new logging adapter on each call. I also think thats an outstanding bug.

All those issues aside. I think there is little chance of avoiding the double expand. Although i think the initial expand done by the LoggingFilter, as its a Message.ToString() Is very harmless because unless im remembering wrong, the object message here is practically always a string.

[Aaronontheweb]

I think there is little chance of avoiding the double expand. Although i think the initial expand done by the LoggingFilter, as its a Message.ToString() Is very harmless because unless im remembering wrong, the object message here is practically always a string.

That was my worry and my conclusion too - thank you for confirming.