Adress PR comments

Author: pakrym

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/AzureWebAppDiagnosticsFactoryExtensions.cs

@@ -17,7 +17,7 @@ public static class AzureWebAppDiagnosticsFactoryExtensions
         /// <param name="factory">The extension method argument</param>
         public static ILoggerFactory AddAzureWebAppDiagnostics(this ILoggerFactory factory)
         {
-            return AddAzureWebAppDiagnostics(factory, null);
+            return AddAzureWebAppDiagnostics(factory, new AzureWebAppDiagnosticsSettings());
         }
 
         /// <summary>

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/AzureWebAppDiagnosticsLoggerProvider.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using Microsoft.Extensions.Logging.Abstractions.Internal;
 using Microsoft.Extensions.Logging.AzureWebAppDiagnostics.Internal;
 using Serilog;
@@ -21,6 +22,11 @@ public class AzureWebAppDiagnosticsLoggerProvider : ILoggerProvider
         /// </summary>
         public AzureWebAppDiagnosticsLoggerProvider(WebAppContext context, AzureWebAppDiagnosticsSettings settings)
         {
+            if (settings == null)
+            {
+                throw new ArgumentNullException(nameof(settings));
+            }
+
             _configurationReader  = new WebAppLogConfigurationReader(context);
 
             var config = _configurationReader.Current;
@@ -32,6 +38,7 @@ public AzureWebAppDiagnosticsLoggerProvider(WebAppContext context, AzureWebAppDi
                 var fileLoggerProvider = new FileLoggerProvider(
                     settings.FileSizeLimit,
                     settings.RetainedFileCountLimit,
+                    settings.BackgroundQueueSize,
                     settings.OutputTemplate);
 
                 _loggerFactory.AddSerilog(fileLoggerProvider.ConfigureLogger(_configurationReader));
@@ -40,8 +47,10 @@ public AzureWebAppDiagnosticsLoggerProvider(WebAppContext context, AzureWebAppDi
                     var blobLoggerProvider = new AzureBlobLoggerProvider(
                         settings.OutputTemplate,
                         context.SiteName,
+                        context.SiteInstanceId,
                         settings.BlobName,
                         settings.BlobBatchSize,
+                        settings.BackgroundQueueSize,
                         settings.BlobCommitPeriod);
                     _loggerFactory.AddSerilog(blobLoggerProvider.ConfigureLogger(_configurationReader));
                 }
@@ -57,7 +66,7 @@ public ILogger CreateLogger(string categoryName)
         /// <inheritdoc />
         public void Dispose()
         {
-            _loggerFactory.Dispose();
+            _loggerFactory?.Dispose();
             _configurationReader.Dispose();
         }
     }

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/AzureWebAppDiagnosticsSettings.cs

@@ -11,33 +11,36 @@ namespace Microsoft.Extensions.Logging.AzureWebAppDiagnostics
     public class AzureWebAppDiagnosticsSettings
     {
         /// <summary>
-        /// A strictly positive value representing the maximum log size in bytes. Once the log is full, no more message will be appended
+        /// Gets or sets a strictly positive value representing the maximum log size in bytes. Once the log is full, no more message will be appended.
         /// </summary>
         public int FileSizeLimit { get; set; } = 10 * 1024 * 1024;
 
         /// <summary>
-        /// A strictly positive value representing the maximum retained file count
+        /// Gets or sets a strictly positive value representing the maximum retained file count.
         /// </summary>
         public int RetainedFileCountLimit { get; set; } = 2;
 
         /// <summary>
-        /// A message template describing the output messages
+        /// Gets or sets a message template describing the output messages.
         /// </summary>
         public string OutputTemplate { get; set; } = "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level}] {Message}{NewLine}{Exception}";
 
         /// <summary>
-        /// The maximum number of events to include in a single blob append batch.
+        /// Gets or sets a maximum number of events to include in a single blob append batch.
         /// </summary>
         public int BlobBatchSize { get; set; } = 32;
 
         /// <summary>
-        /// The time to wait between checking for blob log batches
+        /// Gets or sets a time to wait between checking for blob log batches.
         /// </summary>
         public TimeSpan BlobCommitPeriod { get; set; } = TimeSpan.FromSeconds(5);
 
         /// <summary>
-        /// The last section of log blob name.
+        /// Gets or sets the last section of log blob name.
         /// </summary>
         public string BlobName { get; set; } = "applicationLog.txt";
+
+        /// Gets of sets the maximum size of the background log message queue.
+        public int BackgroundQueueSize { get; set; }
     }
 }

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/AzureBlobLoggerProvider.cs

@@ -10,47 +10,79 @@
 namespace Microsoft.Extensions.Logging.AzureWebAppDiagnostics.Internal
 {
     /// <summary>
-    /// The <see cref="SerilogLoggerProvider"/> implemenation that creates instances of <see cref="Serilog.Core.Logger"/> connected to <see cref="AzureBlobSink"/>.
+    /// The implemenation of logger provider that creates instances of <see cref="Serilog.Core.Logger"/>.
     /// </summary>
-    public class AzureBlobLoggerProvider : SerilogLoggerProvider
+    public class AzureBlobLoggerProvider
     {
         private readonly string _outputTemplate;
         private readonly string _appName;
+        private readonly string _instanceId;
         private readonly string _fileName;
         private readonly int _batchSize;
+        private readonly int _backgroundQueueSize;
         private readonly TimeSpan _period;
 
         /// <summary>
         /// Creates a new instance of the <see cref="AzureBlobLoggerProvider"/> class.
         /// </summary>
-        /// <param name="outputTemplate"></param>
-        /// <param name="appName"></param>
-        /// <param name="fileName"></param>
-        /// <param name="batchSize"></param>
-        /// <param name="period"></param>
-        public AzureBlobLoggerProvider(string outputTemplate, string appName, string fileName, int batchSize, TimeSpan period)
+        /// <param name="outputTemplate">A message template describing the output messages</param>
+        /// <param name="appName">The application name to use in blob name</param>
+        /// <param name="instanceId">The application instance id to use in blob name</param>
+        /// <param name="fileName">The last section in log blob name</param>
+        /// <param name="batchSize">A maximum number of events to include in a single blob append batch</param>
+        /// <param name="backgroundQueueSize">The maximum size of the background queue</param>
+        /// <param name="period">A time to wait between checking for blob log batches</param>
+        public AzureBlobLoggerProvider(string outputTemplate, string appName, string instanceId, string fileName, int batchSize, int backgroundQueueSize, TimeSpan period)
         {
+            if (outputTemplate == null)
+            {
+                throw new ArgumentNullException(nameof(outputTemplate));
+            }
+            if (appName == null)
+            {
+                throw new ArgumentNullException(nameof(appName));
+            }
+            if (instanceId == null)
+            {
+                throw new ArgumentNullException(nameof(instanceId));
+            }
+            if (fileName == null)
+            {
+                throw new ArgumentNullException(nameof(fileName));
+            }
+            if (batchSize <= 0)
+            {
+                throw new ArgumentOutOfRangeException(nameof(batchSize), $"{nameof(batchSize)} should be a positive number.");
+            }
+            if (period <= TimeSpan.Zero)
+            {
+                throw new ArgumentOutOfRangeException(nameof(period), $"{nameof(period)} should be longer than zero.");
+            }
+
             _outputTemplate = outputTemplate;
             _appName = appName;
+            _instanceId = instanceId;
             _fileName = fileName;
             _batchSize = batchSize;
+            _backgroundQueueSize = backgroundQueueSize;
             _period = period;
         }
 
         /// <inheritdoc />
-        public override Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
+        public Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
         {
             var messageFormatter = new MessageTemplateTextFormatter(_outputTemplate, null);
             var container = new CloudBlobContainer(new Uri(reader.Current.BlobContainerUrl));
-            var azureBlobSink = new AzureBlobSink(container, _appName, _fileName, messageFormatter, _batchSize, _period);
-            var backgroundSink = new BackgroundSink(azureBlobSink, BackgroundSink.DefaultLogMessagesQueueSize);
-            LoggerConfiguration loggerConfiguration = new LoggerConfiguration();
+            var fileName = _instanceId + "-" + _fileName;
+            var azureBlobSink = new AzureBlobSink(container, _appName, fileName, messageFormatter, _batchSize, _period);
+            var backgroundSink = new BackgroundSink(azureBlobSink, _backgroundQueueSize);
+            var loggerConfiguration = new LoggerConfiguration();
 
             loggerConfiguration.WriteTo.Sink(backgroundSink);
             loggerConfiguration.MinimumLevel.ControlledBy(new WebConfigurationReaderLevelSwitch(reader,
                 configuration =>
                 {
-                    return configuration.BlobLoggingEnabled ? configuration.BlobLoggingLevel: LogLevel.None;
+                    return configuration.BlobLoggingEnabled ? configuration.BlobLoggingLevel : LogLevel.None;
                 }));
 
             return loggerConfiguration.CreateLogger();

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/AzureBlobSink.cs

@@ -43,13 +43,30 @@ public AzureBlobSink(CloudBlobContainer container,
             int batchSizeLimit,
             TimeSpan period) : base(batchSizeLimit, period)
         {
+            if (appName == null)
+            {
+                throw new ArgumentNullException(nameof(appName));
+            }
+            if (fileName == null)
+            {
+                throw new ArgumentNullException(nameof(fileName));
+            }
+            if (formatter == null)
+            {
+                throw new ArgumentNullException(nameof(formatter));
+            }
+            if (batchSizeLimit <= 0)
+            {
+                throw new ArgumentOutOfRangeException(nameof(batchSizeLimit), $"{nameof(batchSizeLimit)} should be a positive number.");
+            }
+            if (period <= TimeSpan.Zero)
+            {
+                throw new ArgumentOutOfRangeException(nameof(period), $"{nameof(period)} should be longer than zero.");
+            }
+
             _appName = appName;
             _fileName = fileName;
             _formatter = formatter;
-            if (batchSizeLimit < 1)
-            {
-                throw new ArgumentException(nameof(batchSizeLimit));
-            }
             _container = container;
         }
 

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/FileLoggerProvider.cs

@@ -11,12 +11,13 @@
 namespace Microsoft.Extensions.Logging.AzureWebAppDiagnostics.Internal
 {
     /// <summary>
-    /// The <see cref="SerilogLoggerProvider"/> implemenation that creates instances of <see cref="Serilog.Core.Logger"/> connected to <see cref="RollingFileSink"/>.
+    /// The logger provider that creates instances of <see cref="Serilog.Core.Logger"/>.
     /// </summary>
-    public class FileLoggerProvider: SerilogLoggerProvider
+    public class FileLoggerProvider
     {
         private readonly int _fileSizeLimit;
         private readonly int _retainedFileCountLimit;
+        private readonly int _backgroundQueueSize;
         private readonly string _outputTemplate;
 
         private const string FileNamePattern = "diagnostics-{Date}.txt";
@@ -25,17 +26,32 @@ public class FileLoggerProvider: SerilogLoggerProvider
         /// Creates a new instance of the <see cref="FileLoggerProvider"/> class.
         /// </summary>
         /// <param name="fileSizeLimit">A strictly positive value representing the maximum log size in megabytes. Once the log is full, no more message will be appended</param>
-        /// <param name="retainedFileCountLimit"></param>
-        /// <param name="outputTemplate"></param>
-        public FileLoggerProvider(int fileSizeLimit, int retainedFileCountLimit, string outputTemplate)
+        /// <param name="retainedFileCountLimit">A strictly positive value representing the maximum retained file count</param>
+        /// <param name="backgroundQueueSize">The maximum size of the background queue</param>
+        /// <param name="outputTemplate">A message template describing the output messages</param>
+        public FileLoggerProvider(int fileSizeLimit, int retainedFileCountLimit, int backgroundQueueSize, string outputTemplate)
         {
+            if (outputTemplate == null)
+            {
+                throw new ArgumentNullException(nameof(outputTemplate));
+            }
+            if (fileSizeLimit <= 0)
+            {
+                throw new ArgumentOutOfRangeException(nameof(fileSizeLimit), $"{nameof(fileSizeLimit)} should be positive.");
+            }
+            if (retainedFileCountLimit <= 0)
+            {
+                throw new ArgumentOutOfRangeException(nameof(retainedFileCountLimit), $"{nameof(retainedFileCountLimit)} should be positive.");
+            }
+
             _fileSizeLimit = fileSizeLimit;
             _retainedFileCountLimit = retainedFileCountLimit;
+            _backgroundQueueSize = backgroundQueueSize;
             _outputTemplate = outputTemplate;
         }
 
         /// <inheritdoc />
-        public override Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
+        public Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
         {
             var webAppConfiguration = reader.Current;
             if (string.IsNullOrEmpty(webAppConfiguration.FileLoggingFolder))
@@ -53,9 +69,9 @@ public override Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
 
             var messageFormatter = new MessageTemplateTextFormatter(_outputTemplate, null);
             var rollingFileSink = new RollingFileSink(logsFilePattern, messageFormatter, _fileSizeLimit, _retainedFileCountLimit);
-            var backgroundSink = new BackgroundSink(rollingFileSink, BackgroundSink.DefaultLogMessagesQueueSize);
+            var backgroundSink = new BackgroundSink(rollingFileSink, _backgroundQueueSize);
 
-            LoggerConfiguration loggerConfiguration = new LoggerConfiguration();
+            var loggerConfiguration = new LoggerConfiguration();
             loggerConfiguration.WriteTo.Sink(backgroundSink);
             loggerConfiguration.MinimumLevel.ControlledBy(new WebConfigurationReaderLevelSwitch(reader,
                 configuration =>

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/IWebAppContext.cs

@@ -18,6 +18,11 @@ public interface IWebAppContext
         /// </summary>
         string SiteName { get; }
 
+        /// <summary>
+        /// Gets the id of site if running in Azure WebApp
+        /// </summary>
+        string SiteInstanceId { get; }
+
         /// <summary>
         /// Gets a value indicating whether or new we're in an Azure WebApp
         /// </summary>

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/SerilogLoggerProvider.cs

@@ -23,6 +23,9 @@ private WebAppContext() { }
         /// <inheritdoc />
         public string SiteName { get; } = Environment.GetEnvironmentVariable("WEBSITE_SITE_NAME");
 
+        /// <inheritdoc />
+        public string SiteInstanceId { get; } = Environment.GetEnvironmentVariable("WEBSITE_INSTANCE_ID");
+
         /// <inheritdoc />
         public bool IsRunningInAzureWebApp => !string.IsNullOrEmpty(HomeFolder) &&
                                               !string.IsNullOrEmpty(SiteName);

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/WebAppContext.cs

@@ -1,5 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
 using System;
 using Serilog.Core;
 using Serilog.Events;

src/Microsoft.Extensions.Logging.AzureWebAppDiagnostics/Internal/WebConfigurationReaderLevelSwitch.cs

@@ -135,7 +135,7 @@ public void LevelMapping()
             testSink.Setup(m => m.Emit(It.IsAny<LogEvent>()));
 
             var provider = new TestWebAppSerilogLoggerProvider(testSink.Object);
-            var levelsToCheck = new LogLevel[]
+            var levelsToCheck = new []
             {
                 LogLevel.None,
                 LogLevel.Critical,
@@ -151,7 +151,7 @@ public void LevelMapping()
             loggerFactory.AddSerilog(seriloglogger);
             var logger = loggerFactory.CreateLogger("TestLogger");
 
-            for (int i = 0; i < levelsToCheck.Length; i++)
+            for (var i = 0; i < levelsToCheck.Length; i++)
             {
                 var enabledLevel = levelsToCheck[i];
 
@@ -161,7 +161,7 @@ public void LevelMapping()
                 configReader.Raise(m => m.OnConfigurationChanged += (sender, e) => { }, null, currentConfig);
 
                 // Don't try to log "None" (start at 1)
-                for (int j = 1; j < levelsToCheck.Length; j++)
+                for (var j = 1; j < levelsToCheck.Length; j++)
                 {
                     logger.Log(levelsToCheck[j], 1, new object(), null, (state, ex) => string.Empty);
                 }
@@ -177,7 +177,7 @@ public void LevelMapping()
         }
 
 
-        private class TestWebAppSerilogLoggerProvider : SerilogLoggerProvider
+        private class TestWebAppSerilogLoggerProvider
         {
             private readonly ILogEventSink _sink;
 
@@ -186,7 +186,7 @@ public TestWebAppSerilogLoggerProvider(ILogEventSink sink)
                 _sink = sink;
             }
 
-            public override Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
+            public Logger ConfigureLogger(IWebAppLogConfigurationReader reader)
             {
                 var loggerConfiguration = new LoggerConfiguration();
                 loggerConfiguration.WriteTo.Sink(_sink);