Skip to content

configuration overriding problem #70

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

Closed
adnan-kamili opened this issue Oct 18, 2017 · 8 comments
Closed

configuration overriding problem #70

adnan-kamili opened this issue Oct 18, 2017 · 8 comments

Comments

@adnan-kamili
Copy link

adnan-kamili commented Oct 18, 2017

Hi,

I have following serilog config in appsettings.json:

"Serilog": {
    "MinimumLevel": {
      "Default": "Debug",
      "Override": {
        "Microsoft": "Information"
      }
    },
    "WriteTo": [
      {
        "Name": "Async",
        "Args": {
          "configure": [
            {
              "Name": "Console"
            },
            {
              "Name": "RollingFile",
              "Args": {
                "pathFormat": "Logs/log-{Date}.txt",
                "shared": true
              }
            }
          ]
        }
      }
    ]
  }

For production - appsettings.production.json:

  "Serilog": {
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "Microsoft": "Warning"
      }
    },
    "WriteTo": [
      {
        "Name": "Async",
        "Args": {
          "configure": [
            {
              "Name": "Console"
            }
          ]
        }
      }
    ]
  }

As can be seen, I tried to remove logging to file in production, but it still logs to file in production. How can I configure that behavior?

@nblumhardt
Copy link
Member

Hi @adnan-kamili, thanks for getting in touch.

For production - appsettings.developement.json:

I am not sure I understand; production settings are normally in appsettings.Production.json. To turn off logging, it should be enough to have logging configuration in appsettings.Development.json only. Can you send a bit more info about how your solution is set up?

Best regards,
Nick

@adnan-kamili
Copy link
Author

adnan-kamili commented Oct 18, 2017

@nblumhardt it was a typo in the post, fixed that. It is appsettings.production.json wherein I only want console logging in production and disable file logging (as heroku moves console logs to Papertail automatically).

But removing file logging from production json still enables file logging. Seems like it is merging config from appsettings.json.

Note that file logging is only added in appsettings.json to act as default

@nblumhardt
Copy link
Member

Hi @adnan-kamili - this sounds like it's by-design, then. Microsoft.Extensions.Configuration sources are always additive, so if you have file logging in appsettings.json it will always apply everywhere - there's no way for a more specific config file to turn it off. HTH!

@amoerie
Copy link

amoerie commented Oct 26, 2017

Microsoft.Extensions.Configuration sources are always additive, so if you have file logging in appsettings.json it will always apply everywhere - there's no way for a more specific config file to turn it off.

I actually think appsettings.production.json overrides settings with the same key path. See for example https://stackoverflow.com/questions/39215806/asp-net-core-1-appsettings-production-json-not-updating-connection-strings where the asker updates his connection string in production.json

Actually we are running into the same problem with our installers, overriding specific parameters is tricky because the sinks are configured as entries in an array, so instead of being able to "merge" two JSON objects, we would now have to merge the "WriteTo" array based on the value of the Name property of each sink.

Imho, a better approach would be to assign each sink a unique key.

For example, we now have this:

  "Serilog": {
    "Using": [ "Serilog.Sinks.Console", "Serilog.Sinks.File" ],
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "Microsoft": "Warning",
        "System": "Warning"
      }
    },
    "WriteTo": [
      {
        "Name": "Console",
        "Args": {
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "restrictedToMinimumLevel": "Verbose"
        }
      },
      {
        "Name": "File",
        "Args": {
          "path": "C:/OurCompany/OurProduct/Logs/TheWebApplication-.log",
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "fileSizeLimitBytes": 1073741824,
          "retainedFileCountLimit": 62,
          "rollingInterval": "Day",
          "rollOnFileSizeLimit": true,
          "restrictedToMinimumLevel": "Information"
        }
      }
    ],
    "Enrich": [ "FromLogContext" ]
  }

Now you can see, there is a hardcoded path in there to the C:\ drive. However, our product gets installed on enterprise servers where sometimes the C:\ drive is reserved for OS only, and a data drive D:\ is provided to install all applications. In this case, they would want to override this path with D:\OurCompany...

Unfortunately it is not feasible to use appsettings.production.json or any other JSON merging mechanism, because that would only result in 3 entries in the WriteTo array. You can see the limitations of JSON array merging in Newtonsoft JSON.NET: https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_Linq_MergeArrayHandling.htm

If we could assign ids to each sink, the problem disappears:

  "Serilog": {
    "Using": [ "Serilog.Sinks.Console", "Serilog.Sinks.File" ],
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "Microsoft": "Warning",
        "System": "Warning"
      }
    },
    "WriteTo": {
      "Console-Sink-1": {
        "Name": "Console",
        "Args": {
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "restrictedToMinimumLevel": "Verbose"
        }
      },
      "File-Sink-1": {
        "Name": "File",
        "Args": {
          "path": "C:/OurCompany/OurProduct/Logs/TheWebApplication-.log",
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "fileSizeLimitBytes": 1073741824,
          "retainedFileCountLimit": 62,
          "rollingInterval": "Day",
          "rollOnFileSizeLimit": true,
          "restrictedToMinimumLevel": "Information"
        }
      }
    },
    "Enrich": [ "FromLogContext" ]
  }

With this technique, JSON merging becomes easy. All we would need to do to update the path is provide a second JSON object like this:

  "Serilog": {
    "WriteTo": {
      "File-Sink-1": {
        "Args": {
          "path": "D:/OurCompany/OurProduct/Logs/TheWebApplication-.log",
         }
      }
    }
  }

@nblumhardt
Copy link
Member

Thanks @amoerie.

I think your identifier technique will work currently, if you use indices as identifiers:

    "WriteTo": {
      "0": {
        "Name": "Console",
        "Args": {
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "restrictedToMinimumLevel": "Verbose"
        }
      },

@amoerie
Copy link

amoerie commented Oct 27, 2017

Haha that's genius! Thanks!

@Tadimsky
Copy link

Other than doing the index-based workaround, is there any plan to make this support real ids/names? I don't like have 0 or 1 meaning something without it being more clear :)

@skomis-mm
Copy link
Contributor

There was always support for such syntax:

    "WriteTo": {
      "Console-Sink-1": {
        "Name": "Console",
        "Args": {
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "restrictedToMinimumLevel": "Verbose"
        }
      },
      "File-Sink-1": {
        "Name": "File",
        "Args": {
          "path": "C:/OurCompany/OurProduct/Logs/TheWebApplication-.log",
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "fileSizeLimitBytes": 1073741824,
          "retainedFileCountLimit": 62,
          "rollingInterval": "Day",
          "rollOnFileSizeLimit": true,
          "restrictedToMinimumLevel": "Information"
        }
      }
    }

without using indexes. The index comes as convention for a key name when array syntax used:

"WriteTo": [
      {
        "Name": "Console",
        "Args": {
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "restrictedToMinimumLevel": "Verbose"
        }
      },
      {
        "Name": "File",
        "Args": {
          "path": "C:/OurCompany/OurProduct/Logs/TheWebApplication-.log",
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u4}] {SourceContext} {Message}{NewLine}{Exception}",
          "fileSizeLimitBytes": 1073741824,
          "retainedFileCountLimit": 62,
          "rollingInterval": "Day",
          "rollOnFileSizeLimit": true,
          "restrictedToMinimumLevel": "Information"
        }
      }
    ]

where Console has WriteTo:0 key path and File - WriteTo:1.

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

No branches or pull requests

5 participants