serilog-settings-configuration icon indicating copy to clipboard operation
serilog-settings-configuration copied to clipboard

`params` arguments should be regarded as optional

Open sreejith-kulamgarath opened this issue 1 year ago • 7 comments

The following code was working with the previous version of TestCorrelator (3.2.0), but it is failing with v4.

Here is the code.

var configuration = new ConfigurationBuilder()
            .SetBasePath(Directory.GetCurrentDirectory())
            .AddJsonFile(path: "appsettings.Test.json", optional: false, reloadOnChange: true)
            .Build();

const string message = "An event.";
using var context = TestCorrelator.CreateContext();

Log = new LoggerConfiguration().ReadFrom.Configuration(configuration).CreateLogger();

// Act
Log.Information(message);

// Assert
var logEvent = TestCorrelator.GetLogEventsFromCurrentContext().Single();
Assert.Equal(message, logEvent.MessageTemplate.Text);

If I replace ReadFrom.Configuration with this line, the code above works.

Log = new LoggerConfiguration().WriteTo.TestCorrelator().CreateLogger();

Here is my appsettings.Test.json

{
  "Serilog": {
    "Using": [
      "Serilog.Sinks.TestCorrelator"
    ],
    "MinimumLevel": "Debug",
    "WriteTo": [
      {
        "Name": "TestCorrelator",
      }
    ],
    "Enrich": [
      "FromLogContext"
    ],
    "Properties": {
      "Application": "Logging.Tests"
    }
  }
}

sreejith-kulamgarath avatar Oct 22 '24 19:10 sreejith-kulamgarath

Thanks for your message. If the problem is a change is between versions of TestCorrelator, this is best asked over in that project: https://github.com/MitchBodmer/serilog-sinks-testcorrelator. Thanks!

nblumhardt avatar Oct 22 '24 20:10 nblumhardt

I have posted it there now. Let me see if they push it back here :)

sreejith-kulamgarath avatar Oct 22 '24 21:10 sreejith-kulamgarath

For the record, changing the appsettings file as below worked.

"WriteTo": [
      {
        "Name": "TestCorrelator",
        "Args": {
          "ids": []
        }
      }
    ],

sreejith-kulamgarath avatar Oct 22 '24 22:10 sreejith-kulamgarath

@nblumhardt I think this is probably a "bug" in the configuration library, or more specifically a missing feature. It seems (from my quick browsing) that this is happening because the config library doesn't determine that if the parameter is a params array it shouldn't require a value.

I think the fix is to add a check for the ParamArrayAttribute and then provide an empty array of the correct type if one isn't provided in this section.

MitchBodmer avatar Oct 22 '24 22:10 MitchBodmer

Thanks @MitchBodmer! 👍

nblumhardt avatar Oct 22 '24 22:10 nblumhardt

This is also the likely cause of Serilog 4.1's WriteTo.Fallback not being callable via configuration.

See: https://github.com/serilog/serilog/pull/2108#issuecomment-2428268999 via @ariegato

nblumhardt avatar Oct 25 '24 05:10 nblumhardt

@nblumhardt I think it has to do with the fact that FallbackChain is not an extension method e.g. static.

I guess that the WriteTo FallbackChain acts as a sort of Sink, but is not configured the same way as normal sinks. The solution could be to add the extension method in the main library. Not sure that this will fix the params issue.

ArieGato avatar Oct 25 '24 11:10 ArieGato