serilog-mssql / serilog-sinks-mssqlserver

A Serilog sink that writes events to Microsoft SQL Server and Azure SQL
Apache License 2.0
283 stars 148 forks source link

Cannot setup LevelSwitch from config file in MSSqlServerSinkOptions part #470

Closed ogorodnikovInfopulse closed 1 year ago

ogorodnikovInfopulse commented 1 year ago

Bug Report / Support Request Template

Please clearly describe what the SQL Sink is doing incorrectly:

Cannot set levelSwitch from config file

Please clearly describe the expected behavior:

Write levelSwitch in MSSqlServerSinkOptions block in config file

List the names and versions of all Serilog packages used in the project:

Target framework and operating system:

.NET 6 OS: Windows

Provide a simple reproduction of your Serilog configuration code:

Provide a simple reproduction of your Serilog configuration file, if any:


{
"Serilog": {
"Using": [
"Core.BL",
"Serilog.Sinks.Console",
"Serilog.Sinks.Debug",
"Serilog.Sinks.MSSqlServer"
],
"LevelSwitches": {
  "$dbLogLvl": "Warning"
},
"WriteTo:TH_Log": {
  "Name": "DbPerTenant",
  "Args": {
    "levelSwitch": "$dbLogLvl", //Workaround to set levelSwitch
    "tableOptions": {
      //"levelSwitch": "$dbLogLvl", //Should be like this starting from 6.2.0 but not working
      "tableName": "TH_LOG",
      "schemaName": "dbo",
      "autoCreateSqlTable": false
    }
  }
}

} }

>> Provide a *simple* reproduction of your application code:

public static LoggerConfiguration DbPerTenant( this LoggerSinkConfiguration loggerSinkConfiguration, MSSqlServerSinkOptions tableOptions, IConfigurationSection columnOptionsSection, /, ColumnOptions columnOptions/ LoggingLevelSwitch levelSwitch) { tableOptions.LevelSwitch = levelSwitch; //Temp fix return loggerSinkConfiguration .Map(TenantEnricher.TENANT_CONN_STR_PROPERTY_NAME, (connStr, wt) => wt.Async(a => a.MSSqlServer(connStr, sinkOptions: tableOptions, columnOptionsSection: columnOptionsSection/, columnOptions: columnOptions/)) ); }



I expect to write levelSwitch setup in MSSqlServerSinkOptions block but it is not working. 

I guess it is because you forgot to add a levelSwitch param to MSSqlServerSinkOptions constructor 
ckadluba commented 1 year ago

Hello @ogorodnikovInfopulse!

Thanks for your input.

I wonder if there is a real use case for setting LevelSwitch from the configuration. The point of LogLevelSwitch is to pass an object which you can control elsewhere in your program to dynamically adjust the level during runtime from there (see blog post from Nicholas Blumhardt https://nblumhardt.com/2014/10/dynamically-changing-the-serilog-level/).

If you would read LevelSwitch from a static configuration value you would not be able to change the level later at runtime. If you just need to restrict the log level using a fixed configuration value, you can use restrictedToMinimumLevel.

ogorodnikovInfopulse commented 1 year ago

Everything worked on v6.2.0.

I want to have a config file where I can change log level without restarting my app. Now I manually set the levelSwitch (tableOptions.LevelSwitch = levelSwitch). I expect to not set manually tableOptions.LevelSwitch. I expect to read this setting directly from file. I guess if you add param levelSwitch to ctor internal MSSqlServerSinkOptions - everything will work as I do manually.

ckadluba commented 1 year ago

I just realize that Serilog.Settings.Configuration indeed has some support for this.

https://github.com/serilog/serilog-settings-configuration#minimumlevel-levelswitches-overrides-and-dynamic-reload

I will take a look into this.

ckadluba commented 1 year ago

I could not reproduce your workaround with the config you posted. Could you please help us by supplying a full sample project which demonstrates your workaround?

ogorodnikovInfopulse commented 1 year ago

csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="7.0.0" />
    <PackageReference Include="Serilog" Version="3.0.0-dev-02010" />
    <PackageReference Include="Serilog.Settings.Configuration" Version="7.0.0" />
    <PackageReference Include="Serilog.Sinks.MSSqlServer" Version="6.3.0" />
  </ItemGroup>

  <ItemGroup>
    <None Update="serilog.json">
      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
    </None>
  </ItemGroup>

</Project>

Prgram:

using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Options;
using Serilog;

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

var logger = new LoggerConfiguration()
    .ReadFrom
    .Configuration(configuration)
    .CreateLogger();

serilog.json:

{
    "Serilog": {
        "Using": [
            "MsSinkSerilog",
            "Serilog.Sinks.MSSqlServer"
        ],
        "LevelSwitches": {
            "$dbLogLvl": "Warning"
        },
        "WriteTo:SomeName": {
            "Name": "MyConfiguredSink",
            "Args": {
                "levelSwitch": "$dbLogLvl", //Workaround to set levelSwitch
                "tableOptions": {
                    "levelSwitch": "$dbLogLvl", //Should be like this starting from 6.2.0 but not working
                    "tableName": "TH_LOG",
                    "schemaName": "dbo",
                    "autoCreateSqlTable": false
                }
            }
        }
    }
}

MyConfiguredSink:


using Serilog.Configuration;
using Serilog.Core;
using Serilog.Sinks.MSSqlServer;
using Serilog;

namespace MsSinkSerilog;

public static class SerilogConfig
{
    public static LoggerConfiguration MyConfiguredSink(
        this LoggerSinkConfiguration loggerSinkConfiguration
      , LoggingLevelSwitch levelSwitch
      , MSSqlServerSinkOptions tableOptions
        //, IConfigurationSection columnOptionsSection
        /*, ColumnOptions columnOptions*/
        )
    {
        ArgumentNullException.ThrowIfNull(tableOptions);

        var q = tableOptions.LevelSwitch;

        if (q == null || q.MinimumLevel != levelSwitch.MinimumLevel)
        {
            throw new Exception("Need to fix it");
        }

        tableOptions.LevelSwitch = levelSwitch; //Temp fix. Serilog should fix setting tableOptions.LevelSwitch from config file

        return
            loggerSinkConfiguration
            .MSSqlServer("", sinkOptions: tableOptions/*, columnOptionsSection: columnOptionsSection, columnOptions: columnOptions*/)
             ;
    }
}
ckadluba commented 1 year ago

Sorry for the delay. I tried but could not get your approach to work.

Can you implement this and make a PR so we can further investigate?

ckadluba commented 1 year ago

Closing issue due to inactivity.