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

FallbackGroupTarget configured from appsettings.json is not respecting order of the targets. #568

Closed
sebastianstudniczek opened this issue Feb 4, 2022 · 2 comments
Labels

Comments

@sebastianstudniczek
Copy link

sebastianstudniczek commented Feb 4, 2022

Type : Bug
NLog version: 4.7.13
NLog.Extensions.Logging version: 1.7.4
NLog.Web.AspNetCore version: not used

Platform: Net 5.0 (Console App)

Current NLog config json

{
  "Logging": {
    "LogLevel": {
      "Default": "Debug"
    },
    "NLog": {
      "IncludeScopes": true
    }
  },
  "NLog": {
    "autoReload": true,
    "throwConfigExceptions": true,
    "internalLogLevel": "Debug",
    "internalLogFile": "${basedir}/internal-nlog.txt",
    "extensions": [
      { "assembly": "NLog.Extensions.Logging" }
    ],
    "variables": {
      "var_logdir": "c:/temp"
    },
    "targets": {
      "webFallback": {
        "type": "FallbackGroup",
        "returnToFirstOnSuccess": true,
        "targets": {
          "filePrimary": {
            "type": "File",
            "fileName": "${basedir}/logs/filePrimary-${shortdate}.log"
          },
          "fileFallback": {
            "type": "File",
            "fileName": "${basedir}/logs/fileFallback-${shortdate}.log"
          }
        }
      }
    },
    "rules": [
      {
        "logger": "*",
        "minLevel": "Debug",
        "writeTo": "webFallback"
      }
    ]
  }
}
  • What is the current result?
    No matter what is the order of targets in FallbackGroupTarget, fileFallback is configured as first, therefore it's not working as expected.
    appsettings.json Copy To Ouptut Directory is set to "Copy always".

  • What is the expected result?
    Configuration should respect order of the targets in FallbackGroupTarget in order to work properly.

  • Did you checked the Internal log?
    Yes, I checked it at the Debug level. No errors were provided there.

  • Please post full exception details (message, stacktrace, inner exceptions)
    From Logger.Factory -> ConfigurationChanged = {Method = Internal Error evaluating expression}

  • Are there any work arrounds? Partially yes
    Based on provided example, if you change the name of fileFallback to eg. fileTest, it's working as expected. But right now if you switch order of those two targets, filePrimary is configured as the first one.

  • Is there a version in which it did worked?
    Only with configuration from xml config file.

  • Can you help us by writing an unit test?
    Based on provided config example.

[Fact]
public void ShouldSetFilePrimaryTargetAsFirstFallbackGroupTarget() {
    const string expectedFirstTargetName = "filePrimary";
    IConfigurationRoot config = new ConfigurationBuilder()
        .SetBasePath(Directory.GetCurrentDirectory())
        .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
        .Build();

    LogManager.Configuration = new NLogLoggingConfiguration(config.GetSection("NLog"));
    Logger logger = LogManager.GetCurrentClassLogger();

    var fallbackGroupTarget = logger.Factory
        .Configuration
        .AllTargets
        .First(target => target.GetType() == typeof(FallbackGroupTarget))
        as FallbackGroupTarget;
    Target firstTarget = fallbackGroupTarget.Targets.First();
    Assert.Equal(expectedFirstTargetName, firstTarget.Name);
}
@snakefoot
Copy link
Contributor

snakefoot commented Feb 4, 2022

Yes that comes as a hidden surprise for many, that json-dictionaries in appsettings.json actually are sorted string-arrays.

And in the sorted-string-array then fileFallback comes before filePrimary.

If you changed the target-names to 1stFile and 2ndFile then you would get the expected behavior.

So the ordered behavior from xml-config is not available with json-config, because of how Microsoft-Extension-Configuration have implemented json-dictionary.

@sebastianstudniczek
Copy link
Author

Ok, now I get it. Thank for the response and clarification.

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

No branches or pull requests

2 participants