Opened 8 years ago

Closed 8 years ago

#1074 closed enhancement (complete)

Use debug levels consistently

Reported by: stephen Owned by: stephen
Priority: low Milestone: Sprint-20111108
Component: logging Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 6.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

(Summarised from bind10-dev, 29 June 2011)

missing debug number ("A debug message" or "only appear if debug is enabled" is too vague -- when will it be logged at what debug level?)

I think there should be separate "DEBUG" severity for developers too.... now we have debug message useful for admins to debug and then other debug message clearly only useful for admins (even referring to specific code "the RecursiveQuery::resolve method...").

Agreed. Although 100 debug levels are available, for consistency we need to define how they are used. We have discussed the idea of debug levels of 20 or below being used for messages that might be useful to SysAdmins and levels above 20 being reserved for "programmer-level" messages. Perhaps now is the time to formalise that?"

This ticket should be done after all other tickets relating to the addition of logging messages have been completed. The categories of debug messages should be reviewed and debug levels assigned such that the correspondence between category and level is consistent across all modules.

Subtickets

Change History (15)

comment:1 Changed 8 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 6

comment:2 Changed 8 years ago by stephen

  • Owner set to stephen
  • Status changed from new to assigned

comment:3 Changed 8 years ago by stephen

  • Owner changed from stephen to UnAssigned

comment:5 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20110927 to Sprint-20111011

comment:6 Changed 8 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:7 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20111011 to Sprint-20111025

comment:8 Changed 8 years ago by stephen

  • Owner changed from stephen to UnAssigned
  • Status changed from assigned to reviewing

As only a few common debug levels have been set and future modules may need to defined their own debug levels, the following approach has been taken: the symbols used in different modules have been left alone alone, but they are now defined in terms of the common levels.

comment:9 Changed 8 years ago by jinmei

I'm not going to review it (at least not yet), but I've taken a quick
look at the diff, and noticed some redundant white spaces at EOLs.
These should be removed.

comment:10 Changed 8 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:11 Changed 8 years ago by jelte

  • Owner changed from jelte to stephen

log_dbglevels.h: don't know yet if i completely agree with the intro, but
practice may have to show that. For instance, "covers things like
startup-steps and config messages", i think it should also be things like bad
responses from nameservers that do not interrupt operations, but may be useful
for fixing bigger setups etc. from that would follow that 30< for applications,
and 30> for libraries may be too strict a definition.

if such things are to be DEBUG as well, we probably need to add one or more
consts for it as well, but we'll see that when we get there (may become more
relevant once we refocus on resolver)

changed "N.B." to "\note" btw. (and removed a few spaces), please verify commit.

Should we consider replacing the module-specific levels with the general ones everywhere?

Would we want to check all constants in log_test.py? (not sure)

These are just discussion points, and the code looks ok, so if the answer is
'no' to all, it can be merged :)

comment:12 Changed 8 years ago by stephen

  • Owner changed from stephen to jelte

log_dbglevels.h: don't know yet if i completely agree with the intro, but practice may have to show that. For instance, "covers things like startup-steps and config messages", i think it should also be things like bad responses from nameservers that do not interrupt operations, but may be useful for fixing bigger setups etc. from that would follow that 30< for applications, and 30> for libraries may be too strict a definition.

Things like bad responses from nameservers can certainly be included in the category "useful to an administrator". If 30 is too low a value to distinguish between categories, we can certainly shift the boundary.

The idea of "useful to an administrator" was to signal the to the user that it is no use in setting the debug level too high, as the messages that come back are likely to be of no use unless you understand the code, and so will only clog up your log file.

if such things are to be DEBUG as well, we probably need to add one or more consts for it as well, but we'll see that when we get there (may become more relevant once we refocus on resolver)

The current set of constants were created looking at the levels defined in the various modules. I certainly don't think they are completely correct, and they may need to be "tuned" as development progresses. Also, I think it is entirely likely that we will add more levels. However, bear in mind that if we want to track a particular category of conditions, we could always do that via a separate logger using one of the standard debug levels.

changed "N.B." to "\note" btw. (and removed a few spaces), please verify commit.

Thanks.

Should we consider replacing the module-specific levels with the general ones everywhere?

I thought of that, but what decided me against it was:

  1. The possibility that at some time we change the relative numbers of the general constants, which in turn may need some modules having their debug levels modified. With the indirection of module-specific levels, there is just one place where changes will need to be made.
  2. The possibility that modules may want to define custom levels (e.g. a "trace intermediate"). In that case they would use a combination of general debug level constants and module-specific ones. It seemed cleaner to have all the levels for a module defined in one place.

Would we want to check all constants in log_test.py? (not sure)

I didn't do that in case we ever changed some of the values (so would have to alter the tests). The main reason for a test of the value of one of the constants was to ensure that the definitions added to the .cc file were OK.

comment:13 Changed 8 years ago by jelte

  • Owner changed from jelte to stephen

Ok, thanks, please go ahead and merge

comment:14 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20111025 to Sprint-20111108

comment:15 Changed 8 years ago by stephen

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged into master as commit e13d28918a391060d9c1f286d19308cb10975cd9

Note: See TracTickets for help on using tickets.