From 636b523a98aed42a9e9006b13ebe0506b8af83a0 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 19 Apr 2024 15:18:13 +0300 Subject: [PATCH 1/7] Add test to highlight the issue with captured log level by logger --- .../test/common/Internal/Logging/LogTest.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/dotnet/test/common/Internal/Logging/LogTest.cs b/dotnet/test/common/Internal/Logging/LogTest.cs index 5dbdc598b70ae..b50c75909e526 100644 --- a/dotnet/test/common/Internal/Logging/LogTest.cs +++ b/dotnet/test/common/Internal/Logging/LogTest.cs @@ -9,13 +9,27 @@ public class LogTest private TestLogHandler testLogHandler; private ILogger logger; + private void ResetGlobalLog() + { + Log.SetLevel(LogEventLevel.Info); + Log.Handlers.Clear().Handlers.Add(new ConsoleLogHandler()); + } + [SetUp] public void SetUp() { + ResetGlobalLog(); + testLogHandler = new TestLogHandler(); logger = Log.GetLogger(); } + [TearDown] + public void TearDown() + { + ResetGlobalLog(); + } + [Test] public void LoggerShouldEmitEvent() { @@ -141,6 +155,16 @@ public void ContextShouldChangeLevel() Assert.That(logger.Level, Is.EqualTo(LogEventLevel.Warn)); } + [Test] + public void ContextShouldEmitMessages() + { + using var context = Log.CreateContext(LogEventLevel.Trace).Handlers.Add(testLogHandler); + + logger.Trace("test message"); + + Assert.That(testLogHandler.Events.Count, Is.EqualTo(1)); + } + [Test] public void ShouldCreateContextWithCustomHandler() { From dae7454d3e34cf2ab173dc7a78a1e8f664da8772 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:26:22 +0300 Subject: [PATCH 2/7] Determine IsEnabled across local loggers --- dotnet/src/webdriver/Internal/Logging/LogContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Internal/Logging/LogContext.cs b/dotnet/src/webdriver/Internal/Logging/LogContext.cs index bb4d9feede2c5..408e7d19f0213 100644 --- a/dotnet/src/webdriver/Internal/Logging/LogContext.cs +++ b/dotnet/src/webdriver/Internal/Logging/LogContext.cs @@ -98,7 +98,7 @@ public ILogger GetLogger(Type type) public bool IsEnabled(ILogger logger, LogEventLevel level) { - return Handlers != null && level >= _level && level >= logger.Level; + return Handlers != null && level >= _level && level >= _loggers?[logger.Issuer].Level; } public void EmitMessage(ILogger logger, LogEventLevel level, string message) From 77e8a053f3051d070ffc5435e42da9774092ac76 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:27:35 +0300 Subject: [PATCH 3/7] Loggers per context --- dotnet/src/webdriver/Internal/Logging/LogContext.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Internal/Logging/LogContext.cs b/dotnet/src/webdriver/Internal/Logging/LogContext.cs index 408e7d19f0213..3e7843e0d74f5 100644 --- a/dotnet/src/webdriver/Internal/Logging/LogContext.cs +++ b/dotnet/src/webdriver/Internal/Logging/LogContext.cs @@ -46,7 +46,10 @@ public LogContext(LogEventLevel level, ILogContext? parentLogContext, Concurrent _parentLogContext = parentLogContext; - _loggers = loggers; + if (loggers is not null) + { + _loggers = new ConcurrentDictionary(loggers.Select(l => new KeyValuePair(l.Key, new Logger(l.Value.Issuer, level)))); + } if (handlers is not null) { From e3d1ba1859bfc314c37bac0de604aa6fa264ee5d Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 10 Jan 2025 18:51:37 +0300 Subject: [PATCH 4/7] Fix possible NRE --- dotnet/src/webdriver/Internal/Logging/LogContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Internal/Logging/LogContext.cs b/dotnet/src/webdriver/Internal/Logging/LogContext.cs index 3e7843e0d74f5..b7541c68c5126 100644 --- a/dotnet/src/webdriver/Internal/Logging/LogContext.cs +++ b/dotnet/src/webdriver/Internal/Logging/LogContext.cs @@ -101,7 +101,7 @@ public ILogger GetLogger(Type type) public bool IsEnabled(ILogger logger, LogEventLevel level) { - return Handlers != null && level >= _level && level >= _loggers?[logger.Issuer].Level; + return Handlers != null && level >= _level && level >= _loggers?[logger.Issuer]?.Level; } public void EmitMessage(ILogger logger, LogEventLevel level, string message) From df091418a5dca439d85c9c68b6e2102f5e3ebf54 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 10 Jan 2025 19:19:57 +0300 Subject: [PATCH 5/7] Convert to TryGetValue --- dotnet/src/webdriver/Internal/Logging/LogContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Internal/Logging/LogContext.cs b/dotnet/src/webdriver/Internal/Logging/LogContext.cs index b7541c68c5126..a6a27fb9a0c07 100644 --- a/dotnet/src/webdriver/Internal/Logging/LogContext.cs +++ b/dotnet/src/webdriver/Internal/Logging/LogContext.cs @@ -101,7 +101,7 @@ public ILogger GetLogger(Type type) public bool IsEnabled(ILogger logger, LogEventLevel level) { - return Handlers != null && level >= _level && level >= _loggers?[logger.Issuer]?.Level; + return Handlers != null && level >= _level && (_loggers?.TryGetValue(logger.Issuer, out var loggerEntry) != true || level >= loggerEntry?.Level); } public void EmitMessage(ILogger logger, LogEventLevel level, string message) From 6efac848289586a216701c7c5a78d585166258c2 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 10 Jan 2025 19:37:20 +0300 Subject: [PATCH 6/7] Move logic to clone loggers --- .../webdriver/Internal/Logging/LogContext.cs | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/dotnet/src/webdriver/Internal/Logging/LogContext.cs b/dotnet/src/webdriver/Internal/Logging/LogContext.cs index a6a27fb9a0c07..c7ab1217a49a7 100644 --- a/dotnet/src/webdriver/Internal/Logging/LogContext.cs +++ b/dotnet/src/webdriver/Internal/Logging/LogContext.cs @@ -20,6 +20,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; #nullable enable @@ -46,10 +47,7 @@ public LogContext(LogEventLevel level, ILogContext? parentLogContext, Concurrent _parentLogContext = parentLogContext; - if (loggers is not null) - { - _loggers = new ConcurrentDictionary(loggers.Select(l => new KeyValuePair(l.Key, new Logger(l.Value.Issuer, level)))); - } + _loggers = CloneLoggers(loggers, level); if (handlers is not null) { @@ -158,5 +156,24 @@ public void Dispose() Log.CurrentContext = _parentLogContext; } + + [return: NotNullIfNotNull(nameof(loggers))] + private static ConcurrentDictionary? CloneLoggers(ConcurrentDictionary? loggers, LogEventLevel minimumLevel) + { + if (loggers is null) + { + return null; + } + + var cloned = new Dictionary(loggers.Count); + + foreach (KeyValuePair logger in loggers) + { + var clonedLogger = new Logger(logger.Value.Issuer, minimumLevel); + cloned.Add(logger.Key, clonedLogger); + } + + return new ConcurrentDictionary(cloned); + } } } From 5057ec0182796156514af7132946423effe5b8d3 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:28:13 +0300 Subject: [PATCH 7/7] Clone in CreateContext method --- dotnet/src/webdriver/Internal/Logging/LogContext.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/dotnet/src/webdriver/Internal/Logging/LogContext.cs b/dotnet/src/webdriver/Internal/Logging/LogContext.cs index c7ab1217a49a7..081ebbd4c5ab4 100644 --- a/dotnet/src/webdriver/Internal/Logging/LogContext.cs +++ b/dotnet/src/webdriver/Internal/Logging/LogContext.cs @@ -31,7 +31,7 @@ namespace OpenQA.Selenium.Internal.Logging /// Represents a logging context that provides methods for creating sub-contexts, retrieving loggers, emitting log messages, and configuring minimum log levels. /// /// - internal class LogContext : ILogContext + internal sealed class LogContext : ILogContext { private ConcurrentDictionary? _loggers; @@ -66,12 +66,7 @@ public ILogContext CreateContext() public ILogContext CreateContext(LogEventLevel minimumLevel) { - ConcurrentDictionary? loggers = null; - - if (_loggers != null) - { - loggers = new ConcurrentDictionary(_loggers.Select(l => new KeyValuePair(l.Key, new Logger(l.Value.Issuer, minimumLevel)))); - } + ConcurrentDictionary? loggers = CloneLoggers(_loggers, minimumLevel); var context = new LogContext(minimumLevel, this, loggers, Handlers);