NLog config file loading: use process name (e.g. applicationname.exe.nlog) when app.config is not available by snakefoot · Pull Request #3261 · NLog/NLog

snakefoot · 2019-03-30T16:36:58Z

Trying to resolve #3156 for NetCore

codecov · 2019-03-30T18:09:16Z

Codecov Report

Merging #3261 into dev will increase coverage by <1%.
The diff coverage is 73%.

@@          Coverage Diff           @@
##             dev   #3261    +/-   ##
======================================
+ Coverage     80%     80%   +<1%     
======================================
  Files        356     356            
  Lines      28007   28016     +9     
  Branches    3728    3728            
======================================
+ Hits       22408   22438    +30     
+ Misses      4520    4501    -19     
+ Partials    1079    1077     -2

304NotModified · 2019-03-31T21:41:07Z

+        /// <summary>
+        /// Initializes the ThreadIDHelper class.
+        /// </summary>
+        private static ProcessIDHelper CreateInstance()


👍 much better.

FYI: I think "instance" in redundant, and most "factory methods" are called Create - so renamed it

Think the whole instance thing should be removed. But not sure about the reason for the Win32-native-call. Since the values are cached, then I don't understand remark about Win32 is "without the overhead".

snakefoot · 2019-03-31T21:55:53Z

Actually think that Win32ProcessIDHelper could be thrown away. Think the original code was this (Causing overhead by allocating Process-object on every lookup.

Process.GetCurrentProcess().Id

Without any caching. Causing it to be much slower than this code:

NativeMethods.GetCurrentProcessId()

But later caching was added so there no overhead at all for either method. Could be nice to remove some of those Win32-native-methods.

304NotModified · 2019-03-31T21:58:45Z

Could be nice to remove some of those Win32-native-methods.

Agree on that! Less code is better

snakefoot · 2019-04-02T17:07:54Z

@snakefoot Have squashed the commits, and tried to ensure correct rename operations of the files.

304NotModified

Thanks for the changes!

I've a bit s problem with the no-so-lean interface. I know it's only internal, but still it's a degrade of the current interface (IFile). IMO it should be multiple interfaces which has clear separations (Single responsibility principle).

It looks like a lot of code has been moved? It's that really preferred? (Makes review and history more difficult too track)

304NotModified · 2019-04-04T23:34:47Z

maybe we should update the docs - and this unfortunately related: #959

snakefoot · 2019-04-04T23:42:39Z

@304NotModified maybe we should update the docs

Well this PR actually tries to ensure that NetCore follows the Wiki :)

304NotModified · 2019-04-04T23:48:27Z

so supersedes #959? Isn't this a breaking behavior change?

…able

snakefoot · 2019-04-05T00:01:43Z

@304NotModified so supersedes #959? Isn't this a breaking behavior change?

No and No.

#959 actually implements the config loading so it happens in the correct order. It should always use the application-specific process.exe.nlog instead of a random Nlog.config. But this is a breaking change, so you have stalled this fix for NLog 5.0

This PR only fixes NetCore so it behaves like NetFramework. Because AppDomain.SetupInformation.ConfigurationFile is blank on NetCore-platform and needs special help.

304NotModified · 2019-04-05T00:05:32Z

Ok thanks for explaining!

FYI, I like to start with merging 5.0 PRs after 4.6.3 is done.

snakefoot · 2019-04-05T00:06:56Z

FYI, I like to start with merging 5.0 PRs after 4.6.3 is done.

Think we should keep dev and master available for more hotfixes a few more weeks before starting up on NLog 5.0. But your decision.

304NotModified · 2019-04-05T00:12:16Z

Hotfixes could be merged to master then :)

(But features will be 5.0 instead of 4.6.*)

snakefoot · 2019-04-07T10:28:39Z

@304NotModified Is the merge of this PR blocked by something?

304NotModified · 2019-04-07T13:56:48Z

Yes my free time. I try to do some developing next to reviewing. Only reviewing is boring and I'm losing some touch to NLog in that way.

304NotModified

Thanks for the changes!

304NotModified · 2019-04-07T14:22:19Z

+    /// Abstract calls to FileSystem
    /// </summary>
-    internal interface IFile
+    internal interface IFileSystem


FYI, this was not the direction I would take. I think it's better to have 10 small interfaces then 3 medium.

Also, with the introduction of NSubstitute, really small interfaces are nice.

But as it's internal I think this is more than good enough 👍

304NotModified · 2019-04-07T14:24:58Z

@304NotModified Is the merge of this PR blocked by something?

Anyway, pushing has some results ;)

304NotModified · 2019-04-07T14:25:26Z

nice results!

snakefoot · 2019-04-07T15:22:13Z

@304NotModified Yes my free time. I try to do some developing next to reviewing. Only reviewing is boring and I'm losing some touch to NLog in that way.

Very understandable. I also prefer developing over reviewing :) Will try and reduce the number of PRs for the NLog-project, so everything is not about my PRs :)

But thank you for the merge. Now there is some cleanup to do:

Refactor the remaining unit tests in ConfigFileLocatorTests.cs to use the new mock-interface instead of building exe-file (Faster and can be tested on NetCore).
Update Load order precedence fix for config files issue #223 #959 to handle the moving of code from LogFactory to LoggingConfigurationFileLoader

I don't mind fixing these two, unless they should be up-for-grabs?

snakefoot force-pushed the NetCoreNLogConfigLoading branch 6 times, most recently from f1d2f29 to 2cd7e4a Compare March 30, 2019 17:47

snakefoot force-pushed the NetCoreNLogConfigLoading branch 6 times, most recently from 0bec3dc to fb51337 Compare March 30, 2019 20:32

304NotModified self-requested a review March 30, 2019 21:34

304NotModified self-assigned this Mar 31, 2019

304NotModified reviewed Mar 31, 2019

View reviewed changes

Comment thread src/NLog/Config/LoggingConfigurationFileLoader.cs

304NotModified removed their assignment Apr 1, 2019

snakefoot force-pushed the NetCoreNLogConfigLoading branch 3 times, most recently from e5fe536 to 30c8d63 Compare April 2, 2019 17:07

304NotModified added this to the 4.6.3 milestone Apr 2, 2019

304NotModified requested changes Apr 3, 2019

View reviewed changes

Comment thread src/NLog/Config/LoggingConfigurationFileLoader.cs

Comment thread tests/NLog.UnitTests/ConfigFileLocatorTests.cs

Comment thread src/NLog/Internal/Fakeables/IAppEnvironment.cs Outdated

snakefoot force-pushed the NetCoreNLogConfigLoading branch 3 times, most recently from f633c0e to d557d53 Compare April 4, 2019 19:23

304NotModified reviewed Apr 4, 2019

View reviewed changes

304NotModified added the feature label Apr 4, 2019

NLog config file loading using process name when app.config not avail…

d0e19d6

…able

snakefoot force-pushed the NetCoreNLogConfigLoading branch from 4c64ca1 to d0e19d6 Compare April 4, 2019 23:57

304NotModified approved these changes Apr 7, 2019

View reviewed changes

304NotModified changed the title ~~NLog config file loading using process name when app.config not available~~ NLog config file loading: use process name when app.config not available Apr 7, 2019

304NotModified changed the title ~~NLog config file loading: use process name when app.config not available~~ NLog config file loading: use process name (e.g. applicationname.exe.nlog) when app.config not available Apr 7, 2019

304NotModified changed the title ~~NLog config file loading: use process name (e.g. applicationname.exe.nlog) when app.config not available~~ NLog config file loading: use process name (e.g. applicationname.exe.nlog) when app.config is not available Apr 7, 2019

304NotModified merged commit 69c0854 into NLog:dev Apr 7, 2019

304NotModified self-assigned this Apr 7, 2019

snakefoot deleted the NetCoreNLogConfigLoading branch April 4, 2020 12:58

jsm174 mentioned this pull request Jan 4, 2021

NLog crash on Android freezy/VisualPinball.Engine#277

Closed

This was referenced Jan 4, 2021

Layout with {processid} will crash with Mono runtime on Android #3939

Closed

NLog makes application crash on Android platform #4228

Closed

Skip lookup MainModule.FileName on Android platform to avoid crash #4229

Merged

snakefoot mentioned this pull request Mar 3, 2022

Configuration in applicationname.exe.nlog files do not load when single file publish is used #4818

Closed

dependabot bot mentioned this pull request Aug 1, 2025

Bump NLog and 3 others lheimbs/remote-sign-tool#7

Open

dependabot bot mentioned this pull request Sep 2, 2025

Bump NLog.Config and NLog.Schema lheimbs/remote-sign-tool#9

Open

Conversation

snakefoot commented Mar 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

304NotModified Mar 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snakefoot Mar 31, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

snakefoot commented Mar 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

304NotModified commented Mar 31, 2019

Uh oh!

snakefoot commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

304NotModified left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

304NotModified commented Apr 4, 2019

Uh oh!

snakefoot commented Apr 4, 2019

Uh oh!

304NotModified commented Apr 4, 2019

Uh oh!

snakefoot commented Apr 5, 2019

Uh oh!

304NotModified commented Apr 5, 2019

Uh oh!

snakefoot commented Apr 5, 2019

Uh oh!

304NotModified commented Apr 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snakefoot commented Apr 7, 2019

Uh oh!

304NotModified commented Apr 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

304NotModified left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

304NotModified Apr 7, 2019

Choose a reason for hiding this comment

Uh oh!

304NotModified commented Apr 7, 2019

Uh oh!

304NotModified commented Apr 7, 2019

Uh oh!

snakefoot commented Apr 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

snakefoot commented Mar 30, 2019 •

edited

Loading

codecov bot commented Mar 30, 2019 •

edited

Loading

304NotModified Mar 31, 2019 •

edited

Loading

snakefoot commented Mar 31, 2019 •

edited

Loading

snakefoot commented Apr 2, 2019 •

edited

Loading

304NotModified commented Apr 5, 2019 •

edited

Loading

304NotModified commented Apr 7, 2019 •

edited

Loading