Allow an array-style config format for Serilog:MinimumLevel:Override

[jez9999]

At the moment, the way you specify Serilog's minimum level overrides in configuration is in the following format:

"MinimumLevel": {
	"Default": "Information",
	"Override": {
		"Microsoft.AspNetCore.Mvc.Infrastructure": "Debug",
		"Microsoft": "Warning",
		"Microsoft.Hosting.Lifetime": "Information",
		"Microsoft.AspNetCore.Authentication": "Verbose",
	}
}

The source context (or 'namespace') is specified in the setting name, rather than a value. The trouble with this is that not all configuration providers lend themselves to having long, complex configuration key names. For example, I've tried to configure the equivalent to this on an Azure web app (using the double-underscore namespace separator as a replacement for the colon), and the best I could get to was:

"Serilog:MinimumLevel:Override:Microsoft_AspNetCore_Mvc_Infras": "Debug"

On my local machine, which is using appsettings.json, this is how it (correctly) looks:

"Serilog:MinimumLevel:Override:Microsoft.AspNetCore.Mvc.Infrastructure": "Debug"

The dots have been replaced with underscores, and more importantly there is a very restrictive length limit on config key names from these environment variables. So I think the solution is not to try and put complex/long information like a namespace in the key name, but as a key value. Thus, I propose that Serilog support the following syntax for minimum level overrides:

"MinimumLevel": {
	"Default": "Information",
	"Override": [
		{
			"SourceContext": "Microsoft.AspNetCore.Mvc.Infrastructure",
			"Level": "Debug"
		},
		{
			"SourceContext": "Microsoft",
			"Level": "Warning"
		},
		{
			"SourceContext": "Microsoft.Hosting.Lifetime",
			"Level": "Information"
		},
		{
			"SourceContext": "Microsoft.AspNetCore.Authentication",
			"Level": "Verbose"
		}
	]
}

When Override is an array, it takes a set of objects with SourceContext/Level that specify the override level for the given source context. This results in the following kind of config structure, which works even with config providers that have restrictive key names:

"Serilog:MinimumLevel:Override:0:SourceContext": "Microsoft.AspNetCore.Mvc.Infrastructure"
"Serilog:MinimumLevel:Override:0:Level": "Debug"

[sungam3r]

This issue may be moved into serilog-settings-configuration repo.

[skomis-mm]

Hi @jez9999 ,@sungam3r

The new syntax is a bit cumbersome and decreases UX. Since Serilog's overrides API only work with SourceContext's there is no reason to write SourceContext and Level in array elements. Also, array doesn't prevent use of duplicate keys.
I agree that in the current syntax you should know some specifics when you use environment variables on some platforms (need of change : to __), but it is aligned with Microsoft.Extensions.Configuration:

"Logging": {
    "LogLevel": {
      "Default": "Information",
      "Microsoft": "Warning",
      "Microsoft.Hosting.Lifetime": "Information"
    }

Did you hit some strength length limit on keys? In Azure app configuration, the restriction on key-value pair is 10Kb

[jez9999]

"decreases UX"? Nonsense, the old format is still available, this is simply an addition.

I don't know what Azure is doing to my config setting but it seems to limit them to about 61 characters so yes there is a limit.

"array doesn't prevent use of duplicate keys"
Again, so what? You're talking as if this were intended to be used by clueless end users. It's meant for developers to configure their logging.

[sungam3r]

@skomis-mm In general, I agree with your comments and foresaw them, because I myself reason the same way.

The new syntax is a bit cumbersome and decreases UX.

Does it really decrease UX so much?

Since Serilog's overrides API only work with SourceContext's there is no reason to write SourceContext and Level in array elements.

The reason is to assign a some name for the key and value. In fact, the name can be any, but for clarity, I chose the most obvious.

Also, array doesn't prevent use of duplicate keys.

Agree. But if you go up one level, nothing prevents you from setting the same settings key with different values from different providers (file, env, comand line). This is a common practice of overriding configuration. Therefore, it makes little sense to defend against it.

[nblumhardt]

I think any issue Serilog suffers here is going to be shared by Microsoft.Extensions.Logging, which uses the same layout for overrides, so hopefully the Azure web app limitation will be spotted and lifted by the good folks at Microsoft. Raising an issue with them might speed this along, @jez9999.

Adding more variations to the configuration syntax makes it more complicated to define, document, and learn, hence @skomis-mm's concern about UX, I think.

We don't have the time here for the "nonsense", "so what?" combative style of communication; it drains the energy out of open source projects. I've responded to similar recent comments here and here. Life's too short for more of this, let's start afresh if necessary, closing.

[skomis-mm]

New syntax other than adding more verbosity won't do any better for environment variables. For one level override:
MinimumLevel__Override__0__SourceContext=Microsoft.AspNetCore.Mvc.Infrastructure
MinimumLevel__Override__0__Level=Debug

1. Still need to use underscores for some platforms
2. Need to use array indexes elements in the key names which are fragile to use
3. 2 variables for one override instead of 1

We have an old issue to overcome current current syntax and simplify it, not to add complexity to it.

[jez9999]

@nblumhardt

I think any issue Serilog suffers here is going to be shared by Microsoft.Extensions.Logging, which uses the same layout for overrides, so hopefully the Azure web app limitation will be spotted and lifted by the good folks at Microsoft. Raising an issue with them might speed this along, @jez9999.

How is that an argument against this configuration option? People use Serilog becuase it offers stuff the default logging doesn't. If a better configuration is one such thing, so much the better.

Adding more variations to the configuration syntax makes it more complicated to define, document, and learn, hence @skomis-mm's concern about UX, I think.

Sorry, I just don't see this as any kind of problem. It's trivial to document an array-style format.

@skomis-mm

New syntax other than adding more verbosity won't do any better for environment variables. For one level override:
MinimumLevel__Override__0__SourceContext=Microsoft.AspNetCore.Mvc.Infrastructure
MinimumLevel__Override__0__Level=Debug

1. Still need to use underscores for some platforms

2. Need to use array indexes elements in the key names which are fragile to use

3. 2 variables for one override instead of 1

I really get the impression that you're just objecting for the sake of argument here. The point is to have a configuration option that avoids the need for periods in the key name, and avoids the need for long key names. Your objections aren't part of the problem being solved.

We have an old issue to overcome current current syntax and simplify it, not to add complexity to it.

There's no fundamental way to "overcome" the syntax issue of putting the namespace in the key name other than to move it to being a key value.