Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ILoggingBuilder AddNLog extension method with correct dispose and no double logging #325

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Sep 3, 2019

Resolves #324 for ILoggingBuilder. But will NOT help #254 for ILoggerFactory.

Fixes lifetime issue found in NLog/NLog#3284 where calling ILoggingBuilder.AddNLog caused NLogLoggingProvider not being disposed correctly on application shutdown (No flush and shutdown).

@snakefoot
Copy link
Contributor Author

Should probably also make a PR for NLog.Web.AspNetCore so it will also work correctly there.

@codecov-io
Copy link

codecov-io commented Sep 3, 2019

Codecov Report

Merging #325 into master will increase coverage by 0.5%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #325     +/-   ##
=========================================
+ Coverage   81.05%   81.56%   +0.5%     
=========================================
  Files          12       12             
  Lines        1098     1101      +3     
  Branches      193      194      +1     
=========================================
+ Hits          890      898      +8     
+ Misses        143      139      -4     
+ Partials       65       64      -1
Impacted Files Coverage Δ
...tensions.Hosting/Extensions/ConfigureExtensions.cs 93.1% <100%> (+1.1%) ⬆️
...tensions.Logging/Extensions/ConfigureExtensions.cs 31.14% <50%> (+0.5%) ⬆️
...g.Extensions.Logging/Logging/NLogLoggerProvider.cs 95.77% <0%> (+5.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d617f90...e8543ed. Read the comment docs.

@snakefoot snakefoot force-pushed the LoggerBuilderAddNLogLifetime branch from 5def637 to e8543ed Compare September 4, 2019 21:38
@snakefoot
Copy link
Contributor Author

snakefoot commented Sep 4, 2019

@304NotModified Milestone has to be 1.5.4. See also: https://www.nuget.org/packages/NLog.Extensions.Logging/1.5.3

@304NotModified 304NotModified removed this from the 1.5.3 milestone Sep 5, 2019
@304NotModified 304NotModified added this to the 1.5.4 milestone Sep 5, 2019
@304NotModified
Copy link
Member

@304NotModified Milestone has to be 1.5.4. See also: https://www.nuget.org/packages/NLog.Extensions.Logging/1.5.3

Sharp! Thanks! 1.5.3 milestone/github release fixed!

@snakefoot
Copy link
Contributor Author

Created NLog/NLog.Web#459 for NLog.Web.AspNetCore (waits for the 1.5.4 release)

@304NotModified
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If UseNLog and AddNLog are used, two logs will be written
3 participants