Hi list,
looking at the logging code, I was surprised: turns out the 'logging level all everything', though quite a misnomer, was an important counterpart to 'logging level all (debug|...|fatal)'. It worked like this:
Assuming some levels are set...
logging level foo fatal logging level bar debug
Now 'logging level all' would override all of them forcefully:
logging level all error # Now all categories including 'foo' and 'bar' log exactly error,fatal # messages.
logging level all debug # Now all categories including 'foo' and 'bar' log all messages from debug # thru fatal.
# manipulating individual categories has no effect at all now! logging level foo notice logging level bar error # no change in logging behavior! logging level foo fatal logging level bar debug
# the way to get rid of the forced level was: logging level all everything # Now we see above configured behavior of foo == fatal, bar == debug
This is *still the case*, only that we're now ignoring the 'everything' keyword. The consequence is that we can force all categories and clamp them to all-the-same level, but we cannot lift that clamp anymore! :/
I think we should never have accepted the removal of 'everything' as such, it should have been replaced with a more accurate command, like 'no logging level all'. I am considering to bring this back now, ... but
But now that we haven't had 'everything' for a long time and the outrage about it has been limited, I am also drawn towards completely dropping that feature, or at least renaming it. Damage is done, no need to go back to mad naming.
From the logging discussion, we concluded that we want a command to set the log level for all categories at once, one-time; not force-clamping all to one level until the clamp is removed, but as if we set each individual level manually.
I've cracked my head on what name other than "all" we could want this command to have, 'logging level (*|each|any|set-all)'? I keep coming back to 'logging level all (debug|...|fatal)' being by far the best name for this.
Backwards compatibility: what do users see when we modify 'logging level all' to not be a forceful clamp, but instead setting individual levels once-off?
If a user had a config of "all" coming last:
logging level foo debug logging level bar fatal logging level all notice
then the experience is that all categories are logging at level 'notice', and that is what the user most likely also expects to happen. Changing the 'all' command does not change the logging behavior one bit.
If in turn the config has "all" first:
logging level all notice logging level foo debug logging level bar fatal
Then the experience is still that before changing the command, all categories are logging at level 'notice'. Most likely the user expected though to see foo='debug' and bar='fatal', because they were set after the 'all' -- what other reason could a user have to keep these lines in the config file, which don't have any effect as-is? If we change the 'all' command to not be a forceful clamp but just a one-time set, then the logging behavior changes to what the user likely expected when writing this config. However: if these settings were forgotten in the config file, we would suddenly change the logging behavior and might surprise users ... but I still kinda think we would change it to what the user expected, right? And the user would likely go: "ah, finally it's doing what I wanted all the while!" right?
So, I want to make 'logging level all' manipulate each individual category once-off, I want to completely deprecate the 'everything' keyword, and drop the "global clamp" feature entirely.
Maybe I'd re-add the clamp as 'logging level force-all (debug|...|fatal)' and 'no logging level force-all' to switch it off. That would be exactly the old clamping feature just with less confusing names.
The only thing that makes me write here is that I'm only 90% sure on changing the meaning of an existing command, of 'logging level all'. If there is no negative feedback on this here, that would bring me to 100%.
Thanks!
~N
Hi Neels,
On Mon, Sep 10, 2018 at 11:26:46PM +0200, Neels Hofmeyr wrote:
So, I want to make 'logging level all' manipulate each individual category once-off, I want to completely deprecate the 'everything' keyword, and drop the "global clamp" feature entirely.
Maybe I'd re-add the clamp as 'logging level force-all (debug|...|fatal)' and 'no logging level force-all' to switch it off. That would be exactly the old clamping feature just with less confusing names.
The only thing that makes me write here is that I'm only 90% sure on changing the meaning of an existing command, of 'logging level all'. If there is no negative feedback on this here, that would bring me to 100%.
In terms of semantics, old notes/documentation, etc. I would suggest to simply deprecate "all" and introduce a new command instead. If the global clamping is removed, then "logging level all debug" could simply become a no-op, so if people type it in because they are used to it, or have it in some old configs, no harm is done. And if they use "logging level all notice" then they should get a non-fatal warning/notice message that this command is no longer supported.
So in short: I suggest to not overload existing keywords, but introducing new ones with well-defined/documented meaning.
On 11/09/18 05:41, Harald Welte wrote:
Hi Neels,
On Mon, Sep 10, 2018 at 11:26:46PM +0200, Neels Hofmeyr wrote:
So, I want to make 'logging level all' manipulate each individual category once-off, I want to completely deprecate the 'everything' keyword, and drop the "global clamp" feature entirely.
Maybe I'd re-add the clamp as 'logging level force-all (debug|...|fatal)' and 'no logging level force-all' to switch it off. That would be exactly the old clamping feature just with less confusing names.
Well, I was in favour of the "clamp" originally, and I was using it a lot, hence my manual patching for a while of libosmocore to restore "everything" as "clamp-remover".
I use it less now, if ever I do do something like logging level all debug, I know I have to exit the vty and start again to remove the clamp.
The only thing that makes me write here is that I'm only 90% sure on changing the meaning of an existing command, of 'logging level all'. If there is no negative feedback on this here, that would bring me to 100%.
In terms of semantics, old notes/documentation, etc. I would suggest to simply deprecate "all" and introduce a new command instead. If the global clamping is removed, then "logging level all debug" could simply become a no-op, so if people type it in because they are used to it, or have it in some old configs, no harm is done. And if they use "logging level all notice" then they should get a non-fatal warning/notice message that this command is no longer supported.
You /could/ just reinstate the "all everything" as clamp remover.
Note that doing "write" in the vty still writes a "logging level all everything" line to the config, causing a warning on startup: % Ignoring deprecated logging level everything
Then leave logging level all notice meaning "clamp to notice" (as it always was).
I honestly never found this difficult to understand once I understood how it worked, so I'd suggest documenting "all" as a "clamp" and suggesting prefer the use of "set-all"
So in short: I suggest to not overload existing keywords, but introducing new ones with well-defined/documented meaning.
Then add logging level set-all [level]? set-all makes most sense to me out of the suggestions, for a temporary
But again... I really don't mind.. I don't want to wreck your head, Neels, in making decisions. Deprecate 'all' if you want. As I said before I just use expect scripts as shell functions to setup different logging scenarios:
rhizomatica@rccnII:~$ type nitb_cc nitb_cc is a function nitb_cc () { /usr/bin/expect -c 'spawn telnet localhost 4242;send "enable\r";send "logging enable\r";send "logging print category 1\r";send "logging level smpp fatal\r"; send "logging level mncc debug\r"; send "logging level cc debug\r"; send "logging filter all 1\r"; interact' }
I think this is easier that trying to put that functionality into osmocore itself.. which would be something like
logging level foo bar.. [..repeat according to preferences..] then logging scenario save mylogging
logging scenario load mylogging
One has more urgent things to do.. right?
k/
On 10/09/18 23:26, Neels Hofmeyr wrote:
Hi list,
looking at the logging code, I was surprised: turns out the 'logging level all everything', though quite a misnomer, was an important counterpart to 'logging level all (debug|...|fatal)'. It worked like this:
Ahem. https://osmocom.org/issues/71#note-17
# the way to get rid of the forced level was: logging level all everything # Now we see above configured behavior of foo == fatal, bar == debug
This is *still the case*, only that we're now ignoring the 'everything' keyword. The consequence is that we can force all categories and clamp them to all-the-same level, but we cannot lift that clamp anymore! :/
On Tue, Sep 11, 2018 at 09:45:48AM +0200, Keith wrote:
Yes, that looked like a good explanation, but my huge trouble reading that was that I did not grok which log lines were being logged on which log level, so I completely did not get what you were explaining back then :( Now I do :)
This link is dead!? Does gerrit forget things?
~N
On 11/09/18 14:20, Neels Hofmeyr wrote:
On Tue, Sep 11, 2018 at 09:45:48AM +0200, Keith wrote:
This link is dead!? Does gerrit forget things?
Sorry, it was marked "Private"
~N
On 11/09/18 14:20, Neels Hofmeyr wrote:
Yes, that looked like a good explanation, but my huge trouble reading that was that I did not grok which log lines were being logged on which log level,
Understood, without colour and logging print category 1, we never know the log level. :)
On Tue, Sep 11, 2018 at 02:28:52PM +0200, Keith wrote:
On 11/09/18 14:20, Neels Hofmeyr wrote:
Yes, that looked like a good explanation, but my huge trouble reading that was that I did not grok which log lines were being logged on which log level,
Understood, without colour and logging print category 1, we never know the log level. :)
Rather 'logging print level 1', which I think I added pretty much because I didn't understand that explanation :)
~N
Hi,
As I already shared in last discussion about this topic, I agree with keeping and fixing "logging level all" instead of dropping it and creating a new command, because imho most if not all users of "logging level all" in config files expect the fixed behavior (set each category to level X). Dropping and adding a new command is going to add far more breakage than fixing it.
Having said that, I don't want to spend more and more hours in discussions regarding this topic, so I'm fine with whatever fix you want to do as long as current misleading behavior disappears. Have a look at the patches I submitted if you are interested in a similar fix.
On Tue, Sep 11, 2018 at 10:57:16AM +0200, Pau Espin Pedrol wrote:
As I already shared in last discussion about this topic, I agree with keeping and fixing "logging level all" instead of dropping it and creating a new command, because imho most if not all users of "logging level all" in config files expect the fixed behavior (set each category to level X).
But "logging level all" does *not* set each category to level X, instead it sets a global forced level over and above the individually set log levels, which stays clamped on forever regardles of individual logging levels set.
I almost had re-used it to mean "set each", basically accepting your patch as it is, but after all I do agree with laforge that it is better to not re-use the same name to mean something different.
Dropping and adding a new command is going to add far more breakage than fixing it.
Not dropping, just deprecating. They will remain available, but not be prominently visible in vty online doc / command autocompletion.
So ... after all of your input ...
<Dignified Ceremonial Silence>
In Accordance with my Title of Supreme Grand Master of Logging and Reforestation I Decree that we Shall:
- keep 'logging level all', and bring back 'everything', properly documented, but marked as deprecated. That means the old clamping behavior is still there (the code itself is trivial), but the commands do not appear when querying the VTY cmdline doc. Why keep it? So that old config files still work as they did when conceived. Why hide it? because the naming is misleading to most of us, and it's less trouble to just see the new ones with a clear meaning.
- add 'logging level set-all' to set each category once-off.
- add 'logging level force-all <LVL>' and 'no logging level force-all' as aliases for 'all <LVL>' and 'all everything' to not write deprecated commands to file.
I agree that we want to stop discussing this now. This is both an example of Bikeshedding and of why writing new software can be faster than changing old software.
(If you disagree with the above, I don't want to shut you up, everyone is always allowed to talk more, regardless. Just offering to relieve everyone of the burden now.)
~N
-
Harald Welte -
Keith -
Neels Hofmeyr -
Pau Espin Pedrol