Extension methods with IConfiguration parameter are ignored

[andriysavin]

When you define sink configuration extension method with IConfiguration parameter that method is ignored. At the same time if you use IConfigurationSection and specify value in config source, the method is discovered and the parameter is populated. Also, if default value (null) is specified for IConfiguration parameter all works as exepected as well. I believe the source of the problem lies in the code which filters methods and doesn't take into account IConfiguration parameter type:

serilog-settings-configuration/src/Serilog.Settings.Configuration/Settings/Configuration/ConfigurationReader.cs

Line 351 in 51a4fca

.All(p => p.HasDefaultValue || suppliedArgumentValues.Any(s => s.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase))))

[nblumhardt]

Thanks for the report, sounds like something was overlooked 👍

[MV10]

I don't think it's a bug in the config package.. The SQL sink relies on this to find a named connectionstring in a section outside of the Serilog-specific section. Using the dev version of the SQL sink and the 3.0.0 release of the config package, this code and config works and is probably the simplest way to reproduce this without writing your own sink:

var appConfig = new ConfigurationBuilder()
    .SetBasePath(Directory.GetCurrentDirectory())
    .AddJsonFile(path: "appsettings.json")
    .Build();

var logConfig = new LoggerConfiguration();
logConfig.ReadFrom.Configuration(appConfig);
Log.Logger = logConfig.CreateLogger();
Serilog.Debugging.SelfLog.Enable(Console.Error);

Log.Information("An Information message");

Log.CloseAndFlush();

{
  "ConnectionStrings": {
    "DevTest": "Data Source=(LocalDB)\\MSSQLLocalDB;Initial Catalog=SerilogTest;Integrated Security=true"
  },
  "Serilog": {
    "Using": [ "Serilog.Sinks.Console" ],
    "WriteTo": [
      { "Name": "Console" },
      {
        "Name": "MSSqlServer",
        "Args": {
          "connectionString": "DevTest",
          "tableName": "Issue81",
          "autoCreateSqlTable":  true,
          "columnOptionsSection": {
            "customColumns": [
              {
                "ColumnName": "EventType",
                "DataType": "int"
              },
              {
                "ColumnName": "Release",
                "DataType": "varchar",
                "DataLength": 32
              }
            ]
          }
        }
      }
    ]
  }
}

[andriysavin]

@MV10 If you look at the source code (I guess you're the author) of that sink (configuration part), you'll see it assigns a default value null to configuration parameter:

https://github.com/serilog/serilog-sinks-mssqlserver/blob/40e3919ec448f3f5d8673b1a0a9fd959b349099b/src/Serilog.Sinks.MSSqlServer/Configuration/Microsoft.Extensions.Configuration/LoggerConfigurationMSSqlServerExtensions.cs#L57

So this proves what I wrote.

[MV10]

@andriysavin The default value is irrelevant. It works because the SQL sink is able to resolve the named connection string, which is outside of the Serilog section. This means the full IConfiguration parameter was populated correctly.

[andriysavin]

@MV10 correct, but if that parameter is not assigned default value, the method is not called. Try it yourself.

[MV10]

@andriysavin Ah, I missed that part when I read your original mesage, sorry about that. Weird. I'll take a look.

[tsimbalar]

@andriysavin @MV10 I could reproduce that behavior (and fix it) as part of #144.

This should generate a new pre-release of Serilog.Settingc.Configuration v3.0.1-dev00158 no nuget packages are published during a PR build ...

[tsimbalar]

A fix should be available in the pre-release nuget package for v3.0.1 : https://www.nuget.org/packages/Serilog.Settings.Configuration/3.0.1-dev-00160

It would be great if you could give it a try !

[tsimbalar]

Package v3.0.1 has now been released - https://www.nuget.org/packages/Serilog.Settings.Configuration/3.0.1

Thanks for your report !

[andriysavin]

Note that this turns out to be a breaking change for the code which used workaround with assigning default value. In this case the configuration parameter remains null (but the method is now called). For example, if SQL Server sink we discussed above upgrades this package to 3.0.1, I believe it will always get null in appConfiguration parameter.