Deeper LoggingLevelSwitch support

[nblumhardt]

If the root logger's minimum level is controlled by a LoggingLevelSwitch, sinks (the Seq sink in particular) can control this level in response to network messages from a remote server.

It would be great to enable this capability when working with settings (both here and in the JSON variant).

The "declaration" of a switch would need to give it both a name and initial level; subsequent "uses" of the switch would just refer to its name.

E.g.:

<!-- declare the switch -->
<add key="serilog:level-switch:Switch1" value="Debug" />
<!-- use it to control the root logger -->
<add key="serilog:minimum-level:ControlledBy" value="Switch1" />
<add key="serilog:using:Seq" value="Serilog.Sinks.Seq" />
<add key="serilog:write-to:Seq.serverUrl" value="http://localhost:5341" />
<!-- give the sink access to the switch -->
<add key="serilog:write-to:Seq.controlLevelSwitch" value="Switch1" />

This does feel somewhat ad-hoc - opinions welcome. I've held off suggesting this for a while because the usage is quite narrow for the amount of surface area, but I seem to have a question about it every week or two now so opening up the discussion :-)

[SeanKilleen]

Will there ever be more than one LogLevelSwitch in this case?

My theory is that for the majority of use cases, folks are looking for a Seq instance to communicate the logging levels, so they don't necessarily need to worry about the names, as they'd be assuming one switch.

If that's truly the case, could the config be simplified to:

<!-- declare use of the switch -->
<add key="serilog:level-switch" value="true" />

<!-- optional: declare initial minimum level (defaults to something conventionally otherwise -- debug or verbose?) -->
<add key="serilog:level-switch:initialMinLevel" value="Debug" />

If this was provided, would it be possible to create a logging level switch and -- if using seq -- passing the log level switch into the seq wire-up?

I wonder if that could be a good first step. Sure, it relies on conventions a bit, but if it covers a majority of use cases simply, it might be acceptable, especially as a first step.

These are my thoughts as an end-user, but I recognize that implementation-wise, this might be slightly tricky, as I'm assuming a a tighter coupling between Serilog and Seq than may exist.

[nblumhardt]

Thanks for the feedback! I am also a bit unsure about the switch names, but doing it implicitly would still require some way of opting-in to using the switch with a sink.

For example, the configuration below does use a switch, but doesn't let Seq control it:

var sw = new LoggingLevelSwitch();
Log.Logger = new LoggerConfiguration()
    .MinimumLevel.ControlledBy(sw)
    .WriteTo.File(...)
    .WriteTo.Seq(...)
    .CreateLogger();

In this case, the switch controls the root logger and what effectively reaches the file, but this works independently of the level set in Seq.

Having the explicit opt-in means the semantics are the same in code or in configuration - the presence of a switch wouldn't automatically mean that Seq (or another sink accepting a switch) will control it.

[SeanKilleen]

Ahhh, I see -- what I was missing was the fact that there could be a generic switch that Seq didn't control, and that you'd specifically have to wire up a provider to use it (and that we might not want all providers extended to use it).

[SeanKilleen]

In that case, I like your original suggestion, honestly. It's clean-ish, can be documented, and knowing how the switching works now, I could detect if something was wrong with that config. So no suggestions on my end from a client API perspective.

[nvivo]

Is this still on the plan? I'm not sure if this is granular enough, but I was looking for a way to have different log levels at the same server, so I could send a message saying "set customer X or component Y to Verbose for 30 minutes" and not have the entire cluster sending millions of messages to Seq unnecessarily.

[paulomorgado]

It would also be nice to set log level per source.

[nblumhardt]

@nvivo I don't believe this one will be granular enough for what you're after; you might be able to build what you're looking for in code, though, using LoggingLevelSwitch and LoggingFilterSwitch - interested to hear how you go if you give it a try.

@paulomorgado agreed; might need a separate ticket to track that one 👍

Cheers,
Nick

[tsimbalar]

I must say I would love to see that change come sooner rather than later :) It is THE reason why we haven't moved to remotely-controlled level.

It is ok to have a bit of config in the code and a bit of config in the app settings, but I don't believe we can currently mix "seq configuration in the appSettings" but "adding the controlledBy" in the code.

[nvivo]

Just for the record, I did some tests using both LoggingLevelSwitch and LoggingFilterSwitch as recommended by @nblumhardt and although it's not ready out of the box, it seems doable. I didn't implement it in production yet, but nothing seems to prevent me from starting with a default and changing that later remotely. Which barriers did you find @tsimbalar?

[nblumhardt]

Thinking minimum-level:controlled-by would be more consistent with the rest of the syntax than *:ControlledBy as in the first example.

This would make the syntax:

<!-- declare the switch -->
<add key="serilog:level-switch:Switch1" value="Debug" />
<!-- use it to control the root logger -->
<add key="serilog:minimum-level:controlled-by" value="Switch1" />
<add key="serilog:using:Seq" value="Serilog.Sinks.Seq" />
<add key="serilog:write-to:Seq.serverUrl" value="http://localhost:5341" />
<!-- give the sink access to the switch -->
<add key="serilog:write-to:Seq.controlLevelSwitch" value="Switch1" />

Seems like there's heaps of support for this one - don't suppose anyone has the opportunity to try an implementation/PR?

[tsimbalar]

@nblumhardt oh, I thought it was already implemented somewhere, but just waiting for feedback about the the final syntax in the config file

I would like to give it a try and implement it.
I would probably need a bit of help to find my way around the code base(s) , and also more generally about the process (PRs and so ... I'm pretty much a noob with OSS, better late than never I guess ;) )

The way I see it, because ReadFrom.AppSettings relies on ReadFrom.KeyValuePairs, the implementation for this issue would be in the serilog core repo, and not in this one.

This would go into https://github.com/serilog/serilog/blob/6118a6f5958fc42b4d74e80236bc7151983ffdbf/src/Serilog/Settings/KeyValuePairs/KeyValuePairSettings.cs and matching tests in https://github.com/serilog/serilog/blob/e42245c0db0931e1fffdd72a0729361c60e877bc/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs , is this correct ?

[nblumhardt]

@tsimbalar awesome! :-)

Yes, that's correct.

[tsimbalar]

I'm currently working on it (here - work in progress :p).

I am having second thoughts about the syntax for the app settings for the LoggingLevelSwitch declaration and reference. I'm probably not following YAGNI, but anyway, here are my thoughts ...

1 - Variable Declaration

I'm wondering if the "variable declaration" syntax should not be more generic and allow to instantiate any type from its constructor, and then end up with something like

<add key="serilog:variable:Switch1:LoggingSwitchLevel" value="Debug" />

instead of

<add key="serilog:level-switch:Switch1" value="Debug" />

this would effectively say "create an instance of LoggingSwitchLevel by calling its constructor with one param, doing the usual conversion from string to LogEventLevel and store it in variable Switch1".

This would actually allow to also define variables of types other than LoggingSwitchLevel. I'm really not sure if there is any use case for this, so there might be no point in being overly generic ... but we can still make the syntax generic even though we support only the type LogginSwitchLevel for now.

2 - Variable Reference

I'm wondering if referencing a variable should be made a bit more explicit. What I mean is that the syntax should make it unambiguous whether the param we are passing is a value (like a string, a LogEventLevel, a Uri) or a reference to a variable (like the LoggingSwitchLevel in our case).

For instance using a special symbol (like $ ?) when referencing a variable

<add key="serilog:minimum-level:controlled-by" value="$Switch1" />
<add key="serilog:using:Seq" value="Serilog.Sinks.Seq" />
<!-- give the sink access to the switch -->
<add key="serilog:write-to:Seq.controlLevelSwitch" value="$Switch1" />

this introduces a bit of complexity in the syntax for a newcomer but :

• this allows the configuration-parsing code to clearly report errors like "There is no such variable as Switch1" instead of having to first look if a variable exists with that name and fall back to a value conversion (which might also be problematic and confusing if for instance someone declares a level switch with the name Information ... in that case where is the variable and where is the value ... I know why would anyone do that... )
• this is a bit more intent-revealing, as in "I explicityly want to pass this variable to this method, this is not a string"
• that is an advanced configuration scenario already

Possibly, the special symbol could also be used when declaring the variable as in :

<add key="serilog:level-switch:$Switch1" value="Debug" />

Options

I don't have particularly strong opinions on both topics, but I believe it would be good to choose a syntax that will not prevent us from extending in the future.

To sum up, here are the possible syntax :

Option 1 - original

<!-- declare the switch -->
<add key="serilog:level-switch:Switch1" value="Debug" />
<!-- use it to control the root logger -->
<add key="serilog:minimum-level:controlled-by" value="Switch1" />
<add key="serilog:using:Seq" value="Serilog.Sinks.Seq" />
<add key="serilog:write-to:Seq.serverUrl" value="http://localhost:5341" />
<!-- give the sink access to the switch -->
<add key="serilog:write-to:Seq.controlLevelSwitch" value="Switch1" />

Option 2 - explicit variable declaration and references

for instance using a $ sign

<!-- declare the switch -->
<add key="serilog:level-switch:$Switch1" value="Debug" />
<!-- use it to control the root logger -->
<add key="serilog:minimum-level:controlled-by" value="$Switch1" />
<add key="serilog:using:Seq" value="Serilog.Sinks.Seq" />
<add key="serilog:write-to:Seq.serverUrl" value="http://localhost:5341" />
<!-- give the sink access to the switch -->
<add key="serilog:write-to:Seq.controlLevelSwitch" value="$Switch1" />

(other possibilities : {variableName}, $(variableName), ${variableName} NLog does that )

Option 3 - generic variable declaration

<!-- declare the switch -->
<add key="serilog:variable:Switch1:LoggingLevelSwitch" value="Debug" />
<!-- use it to control the root logger -->
<add key="serilog:minimum-level:controlled-by" value="$Switch1" />
<add key="serilog:using:Seq" value="Serilog.Sinks.Seq" />
<add key="serilog:write-to:Seq.serverUrl" value="http://localhost:5341" />
<!-- give the sink access to the switch -->
<add key="serilog:write-to:Seq.controlLevelSwitch" value="$Switch1" />

possible variations : serilog:variable:variableName:variableType, serilog:declare:variableName:variableType, serilog:var:variableName:variableType , serilog:declare:$variableName:variableType, serilog:var:variableType:$variableName:,, serilog:new:variableType:$variableName:

I quite like Option 2 :)
Any thoughts ?

For now, I'm still implementing the original proposal by @nblumhardt

[nblumhardt]

Interesting!

I think to do variables successfully, we'd need to go pretty deep so that we don't risk adding a general feature that only ever supports the one use case. I.e. we'd need to make sure values with $ in them were not ambiguous, deal with types consistently, and ideally keep the option open to do a string-substitution syntax for things like common log file paths a-la NLog.

It would also help to list other scenarios where variables would save the user enough effort that learning the variable system would pay off.

My immediate reaction is - ah, that's elegant, we should definitely do variables - but, there's a lot of effort involved, so being realistic, I think we'd need to work on a pretty strong case/proposal to go forward with it.

Thoughts?