Let's be stricter (and more helpful) in Debug mode
Hi guys, I've already mentioned this at some calls before, but this is actually the right channel for discussing the idea. I've seen many times before, that there is, for instance, a media layout error, but we simply report that problem into the log and try to continue. Usually it's nothing critical in our eyes. As an example, we've recently seen that someone reported a missing Czech license in installer. Ladislav found out that the license actually exists, but wrongly uses "cz" instead of "cs" in its filename. The installer actually reported a warning into the log. Something like "unknown language 'cz'". So, what is the problem? It could have been spotted 3 months earlier. Plus it could have been fixed without us debugging the problem. Proposed solution: In case of Y2DEBUG==1, raise an internal error or rather show a pop-up error/warning message. This would not influence common installations, but openQA would catch and record it. With a little piece of work openQA could even skip it if needed. I would like to have a separate Y2SOMETHING variable to control the behavior (by default, X==1 in case of Y2DEBUG==1). Obviously, this means more work now and from time to time for adding such warning/error reports one by one. But that IMO pays off mid-to-long term. At the end we could have more time to fix "our" bugs :) What do you think? Thx Lukas
On Fri, 15 Jan 2021 16:29:11 +0100
Lukas Ocilka
Hi guys,
I've already mentioned this at some calls before, but this is actually the right channel for discussing the idea.
I've seen many times before, that there is, for instance, a media layout error, but we simply report that problem into the log and try to continue. Usually it's nothing critical in our eyes. As an example, we've recently seen that someone reported a missing Czech license in installer. Ladislav found out that the license actually exists, but wrongly uses "cz" instead of "cs" in its filename. The installer actually reported a warning into the log. Something like "unknown language 'cz'".
So, what is the problem? It could have been spotted 3 months earlier. Plus it could have been fixed without us debugging the problem.
Proposed solution: In case of Y2DEBUG==1, raise an internal error or rather show a pop-up error/warning message. This would not influence common installations, but openQA would catch and record it. With a little piece of work openQA could even skip it if needed. I would like to have a separate Y2SOMETHING variable to control the behavior (by default, X==1 in case of Y2DEBUG==1).
Obviously, this means more work now and from time to time for adding such warning/error reports one by one. But that IMO pays off mid-to-long term. At the end we could have more time to fix "our" bugs :)
What do you think? Thx Lukas
I quite like idea. Something like assert call in C which check conditions and if it failed then it log warning or raise exception depending on ENV variable. Josef
On 1/18/21 8:14 AM, josef Reidinger wrote:
On Fri, 15 Jan 2021 16:29:11 +0100 Lukas Ocilka
wrote: Hi guys,
I've already mentioned this at some calls before, but this is actually the right channel for discussing the idea.
I've seen many times before, that there is, for instance, a media layout error, but we simply report that problem into the log and try to continue. Usually it's nothing critical in our eyes. As an example, we've recently seen that someone reported a missing Czech license in installer. Ladislav found out that the license actually exists, but wrongly uses "cz" instead of "cs" in its filename. The installer actually reported a warning into the log. Something like "unknown language 'cz'".
So, what is the problem? It could have been spotted 3 months earlier. Plus it could have been fixed without us debugging the problem.
Proposed solution: In case of Y2DEBUG==1, raise an internal error or rather show a pop-up error/warning message. This would not influence common installations, but openQA would catch and record it. With a little piece of work openQA could even skip it if needed. I would like to have a separate Y2SOMETHING variable to control the behavior (by default, X==1 in case of Y2DEBUG==1).
Obviously, this means more work now and from time to time for adding such warning/error reports one by one. But that IMO pays off mid-to-long term. At the end we could have more time to fix "our" bugs :)
What do you think? Thx Lukas
I quite like idea. Something like assert call in C which check conditions and if it failed then it log warning or raise exception depending on ENV variable.
Josef
+1 -- José Iván López González YaST Team at SUSE LINUX GmbH IRC: jilopez
Obviously, this means more work now and from time to time for adding such warning/error reports one by one. But that IMO pays off mid-to-long term. At the end we could have more time to fix "our" bugs :)
What do you think? Thx Lukas
I quite like idea. Something like assert call in C which check conditions and if it failed then it log warning or raise exception depending on ENV variable.
Unrelated, but something we adopted in 389-ds is not just to log warnings/errors, but to follow a formula of: 1: What went wrong? 2: Why did it go wrong? 3: How can you correct/remidiate this? That really helps compared to most errors which are just step 1, since there is now an actionable path for a user. For example, compare: Error: sync_read_entry_from_changelog - Retro Changelog does not provide entryuuid. This tells us something what went wrong, and a bit of why - but without knowing what the retro changelog is, or why entryuuid's are involved what should an admin do? Instead, our log message is now: sync_read_entry_from_changelog - Retro Changelog does not provide entryuuid. Check 'cn=Retro Changelog Plugin,cn=plugins,cn=config' contains 'nsslapd-attribute: entryuuid:targetEntryUUID' Now there is something actionable. The admin knows to check their cn=config, which section of that config, and what needs to be added to resolve the problem. So maybe this is something you could use in your warning/error improvements here? — Sincerely, William Brown Senior Software Engineer, 389 Directory Server SUSE Labs, Australia
On 2021-01-15 16:29, Lukas Ocilka wrote:
[errors/warnings in the y2log going unnoticed for a long time]
IMHO the major problem with this is that we are neglecting the y2log.
Lots of cruft get logged all the time in certain parts of the code while
other parts of the code don't do ANY logging at all.
Try turning Y2DEBUG on and watch in amazement the wall of text that
keeps scrolling by. That's where things go downhill. In that wasteland
of useless messages it's impossible to find anything, so we stopped
caring.
We need better discipline with logging. When you are done developing
some piece of code, turn excessive logging off. I normally simply
comment out the log lines so they are still available when I need to get
back to that part to continue developing; or for serious debugging.
Some months ago, for example, I threw out a ton of y2debug() from
libyui-ncurses; that level of debugging was close to what gdb will give
you if you let it. We don't need to log every constructor and
destructor. We don't need to dump all the internal status of an object
in every function.
When y2debug no longer keeps flooding people with useless stuff, they
will be more inclined to watch the y2log, so errors and warnings are
spotted more easily.
Also, if a warning or error occurs on a regular basis, it's probably not
an error or a warning, but a normal message. During normal operation,
there should be NO error and very few warnings.
We need to get over throwing and catching exceptions on a regular basis;
during normal operation, there should be no exceptions. In certain parts
of our code (libstorage?) we may need more check functions; or maybe one
or more flags meaning something like "I know this can go wrong, so don't
complain (too loudly)". Probing comes to mind as an example.
Many years ago, we used to check the y2log for warnings and errors
before each release, and if there were any, we decided to either
downgrade the message to y2milestone or to fix the underlying problem.
IMHO this is still the way to go.
So, IMHO rather than inventing yet another level of problem reporting,
we should clean up what we already have. Since we have been neglecting
that for so many years, it will take a while; but still this is clearly
necessary. Please let's not work around that problem with yet another
layer of indirection. Remember RFC 1925? ;-)
Kind regards
--
Stefan Hundhammer
participants (5)
-
josef Reidinger
-
José Iván López González
-
Lukas Ocilka
-
Stefan Hundhammer
-
William Brown