From 130fcebe3067417ce2c8be86a957ec00da0104b1 Mon Sep 17 00:00:00 2001 From: Rolf Kristensen Date: Tue, 28 May 2019 08:04:22 +0200 Subject: [PATCH] Added "Properties" property on Logger for reading and editing properties. (#3430) * Logger - Properties added using WithProperty can now be inspected safely * small code improvement --- src/NLog/Config/LoggingConfiguration.cs | 7 +- src/NLog/GlobalDiagnosticsContext.cs | 9 +- src/NLog/Internal/ThreadSafeDictionary.cs | 230 ++++++++++++++++++++++ src/NLog/Logger.cs | 66 ++++--- 4 files changed, 278 insertions(+), 34 deletions(-) create mode 100644 src/NLog/Internal/ThreadSafeDictionary.cs diff --git a/src/NLog/Config/LoggingConfiguration.cs b/src/NLog/Config/LoggingConfiguration.cs index 94b47b5585..fe3817b5f8 100644 --- a/src/NLog/Config/LoggingConfiguration.cs +++ b/src/NLog/Config/LoggingConfiguration.cs @@ -61,7 +61,7 @@ public class LoggingConfiguration /// /// Variables defined in xml or in API. name is case case insensitive. /// - private readonly Dictionary _variables = new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly ThreadSafeDictionary _variables = new ThreadSafeDictionary(StringComparer.OrdinalIgnoreCase); /// /// Gets the factory that will be configured @@ -812,10 +812,7 @@ private List GetSupportsInitializes(bool reverse = false) /// Master variables dictionary internal void CopyVariables(IDictionary masterVariables) { - foreach (var variable in masterVariables) - { - Variables[variable.Key] = variable.Value; - } + _variables.CopyFrom(masterVariables); } /// diff --git a/src/NLog/GlobalDiagnosticsContext.cs b/src/NLog/GlobalDiagnosticsContext.cs index d8b0488e30..93cfda99ce 100644 --- a/src/NLog/GlobalDiagnosticsContext.cs +++ b/src/NLog/GlobalDiagnosticsContext.cs @@ -43,6 +43,7 @@ namespace NLog /// public static class GlobalDiagnosticsContext { + private static readonly object _lockObject = new object(); private static Dictionary _dict = new Dictionary(); private static Dictionary _dictReadOnly; // Reset cache on change @@ -63,7 +64,7 @@ public static void Set(string item, string value) /// Item value. public static void Set(string item, object value) { - lock (_dict) + lock (_lockObject) { bool requireCopyOnWrite = _dictReadOnly != null && !_dict.ContainsKey(item); // Overwiting existing value is ok (no resize) GetWritableDict(requireCopyOnWrite)[item] = value; @@ -129,7 +130,7 @@ public static bool Contains(string item) /// Item name. public static void Remove(string item) { - lock (_dict) + lock (_lockObject) { bool requireCopyOnWrite = _dictReadOnly != null && _dict.ContainsKey(item); GetWritableDict(requireCopyOnWrite).Remove(item); @@ -141,7 +142,7 @@ public static void Remove(string item) /// public static void Clear() { - lock (_dict) + lock (_lockObject) { bool requireCopyOnWrite = _dictReadOnly != null && _dict.Count > 0; GetWritableDict(requireCopyOnWrite, true).Clear(); @@ -153,7 +154,7 @@ public static void Clear() var readOnly = _dictReadOnly; if (readOnly == null) { - lock (_dict) + lock (_lockObject) { readOnly = _dictReadOnly = _dict; } diff --git a/src/NLog/Internal/ThreadSafeDictionary.cs b/src/NLog/Internal/ThreadSafeDictionary.cs new file mode 100644 index 0000000000..811576600c --- /dev/null +++ b/src/NLog/Internal/ThreadSafeDictionary.cs @@ -0,0 +1,230 @@ +// +// Copyright (c) 2004-2019 Jaroslaw Kowalski , Kim Christensen, Julian Verdurmen +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions +// are met: +// +// * Redistributions of source code must retain the above copyright notice, +// this list of conditions and the following disclaimer. +// +// * Redistributions in binary form must reproduce the above copyright notice, +// this list of conditions and the following disclaimer in the documentation +// and/or other materials provided with the distribution. +// +// * Neither the name of Jaroslaw Kowalski nor the names of its +// contributors may be used to endorse or promote products derived from this +// software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF +// THE POSSIBILITY OF SUCH DAMAGE. +// + +namespace NLog.Internal +{ + using System; + using System.Collections; + using System.Collections.Generic; + using System.Linq; + + internal class ThreadSafeDictionary : IDictionary + { + private readonly object _lockObject = new object(); + private readonly IEqualityComparer _comparer; + private Dictionary _dict; + private Dictionary _dictReadOnly; // Reset cache on change + + public ThreadSafeDictionary() + :this(EqualityComparer.Default) + { + } + + public ThreadSafeDictionary(IEqualityComparer comparer) + { + _comparer = comparer; + _dict = new Dictionary(_comparer); + } + + public ThreadSafeDictionary(ThreadSafeDictionary source) + { + _comparer = source._comparer; + _dict = source.GetReadOnlyDict(); + GetWritableDict(); // Clone + } + + public TValue this[TKey key] + { + get => GetReadOnlyDict()[key]; + set + { + lock (_lockObject) + { + GetWritableDict()[key] = value; + } + } + } + + public ICollection Keys => GetReadOnlyDict().Keys; + + public ICollection Values => GetReadOnlyDict().Values; + + public int Count => GetReadOnlyDict().Count; + + public bool IsReadOnly => false; + + public void Add(TKey key, TValue value) + { + lock (_lockObject) + { + GetWritableDict().Add(key, value); + } + } + + public void Add(KeyValuePair item) + { + lock (_lockObject) + { + GetWritableDict().Add(item.Key, item.Value); + } + } + + public void Clear() + { + lock (_lockObject) + { + GetWritableDict(true); + } + } + + public bool Contains(KeyValuePair item) + { + return GetReadOnlyDict().Contains(item); + } + + public bool ContainsKey(TKey key) + { + return GetReadOnlyDict().ContainsKey(key); + } + + public void CopyTo(KeyValuePair[] array, int arrayIndex) + { + ((IDictionary)GetReadOnlyDict()).CopyTo(array, arrayIndex); + } + + public void CopyFrom(IDictionary source) + { + if (!ReferenceEquals(this, source) && source?.Count > 0) + { + lock (_lockObject) + { + var destDict = GetWritableDict(); + foreach (var item in source) + destDict[item.Key] = item.Value; + } + } + } + + public bool Remove(TKey key) + { + lock (_lockObject) + { + return GetWritableDict().Remove(key); + } + } + + public bool Remove(KeyValuePair item) + { + lock (_lockObject) + { + return GetWritableDict().Remove(item); + } + } + + public bool TryGetValue(TKey key, out TValue value) + { + return GetReadOnlyDict().TryGetValue(key, out value); + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + return GetReadOnlyDict().GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetReadOnlyDict().GetEnumerator(); + } + + public Enumerator GetEnumerator() + { + return new Enumerator(GetReadOnlyDict().GetEnumerator()); + } + + public struct Enumerator : IEnumerator> + { + Dictionary.Enumerator _enumerator; + + public Enumerator(Dictionary.Enumerator enumerator) + { + _enumerator = enumerator; + } + + public KeyValuePair Current => _enumerator.Current; + + object IEnumerator.Current => _enumerator.Current; + + public void Dispose() + { + _enumerator.Dispose(); + } + + public bool MoveNext() + { + return _enumerator.MoveNext(); + } + + void IEnumerator.Reset() + { + ((IEnumerator)_enumerator).Reset(); + } + } + + private Dictionary GetReadOnlyDict() + { + var readOnly = _dictReadOnly; + if (readOnly == null) + { + lock (_lockObject) + { + readOnly = _dictReadOnly = _dict; + } + } + return readOnly; + } + + private IDictionary GetWritableDict(bool clearDictionary = false) + { + var newDict = new Dictionary(clearDictionary ? 0 : _dict.Count + 1, _comparer); + if (!clearDictionary) + { + // Less allocation with enumerator than Dictionary-constructor + foreach (var item in _dict) + newDict[item.Key] = item.Value; + } + _dict = newDict; + _dictReadOnly = null; + return newDict; + } + } +} diff --git a/src/NLog/Logger.cs b/src/NLog/Logger.cs index 4f4a693d29..ca727f952c 100644 --- a/src/NLog/Logger.cs +++ b/src/NLog/Logger.cs @@ -50,7 +50,7 @@ public partial class Logger : ILogger { internal static readonly Type DefaultLoggerType = typeof(Logger); private Logger _contextLogger; - private Dictionary _contextProperties; + private ThreadSafeDictionary _contextProperties; private LoggerConfiguration _configuration; private volatile bool _isTraceEnabled; private volatile bool _isDebugEnabled; @@ -82,6 +82,15 @@ protected internal Logger() /// public LogFactory Factory { get; private set; } + /// + /// Collection of context properties for the Logger. The logger will append it for all log events + /// + /// + /// It is recommended to use for modifying context properties + /// when same named logger is used at multiple locations or shared by different thread contexts. + /// + public IDictionary Properties => _contextProperties ?? System.Threading.Interlocked.CompareExchange(ref _contextProperties, CreateContextPropertiesDictionary(null), null) ?? _contextProperties; + /// /// Gets a value indicating whether logging is enabled for the specified level. /// @@ -110,34 +119,41 @@ public Logger WithProperty(string propertyKey, object propertyValue) Logger newLogger = Factory.CreateNewLogger(GetType()) ?? new Logger(); newLogger.Initialize(Name, _configuration, Factory); - newLogger._contextProperties = CopyOnWrite(propertyKey, propertyValue); + newLogger._contextProperties = CreateContextPropertiesDictionary(_contextProperties); + newLogger._contextProperties[propertyKey] = propertyValue; newLogger._contextLogger = _contextLogger; // Use the LoggerConfiguration of the parent Logger return newLogger; } /// - /// Updates the specified context property for the current logger. The logger will append it for all log events + /// Updates the specified context property for the current logger. The logger will append it for all log events. + /// + /// It could be rendered with ${event-properties:YOURNAME} + /// + /// With property, all properties could be changed. /// /// /// Will affect all locations/contexts that makes use of the same named logger object. /// /// Property Name /// Property Value + /// + /// It is recommended to use for modifying context properties + /// when same named logger is used at multiple locations or shared by different thread contexts. + /// public void SetProperty(string propertyKey, object propertyValue) { if (string.IsNullOrEmpty(propertyKey)) throw new ArgumentException(nameof(propertyKey)); - _contextProperties = CopyOnWrite(propertyKey, propertyValue); + Properties[propertyKey] = propertyValue; } - private Dictionary CopyOnWrite(string propertyKey, object propertyValue) + private static ThreadSafeDictionary CreateContextPropertiesDictionary(ThreadSafeDictionary contextProperties) { - var contextProperties = _contextProperties; contextProperties = contextProperties != null - ? new Dictionary(contextProperties) - : new Dictionary(); - contextProperties[propertyKey] = propertyValue; + ? new ThreadSafeDictionary(contextProperties) + : new ThreadSafeDictionary(); return contextProperties; } @@ -246,10 +262,10 @@ public void LogException(LogLevel level, [Localizable(false)] string message, Ex /// Arguments to format. [MessageTemplateFormatMethod("message")] public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(false)] string message, params object[] args) - { + { if (IsEnabled(level)) { - WriteToTargets(level, formatProvider, message, args); + WriteToTargets(level, formatProvider, message, args); } } @@ -258,8 +274,8 @@ public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(fal /// /// The log level. /// Log message. - public void Log(LogLevel level, [Localizable(false)] string message) - { + public void Log(LogLevel level, [Localizable(false)] string message) + { if (IsEnabled(level)) { WriteToTargets(level, null, message); @@ -273,8 +289,8 @@ public void Log(LogLevel level, [Localizable(false)] string message) /// A containing format items. /// Arguments to format. [MessageTemplateFormatMethod("message")] - public void Log(LogLevel level, [Localizable(false)] string message, params object[] args) - { + public void Log(LogLevel level, [Localizable(false)] string message, params object[] args) + { if (IsEnabled(level)) { WriteToTargets(level, message, args); @@ -340,10 +356,10 @@ public void Log(LogLevel level, Exception exception, IFormatProvider formatProvi /// The argument to format. [MessageTemplateFormatMethod("message")] public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(false)] string message, TArgument argument) - { + { if (IsEnabled(level)) { - WriteToTargets(level, formatProvider, message, new object[] { argument }); + WriteToTargets(level, formatProvider, message, new object[] { argument }); } } @@ -374,11 +390,11 @@ public void Log(LogLevel level, [Localizable(false)] string message, /// The first argument to format. /// The second argument to format. [MessageTemplateFormatMethod("message")] - public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(false)] string message, TArgument1 argument1, TArgument2 argument2) - { + public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(false)] string message, TArgument1 argument1, TArgument2 argument2) + { if (IsEnabled(level)) { - WriteToTargets(level, formatProvider, message, new object[] { argument1, argument2 }); + WriteToTargets(level, formatProvider, message, new object[] { argument1, argument2 }); } } @@ -393,7 +409,7 @@ public void Log(LogLevel level, [Localizable(false)] string message, /// The second argument to format. [MessageTemplateFormatMethod("message")] public void Log(LogLevel level, [Localizable(false)] string message, TArgument1 argument1, TArgument2 argument2) - { + { if (IsEnabled(level)) { WriteToTargets(level, message, new object[] { argument1, argument2 }); @@ -413,11 +429,11 @@ public void Log(LogLevel level, [Localizable(false)] string message, /// The second argument to format. /// The third argument to format. [MessageTemplateFormatMethod("message")] - public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(false)] string message, TArgument1 argument1, TArgument2 argument2, TArgument3 argument3) - { + public void Log(LogLevel level, IFormatProvider formatProvider, [Localizable(false)] string message, TArgument1 argument1, TArgument2 argument2, TArgument3 argument3) + { if (IsEnabled(level)) { - WriteToTargets(level, formatProvider, message, new object[] { argument1, argument2, argument3 }); + WriteToTargets(level, formatProvider, message, new object[] { argument1, argument2, argument3 }); } } @@ -434,7 +450,7 @@ public void Log(LogLevel level, [Localizable(false)] string message, /// The third argument to format. [MessageTemplateFormatMethod("message")] public void Log(LogLevel level, [Localizable(false)] string message, TArgument1 argument1, TArgument2 argument2, TArgument3 argument3) - { + { if (IsEnabled(level)) { WriteToTargets(level, message, new object[] { argument1, argument2, argument3 });