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 <lukas.ocilka@suse.com> 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
On 1/18/21 8:14 AM, josef Reidinger wrote:
On Fri, 15 Jan 2021 16:29:11 +0100 Lukas Ocilka <lukas.ocilka@suse.com> 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 <shundhammer@suse.de> YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
participants (5)
-
josef Reidinger -
José Iván López González -
Lukas Ocilka -
Stefan Hundhammer -
William Brown