From 3a45e156a8524a9a216de413e30ed39bf8a15486 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 2 Jun 2022 15:52:56 -0700 Subject: [PATCH 01/20] Enable configuring console logger's queue length and mode - Adds new APIs for the logging console queue processor - Replaces BlockingCollection with Queue implementation - Adds code coverage for ConsoleLoggerProcessor - Add queue name and metrics - Throw exception when new queue length is invalid --- .../Microsoft.Extensions.Logging.Console.cs | 7 + .../src/ConsoleLoggerOptions.cs | 24 +++ .../src/ConsoleLoggerProcessor.cs | 148 +++++++++++--- .../src/ConsoleLoggerProvider.cs | 17 +- .../src/ConsoleLoggerQueueFullMode.cs | 20 ++ .../ConsoleFormatterTests.cs | 4 +- .../ConsoleLoggerExtensionsTests.cs | 45 ++++- .../ConsoleLoggerProcessorTests.cs | 183 ++++++++++++++++++ .../ConsoleLoggerTest.cs | 44 ++--- 9 files changed, 437 insertions(+), 55 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs b/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs index aa9077333e6c54..62d2001f7ef774 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs @@ -62,6 +62,8 @@ public ConsoleLoggerOptions() { } [System.ObsoleteAttribute("ConsoleLoggerOptions.IncludeScopes has been deprecated. Use ConsoleFormatterOptions.IncludeScopes instead.")] public bool IncludeScopes { get { throw null; } set { } } public Microsoft.Extensions.Logging.LogLevel LogToStandardErrorThreshold { get { throw null; } set { } } + public int MaxQueueLength { get { throw null; } set { } } + public Microsoft.Extensions.Logging.Console.ConsoleLoggerBufferFullMode BufferFullMode { get { throw null; } set { } } [System.ObsoleteAttribute("ConsoleLoggerOptions.TimestampFormat has been deprecated. Use ConsoleFormatterOptions.TimestampFormat instead.")] public string? TimestampFormat { get { throw null; } set { } } [System.ObsoleteAttribute("ConsoleLoggerOptions.UseUtcTimestamp has been deprecated. Use ConsoleFormatterOptions.UseUtcTimestamp instead.")] @@ -77,6 +79,11 @@ public ConsoleLoggerProvider(Microsoft.Extensions.Options.IOptionsMonitor [System.ObsoleteAttribute("ConsoleLoggerOptions.UseUtcTimestamp has been deprecated. Use ConsoleFormatterOptions.UseUtcTimestamp instead.")] public bool UseUtcTimestamp { get; set; } + + /// + /// Gets or sets the desired console logger behavior when buffer becomes full. Defaults to Wait. + /// + public ConsoleLoggerBufferFullMode BufferFullMode { get; set; } + + private int _maxQueuedMessages = 1048576; + + /// + /// Gets or sets the maximum number of enqueued messages. Defaults to 1048576. + /// + public int MaxQueueLength + { + get => _maxQueuedMessages; + set + { + if (value <= 0) + { + throw new ArgumentException(nameof(MaxQueueLength)); + } + + _maxQueuedMessages = value; + } + } } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 8797a5058d4801..0f18248d8faaf5 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -2,8 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Concurrent; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Diagnostics.Metrics; using System.Runtime.Versioning; using System.Threading; @@ -12,16 +13,41 @@ namespace Microsoft.Extensions.Logging.Console [UnsupportedOSPlatform("browser")] internal class ConsoleLoggerProcessor : IDisposable { - private const int _maxQueuedMessages = 1024; + private static Meter _meter = new Meter("Microsoft-Extension-Logging-Console-Queue", "1.0.0"); + private readonly Queue _messageQueue; + private int _messagesDropped; + private bool _isAddingCompleted; + private int _maxQueuedMessages = 1048576; + public int MaxQueueLength + { + get => _maxQueuedMessages; + set + { + if (value <= 0) + { + throw new ArgumentException(nameof(MaxQueueLength)); + } - private readonly BlockingCollection _messageQueue = new BlockingCollection(_maxQueuedMessages); + lock (_messageQueue) + { + _maxQueuedMessages = value; + Monitor.PulseAll(_messageQueue); + } + } + } + public ConsoleLoggerBufferFullMode FullMode { get; set; } + private readonly string _queueName; private readonly Thread _outputThread; public IConsole Console { get; } public IConsole ErrorConsole { get; } - public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole) + public ConsoleLoggerProcessor(string queueName, IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) { + _queueName = queueName; + _messageQueue = new Queue(); + FullMode = fullMode; + MaxQueueLength = maxQueueLength; Console = console; ErrorConsole = errorConsole; // Start Console message queue processor @@ -31,57 +57,110 @@ public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole) Name = "Console logger queue processing thread" }; _outputThread.Start(); + _meter.CreateObservableGauge("queue-size", GetQueueSize); } public virtual void EnqueueMessage(LogMessageEntry message) { - if (!_messageQueue.IsAddingCompleted) - { - try - { - _messageQueue.Add(message); - return; - } - catch (InvalidOperationException) { } - } - - // Adding is completed so just log the message - try + // cannot enqueue when adding is completed + if (!Enqueue(message)) { WriteMessage(message); } - catch (Exception) { } } // for testing internal void WriteMessage(LogMessageEntry entry) { - IConsole console = entry.LogAsError ? ErrorConsole : Console; - console.Write(entry.Message); + try + { + var messagesDropped = Interlocked.Exchange(ref _messagesDropped, 0); + if (messagesDropped != 0) + { + System.Console.Error.WriteLine($"{messagesDropped} message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this.{Environment.NewLine}"); + } + + IConsole console = entry.LogAsError ? ErrorConsole : Console; + console.Write(entry.Message); + } + catch (Exception) + { + CompleteAdding(); + } } private void ProcessLogQueue() { - try + while (!_isAddingCompleted || _messageQueue.Count > 0) { - foreach (LogMessageEntry message in _messageQueue.GetConsumingEnumerable()) + if (TryDequeue(out LogMessageEntry message)) { WriteMessage(message); } } - catch + } + + public bool Enqueue(LogMessageEntry item) + { + lock (_messageQueue) { - try + while (_messageQueue.Count >= MaxQueueLength && !_isAddingCompleted) { - _messageQueue.CompleteAdding(); + if (FullMode == ConsoleLoggerBufferFullMode.DropWrite) + { + Interlocked.Increment(ref _messagesDropped); + return true; + } + + Monitor.Wait(_messageQueue); + } + + if (!_isAddingCompleted) + { + _messageQueue.Enqueue(item); + + if (_messageQueue.Count == 1) + { + // pulse for wait in Dequeue + Monitor.PulseAll(_messageQueue); + } + + return true; } - catch { } + } + + return false; + } + + public bool TryDequeue(out LogMessageEntry item) + { + lock (_messageQueue) + { + while (_messageQueue.Count == 0 && !_isAddingCompleted) + { + Monitor.Wait(_messageQueue); + } + + if (!_isAddingCompleted) + { + item = _messageQueue.Dequeue(); + if (_messageQueue.Count == MaxQueueLength - 1) + { + // pulse for wait in Enqueue + Monitor.PulseAll(_messageQueue); + } + + return true; + } + + item = default; + return false; } } public void Dispose() { - _messageQueue.CompleteAdding(); + CompleteAdding(); try { @@ -89,5 +168,22 @@ public void Dispose() } catch (ThreadStateException) { } } + + private void CompleteAdding() + { + lock (_messageQueue) + { + Monitor.PulseAll(_messageQueue); + _isAddingCompleted = true; + } + } + + private IEnumerable> GetQueueSize() + { + return new Measurement[] + { + new Measurement(_messageQueue.Count, new KeyValuePair("queue-name", _queueName)), + }; + } } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs index abed24fa2e11ea..150aabfe1a92ce 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs @@ -43,10 +43,6 @@ public ConsoleLoggerProvider(IOptionsMonitor options, IEnu _options = options; _loggers = new ConcurrentDictionary(); SetFormatters(formatters); - - ReloadLoggerOptions(options.CurrentValue); - _optionsReloadToken = _options.OnChange(ReloadLoggerOptions); - IConsole? console; IConsole? errorConsole; if (DoesConsoleSupportAnsi()) @@ -59,7 +55,15 @@ public ConsoleLoggerProvider(IOptionsMonitor options, IEnu console = new AnsiParsingLogConsole(); errorConsole = new AnsiParsingLogConsole(stdErr: true); } - _messageQueue = new ConsoleLoggerProcessor(console, errorConsole); + _messageQueue = new ConsoleLoggerProcessor( + "LoggingConsoleQueue", + console, + errorConsole, + options.CurrentValue.BufferFullMode, + options.CurrentValue.MaxQueueLength); + + ReloadLoggerOptions(options.CurrentValue); + _optionsReloadToken = _options.OnChange(ReloadLoggerOptions); } [UnsupportedOSPlatformGuard("windows")] @@ -123,6 +127,9 @@ private void ReloadLoggerOptions(ConsoleLoggerOptions options) #pragma warning restore CS0618 } + _messageQueue.FullMode = options.BufferFullMode; + _messageQueue.MaxQueueLength = options.MaxQueueLength; + foreach (KeyValuePair logger in _loggers) { logger.Value.Options = options; diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs new file mode 100644 index 00000000000000..e8bcd530df286b --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Extensions.Logging.Console +{ + /// + /// Determines the console logger behavior when buffer becomes full. + /// + public enum ConsoleLoggerBufferFullMode + { + /// + /// Blocks the console thread once the buffer limit is reached + /// + Wait, + /// + /// Drops new log messages when the buffer is full + /// + DropWrite + } +} diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs index 8c4b68ba6193b8..154c1a31b34e13 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs @@ -38,7 +38,9 @@ internal static (ConsoleLogger Logger, ConsoleSink Sink, ConsoleSink ErrorSink, var errorSink = new ConsoleSink(); var console = new TestConsole(sink); var errorConsole = new TestConsole(errorSink); - var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole); + var bufferMode = options == null ? ConsoleLoggerBufferFullMode.Wait : options.BufferFullMode; + var maxQueueLength = options == null ? ConsoleLoggerProcessorTests.DefaultMaxQueueLengthValue : options.MaxQueueLength; + var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, bufferMode, maxQueueLength); var formatters = new ConcurrentDictionary(ConsoleLoggerTest.GetFormatters(simpleOptions, systemdOptions, jsonOptions).ToDictionary(f => f.Name)); diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs index 739978be7d069e..533cc6090bb66d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Reflection; using System.Text.Encodings.Web; using System.Text.Json; using Microsoft.Extensions.Logging.Abstractions; @@ -15,7 +16,7 @@ using Microsoft.Extensions.Options; using Xunit; -namespace Microsoft.Extensions.Logging.Test +namespace Microsoft.Extensions.Logging.Console.Test { public class ConsoleLoggerExtensionsTests { @@ -356,6 +357,48 @@ public void AddConsole_NullFormatterNameUsingSystemdFormat_AnyDeprecatedProperti Assert.Equal("HH:mm:ss ", formatter.FormatterOptions.TimestampFormat); } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + [InlineData(-1)] + [InlineData(0)] + public void AddConsole_MaxQueueLengthSetToNegativeOrZero_IgnoresValue(int invalidMaxQueueLength) + { + var configs = new[] { + new KeyValuePair("Console:MaxQueueLength", invalidMaxQueueLength.ToString()), + }; + var configuration = new ConfigurationBuilder().AddInMemoryCollection(configs).Build(); + + IServiceProvider serviceProvider = new ServiceCollection() + .AddLogging(builder => builder + .AddConfiguration(configuration) + .AddConsole(o => { }) + ) + .BuildServiceProvider(); + + // the configuration binder throws TargetInvocationException when setting options property MaxQueueLength throws exception + Assert.Throws(() => serviceProvider.GetRequiredService()); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + public void AddConsole_MaxQueueLengthLargerThanZero_ConfiguredProperly() + { + var configs = new[] { + new KeyValuePair("Console:MaxQueueLength", "12345"), + }; + var configuration = new ConfigurationBuilder().AddInMemoryCollection(configs).Build(); + + var loggerProvider = new ServiceCollection() + .AddLogging(builder => builder + .AddConfiguration(configuration) + .AddConsole(o => { }) + ) + .BuildServiceProvider() + .GetRequiredService(); + + var consoleLoggerProvider = Assert.IsType(loggerProvider); + var logger = (ConsoleLogger)consoleLoggerProvider.CreateLogger("Category"); + Assert.Equal(12345, logger.Options.MaxQueueLength); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void AddConsole_NullFormatterName_UsingSystemdFormat_IgnoreFormatterOptionsAndUseDeprecatedInstead() { diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs new file mode 100644 index 00000000000000..904d14aee8f43e --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -0,0 +1,183 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.IO; +using System.Linq; +using Microsoft.DotNet.RemoteExecutor; +using Microsoft.Extensions.Logging.Test.Console; +using Xunit; +#pragma warning disable CS0618 + +namespace Microsoft.Extensions.Logging.Console.Test +{ + public class ConsoleLoggerProcessorTests + { + internal const int DefaultMaxQueueLengthValue = 1048576; + private const string _loggerName = "test"; + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + public void LogAfterDisposeWritesLog() + { + // Arrange + var sink = new ConsoleSink(); + var console = new TestConsole(sink); + var processor = new ConsoleLoggerProcessor(nameof(LogAfterDisposeWritesLog), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + + var logger = new ConsoleLogger(_loggerName, loggerProcessor: processor, + new SimpleConsoleFormatter(new TestFormatterOptionsMonitor(new SimpleConsoleFormatterOptions())), + null, new ConsoleLoggerOptions()); + Assert.Null(logger.Options.FormatterName); + UpdateFormatterOptions(logger.Formatter, logger.Options); + + // Act + processor.Dispose(); + logger.LogInformation("Logging after dispose"); + + // Assert + Assert.Equal(2, sink.Writes.Count); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + public static void SetNegativeMaxQueueLength_IgnoresWhenNotPositive() + { + // Arrange + var sink = new ConsoleSink(); + var console = new TestConsole(sink); + var processor = new ConsoleLoggerProcessor(nameof(SetNegativeMaxQueueLength_IgnoresWhenNotPositive), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( + new SimpleConsoleFormatterOptions())); + + var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); + Assert.Null(logger.Options.FormatterName); + UpdateFormatterOptions(logger.Formatter, logger.Options); + + // Act & Assert + Assert.Throws(() => processor.MaxQueueLength = -1); + } + + [ConditionalTheory(nameof(IsThreadingAndRemoteExecutorSupported))] + [InlineData(true)] + [InlineData(false)] + public void CheckForNotificationWhenQueueIsFull(bool dropWrite) + { + RemoteExecutor.Invoke((okToDropMessages) => + { + using (var stringWriter = new StringWriter()) + { + System.Console.SetError(stringWriter); + bool okToDrop = bool.Parse(okToDropMessages); + using (var listener = new ConsoleTraceListener(useErrorStream: true)) + { + // Arrange + var sink = new ConsoleSink(); + var console = new TestConsole(sink); + string queueName = nameof(CheckForNotificationWhenQueueIsFull) + (okToDrop ? "InDropWriteMode" : "InWaitMode"); + var processor = new ConsoleLoggerProcessor(queueName, console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( + new SimpleConsoleFormatterOptions())); + + var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); + Assert.Null(logger.Options.FormatterName); + UpdateFormatterOptions(logger.Formatter, logger.Options); + string messageTemplate = string.Join(", ", Enumerable.Range(1, 100).Select(x => "{A" + x + "}")); + object[] messageParams = Enumerable.Range(1, 100).Select(x => (object)x).ToArray(); + + // Act + processor.MaxQueueLength = 1; + processor.FullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; + for (int i = 0; i < 2000; i++) + { + logger.LogInformation(messageTemplate, messageParams); + } + + // Assert + if (okToDrop) + { + Assert.Contains("dropped because of queue size limit", stringWriter.ToString()); + } + else + { + Assert.DoesNotContain("dropped because of queue size limit", stringWriter.ToString()); + } + } + } + }, dropWrite.ToString()).Dispose(); + } + + private class TimesWriteCalledConsole : IConsole + { + public volatile int TimesWriteCalled; + public void Write(string message) + { + TimesWriteCalled++; + } + } + + private class WriteThrowingConsole : IConsole + { + public void Write(string message) + { + throw new InvalidOperationException(); + } + } + + [OuterLoop] + [ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))] + public void ThrowDuringProcessLog_ShutsDownGracefully() + { + var console = new TimesWriteCalledConsole(); + var writeThrowingConsole = new WriteThrowingConsole(); + var processor = new ConsoleLoggerProcessor( + nameof(ThrowDuringProcessLog_ShutsDownGracefully), + console, + writeThrowingConsole, + ConsoleLoggerBufferFullMode.Wait, + 1024); + + var formatter = new SimpleConsoleFormatter( + new TestFormatterOptionsMonitor( + new SimpleConsoleFormatterOptions())); + + var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions() + { + LogToStandardErrorThreshold = LogLevel.Error + }); + + Assert.Null(logger.Options.FormatterName); + UpdateFormatterOptions(logger.Formatter, logger.Options); + logger.LogInformation("Process 1st log normally using {DesiredConsole}", nameof(TimesWriteCalledConsole)); + logger.LogInformation("Process 2nd log normally using {DesiredConsole}", nameof(TimesWriteCalledConsole)); + while (console.TimesWriteCalled != 2); // wait until the logs are processed + Assert.Equal(2, console.TimesWriteCalled); + logger.LogError("Causing exception to throw in {ClassName} using {DesiredConsole}", nameof(ConsoleLoggerProcessor), nameof(WriteThrowingConsole)); + logger.LogInformation("After the write logic threw exception, {ClassName} stopped gracefully, no longer processing next logs", nameof(ConsoleLoggerProcessor)); + Assert.Equal(2, console.TimesWriteCalled); + } + + private static void UpdateFormatterOptions(ConsoleFormatter formatter, ConsoleLoggerOptions deprecatedFromOptions) + { + // kept for deprecated apis: + if (formatter is SimpleConsoleFormatter defaultFormatter) + { + defaultFormatter.FormatterOptions.ColorBehavior = deprecatedFromOptions.DisableColors ? + LoggerColorBehavior.Disabled : LoggerColorBehavior.Enabled; + defaultFormatter.FormatterOptions.IncludeScopes = deprecatedFromOptions.IncludeScopes; + defaultFormatter.FormatterOptions.TimestampFormat = deprecatedFromOptions.TimestampFormat; + defaultFormatter.FormatterOptions.UseUtcTimestamp = deprecatedFromOptions.UseUtcTimestamp; + } + else + if (formatter is SystemdConsoleFormatter systemdFormatter) + { + systemdFormatter.FormatterOptions.IncludeScopes = deprecatedFromOptions.IncludeScopes; + systemdFormatter.FormatterOptions.TimestampFormat = deprecatedFromOptions.TimestampFormat; + systemdFormatter.FormatterOptions.UseUtcTimestamp = deprecatedFromOptions.UseUtcTimestamp; + } + } + + public static bool IsThreadingAndRemoteExecutorSupported => + PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported; + } +} +#pragma warning restore CS0618 diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index b611d6cc952509..a08f06801ef0d9 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -47,12 +47,12 @@ private static (ConsoleLogger Logger, ConsoleSink Sink, ConsoleSink ErrorSink, F var errorSink = new ConsoleSink(); var console = new TestConsole(sink); var errorConsole = new TestConsole(errorSink); - var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole); var formatters = new ConcurrentDictionary(GetFormatters().ToDictionary(f => f.Name)); ConsoleFormatter? formatter = null; var loggerOptions = options ?? new ConsoleLoggerOptions(); + var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, loggerOptions.BufferFullMode, loggerOptions.MaxQueueLength); Func levelAsString; int writesPerMsg; switch (loggerOptions.Format) @@ -1139,26 +1139,6 @@ public void WriteCore_NullMessageWithNullException(LogLevel level) Assert.Empty(sink.Writes); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] - public void LogAfterDisposeWritesLog() - { - // Arrange - var sink = new ConsoleSink(); - var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(console, null!); - var formatters = new ConcurrentDictionary(GetFormatters().ToDictionary(f => f.Name)); - - var logger = new ConsoleLogger(_loggerName, loggerProcessor: processor, formatters[ConsoleFormatterNames.Simple], null, new ConsoleLoggerOptions()); - Assert.Null(logger.Options.FormatterName); - UpdateFormatterOptions(logger.Formatter, logger.Options); - // Act - processor.Dispose(); - logger.LogInformation("Logging after dispose"); - - // Assert - Assert.True(sink.Writes.Count == 2); - } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public static void IsEnabledReturnsCorrectValue() { @@ -1300,6 +1280,25 @@ public void ConsoleLoggerOptions_LogAsErrorLevel_IsAppliedToLoggers() Assert.Equal(LogLevel.Error, logger.Options.LogToStandardErrorThreshold); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + public void ConsoleLoggerOptions_UpdateQueueOptions_UpdatesConsoleLoggerProcessorProperties() + { + // Arrange + var monitor = new TestOptionsMonitor(new ConsoleLoggerOptions()); + var loggerProvider = new ConsoleLoggerProvider(monitor); + var logger = (ConsoleLogger)loggerProvider.CreateLogger("Name"); + + // Act & Assert + Assert.Equal(ConsoleLoggerBufferFullMode.Wait, logger.Options.BufferFullMode); + Assert.Equal(ConsoleLoggerProcessorTests.DefaultMaxQueueLengthValue, logger.Options.MaxQueueLength); + monitor.Set(new ConsoleLoggerOptions() { + BufferFullMode = ConsoleLoggerBufferFullMode.DropWrite, + MaxQueueLength = 10 + }); + Assert.Equal(ConsoleLoggerBufferFullMode.DropWrite, logger.Options.BufferFullMode); + Assert.Equal(10, logger.Options.MaxQueueLength); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void ConsoleLoggerOptions_UseUtcTimestamp_IsAppliedToLoggers() { @@ -1404,7 +1403,8 @@ private string CreateHeader(ConsoleLoggerFormat format, int eventId = 0) internal class TestLoggerProcessor : ConsoleLoggerProcessor { - public TestLoggerProcessor(IConsole console, IConsole errorConsole) : base(console, errorConsole) + public TestLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) + : base(nameof(TestLoggerProcessor), console, errorConsole, fullMode, maxQueueLength) { } From 0da2fd109b31de87af14488e29b2038793bf7888 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 3 Jun 2022 13:57:13 -0700 Subject: [PATCH 02/20] Apply PR feedback --- .../src/ConsoleLoggerOptions.cs | 21 ++++++++++--- .../src/ConsoleLoggerProcessor.cs | 24 +++++++++++--- .../ConsoleLoggerExtensionsTests.cs | 2 +- .../ConsoleLoggerProcessorTests.cs | 31 ++++++++++++++++--- .../ConsoleLoggerTest.cs | 14 +++++++++ 5 files changed, 77 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs index a600f10e119556..3f87401d2479ae 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs @@ -64,15 +64,28 @@ public ConsoleLoggerFormat Format [System.ObsoleteAttribute("ConsoleLoggerOptions.UseUtcTimestamp has been deprecated. Use ConsoleFormatterOptions.UseUtcTimestamp instead.")] public bool UseUtcTimestamp { get; set; } + private ConsoleLoggerBufferFullMode _bufferFullMode = ConsoleLoggerBufferFullMode.Wait; /// /// Gets or sets the desired console logger behavior when buffer becomes full. Defaults to Wait. /// - public ConsoleLoggerBufferFullMode BufferFullMode { get; set; } + public ConsoleLoggerBufferFullMode BufferFullMode + { + get => _bufferFullMode; + set + { + if (value != ConsoleLoggerBufferFullMode.Wait && value != ConsoleLoggerBufferFullMode.DropWrite) + { + throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); + } + _bufferFullMode = value; + } + } - private int _maxQueuedMessages = 1048576; + internal const int DefaultMaxQueueLengthValue = 1024 * 1024; + private int _maxQueuedMessages = DefaultMaxQueueLengthValue; /// - /// Gets or sets the maximum number of enqueued messages. Defaults to 1048576. + /// Gets or sets the maximum number of enqueued messages. Defaults to 1024 * 1024. /// public int MaxQueueLength { @@ -81,7 +94,7 @@ public int MaxQueueLength { if (value <= 0) { - throw new ArgumentException(nameof(MaxQueueLength)); + throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} must be larger than zero."); } _maxQueuedMessages = value; diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 0f18248d8faaf5..98837a6ff159d0 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Metrics; using System.Runtime.Versioning; @@ -13,11 +14,11 @@ namespace Microsoft.Extensions.Logging.Console [UnsupportedOSPlatform("browser")] internal class ConsoleLoggerProcessor : IDisposable { - private static Meter _meter = new Meter("Microsoft-Extension-Logging-Console-Queue", "1.0.0"); + private static readonly Meter s_meter = new Meter("Microsoft-Extension-Logging-Console-Queue", "1.0.0"); private readonly Queue _messageQueue; private int _messagesDropped; private bool _isAddingCompleted; - private int _maxQueuedMessages = 1048576; + private int _maxQueuedMessages = ConsoleLoggerOptions.DefaultMaxQueueLengthValue; public int MaxQueueLength { get => _maxQueuedMessages; @@ -25,7 +26,7 @@ public int MaxQueueLength { if (value <= 0) { - throw new ArgumentException(nameof(MaxQueueLength)); + throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} must be larger than zero."); } lock (_messageQueue) @@ -35,7 +36,19 @@ public int MaxQueueLength } } } - public ConsoleLoggerBufferFullMode FullMode { get; set; } + private ConsoleLoggerBufferFullMode _fullMode = ConsoleLoggerBufferFullMode.Wait; + public ConsoleLoggerBufferFullMode FullMode + { + get => _fullMode; + set + { + if (value != ConsoleLoggerBufferFullMode.Wait && value != ConsoleLoggerBufferFullMode.DropWrite) + { + throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); + } + _fullMode = value; + } + } private readonly string _queueName; private readonly Thread _outputThread; @@ -57,7 +70,7 @@ public ConsoleLoggerProcessor(string queueName, IConsole console, IConsole error Name = "Console logger queue processing thread" }; _outputThread.Start(); - _meter.CreateObservableGauge("queue-size", GetQueueSize); + s_meter.CreateObservableGauge("queue-size", GetQueueSize); } public virtual void EnqueueMessage(LogMessageEntry message) @@ -112,6 +125,7 @@ public bool Enqueue(LogMessageEntry item) return true; } + Debug.Assert(FullMode == ConsoleLoggerBufferFullMode.Wait); Monitor.Wait(_messageQueue); } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs index 533cc6090bb66d..a89c53f5322a88 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs @@ -360,7 +360,7 @@ public void AddConsole_NullFormatterNameUsingSystemdFormat_AnyDeprecatedProperti [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] [InlineData(-1)] [InlineData(0)] - public void AddConsole_MaxQueueLengthSetToNegativeOrZero_IgnoresValue(int invalidMaxQueueLength) + public void AddConsole_MaxQueueLengthSetToNegativeOrZero_Throws(int invalidMaxQueueLength) { var configs = new[] { new KeyValuePair("Console:MaxQueueLength", invalidMaxQueueLength.ToString()), diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index 904d14aee8f43e..b534312f6080a5 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -14,7 +14,7 @@ namespace Microsoft.Extensions.Logging.Console.Test { public class ConsoleLoggerProcessorTests { - internal const int DefaultMaxQueueLengthValue = 1048576; + internal const int DefaultMaxQueueLengthValue = ConsoleLoggerOptions.DefaultMaxQueueLengthValue; private const string _loggerName = "test"; [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] @@ -39,13 +39,33 @@ public void LogAfterDisposeWritesLog() Assert.Equal(2, sink.Writes.Count); } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + [InlineData(-1)] + [InlineData(0)] + public static void MaxQueueLength_SetInvalid_Throws(int invalidMaxQueueLength) + { + // Arrange + var sink = new ConsoleSink(); + var console = new TestConsole(sink); + var processor = new ConsoleLoggerProcessor(nameof(MaxQueueLength_SetInvalid_Throws), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( + new SimpleConsoleFormatterOptions())); + + var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); + Assert.Null(logger.Options.FormatterName); + UpdateFormatterOptions(logger.Formatter, logger.Options); + + // Act & Assert + Assert.Throws(() => processor.MaxQueueLength = invalidMaxQueueLength); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] - public static void SetNegativeMaxQueueLength_IgnoresWhenNotPositive() + public static void FullMode_SetInvalid_Throws() { // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(nameof(SetNegativeMaxQueueLength_IgnoresWhenNotPositive), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(nameof(FullMode_SetInvalid_Throws), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( new SimpleConsoleFormatterOptions())); @@ -54,9 +74,10 @@ public static void SetNegativeMaxQueueLength_IgnoresWhenNotPositive() UpdateFormatterOptions(logger.Formatter, logger.Options); // Act & Assert - Assert.Throws(() => processor.MaxQueueLength = -1); + Assert.Throws(() => processor.FullMode = (ConsoleLoggerBufferFullMode)10); } + [OuterLoop] [ConditionalTheory(nameof(IsThreadingAndRemoteExecutorSupported))] [InlineData(true)] [InlineData(false)] @@ -87,7 +108,7 @@ public void CheckForNotificationWhenQueueIsFull(bool dropWrite) // Act processor.MaxQueueLength = 1; processor.FullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; - for (int i = 0; i < 2000; i++) + for (int i = 0; i < 20000; i++) { logger.LogInformation(messageTemplate, messageParams); } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index a08f06801ef0d9..a82d54d89a5518 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -1173,6 +1173,20 @@ public void ConsoleLoggerOptions_InvalidFormat_Throws() Assert.Throws(() => new ConsoleLoggerOptions() { Format = (ConsoleLoggerFormat)10 }); } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + [InlineData(-1)] + [InlineData(0)] + public void ConsoleLoggerOptions_SetInvalidMaxQueueLength_Throws(int invalidMaxQueueLength) + { + Assert.Throws(() => new ConsoleLoggerOptions() { MaxQueueLength = invalidMaxQueueLength }); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + public void ConsoleLoggerOptions_SetInvalidBufferMode_Throws() + { + Assert.Throws(() => new ConsoleLoggerOptions() { BufferFullMode = (ConsoleLoggerBufferFullMode)10 }); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void ConsoleLoggerOptions_DisableColors_IsReadFromLoggingConfiguration() { From ace65f078c619c24210a8518c905b1f3198ce6c3 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 3 Jun 2022 14:28:26 -0700 Subject: [PATCH 03/20] More PR feedback --- .../src/ConsoleLoggerProcessor.cs | 4 ++-- .../ConsoleLoggerProcessorTests.cs | 12 ------------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 98837a6ff159d0..82e5b1215de5a9 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -129,7 +129,7 @@ public bool Enqueue(LogMessageEntry item) Monitor.Wait(_messageQueue); } - if (!_isAddingCompleted) + if (_messageQueue.Count < MaxQueueLength) { _messageQueue.Enqueue(item); @@ -155,7 +155,7 @@ public bool TryDequeue(out LogMessageEntry item) Monitor.Wait(_messageQueue); } - if (!_isAddingCompleted) + if (_messageQueue.Count > 0) { item = _messageQueue.Dequeue(); if (_messageQueue.Count == MaxQueueLength - 1) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index b534312f6080a5..0145f9992905c3 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -48,12 +48,6 @@ public static void MaxQueueLength_SetInvalid_Throws(int invalidMaxQueueLength) var sink = new ConsoleSink(); var console = new TestConsole(sink); var processor = new ConsoleLoggerProcessor(nameof(MaxQueueLength_SetInvalid_Throws), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); - var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( - new SimpleConsoleFormatterOptions())); - - var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); - Assert.Null(logger.Options.FormatterName); - UpdateFormatterOptions(logger.Formatter, logger.Options); // Act & Assert Assert.Throws(() => processor.MaxQueueLength = invalidMaxQueueLength); @@ -66,12 +60,6 @@ public static void FullMode_SetInvalid_Throws() var sink = new ConsoleSink(); var console = new TestConsole(sink); var processor = new ConsoleLoggerProcessor(nameof(FullMode_SetInvalid_Throws), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); - var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( - new SimpleConsoleFormatterOptions())); - - var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); - Assert.Null(logger.Options.FormatterName); - UpdateFormatterOptions(logger.Formatter, logger.Options); // Act & Assert Assert.Throws(() => processor.FullMode = (ConsoleLoggerBufferFullMode)10); From 00d3e8d1de4c364c24e8aa4b99e0b2d72b1d6b37 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 3 Jun 2022 14:41:07 -0700 Subject: [PATCH 04/20] Code cleanup --- .../ConsoleFormatterTests.cs | 2 +- .../ConsoleLoggerProcessorTests.cs | 1 - .../ConsoleLoggerTest.cs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs index 154c1a31b34e13..327acfef288f21 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs @@ -39,7 +39,7 @@ internal static (ConsoleLogger Logger, ConsoleSink Sink, ConsoleSink ErrorSink, var console = new TestConsole(sink); var errorConsole = new TestConsole(errorSink); var bufferMode = options == null ? ConsoleLoggerBufferFullMode.Wait : options.BufferFullMode; - var maxQueueLength = options == null ? ConsoleLoggerProcessorTests.DefaultMaxQueueLengthValue : options.MaxQueueLength; + var maxQueueLength = options == null ? ConsoleLoggerOptions.DefaultMaxQueueLengthValue : options.MaxQueueLength; var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, bufferMode, maxQueueLength); var formatters = new ConcurrentDictionary(ConsoleLoggerTest.GetFormatters(simpleOptions, systemdOptions, jsonOptions).ToDictionary(f => f.Name)); diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index 0145f9992905c3..f561dba26b335a 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -14,7 +14,6 @@ namespace Microsoft.Extensions.Logging.Console.Test { public class ConsoleLoggerProcessorTests { - internal const int DefaultMaxQueueLengthValue = ConsoleLoggerOptions.DefaultMaxQueueLengthValue; private const string _loggerName = "test"; [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index a82d54d89a5518..b25c0e9291b402 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -1304,7 +1304,7 @@ public void ConsoleLoggerOptions_UpdateQueueOptions_UpdatesConsoleLoggerProcesso // Act & Assert Assert.Equal(ConsoleLoggerBufferFullMode.Wait, logger.Options.BufferFullMode); - Assert.Equal(ConsoleLoggerProcessorTests.DefaultMaxQueueLengthValue, logger.Options.MaxQueueLength); + Assert.Equal(ConsoleLoggerOptions.DefaultMaxQueueLengthValue, logger.Options.MaxQueueLength); monitor.Set(new ConsoleLoggerOptions() { BufferFullMode = ConsoleLoggerBufferFullMode.DropWrite, MaxQueueLength = 10 From 1ae14406f45a817fd4228eb4f621a76a7a8dc125 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 3 Jun 2022 15:47:11 -0700 Subject: [PATCH 05/20] Correct doc summary --- .../src/ConsoleLoggerQueueFullMode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs index e8bcd530df286b..2ef0adbf894bdb 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs @@ -9,7 +9,7 @@ namespace Microsoft.Extensions.Logging.Console public enum ConsoleLoggerBufferFullMode { /// - /// Blocks the console thread once the buffer limit is reached + /// Blocks the logging threads once the buffer limit is reached. /// Wait, /// From e963406401b8f6b0fc017b087f7a2b4af0c26b73 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 3 Jun 2022 16:25:00 -0700 Subject: [PATCH 06/20] bugfix --- .../src/ConsoleLoggerProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 82e5b1215de5a9..70846b33cfb51a 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -129,7 +129,7 @@ public bool Enqueue(LogMessageEntry item) Monitor.Wait(_messageQueue); } - if (_messageQueue.Count < MaxQueueLength) + if (_messageQueue.Count < MaxQueueLength && !_isAddingCompleted) { _messageQueue.Enqueue(item); From 99601edbcafef409bed7917cd342fb1fad4312e2 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 3 Jun 2022 16:57:13 -0700 Subject: [PATCH 07/20] Write dropped notification to error console --- .../src/ConsoleLoggerProcessor.cs | 2 +- .../ConsoleLoggerProcessorTests.cs | 82 ++++++++----------- ...ft.Extensions.Logging.Console.Tests.csproj | 1 - 3 files changed, 35 insertions(+), 50 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 70846b33cfb51a..d7a6027ba6ce4d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -90,7 +90,7 @@ internal void WriteMessage(LogMessageEntry entry) var messagesDropped = Interlocked.Exchange(ref _messagesDropped, 0); if (messagesDropped != 0) { - System.Console.Error.WriteLine($"{messagesDropped} message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this.{Environment.NewLine}"); + ErrorConsole.Write($"{messagesDropped} message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this.{Environment.NewLine}"); } IConsole console = entry.LogAsError ? ErrorConsole : Console; diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index f561dba26b335a..d7d2ed25fda2ea 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.IO; using System.Linq; -using Microsoft.DotNet.RemoteExecutor; using Microsoft.Extensions.Logging.Test.Console; using Xunit; #pragma warning disable CS0618 @@ -65,53 +64,43 @@ public static void FullMode_SetInvalid_Throws() } [OuterLoop] - [ConditionalTheory(nameof(IsThreadingAndRemoteExecutorSupported))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] [InlineData(true)] [InlineData(false)] - public void CheckForNotificationWhenQueueIsFull(bool dropWrite) + public void CheckForNotificationWhenQueueIsFull(bool okToDrop) { - RemoteExecutor.Invoke((okToDropMessages) => + // Arrange + var sink = new ConsoleSink(); + var console = new TestConsole(sink); + var errorConsole = new TimesWriteCalledConsole(); + string queueName = nameof(CheckForNotificationWhenQueueIsFull) + (okToDrop ? "InDropWriteMode" : "InWaitMode"); + var processor = new ConsoleLoggerProcessor(queueName, console, errorConsole, ConsoleLoggerBufferFullMode.Wait, 1024); + var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( + new SimpleConsoleFormatterOptions())); + + var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); + Assert.Null(logger.Options.FormatterName); + UpdateFormatterOptions(logger.Formatter, logger.Options); + string messageTemplate = string.Join(", ", Enumerable.Range(1, 100).Select(x => "{A" + x + "}")); + object[] messageParams = Enumerable.Range(1, 100).Select(x => (object)x).ToArray(); + + // Act + processor.MaxQueueLength = 1; + processor.FullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; + for (int i = 0; i < 20000; i++) { - using (var stringWriter = new StringWriter()) - { - System.Console.SetError(stringWriter); - bool okToDrop = bool.Parse(okToDropMessages); - using (var listener = new ConsoleTraceListener(useErrorStream: true)) - { - // Arrange - var sink = new ConsoleSink(); - var console = new TestConsole(sink); - string queueName = nameof(CheckForNotificationWhenQueueIsFull) + (okToDrop ? "InDropWriteMode" : "InWaitMode"); - var processor = new ConsoleLoggerProcessor(queueName, console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); - var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( - new SimpleConsoleFormatterOptions())); - - var logger = new ConsoleLogger(_loggerName, processor, formatter, null, new ConsoleLoggerOptions()); - Assert.Null(logger.Options.FormatterName); - UpdateFormatterOptions(logger.Formatter, logger.Options); - string messageTemplate = string.Join(", ", Enumerable.Range(1, 100).Select(x => "{A" + x + "}")); - object[] messageParams = Enumerable.Range(1, 100).Select(x => (object)x).ToArray(); - - // Act - processor.MaxQueueLength = 1; - processor.FullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; - for (int i = 0; i < 20000; i++) - { - logger.LogInformation(messageTemplate, messageParams); - } - - // Assert - if (okToDrop) - { - Assert.Contains("dropped because of queue size limit", stringWriter.ToString()); - } - else - { - Assert.DoesNotContain("dropped because of queue size limit", stringWriter.ToString()); - } - } - } - }, dropWrite.ToString()).Dispose(); + logger.LogInformation(messageTemplate, messageParams); + } + + // Assert + if (okToDrop) + { + Assert.True(errorConsole.TimesWriteCalled > 0); + } + else + { + Assert.Equal(0, errorConsole.TimesWriteCalled); + } } private class TimesWriteCalledConsole : IConsole @@ -132,7 +121,7 @@ public void Write(string message) } [OuterLoop] - [ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void ThrowDuringProcessLog_ShutsDownGracefully() { var console = new TimesWriteCalledConsole(); @@ -183,9 +172,6 @@ private static void UpdateFormatterOptions(ConsoleFormatter formatter, ConsoleLo systemdFormatter.FormatterOptions.UseUtcTimestamp = deprecatedFromOptions.UseUtcTimestamp; } } - - public static bool IsThreadingAndRemoteExecutorSupported => - PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported; } } #pragma warning restore CS0618 diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj index 5f182cf1cc3b17..d762d320452a36 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj @@ -1,7 +1,6 @@ - true $(NetCoreAppCurrent);$(NetFrameworkMinimum) true true From 49a27e004b52b73abb6b6fcfe22ea19c7a788c53 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 8 Jun 2022 11:41:37 -0700 Subject: [PATCH 08/20] Apply PR feedback - Changes default max queue length value - Supports metrics for multiple console providers --- .../src/ConsoleLoggerOptions.cs | 4 +-- .../src/ConsoleLoggerProcessor.cs | 36 ++++++++++++------- .../src/ConsoleLoggerProvider.cs | 1 - .../ConsoleLoggerProcessorTests.cs | 9 +++-- .../ConsoleLoggerTest.cs | 2 +- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs index 3f87401d2479ae..52b90042c7c2ca 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs @@ -81,11 +81,11 @@ public ConsoleLoggerBufferFullMode BufferFullMode } } - internal const int DefaultMaxQueueLengthValue = 1024 * 1024; + internal const int DefaultMaxQueueLengthValue = 2500; private int _maxQueuedMessages = DefaultMaxQueueLengthValue; /// - /// Gets or sets the maximum number of enqueued messages. Defaults to 1024 * 1024. + /// Gets or sets the maximum number of enqueued messages. Defaults to 2500. /// public int MaxQueueLength { diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index d7a6027ba6ce4d..ac7044a2c4fb28 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -14,7 +14,10 @@ namespace Microsoft.Extensions.Logging.Console [UnsupportedOSPlatform("browser")] internal class ConsoleLoggerProcessor : IDisposable { + private static int s_processorCount; + private int _processorId; private static readonly Meter s_meter = new Meter("Microsoft-Extension-Logging-Console-Queue", "1.0.0"); + private const string WarningMessageOnDrop = " message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages."; private readonly Queue _messageQueue; private int _messagesDropped; private bool _isAddingCompleted; @@ -46,18 +49,17 @@ public ConsoleLoggerBufferFullMode FullMode { throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); } + _fullMode = value; } } - private readonly string _queueName; private readonly Thread _outputThread; public IConsole Console { get; } public IConsole ErrorConsole { get; } - public ConsoleLoggerProcessor(string queueName, IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) + public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) { - _queueName = queueName; _messageQueue = new Queue(); FullMode = fullMode; MaxQueueLength = maxQueueLength; @@ -70,6 +72,7 @@ public ConsoleLoggerProcessor(string queueName, IConsole console, IConsole error Name = "Console logger queue processing thread" }; _outputThread.Start(); + _processorId = ++s_processorCount; s_meter.CreateObservableGauge("queue-size", GetQueueSize); } @@ -87,12 +90,6 @@ internal void WriteMessage(LogMessageEntry entry) { try { - var messagesDropped = Interlocked.Exchange(ref _messagesDropped, 0); - if (messagesDropped != 0) - { - ErrorConsole.Write($"{messagesDropped} message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this.{Environment.NewLine}"); - } - IConsole console = entry.LogAsError ? ErrorConsole : Console; console.Write(entry.Message); } @@ -121,7 +118,7 @@ public bool Enqueue(LogMessageEntry item) { if (FullMode == ConsoleLoggerBufferFullMode.DropWrite) { - Interlocked.Increment(ref _messagesDropped); + _messagesDropped++; return true; } @@ -131,9 +128,24 @@ public bool Enqueue(LogMessageEntry item) if (_messageQueue.Count < MaxQueueLength && !_isAddingCompleted) { + bool startedEmpty = _messageQueue.Count == 0; + if (_messagesDropped > 0) + { + _messageQueue.Enqueue(new LogMessageEntry( + message: _messagesDropped + WarningMessageOnDrop + Environment.NewLine, + logAsError: true + )); + + _messagesDropped = 0; + } + + // if we just logged the dropped message warning this could push the queue size to + // MaxLength + 1, that shouldn't be a problem. It won't grow any further until it is less than + // MaxLength once again. _messageQueue.Enqueue(item); - if (_messageQueue.Count == 1) + // if the queue started empty it could be at 1 or 2 now + if (startedEmpty) { // pulse for wait in Dequeue Monitor.PulseAll(_messageQueue); @@ -196,7 +208,7 @@ private IEnumerable> GetQueueSize() { return new Measurement[] { - new Measurement(_messageQueue.Count, new KeyValuePair("queue-name", _queueName)), + new Measurement(_messageQueue.Count, new KeyValuePair("queue-name", nameof(ConsoleLoggerProcessor) + _processorId)), }; } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs index 150aabfe1a92ce..bb92d2a45c5b07 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs @@ -56,7 +56,6 @@ public ConsoleLoggerProvider(IOptionsMonitor options, IEnu errorConsole = new AnsiParsingLogConsole(stdErr: true); } _messageQueue = new ConsoleLoggerProcessor( - "LoggingConsoleQueue", console, errorConsole, options.CurrentValue.BufferFullMode, diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index d7d2ed25fda2ea..2f9ce563e7355c 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -21,7 +21,7 @@ public void LogAfterDisposeWritesLog() // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(nameof(LogAfterDisposeWritesLog), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); var logger = new ConsoleLogger(_loggerName, loggerProcessor: processor, new SimpleConsoleFormatter(new TestFormatterOptionsMonitor(new SimpleConsoleFormatterOptions())), @@ -45,7 +45,7 @@ public static void MaxQueueLength_SetInvalid_Throws(int invalidMaxQueueLength) // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(nameof(MaxQueueLength_SetInvalid_Throws), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); // Act & Assert Assert.Throws(() => processor.MaxQueueLength = invalidMaxQueueLength); @@ -57,7 +57,7 @@ public static void FullMode_SetInvalid_Throws() // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(nameof(FullMode_SetInvalid_Throws), console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); // Act & Assert Assert.Throws(() => processor.FullMode = (ConsoleLoggerBufferFullMode)10); @@ -74,7 +74,7 @@ public void CheckForNotificationWhenQueueIsFull(bool okToDrop) var console = new TestConsole(sink); var errorConsole = new TimesWriteCalledConsole(); string queueName = nameof(CheckForNotificationWhenQueueIsFull) + (okToDrop ? "InDropWriteMode" : "InWaitMode"); - var processor = new ConsoleLoggerProcessor(queueName, console, errorConsole, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, errorConsole, ConsoleLoggerBufferFullMode.Wait, 1024); var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( new SimpleConsoleFormatterOptions())); @@ -127,7 +127,6 @@ public void ThrowDuringProcessLog_ShutsDownGracefully() var console = new TimesWriteCalledConsole(); var writeThrowingConsole = new WriteThrowingConsole(); var processor = new ConsoleLoggerProcessor( - nameof(ThrowDuringProcessLog_ShutsDownGracefully), console, writeThrowingConsole, ConsoleLoggerBufferFullMode.Wait, diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index b25c0e9291b402..ce32df062e2472 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -1418,7 +1418,7 @@ private string CreateHeader(ConsoleLoggerFormat format, int eventId = 0) internal class TestLoggerProcessor : ConsoleLoggerProcessor { public TestLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) - : base(nameof(TestLoggerProcessor), console, errorConsole, fullMode, maxQueueLength) + : base(console, errorConsole, fullMode, maxQueueLength) { } From c559455bbf87e24e8f43178c6400c64da00aac55 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 10 Jun 2022 10:45:32 -0700 Subject: [PATCH 09/20] Apply remaining pr feedback --- .../src/ConsoleLoggerProcessor.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index ac7044a2c4fb28..a5e93a403c82a7 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -16,7 +16,8 @@ internal class ConsoleLoggerProcessor : IDisposable { private static int s_processorCount; private int _processorId; - private static readonly Meter s_meter = new Meter("Microsoft-Extension-Logging-Console-Queue", "1.0.0"); + private static List s_processors = new List(); + private static readonly Meter s_meter = new Meter("Microsoft.Extensions.Logging.Console", "1.0.0"); private const string WarningMessageOnDrop = " message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages."; private readonly Queue _messageQueue; private int _messagesDropped; @@ -73,6 +74,11 @@ public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLo }; _outputThread.Start(); _processorId = ++s_processorCount; + s_processors.Add(this); + } + + static ConsoleLoggerProcessor() + { s_meter.CreateObservableGauge("queue-size", GetQueueSize); } @@ -204,12 +210,16 @@ private void CompleteAdding() } } - private IEnumerable> GetQueueSize() + private static IEnumerable> GetQueueSize() { - return new Measurement[] + var measures = new Measurement[s_processorCount]; + for (int i = 0; i < s_processorCount; i++) { - new Measurement(_messageQueue.Count, new KeyValuePair("queue-name", nameof(ConsoleLoggerProcessor) + _processorId)), - }; + ConsoleLoggerProcessor p = s_processors[i]; + measures[i] = new Measurement(p._messageQueue.Count, new KeyValuePair("queue-name", nameof(ConsoleLoggerProcessor) + p._processorId)); + } + + return measures; } } } From a4f9176cf0839038c4c8adfa330e8ab24fd207e0 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 14 Jun 2022 12:59:42 -0700 Subject: [PATCH 10/20] Remove static logic for metrics --- .../src/ConsoleLoggerProcessor.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index a5e93a403c82a7..674435964454a6 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -14,10 +14,6 @@ namespace Microsoft.Extensions.Logging.Console [UnsupportedOSPlatform("browser")] internal class ConsoleLoggerProcessor : IDisposable { - private static int s_processorCount; - private int _processorId; - private static List s_processors = new List(); - private static readonly Meter s_meter = new Meter("Microsoft.Extensions.Logging.Console", "1.0.0"); private const string WarningMessageOnDrop = " message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages."; private readonly Queue _messageQueue; private int _messagesDropped; @@ -73,13 +69,6 @@ public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLo Name = "Console logger queue processing thread" }; _outputThread.Start(); - _processorId = ++s_processorCount; - s_processors.Add(this); - } - - static ConsoleLoggerProcessor() - { - s_meter.CreateObservableGauge("queue-size", GetQueueSize); } public virtual void EnqueueMessage(LogMessageEntry message) @@ -209,17 +198,5 @@ private void CompleteAdding() _isAddingCompleted = true; } } - - private static IEnumerable> GetQueueSize() - { - var measures = new Measurement[s_processorCount]; - for (int i = 0; i < s_processorCount; i++) - { - ConsoleLoggerProcessor p = s_processors[i]; - measures[i] = new Measurement(p._messageQueue.Count, new KeyValuePair("queue-name", nameof(ConsoleLoggerProcessor) + p._processorId)); - } - - return measures; - } } } From b55de82b8dfece49d6cbdd6acf07723666581b5e Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 20 Jun 2022 13:46:05 -0700 Subject: [PATCH 11/20] Apply feedback - Rename to QueueFullMode --- .../ref/Microsoft.Extensions.Logging.Console.cs | 2 +- .../src/ConsoleLoggerOptions.cs | 2 +- .../src/ConsoleLoggerProvider.cs | 4 ++-- .../ConsoleFormatterTests.cs | 2 +- .../ConsoleLoggerTest.cs | 12 ++++++------ ...Microsoft.Extensions.Logging.Console.Tests.csproj | 1 + 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs b/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs index 62d2001f7ef774..ec088a31b8ca5d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs @@ -63,7 +63,7 @@ public ConsoleLoggerOptions() { } public bool IncludeScopes { get { throw null; } set { } } public Microsoft.Extensions.Logging.LogLevel LogToStandardErrorThreshold { get { throw null; } set { } } public int MaxQueueLength { get { throw null; } set { } } - public Microsoft.Extensions.Logging.Console.ConsoleLoggerBufferFullMode BufferFullMode { get { throw null; } set { } } + public Microsoft.Extensions.Logging.Console.ConsoleLoggerBufferFullMode QueueFullMode { get { throw null; } set { } } [System.ObsoleteAttribute("ConsoleLoggerOptions.TimestampFormat has been deprecated. Use ConsoleFormatterOptions.TimestampFormat instead.")] public string? TimestampFormat { get { throw null; } set { } } [System.ObsoleteAttribute("ConsoleLoggerOptions.UseUtcTimestamp has been deprecated. Use ConsoleFormatterOptions.UseUtcTimestamp instead.")] diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs index 52b90042c7c2ca..2fbe835324fe7b 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs @@ -68,7 +68,7 @@ public ConsoleLoggerFormat Format /// /// Gets or sets the desired console logger behavior when buffer becomes full. Defaults to Wait. /// - public ConsoleLoggerBufferFullMode BufferFullMode + public ConsoleLoggerBufferFullMode QueueFullMode { get => _bufferFullMode; set diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs index bb92d2a45c5b07..6bbb9b7e3f46cb 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs @@ -58,7 +58,7 @@ public ConsoleLoggerProvider(IOptionsMonitor options, IEnu _messageQueue = new ConsoleLoggerProcessor( console, errorConsole, - options.CurrentValue.BufferFullMode, + options.CurrentValue.QueueFullMode, options.CurrentValue.MaxQueueLength); ReloadLoggerOptions(options.CurrentValue); @@ -126,7 +126,7 @@ private void ReloadLoggerOptions(ConsoleLoggerOptions options) #pragma warning restore CS0618 } - _messageQueue.FullMode = options.BufferFullMode; + _messageQueue.FullMode = options.QueueFullMode; _messageQueue.MaxQueueLength = options.MaxQueueLength; foreach (KeyValuePair logger in _loggers) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs index 327acfef288f21..6a1a7e8b2e4fd9 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs @@ -38,7 +38,7 @@ internal static (ConsoleLogger Logger, ConsoleSink Sink, ConsoleSink ErrorSink, var errorSink = new ConsoleSink(); var console = new TestConsole(sink); var errorConsole = new TestConsole(errorSink); - var bufferMode = options == null ? ConsoleLoggerBufferFullMode.Wait : options.BufferFullMode; + var bufferMode = options == null ? ConsoleLoggerBufferFullMode.Wait : options.QueueFullMode; var maxQueueLength = options == null ? ConsoleLoggerOptions.DefaultMaxQueueLengthValue : options.MaxQueueLength; var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, bufferMode, maxQueueLength); diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index ce32df062e2472..6237377c93f5ef 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -52,7 +52,7 @@ private static (ConsoleLogger Logger, ConsoleSink Sink, ConsoleSink ErrorSink, F ConsoleFormatter? formatter = null; var loggerOptions = options ?? new ConsoleLoggerOptions(); - var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, loggerOptions.BufferFullMode, loggerOptions.MaxQueueLength); + var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, loggerOptions.QueueFullMode, loggerOptions.MaxQueueLength); Func levelAsString; int writesPerMsg; switch (loggerOptions.Format) @@ -424,7 +424,7 @@ public void AddConsole_IsOutputRedirected_ColorDisabled() var sink = new ConsoleSink(); var console = new TestConsole(sink); var loggerOptions = new ConsoleLoggerOptions(); - var consoleLoggerProcessor = new TestLoggerProcessor(console, null!); + var consoleLoggerProcessor = new TestLoggerProcessor(console, null!, loggerOptions.QueueFullMode, loggerOptions.MaxQueueLength); var loggerProvider = new ServiceCollection() .AddLogging(builder => builder @@ -1184,7 +1184,7 @@ public void ConsoleLoggerOptions_SetInvalidMaxQueueLength_Throws(int invalidMaxQ [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void ConsoleLoggerOptions_SetInvalidBufferMode_Throws() { - Assert.Throws(() => new ConsoleLoggerOptions() { BufferFullMode = (ConsoleLoggerBufferFullMode)10 }); + Assert.Throws(() => new ConsoleLoggerOptions() { QueueFullMode = (ConsoleLoggerBufferFullMode)10 }); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] @@ -1303,13 +1303,13 @@ public void ConsoleLoggerOptions_UpdateQueueOptions_UpdatesConsoleLoggerProcesso var logger = (ConsoleLogger)loggerProvider.CreateLogger("Name"); // Act & Assert - Assert.Equal(ConsoleLoggerBufferFullMode.Wait, logger.Options.BufferFullMode); + Assert.Equal(ConsoleLoggerBufferFullMode.Wait, logger.Options.QueueFullMode); Assert.Equal(ConsoleLoggerOptions.DefaultMaxQueueLengthValue, logger.Options.MaxQueueLength); monitor.Set(new ConsoleLoggerOptions() { - BufferFullMode = ConsoleLoggerBufferFullMode.DropWrite, + QueueFullMode = ConsoleLoggerBufferFullMode.DropWrite, MaxQueueLength = 10 }); - Assert.Equal(ConsoleLoggerBufferFullMode.DropWrite, logger.Options.BufferFullMode); + Assert.Equal(ConsoleLoggerBufferFullMode.DropWrite, logger.Options.QueueFullMode); Assert.Equal(10, logger.Options.MaxQueueLength); } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj index d762d320452a36..c1f8f924a4b2ee 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj @@ -4,6 +4,7 @@ $(NetCoreAppCurrent);$(NetFrameworkMinimum) true true + true From a3d550a99237048d4ffaa5944c5cb2e75ee4bebc Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 20 Jun 2022 13:55:29 -0700 Subject: [PATCH 12/20] Apply feedback - regex + pragma warning --- .../src/ConsoleLoggerOptions.cs | 2 +- .../src/Resources/Strings.resx | 3 +++ .../ConsoleLoggerProcessorTests.cs | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs index 2fbe835324fe7b..6a4c8a9df83e20 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs @@ -75,7 +75,7 @@ public ConsoleLoggerBufferFullMode QueueFullMode { if (value != ConsoleLoggerBufferFullMode.Wait && value != ConsoleLoggerBufferFullMode.DropWrite) { - throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); + throw new ArgumentOutOfRangeException(SR.Format(SR.BufferModeNotSupported, nameof(value))); } _bufferFullMode = value; } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx index 5ef4b889b114c3..77dc8abdd55c12 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx @@ -120,4 +120,7 @@ Cannot allocate a buffer of size {0}. + + {0} is not a supported buffer mode value. + \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index 2f9ce563e7355c..8309436471847f 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -7,7 +7,6 @@ using System.Linq; using Microsoft.Extensions.Logging.Test.Console; using Xunit; -#pragma warning disable CS0618 namespace Microsoft.Extensions.Logging.Console.Test { @@ -154,6 +153,7 @@ public void ThrowDuringProcessLog_ShutsDownGracefully() private static void UpdateFormatterOptions(ConsoleFormatter formatter, ConsoleLoggerOptions deprecatedFromOptions) { +#pragma warning disable CS0618 // kept for deprecated apis: if (formatter is SimpleConsoleFormatter defaultFormatter) { @@ -170,7 +170,7 @@ private static void UpdateFormatterOptions(ConsoleFormatter formatter, ConsoleLo systemdFormatter.FormatterOptions.TimestampFormat = deprecatedFromOptions.TimestampFormat; systemdFormatter.FormatterOptions.UseUtcTimestamp = deprecatedFromOptions.UseUtcTimestamp; } +#pragma warning restore CS0618 } } } -#pragma warning restore CS0618 From 60f5f3b455e9731b7907dda1762285d27e99107e Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 20 Jun 2022 14:00:01 -0700 Subject: [PATCH 13/20] Apply PR feedback - correct test --- .../ConsoleLoggerProcessorTests.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index 8309436471847f..39d98652f2477a 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -63,7 +63,7 @@ public static void FullMode_SetInvalid_Throws() } [OuterLoop] - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] [InlineData(true)] [InlineData(false)] public void CheckForNotificationWhenQueueIsFull(bool okToDrop) @@ -73,7 +73,8 @@ public void CheckForNotificationWhenQueueIsFull(bool okToDrop) var console = new TestConsole(sink); var errorConsole = new TimesWriteCalledConsole(); string queueName = nameof(CheckForNotificationWhenQueueIsFull) + (okToDrop ? "InDropWriteMode" : "InWaitMode"); - var processor = new ConsoleLoggerProcessor(console, errorConsole, ConsoleLoggerBufferFullMode.Wait, 1024); + var fullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; + var processor = new ConsoleLoggerProcessor(console, errorConsole, fullMode, maxQueueLength: 1); var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( new SimpleConsoleFormatterOptions())); @@ -84,8 +85,6 @@ public void CheckForNotificationWhenQueueIsFull(bool okToDrop) object[] messageParams = Enumerable.Range(1, 100).Select(x => (object)x).ToArray(); // Act - processor.MaxQueueLength = 1; - processor.FullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; for (int i = 0; i < 20000; i++) { logger.LogInformation(messageTemplate, messageParams); From f200c8c81287f2ab2c26061465c388f6021f45e2 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 20 Jun 2022 14:04:37 -0700 Subject: [PATCH 14/20] Apply PR feedback - resx --- .../src/ConsoleLoggerProcessor.cs | 3 +-- .../src/Resources/Strings.resx | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 674435964454a6..95a9c7ef43fbed 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -14,7 +14,6 @@ namespace Microsoft.Extensions.Logging.Console [UnsupportedOSPlatform("browser")] internal class ConsoleLoggerProcessor : IDisposable { - private const string WarningMessageOnDrop = " message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages."; private readonly Queue _messageQueue; private int _messagesDropped; private bool _isAddingCompleted; @@ -127,7 +126,7 @@ public bool Enqueue(LogMessageEntry item) if (_messagesDropped > 0) { _messageQueue.Enqueue(new LogMessageEntry( - message: _messagesDropped + WarningMessageOnDrop + Environment.NewLine, + message: _messagesDropped + SR.WarningMessageOnDrop + Environment.NewLine, logAsError: true )); diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx index 77dc8abdd55c12..2202ba914db4be 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx @@ -123,4 +123,7 @@ {0} is not a supported buffer mode value. + + message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages. + \ No newline at end of file From f74dd419a6670ced4355ebda509c536df72bec7f Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 20 Jun 2022 15:06:15 -0700 Subject: [PATCH 15/20] Apply more feedback --- .../src/ConsoleLoggerProcessor.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 95a9c7ef43fbed..6eca74eaa40607 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -15,7 +15,7 @@ namespace Microsoft.Extensions.Logging.Console internal class ConsoleLoggerProcessor : IDisposable { private readonly Queue _messageQueue; - private int _messagesDropped; + private volatile int _messagesDropped; private bool _isAddingCompleted; private int _maxQueuedMessages = ConsoleLoggerOptions.DefaultMaxQueueLengthValue; public int MaxQueueLength @@ -79,7 +79,7 @@ public virtual void EnqueueMessage(LogMessageEntry message) } } - // for testing + // internal for testing internal void WriteMessage(LogMessageEntry entry) { try @@ -87,7 +87,7 @@ internal void WriteMessage(LogMessageEntry entry) IConsole console = entry.LogAsError ? ErrorConsole : Console; console.Write(entry.Message); } - catch (Exception) + catch { CompleteAdding(); } @@ -120,8 +120,9 @@ public bool Enqueue(LogMessageEntry item) Monitor.Wait(_messageQueue); } - if (_messageQueue.Count < MaxQueueLength && !_isAddingCompleted) + if (!_isAddingCompleted) { + Debug.Assert(_messageQueue.Count < MaxQueueLength); bool startedEmpty = _messageQueue.Count == 0; if (_messagesDropped > 0) { From b4458be36d6e9baea0c0ee9c54ec3b51d7bfa901 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 20 Jun 2022 16:04:15 -0700 Subject: [PATCH 16/20] Address thread safety issue --- .../src/ConsoleLoggerProcessor.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index 6eca74eaa40607..bc069e1dbd469d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -46,7 +46,12 @@ public ConsoleLoggerBufferFullMode FullMode throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); } - _fullMode = value; + lock (_messageQueue) + { + // _fullMode is used inside the lock and is safer to guard setter with lock as well + // this set is not expected to happen frequently + _fullMode = value; + } } } private readonly Thread _outputThread; @@ -95,12 +100,9 @@ internal void WriteMessage(LogMessageEntry entry) private void ProcessLogQueue() { - while (!_isAddingCompleted || _messageQueue.Count > 0) + while (TryDequeue(out LogMessageEntry message)) { - if (TryDequeue(out LogMessageEntry message)) - { - WriteMessage(message); - } + WriteMessage(message); } } @@ -162,7 +164,7 @@ public bool TryDequeue(out LogMessageEntry item) Monitor.Wait(_messageQueue); } - if (_messageQueue.Count > 0) + if (_messageQueue.Count > 0 && !_isAddingCompleted) { item = _messageQueue.Dequeue(); if (_messageQueue.Count == MaxQueueLength - 1) From 5ded943aca65e74fcb41abc9dd0669f62505efbc Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 21 Jun 2022 07:08:28 -0700 Subject: [PATCH 17/20] rename --- .../ref/Microsoft.Extensions.Logging.Console.cs | 4 ++-- .../src/ConsoleLoggerOptions.cs | 6 +++--- .../src/ConsoleLoggerProcessor.cs | 12 ++++++------ .../src/ConsoleLoggerQueueFullMode.cs | 2 +- .../src/Resources/Strings.resx | 2 +- .../ConsoleFormatterTests.cs | 2 +- .../ConsoleLoggerProcessorTests.cs | 12 ++++++------ .../ConsoleLoggerTest.cs | 10 +++++----- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs b/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs index ec088a31b8ca5d..dabec63ae00d39 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/ref/Microsoft.Extensions.Logging.Console.cs @@ -63,7 +63,7 @@ public ConsoleLoggerOptions() { } public bool IncludeScopes { get { throw null; } set { } } public Microsoft.Extensions.Logging.LogLevel LogToStandardErrorThreshold { get { throw null; } set { } } public int MaxQueueLength { get { throw null; } set { } } - public Microsoft.Extensions.Logging.Console.ConsoleLoggerBufferFullMode QueueFullMode { get { throw null; } set { } } + public Microsoft.Extensions.Logging.Console.ConsoleLoggerQueueFullMode QueueFullMode { get { throw null; } set { } } [System.ObsoleteAttribute("ConsoleLoggerOptions.TimestampFormat has been deprecated. Use ConsoleFormatterOptions.TimestampFormat instead.")] public string? TimestampFormat { get { throw null; } set { } } [System.ObsoleteAttribute("ConsoleLoggerOptions.UseUtcTimestamp has been deprecated. Use ConsoleFormatterOptions.UseUtcTimestamp instead.")] @@ -79,7 +79,7 @@ public ConsoleLoggerProvider(Microsoft.Extensions.Options.IOptionsMonitor /// Gets or sets the desired console logger behavior when buffer becomes full. Defaults to Wait. /// - public ConsoleLoggerBufferFullMode QueueFullMode + public ConsoleLoggerQueueFullMode QueueFullMode { get => _bufferFullMode; set { - if (value != ConsoleLoggerBufferFullMode.Wait && value != ConsoleLoggerBufferFullMode.DropWrite) + if (value != ConsoleLoggerQueueFullMode.Wait && value != ConsoleLoggerQueueFullMode.DropWrite) { throw new ArgumentOutOfRangeException(SR.Format(SR.BufferModeNotSupported, nameof(value))); } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index bc069e1dbd469d..fbb938b147cbf5 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -35,13 +35,13 @@ public int MaxQueueLength } } } - private ConsoleLoggerBufferFullMode _fullMode = ConsoleLoggerBufferFullMode.Wait; - public ConsoleLoggerBufferFullMode FullMode + private ConsoleLoggerQueueFullMode _fullMode = ConsoleLoggerQueueFullMode.Wait; + public ConsoleLoggerQueueFullMode FullMode { get => _fullMode; set { - if (value != ConsoleLoggerBufferFullMode.Wait && value != ConsoleLoggerBufferFullMode.DropWrite) + if (value != ConsoleLoggerQueueFullMode.Wait && value != ConsoleLoggerQueueFullMode.DropWrite) { throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); } @@ -59,7 +59,7 @@ public ConsoleLoggerBufferFullMode FullMode public IConsole Console { get; } public IConsole ErrorConsole { get; } - public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) + public ConsoleLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerQueueFullMode fullMode, int maxQueueLength) { _messageQueue = new Queue(); FullMode = fullMode; @@ -112,13 +112,13 @@ public bool Enqueue(LogMessageEntry item) { while (_messageQueue.Count >= MaxQueueLength && !_isAddingCompleted) { - if (FullMode == ConsoleLoggerBufferFullMode.DropWrite) + if (FullMode == ConsoleLoggerQueueFullMode.DropWrite) { _messagesDropped++; return true; } - Debug.Assert(FullMode == ConsoleLoggerBufferFullMode.Wait); + Debug.Assert(FullMode == ConsoleLoggerQueueFullMode.Wait); Monitor.Wait(_messageQueue); } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs index 2ef0adbf894bdb..74d8ab577feb7b 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs @@ -6,7 +6,7 @@ namespace Microsoft.Extensions.Logging.Console /// /// Determines the console logger behavior when buffer becomes full. /// - public enum ConsoleLoggerBufferFullMode + public enum ConsoleLoggerQueueFullMode { /// /// Blocks the logging threads once the buffer limit is reached. diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx index 2202ba914db4be..891c5a8da518f3 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx @@ -124,6 +124,6 @@ {0} is not a supported buffer mode value. - message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages. + message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerQueueFullMode` to stop dropping messages. \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs index 6a1a7e8b2e4fd9..6da081c4e86ab0 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs @@ -38,7 +38,7 @@ internal static (ConsoleLogger Logger, ConsoleSink Sink, ConsoleSink ErrorSink, var errorSink = new ConsoleSink(); var console = new TestConsole(sink); var errorConsole = new TestConsole(errorSink); - var bufferMode = options == null ? ConsoleLoggerBufferFullMode.Wait : options.QueueFullMode; + var bufferMode = options == null ? ConsoleLoggerQueueFullMode.Wait : options.QueueFullMode; var maxQueueLength = options == null ? ConsoleLoggerOptions.DefaultMaxQueueLengthValue : options.MaxQueueLength; var consoleLoggerProcessor = new TestLoggerProcessor(console, errorConsole, bufferMode, maxQueueLength); diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs index 39d98652f2477a..0dce61f7884d26 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerProcessorTests.cs @@ -20,7 +20,7 @@ public void LogAfterDisposeWritesLog() // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerQueueFullMode.Wait, 1024); var logger = new ConsoleLogger(_loggerName, loggerProcessor: processor, new SimpleConsoleFormatter(new TestFormatterOptionsMonitor(new SimpleConsoleFormatterOptions())), @@ -44,7 +44,7 @@ public static void MaxQueueLength_SetInvalid_Throws(int invalidMaxQueueLength) // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerQueueFullMode.Wait, 1024); // Act & Assert Assert.Throws(() => processor.MaxQueueLength = invalidMaxQueueLength); @@ -56,10 +56,10 @@ public static void FullMode_SetInvalid_Throws() // Arrange var sink = new ConsoleSink(); var console = new TestConsole(sink); - var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerBufferFullMode.Wait, 1024); + var processor = new ConsoleLoggerProcessor(console, null!, ConsoleLoggerQueueFullMode.Wait, 1024); // Act & Assert - Assert.Throws(() => processor.FullMode = (ConsoleLoggerBufferFullMode)10); + Assert.Throws(() => processor.FullMode = (ConsoleLoggerQueueFullMode)10); } [OuterLoop] @@ -73,7 +73,7 @@ public void CheckForNotificationWhenQueueIsFull(bool okToDrop) var console = new TestConsole(sink); var errorConsole = new TimesWriteCalledConsole(); string queueName = nameof(CheckForNotificationWhenQueueIsFull) + (okToDrop ? "InDropWriteMode" : "InWaitMode"); - var fullMode = okToDrop ? ConsoleLoggerBufferFullMode.DropWrite : ConsoleLoggerBufferFullMode.Wait; + var fullMode = okToDrop ? ConsoleLoggerQueueFullMode.DropWrite : ConsoleLoggerQueueFullMode.Wait; var processor = new ConsoleLoggerProcessor(console, errorConsole, fullMode, maxQueueLength: 1); var formatter = new SimpleConsoleFormatter(new TestFormatterOptionsMonitor( new SimpleConsoleFormatterOptions())); @@ -127,7 +127,7 @@ public void ThrowDuringProcessLog_ShutsDownGracefully() var processor = new ConsoleLoggerProcessor( console, writeThrowingConsole, - ConsoleLoggerBufferFullMode.Wait, + ConsoleLoggerQueueFullMode.Wait, 1024); var formatter = new SimpleConsoleFormatter( diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index 6237377c93f5ef..0994980590122b 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -1184,7 +1184,7 @@ public void ConsoleLoggerOptions_SetInvalidMaxQueueLength_Throws(int invalidMaxQ [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] public void ConsoleLoggerOptions_SetInvalidBufferMode_Throws() { - Assert.Throws(() => new ConsoleLoggerOptions() { QueueFullMode = (ConsoleLoggerBufferFullMode)10 }); + Assert.Throws(() => new ConsoleLoggerOptions() { QueueFullMode = (ConsoleLoggerQueueFullMode)10 }); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] @@ -1303,13 +1303,13 @@ public void ConsoleLoggerOptions_UpdateQueueOptions_UpdatesConsoleLoggerProcesso var logger = (ConsoleLogger)loggerProvider.CreateLogger("Name"); // Act & Assert - Assert.Equal(ConsoleLoggerBufferFullMode.Wait, logger.Options.QueueFullMode); + Assert.Equal(ConsoleLoggerQueueFullMode.Wait, logger.Options.QueueFullMode); Assert.Equal(ConsoleLoggerOptions.DefaultMaxQueueLengthValue, logger.Options.MaxQueueLength); monitor.Set(new ConsoleLoggerOptions() { - QueueFullMode = ConsoleLoggerBufferFullMode.DropWrite, + QueueFullMode = ConsoleLoggerQueueFullMode.DropWrite, MaxQueueLength = 10 }); - Assert.Equal(ConsoleLoggerBufferFullMode.DropWrite, logger.Options.QueueFullMode); + Assert.Equal(ConsoleLoggerQueueFullMode.DropWrite, logger.Options.QueueFullMode); Assert.Equal(10, logger.Options.MaxQueueLength); } @@ -1417,7 +1417,7 @@ private string CreateHeader(ConsoleLoggerFormat format, int eventId = 0) internal class TestLoggerProcessor : ConsoleLoggerProcessor { - public TestLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerBufferFullMode fullMode, int maxQueueLength) + public TestLoggerProcessor(IConsole console, IConsole errorConsole, ConsoleLoggerQueueFullMode fullMode, int maxQueueLength) : base(console, errorConsole, fullMode, maxQueueLength) { } From 10fbf54fb8772398536e1fd1d3fe463ef40521bd Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 21 Jun 2022 10:10:27 -0400 Subject: [PATCH 18/20] Apply suggestions from code review Co-authored-by: Eric Erhardt --- .../src/ConsoleLoggerOptions.cs | 2 +- .../src/ConsoleLoggerQueueFullMode.cs | 6 +++--- .../src/Resources/Strings.resx | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs index 6a4c8a9df83e20..99fd16de99a10f 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs @@ -66,7 +66,7 @@ public ConsoleLoggerFormat Format private ConsoleLoggerBufferFullMode _bufferFullMode = ConsoleLoggerBufferFullMode.Wait; /// - /// Gets or sets the desired console logger behavior when buffer becomes full. Defaults to Wait. + /// Gets or sets the desired console logger behavior when the queue becomes full. Defaults to Wait. /// public ConsoleLoggerBufferFullMode QueueFullMode { diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs index 2ef0adbf894bdb..14f5880a62cc46 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerQueueFullMode.cs @@ -4,16 +4,16 @@ namespace Microsoft.Extensions.Logging.Console { /// - /// Determines the console logger behavior when buffer becomes full. + /// Determines the console logger behavior when the queue becomes full. /// public enum ConsoleLoggerBufferFullMode { /// - /// Blocks the logging threads once the buffer limit is reached. + /// Blocks the logging threads once the queue limit is reached. /// Wait, /// - /// Drops new log messages when the buffer is full + /// Drops new log messages when the queue is full. /// DropWrite } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx index 2202ba914db4be..73e5b222f01038 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx @@ -121,7 +121,7 @@ Cannot allocate a buffer of size {0}. - {0} is not a supported buffer mode value. + {0} is not a supported queue mode value. message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerBufferFullMode` to stop dropping messages. From a369c543e8b20e917e5bce59e1c71f61d7b45f54 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 21 Jun 2022 07:22:28 -0700 Subject: [PATCH 19/20] More feedback --- .../src/ConsoleLoggerOptions.cs | 10 +++++----- .../src/ConsoleLoggerProcessor.cs | 6 +++--- .../src/Resources/Strings.resx | 7 +++++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs index 6acbb48f0d2925..13aa8311ff679e 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs @@ -64,20 +64,20 @@ public ConsoleLoggerFormat Format [System.ObsoleteAttribute("ConsoleLoggerOptions.UseUtcTimestamp has been deprecated. Use ConsoleFormatterOptions.UseUtcTimestamp instead.")] public bool UseUtcTimestamp { get; set; } - private ConsoleLoggerQueueFullMode _bufferFullMode = ConsoleLoggerQueueFullMode.Wait; + private ConsoleLoggerQueueFullMode _queueFullMode = ConsoleLoggerQueueFullMode.Wait; /// /// Gets or sets the desired console logger behavior when the queue becomes full. Defaults to Wait. /// public ConsoleLoggerQueueFullMode QueueFullMode { - get => _bufferFullMode; + get => _queueFullMode; set { if (value != ConsoleLoggerQueueFullMode.Wait && value != ConsoleLoggerQueueFullMode.DropWrite) { - throw new ArgumentOutOfRangeException(SR.Format(SR.BufferModeNotSupported, nameof(value))); + throw new ArgumentOutOfRangeException(SR.Format(SR.QueueModeNotSupported, nameof(value))); } - _bufferFullMode = value; + _queueFullMode = value; } } @@ -94,7 +94,7 @@ public int MaxQueueLength { if (value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} must be larger than zero."); + throw new ArgumentOutOfRangeException(SR.Format(SR.MaxQueueLengthBadValue, nameof(value))); } _maxQueuedMessages = value; diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index fbb938b147cbf5..a021b912cd280d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -25,7 +25,7 @@ public int MaxQueueLength { if (value <= 0) { - throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} must be larger than zero."); + throw new ArgumentOutOfRangeException(SR.Format(SR.MaxQueueLengthBadValue, nameof(value))); } lock (_messageQueue) @@ -43,7 +43,7 @@ public ConsoleLoggerQueueFullMode FullMode { if (value != ConsoleLoggerQueueFullMode.Wait && value != ConsoleLoggerQueueFullMode.DropWrite) { - throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} is not a supported buffer mode value."); + throw new ArgumentOutOfRangeException(SR.Format(SR.QueueModeNotSupported, nameof(value))); } lock (_messageQueue) @@ -129,7 +129,7 @@ public bool Enqueue(LogMessageEntry item) if (_messagesDropped > 0) { _messageQueue.Enqueue(new LogMessageEntry( - message: _messagesDropped + SR.WarningMessageOnDrop + Environment.NewLine, + message: SR.Format(SR.WarningMessageOnDrop + Environment.NewLine, _messagesDropped), logAsError: true )); diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx index fef7817847a004..c3d2473f20cefa 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx @@ -120,10 +120,13 @@ Cannot allocate a buffer of size {0}. - + {0} is not a supported queue mode value. + + {0} must be larger than zero. + - message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerQueueFullMode` to stop dropping messages. + {0} message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerQueueFullMode` to stop dropping messages. \ No newline at end of file From 04d9fb58f5a29d2b61c0a34ca9292fd0e7798086 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 21 Jun 2022 07:26:00 -0700 Subject: [PATCH 20/20] feedback --- .../src/ConsoleLoggerProcessor.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs index a021b912cd280d..ff48c3880c418c 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProcessor.cs @@ -51,6 +51,7 @@ public ConsoleLoggerQueueFullMode FullMode // _fullMode is used inside the lock and is safer to guard setter with lock as well // this set is not expected to happen frequently _fullMode = value; + Monitor.PulseAll(_messageQueue); } } } @@ -196,8 +197,8 @@ private void CompleteAdding() { lock (_messageQueue) { - Monitor.PulseAll(_messageQueue); _isAddingCompleted = true; + Monitor.PulseAll(_messageQueue); } } }