UseSerilog() disables Azure Application Logging

[cnelsonakgov]

Unsure why serilog-aspnetcore with Azure doesn't support basic Azure Application Logging. UseSerilog() seems to disable Application Logging feature and perhaps by design but there is no documentation on how to re-configure or enable it and no sink to support it.

Azure Application logging supports blob storage and log retention and without Serilog "just works" with .Net Core 2.0+ logger. It seems a shame that this package doesn't support it since .Net Core should play well with Azure and the serilog-aspnetcore package is preferred for .Net Core applications. Not sure if this is documentation issue, a by-design issue, or just a missing sink that needs to be developed.

Similar to issue #42 but different as that involves ways to enable the Log Stream.

[mtorres253]

Thank you @cnelsonakgov. That's really helpful. What would next steps look like here?

[nblumhardt]

Thanks for the feedback @cnelsonakgov .

We stopped trying to fit within the Microsoft.Extensions.Logging infrastructure, and replace it entirely, since at least as of the MEL v2 it's been hard to integrate deeply without delivering a second-class experience (duplicated level control, filtering, configuration, ...). The flip-side of this is that Serilog sinks need to be available for each logging target - which, with several hundred sinks available, mostly works fine, but there could be a gap here.

We would be open to improving the out-of-the-box experience, perhaps by having MEL "providers" plug into Serilog somehow, but as yet there's no proposal in the works. If anyone is keen to take a look, I think the first place to investigate is:

https://github.com/serilog/serilog-aspnetcore/blob/dev/src/Serilog.AspNetCore/AspNetCore/SerilogLoggerFactory.cs#L66

(It would be awesome to move this forward; the current situation really isn't ideal for anybody - happy to put time into this if there's a concrete plan.)

CC @glennc

[cnelsonakgov]

Thanks for the feedback @nblumhardt.

I have a basic solution working using a sink. I'm not sure if that is the best way to implement it but it does solve some of the issues with UseSerilog() clobbering the Microsoft.Extentions.Logging. It basically recreates the ILoggerFactory just for the AzureWebAppDiagnostics provider. It requires explicit declaration of the provider and thus uses the Microsoft.Extensions.Logging.AzureAppServices. The benefit is that you don't need to use any of the MEL configurations and you can still call 'UseSerilog()`. I will make it available for review. I'm pretty sure it needs some work as I just hacked it together.

[robfe]

Some context for others (I'm assuming this isn't news to @cnelsonakgov or @nblumhardt), Microsoft.Extensions.Logging.AzureAppServices has an extension method on ILoggerFactory called AddAzureWebAppDiagnostics
https://github.com/aspnet/Logging/blob/master/src/Microsoft.Extensions.Logging.AzureAppServices/AzureAppServicesLoggerFactoryExtensions.cs#L107

Calling this method (or sometimes simply referencing the right packages, see dotnet/AspNetCore.Docs#5016) is meant to make your web app respond to logging reconfiguration within the azure web portal (without requiring an app restart to pick up the new config)

It works on the default ILoggerFactory, but not on the Serilog implementation.

So the upshot is that calling .UseSerilog() on your IWebHostBuilder will stop you being able to dynamically reconfigure logging on the azure portal. You can still view streaming logs in the portal if you set up a file sink as per #42 (comment)

[cnelsonakgov]

I've put my initial implementation of a sink here, serilog-sinks-azureapp. I also put it out on NuGet for testing, Serilog.Sinks.AzureApp. Would be happy to get feedback.

After enabling the sink, Serilog events will show up in the Log Stream (File System) and Blob storage if enabled in the Diagnostics Logs.

cc @nblumhardt @robfe

[robfe]

Hi @cnelsonakgov, that's really helpful thanks.

I tweaked the app sink a little bit to try and get the right SourceContext from serilog mapped into the right Category in the ILogger:

    class AzureAppSink : ILogEventSink
    {
        /// <summary>
        /// Our very own Microsoft.Extensions.LoggerFactory, this is where we'll send Serilog events so that Azure can pick up the logs.
        /// We expect that Serilog has replaced this in the app's services.
        /// </summary>
        static ILoggerFactory CoreLoggerFactory { get; } = new LoggerFactory().AddAzureWebAppDiagnostics();

        /// <summary>
        /// The Microsoft.Extensions.LoggerFactory implementation of CreateLogger(string category) uses lock(_sync) before looking in its dictionary.
        /// We'll use our own ConcurrentDictionary for performance, since we lookup the category on every log write.
        /// </summary>
        readonly ConcurrentDictionary<string, ILogger> loggerCategories = new ConcurrentDictionary<string, ILogger>();

        readonly ITextFormatter textFormatter;

        public AzureAppSink(ITextFormatter textFormatter)
        {
            this.textFormatter = textFormatter ?? throw new ArgumentNullException(nameof(textFormatter));
        }

        public void Emit(LogEvent logEvent)
        {
            if (logEvent == null)
                throw new ArgumentNullException(nameof(logEvent));

            var sr = new StringWriter();
            textFormatter.Format(logEvent, sr);
            var text = sr.ToString().Trim();

            var category = logEvent.Properties.TryGetValue("SourceContext", out var value) ? value.ToString() : "";
            var logger = loggerCategories.GetOrAdd(category, s => CoreLoggerFactory.CreateLogger(s));

            switch (logEvent.Level)
            {
                case LogEventLevel.Fatal:
                    logger.LogCritical(text);
                    break;
                case LogEventLevel.Error:
                    logger.LogError(text);
                    break;
                case LogEventLevel.Warning:
                    logger.LogWarning(text);
                    break;
                case LogEventLevel.Information:
                    logger.LogInformation(text);
                    break;
                case LogEventLevel.Debug:
                    logger.LogDebug(text);
                    break;
                case LogEventLevel.Verbose:
                    logger.LogTrace(text);
                    break;
                default:
                    throw new ArgumentOutOfRangeException();
            }
        }
    }

[cnelsonakgov]

@nblumhardt I incorporated the suggestions from @robfe as a pull request and deployed an updated 2.1.1.1 version of the Sink to NuGet. The SourceContext change is very nice and re-factoring the LoggerFactory seems to improve the design.

[nblumhardt]

Nice! Thanks for the heads-up, Curtis 👍

Let's leave this issue open for discussing a better out-of-the-box experience; sounds like the main challenges have been overcome now, which is great.

[glennc]

So the current state of this is that there is a Serilog package that supports all the App Service things if you add it to your app. Right?

Are you thinking of trying for something like the light up experience that MEL has to turn it on post-deployment without adding it to the app?

[cnelsonakgov]

@glennc Correct, if you add the package, it will support all the Diagnostic Web Application log things (Stream, File, and Blob).

As for the light up experience, I believe you're talking about the guidance discussed here that says one should not "explicitly call AddAzureWebAppDiagnostics. The provider is automatically made available to the app when the app is deployed to Azure App Service."

Unfortunately, in my testing, it appears UseSerilog() makes this unavailable and so we do have to explicitly call the AddAzureWebAppDiagnostics provider when we re-create our own MEL LoggerFactory in the sink.

The sink does, however, accommodate turning the logging on and off in the Azure Portal interface -- stream, filesystem, blob -- so other than adding the NuGet package and .WriteTo.AzureApp() to the Startup.cs file the general experience and features available in the Azure Portal are the same as what one would get using the MEL.

[glennc]

There are two ways I can see us try and approach something like this:

1. Build a serilog version of the same code that enables the MEL provider in App Service, this would change the experience to installing that extension instead of adding the package and the line of code.
2. Work out some sort of MEL Sink that's added when using serilog.aspnetcore that effectively logs to MEL. Allowing MEL loggers to also work when otherwise replacing the system with Serilog.

Do either of those sound interesting?

[cnelsonakgov]

I think I'd let @nblumhardt weigh in here as to how Serilog team thinks this design should evolve to support the Azure App Service. But here are my comments:

1. I think this is an interesting approach, though I don't think I have the technical insight or exposure to the Azure environment to pull off writing an extension. Also, it does seem a little odd to me to treat this use case so differently than the others from the viewpoint of the Serilog project. Any other sink you would just reference and add to the configuration. Why should this one be different? It may be that from the Azure development perspective, App Service logging is supposed to be transparent, and thus would prefer an extension.

2. Also interesting, and the current implementation of the AzureApp sink could probably be updated to configure other providers instead of just the AzureWebAppDiagnostics provider. This would allow the creation of a generic MEL sink that could be configured in code or configuration file. Though I'm not sure what other MEL loggers are needed that aren't provided by the Serilog sinks.

I'll just note that we were building an app for Azure and .Net Core and decided to use Serilog. Our team was originally under the impression that the Trace sink was going to accommodate all the logging needs once we deployed to Azure. Our development proceeded with the Serilog package and the Trace and Console sinks. When we finally deployed to Azure, we realized that it wasn't working quite as we wanted so dumping the Trace sink and adding an AzureApp sink seemed like a straightforward exercise. At the time, my only other option was to rip out all the Serilog code and replace with MEL, so I'm happy we were able to successfully add the sink.

[nblumhardt]

It's a really tricky one. @glennc the second approach is closer to what I initially had in mind, and something we could explore further, but, zooming out, here's another angle we could look at this from.

It seems inevitable that Microsoft is going to develop many more log sinks over the coming years. But then we still have:

Serilog is used by a large slice of developers on .NET Core (as is NLog). Why the hard dependency on MEL specifically with these new diagnostic targets?

I don't mean that in the dummy spit way - this is a tough problem to crack and a great discussion to be having :-) ... I mean, why not ship a provider-agnostic AzureWebAppDiagnostics core package, and then reach the widest possible audience by accompanying it with some thin wrappers for MEL (default, lights up on Azure), Serilog (just add a WriteTo.X()) and NLog?

Behind the scenes, the logger-specific packages would just be thin facades that adapt a pretty narrow mapping from one structured event format to another very similar one.

The end result wouldn't have to be much more complex than the internals of the adapter package used above, which serves to prove that the concept is realizable, but it'd set a great precedent for nourishing the ecosystem rather than forcing hard choices between the default provider (with all of Microsoft's targets given first-class support) and Serilog (with hundreds of sinks, most for non-Microsoft technologies, which stand very little chance of having MEL providers written specifically for them).

Not intending to push this over either of the other two suggestions, which I'd be really happy to help bring to fruition, but just wanted to try to get a broader perspective and make sure we're solving the right problem. What do you think?

[nblumhardt]

Hmmm and now when I re-read #60 (comment), I think I could just be re-stating option 1. Sigh :-) 👍

[nblumhardt]

Hope my somewhat obtuse stream-of-consciousness reply didn't stall this thread @glennc ... The editor must have been taking the day off on Friday! 😅

Is there general agreement that getting direct support for Serilog as a source for Azure Web App Diagnostics (WriteTo...) is the way to go? Is there anything the Serilog project could do to make that possible, especially if it would mean enabling a similar user experience for other logging targets down the line?

[PureKrome]

polite ping to @glennc (and other MS team members) on thoughts about this discussion

[nblumhardt]

We ended up going over this again in serilog/serilog-extensions-logging#130 - still not really any consensus on a solution, but I'm working on serilog/serilog-extensions-logging#132 in the hope of achieving something better than the status quo. Could use input from someone more familiar with the space, if anyone out there is keen to help.