Skip to content

FilteringTargetWrapper: Add support for batch writing + PostFilteringTargetWrapper: performance optimizations#3405

Merged
304NotModified merged 9 commits intoNLog:masterfrom
snakefoot:repeatedfilterwrapper
May 20, 2019
Merged

FilteringTargetWrapper: Add support for batch writing + PostFilteringTargetWrapper: performance optimizations#3405
304NotModified merged 9 commits intoNLog:masterfrom
snakefoot:repeatedfilterwrapper

Conversation

@snakefoot
Copy link
Copy Markdown
Contributor

@snakefoot snakefoot commented May 17, 2019 •
edited
Loading

And allow use of WhenRepeated as Filter:

      <target name="default" xsi:type="FilteringWrapper">
        <filter xsi:type="whenRepeated" layout="${message}" timeoutSeconds="30" action="Ignore" />
        <target name="console" xsi:type="Console" />
      </target>

Also made some optimizations to PostFilteringTargetWrapper so it also supports OptimizeBufferReuse = true.

And also "fixed" PostFilteringTargetWrapper so it also works when writing a single LogEvent, without using BufferingWrapper or AsyncWrapper (More intuitive and user-friendly)

@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch 3 times, most recently from c406e16 to 558eb0f Compare May 18, 2019 00:08
@snakefoot snakefoot changed the base branch from dev to master May 18, 2019 00:11
@snakefoot snakefoot changed the base branch from master to dev May 18, 2019 00:11
@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch from 558eb0f to 2328606 Compare May 18, 2019 00:13
@snakefoot snakefoot changed the base branch from dev to master May 18, 2019 00:13
@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch 2 times, most recently from 349b1ab to c2e4c90 Compare May 18, 2019 00:42
@codecov-io
Copy link
Copy Markdown

codecov-io commented May 18, 2019 •
edited
Loading

Codecov Report

Merging #3405 into master will increase coverage by <1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #3405    +/-   ##
=======================================
+ Coverage      80%     80%   +<1%     
=======================================
  Files         356     357     +1     
  Lines       28205   28281    +76     
  Branches     3750    3769    +19     
=======================================
+ Hits        22559   22660   +101     
+ Misses       4540    4535     -5     
+ Partials     1106    1086    -20

@304NotModified 304NotModified added this to the 4.6.4 milestone May 18, 2019
@304NotModified 304NotModified self-requested a review May 18, 2019 02:59
@304NotModified 304NotModified changed the title FilteringTargetWrapper - Add support for batch writing FilteringTargetWrapper: Add support for batch writing + PostFilteringTargetWrapper: performance optimizations May 18, 2019
@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 18, 2019

Thanks!

Is this a Breaking change? (Removal of ConditionExpression Condition)

@304NotModified 304NotModified added the breaking change Breaking API change (different to semantic change) label May 18, 2019
@304NotModified 304NotModified removed this from the 4.6.4 milestone May 18, 2019
@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 18, 2019 •
edited
Loading

Is this a Breaking change? (Removal of ConditionExpression Condition)

No breaking change. The existing property:

public ConditionExpression Condition { get; set; }

Is now forwarding into the new Filter-property using ConditionBasedFilter as wrapper

@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch 3 times, most recently from 2ed42c9 to 9324129 Compare May 18, 2019 08:03
@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 18, 2019

@304NotModified Please include for ver. 4.6.4

@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch from 9324129 to 89def84 Compare May 18, 2019 08:25
@304NotModified 304NotModified removed the breaking change Breaking API change (different to semantic change) label May 18, 2019
@304NotModified 304NotModified added this to the 4.6.4 milestone May 18, 2019
@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch from 89def84 to c59754d Compare May 19, 2019 12:35
@snakefoot snakefoot force-pushed the repeatedfilterwrapper branch from c59754d to 3cb0d55 Compare May 19, 2019 13:13
@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 19, 2019

It would be nice if we could deduplicate PostFilteringTargetWrapper.ApplyFilter and FilteringTargetWrapper.Write - any idea how?

304NotModified
304NotModified reviewed May 19, 2019
View reviewed changes
Comment thread src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs Outdated
Comment thread src/NLog/Targets/Wrappers/FilteringTargetWrapper.cs
304NotModified
304NotModified reviewed May 19, 2019
View reviewed changes
Comment thread src/NLog/Config/LoggingConfigurationParser.cs Outdated
@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 19, 2019

It would be nice if we could deduplicate PostFilteringTargetWrapper.ApplyFilter and FilteringTargetWrapper.Write - any idea how?

Made a commit

@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 19, 2019

cool,

what do you think of snakefoot#8 ? Or are lambdas bad for performance?

@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 19, 2019 •
edited
Loading

Or are lambdas bad for performance?

Yes bad for performance when doing capture. Was also doing it first (With delegate-caching). Until the other target comes with a dynamic-filter-object. Then it became a TState-parameter (Because delegate-caching was not easy).

@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 19, 2019 •
edited
Loading

OK! Added some docs and now this is perfect IMO :)

@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 19, 2019

OK! Added some docs and now this is perfect IMO :)

Also happy with the result :)

@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 19, 2019

@304NotModified Guess you need to restart the appveyor-build

@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 20, 2019

restarted 2nd time 👼

@304NotModified 304NotModified merged commit dfd3845 into NLog:master May 20, 2019
@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 20, 2019

Oops the idea was to merge this on to release/4.6.4. Anyway, canceled the build so no problem.

@snakefoot
Copy link
Copy Markdown
Contributor Author

snakefoot commented May 20, 2019

Updated the documentation: https://github.com/NLog/NLog/wiki/FilteringWrapper-target and added link from https://github.com/NLog/NLog/wiki/WhenRepeated-Filter

@304NotModified
Copy link
Copy Markdown
Member

304NotModified commented May 20, 2019 •
edited
Loading

Ah this is also a xsd update

@304NotModified 304NotModified mentioned this pull request May 20, 2019
Merged
1 task
@snakefoot snakefoot mentioned this pull request May 27, 2020
Merged
@dependabot dependabot bot mentioned this pull request Aug 1, 2025
Open
@dependabot dependabot bot mentioned this pull request Sep 2, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@304NotModified 304NotModified 304NotModified approved these changes

Assignees

No one assigned

Labels

feature performance

Projects

None yet

Milestone

4.6.4

Development

Successfully merging this pull request may close these issues.

3 participants

@snakefoot @codecov-io @304NotModified