From c2e4c90fef1ff638e334be0d062a096f7e362f46 Mon Sep 17 00:00:00 2001 From: Rolf Kristensen Date: Sat, 18 May 2019 02:13:02 +0200 Subject: [PATCH] FilteringTargetWrapper - Add support for batch writing and use of whenRepeated as Filter --- src/NLog/Config/LoggingConfigurationParser.cs | 29 ++++++ src/NLog/Filters/ConditionBasedFilter.cs | 4 +- .../Wrappers/FilteringTargetWrapper.cs | 92 ++++++++++++++++--- .../Wrappers/PostFilteringTargetWrapper.cs | 77 +++++++++------- tests/NLog.UnitTests/Targets/TargetTests.cs | 4 +- .../Wrappers/FilteringTargetWrapperTests.cs | 32 +++++++ tools/MakeNLogXSD/XsdFileGenerator.cs | 3 + 7 files changed, 194 insertions(+), 47 deletions(-) diff --git a/src/NLog/Config/LoggingConfigurationParser.cs b/src/NLog/Config/LoggingConfigurationParser.cs index a193162a42..fb2bceb595 100644 --- a/src/NLog/Config/LoggingConfigurationParser.cs +++ b/src/NLog/Config/LoggingConfigurationParser.cs @@ -1021,6 +1021,11 @@ private void SetPropertyFromElement(object o, ILoggingConfigurationElement eleme return; } + if (SetFilterFromElement(o, propInfo, element)) + { + return; + } + SetItemFromElement(o, propInfo, element); } @@ -1095,6 +1100,30 @@ private Layout TryCreateLayoutInstance(ILoggingConfigurationElement element, Typ return _configurationItemFactory.Layouts.CreateInstance(ExpandSimpleVariables(layoutTypeName)); } + private bool SetFilterFromElement(object o, PropertyInfo propInfo, ILoggingConfigurationElement layoutElement) + { + // Check if it is a Layout + if (!typeof(Filter).IsAssignableFrom(propInfo.PropertyType)) + return false; + + // Check if the 'type' attribute has been specified + string filterTypeName = GetConfigItemTypeAttribute(layoutElement); + if (filterTypeName == null) + return false; + + Filter filter = _configurationItemFactory.Filters.CreateInstance(ExpandSimpleVariables(filterTypeName)); + // and is a Layout and 'type' attribute has been specified + if (filter != null) + { + ConfigureObjectFromAttributes(filter, layoutElement, true); + ConfigureObjectFromElement(filter, layoutElement); + propInfo.SetValue(o, filter, null); + return true; + } + + return false; + } + private void SetItemFromElement(object o, PropertyInfo propInfo, ILoggingConfigurationElement element) { object item = propInfo.GetValue(o, null); diff --git a/src/NLog/Filters/ConditionBasedFilter.cs b/src/NLog/Filters/ConditionBasedFilter.cs index 5b6be277e9..a629008e8f 100644 --- a/src/NLog/Filters/ConditionBasedFilter.cs +++ b/src/NLog/Filters/ConditionBasedFilter.cs @@ -55,6 +55,8 @@ public class ConditionBasedFilter : Filter [RequiredParameter] public ConditionExpression Condition { get; set; } + internal FilterResult DefaultFilterResult { get; set; } = FilterResult.Neutral; + /// /// Checks whether log event should be logged or not. /// @@ -72,7 +74,7 @@ protected override FilterResult Check(LogEventInfo logEvent) return Action; } - return FilterResult.Neutral; + return DefaultFilterResult; } } } diff --git a/src/NLog/Targets/Wrappers/FilteringTargetWrapper.cs b/src/NLog/Targets/Wrappers/FilteringTargetWrapper.cs index 0e8404f6e0..ffeb5bc863 100644 --- a/src/NLog/Targets/Wrappers/FilteringTargetWrapper.cs +++ b/src/NLog/Targets/Wrappers/FilteringTargetWrapper.cs @@ -33,11 +33,11 @@ namespace NLog.Targets.Wrappers { - using System; - using Common; - using Conditions; - using Config; - using Internal; + using System.Collections.Generic; + using NLog.Common; + using NLog.Conditions; + using NLog.Config; + using NLog.Filters; /// /// Filters log entries based on a condition. @@ -59,8 +59,6 @@ namespace NLog.Targets.Wrappers [Target("FilteringWrapper", IsWrapper = true)] public class FilteringTargetWrapper : WrapperTargetBase { - private static readonly object boxedBooleanTrue = true; - /// /// Initializes a new instance of the class. /// @@ -90,7 +88,6 @@ public FilteringTargetWrapper(Target wrappedTarget, ConditionExpression conditio { WrappedTarget = wrappedTarget; Condition = condition; - OptimizeBufferReuse = GetType() == typeof(FilteringTargetWrapper); // Class not sealed, reduce breaking changes } /// @@ -98,8 +95,25 @@ public FilteringTargetWrapper(Target wrappedTarget, ConditionExpression conditio /// to the wrapped target. /// /// + public ConditionExpression Condition { get => (Filter as ConditionBasedFilter)?.Condition; set => Filter = value != null ? new ConditionBasedFilter() { Condition = value, DefaultFilterResult = FilterResult.Ignore } : null; } + + /// + /// Gets or sets the filter. Log events who evaluates to will be discarded + /// + /// [RequiredParameter] - public ConditionExpression Condition { get; set; } + public Filter Filter { get; set; } + + /// + protected override void InitializeTarget() + { + base.InitializeTarget(); + + if (!OptimizeBufferReuse && WrappedTarget != null && WrappedTarget.OptimizeBufferReuse) + { + OptimizeBufferReuse = GetType() == typeof(FilteringTargetWrapper); // Class not sealed, reduce breaking changes + } + } /// /// Checks the condition against the passed log event. @@ -109,8 +123,7 @@ public FilteringTargetWrapper(Target wrappedTarget, ConditionExpression conditio /// Log event. protected override void Write(AsyncLogEventInfo logEvent) { - object v = Condition.Evaluate(logEvent.LogEvent); - if (boxedBooleanTrue.Equals(v)) + if (FilterIncludeLogEvent(logEvent.LogEvent)) { WrappedTarget.WriteAsyncLogEvent(logEvent); } @@ -119,5 +132,62 @@ protected override void Write(AsyncLogEventInfo logEvent) logEvent.Continuation(null); } } + + /// + protected override void Write(IList logEvents) + { + bool hasIgnoredLogEvents = false; + IList filterLogEvents = null; + AsyncLogEventInfo filterLogEvent = default(AsyncLogEventInfo); + for (int i = 0; i < logEvents.Count; ++i) + { + if (FilterIncludeLogEvent(logEvents[i].LogEvent)) + { + if (hasIgnoredLogEvents && filterLogEvents == null) + { + if (filterLogEvent.LogEvent != null) + { + filterLogEvents = new List(); + filterLogEvents.Add(filterLogEvent); + filterLogEvent = default(AsyncLogEventInfo); + } + else + { + filterLogEvent = logEvents[i]; + } + } + + if (filterLogEvents != null) + filterLogEvents.Add(logEvents[i]); + } + else + { + if (!hasIgnoredLogEvents && i > 0) + { + filterLogEvents = new List(); + for (int j = 0; j < i; ++j) + filterLogEvents.Add(logEvents[j]); + } + hasIgnoredLogEvents = true; + logEvents[i].Continuation(null); + } + } + + if (!hasIgnoredLogEvents) + WrappedTarget.WriteAsyncLogEvents(logEvents); + else if (filterLogEvents != null) + WrappedTarget.WriteAsyncLogEvents(filterLogEvents); + else if (filterLogEvent.LogEvent != null) + WrappedTarget.WriteAsyncLogEvent(filterLogEvent); + } + + private bool FilterIncludeLogEvent(LogEventInfo logEvent) + { + var filterResult = Filter.GetFilterResult(logEvent); + if (filterResult == FilterResult.Ignore || filterResult == FilterResult.IgnoreFinal) + return false; + else + return true; + } } } diff --git a/src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs b/src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs index f027e76bb0..6fc3851662 100644 --- a/src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs +++ b/src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs @@ -35,10 +35,10 @@ namespace NLog.Targets.Wrappers { using System; using System.Collections.Generic; - using Common; - using Conditions; - using Config; - using Internal; + using NLog.Common; + using NLog.Conditions; + using NLog.Config; + using NLog.Internal; /// /// Filters buffered log entries based on a set of conditions that are evaluated on a group of events. @@ -75,18 +75,17 @@ public class PostFilteringTargetWrapper : WrapperTargetBase /// /// Initializes a new instance of the class. /// - public PostFilteringTargetWrapper() : this(null) + public PostFilteringTargetWrapper() + : this(null) { - Rules = new List(); } /// /// Initializes a new instance of the class. /// public PostFilteringTargetWrapper(Target wrappedTarget) + : this(null, wrappedTarget) { - Rules = new List(); - WrappedTarget = wrappedTarget; } /// @@ -95,9 +94,10 @@ public PostFilteringTargetWrapper(Target wrappedTarget) /// Name of the target. /// The wrapped target. public PostFilteringTargetWrapper(string name, Target wrappedTarget) - : this(wrappedTarget) { Name = name; + WrappedTarget = wrappedTarget; + Rules = new List(); } /// @@ -115,6 +115,17 @@ public PostFilteringTargetWrapper(string name, Target wrappedTarget) [ArrayParameter(typeof(FilteringRule), "when")] public IList Rules { get; private set; } + /// + protected override void InitializeTarget() + { + base.InitializeTarget(); + + if (!OptimizeBufferReuse && WrappedTarget != null && WrappedTarget.OptimizeBufferReuse) + { + OptimizeBufferReuse = GetType() == typeof(PostFilteringTargetWrapper); // Class not sealed, reduce breaking changes + } + } + /// /// NOTE! Obsolete, instead override Write(IList{AsyncLogEventInfo} logEvents) /// @@ -138,12 +149,9 @@ protected override void Write(AsyncLogEventInfo[] logEvents) /// Array of log events to be post-filtered. protected override void Write(IList logEvents) { - - InternalLogger.Trace("PostFilteringWrapper(Name={0}): Running on {1} events", Name, logEvents.Count); var resultFilter = EvaluateAllRules(logEvents) ?? DefaultFilter; - if (resultFilter == null) { WrappedTarget.WriteAsyncLogEvents(logEvents); @@ -151,9 +159,7 @@ protected override void Write(IList logEvents) else { InternalLogger.Trace("PostFilteringWrapper(Name={0}): Filter to apply: {1}", Name, resultFilter); - var resultBuffer = ApplyFilter(logEvents, resultFilter); - InternalLogger.Trace("PostFilteringWrapper(Name={0}): After filtering: {1} events.", Name, resultBuffer.Count); if (resultBuffer.Count > 0) { @@ -169,25 +175,38 @@ protected override void Write(IList logEvents) /// /// /// - private static List ApplyFilter(IList logEvents, ConditionExpression resultFilter) + private static IList ApplyFilter(IList logEvents, ConditionExpression resultFilter) { - var resultBuffer = new List(); - + bool hasIgnoredLogEvents = false; + IList resultBuffer = null; for (int i = 0; i < logEvents.Count; ++i) { object v = resultFilter.Evaluate(logEvents[i].LogEvent); if (boxedTrue.Equals(v)) { - resultBuffer.Add(logEvents[i]); + if (hasIgnoredLogEvents && resultBuffer == null) + { + resultBuffer = new List(); + } + + if (resultBuffer != null) + resultBuffer.Add(logEvents[i]); } else { + if (!hasIgnoredLogEvents && i > 0) + { + resultBuffer = new List(); + for (int j = 0; j < i; ++j) + resultBuffer.Add(logEvents[j]); + } + hasIgnoredLogEvents = true; // anything not passed down will be notified about successful completion logEvents[i].Continuation(null); } } - return resultBuffer; + return resultBuffer ?? (hasIgnoredLogEvents ? ArrayHelper.Empty() : logEvents); } /// @@ -197,30 +216,24 @@ private static List ApplyFilter(IList logE /// private ConditionExpression EvaluateAllRules(IList logEvents) { - ConditionExpression resultFilter = null; - + if (Rules.Count == 0) + return null; + for (int i = 0; i < logEvents.Count; ++i) { - foreach (FilteringRule rule in Rules) + for (int j = 0; j < Rules.Count; ++j) { + var rule = Rules[j]; object v = rule.Exists.Evaluate(logEvents[i].LogEvent); - if (boxedTrue.Equals(v)) { InternalLogger.Trace("PostFilteringWrapper(Name={0}): Rule matched: {1}", Name, rule.Exists); - - resultFilter = rule.Filter; - break; + return rule.Filter; } } - - if (resultFilter != null) - { - break; - } } - return resultFilter; + return null; } } } diff --git a/tests/NLog.UnitTests/Targets/TargetTests.cs b/tests/NLog.UnitTests/Targets/TargetTests.cs index b9da4143ff..9f6e0f6b16 100644 --- a/tests/NLog.UnitTests/Targets/TargetTests.cs +++ b/tests/NLog.UnitTests/Targets/TargetTests.cs @@ -84,8 +84,6 @@ public void TargetContructorWithNameTest() var args = new List { fileTarget }; - - //default ctor var defaultConstructedTarget = (WrapperTargetBase)Activator.CreateInstance(targetType); defaultConstructedTarget.Name = name; @@ -94,7 +92,7 @@ public void TargetContructorWithNameTest() //specials cases if (targetType == typeof(FilteringTargetWrapper)) { - var cond = new ConditionLoggerNameExpression(); + ConditionLoggerNameExpression cond = null; args.Add(cond); var target = (FilteringTargetWrapper) defaultConstructedTarget; target.Condition = cond; diff --git a/tests/NLog.UnitTests/Targets/Wrappers/FilteringTargetWrapperTests.cs b/tests/NLog.UnitTests/Targets/Wrappers/FilteringTargetWrapperTests.cs index b69ea31ea5..2d6b1ffc9b 100644 --- a/tests/NLog.UnitTests/Targets/Wrappers/FilteringTargetWrapperTests.cs +++ b/tests/NLog.UnitTests/Targets/Wrappers/FilteringTargetWrapperTests.cs @@ -37,6 +37,7 @@ namespace NLog.UnitTests.Targets.Wrappers using System.Threading; using NLog.Common; using NLog.Conditions; + using NLog.Config; using NLog.Targets; using NLog.Targets.Wrappers; using Xunit; @@ -267,6 +268,37 @@ public void FilteringTargetWrapperAsyncWithExceptionTest2() Assert.Equal(2, myMockCondition.CallCount); } + [Fact] + public void FilteringTargetWrapperWhenRepeatedFilter() + { + var myTarget = new MyTarget(); + var wrapper = new FilteringTargetWrapper + { + WrappedTarget = myTarget, + Filter = new NLog.Filters.WhenRepeatedFilter() { Layout = "${level}${message}", Action = NLog.Filters.FilterResult.Ignore }, + }; + var bufferingWrapper = new BufferingTargetWrapper + { + WrappedTarget = wrapper, + }; + var logFactory = new LogFactory(); + var loggingConfiguration = new LoggingConfiguration(logFactory); + loggingConfiguration.AddRuleForAllLevels(bufferingWrapper); + logFactory.Configuration = loggingConfiguration; + var logger = logFactory.GetLogger(nameof(FilteringTargetWrapperWhenRepeatedFilter)); + logger.Info("Hello World"); + logger.Info("Hello World"); // Will be ignored + logger.Info("Goodbye World"); + logger.Warn("Goodbye World"); + logFactory.Flush(); + Assert.Equal(3, myTarget.WriteCount); + logger.Info("Hello World"); // Will be ignored + logger.Error("Goodbye World"); + logger.Fatal("Goodbye World"); + logFactory.Flush(); + Assert.Equal(5, myTarget.WriteCount); + } + class MyAsyncTarget : Target { public int WriteCount { get; private set; } diff --git a/tools/MakeNLogXSD/XsdFileGenerator.cs b/tools/MakeNLogXSD/XsdFileGenerator.cs index 26bc2d6460..f9069e47e5 100644 --- a/tools/MakeNLogXSD/XsdFileGenerator.cs +++ b/tools/MakeNLogXSD/XsdFileGenerator.cs @@ -312,6 +312,9 @@ private static string GetXsdType(string apiTypeName, bool attribute) case "Layout": return attribute ? "SimpleLayoutAttribute" : "Layout"; + case "NLog.Filters.Filter": + return "Filter"; + case "Condition": return "Condition";