Logging system rework #59
Conversation
bieryAtFnal
added
the
miscellaneous deliverable
| TLVL_VERY_IMPORTANT = 1, | ||
| TLVL_IMPORTANT = 2, | ||
| TLVL_GENERAL = 3, | ||
| TLVL_DEBUG_INFO = 4, | ||
| TLVL_DEBUG_LOW = 5, | ||
| TLVL_DEBUG_MEDIUM = 10, | ||
| TLVL_DEBUG_HIGH = 15, | ||
| TLVL_DEBUG_ALL = 20 |
There was a problem hiding this comment.
IMPORTANT and VERY_IMPORTANT is strange naming (I guess GENERAL and DEBUG_INFO are too). Is the naming similar across the rest of the DAQ? The usual naming for different logging verbosity in the industry is:
TRACEfor everything, soTLVL_DEBUG_ALLDEBUGfor the debugging messages (here it makes sense to have low, medium, high)INFOwould be yourVERY_IMPORTANT,IMPORTANTand/orDEBUG_INFO,WARNfor warnings (we have different system for that),ERRORfor errors (we have different system for that),FATALfor fatal errors that should crash (we have different system for that).
What are the namings in the rest of the DAQ?
There was a problem hiding this comment.
This naming is completely custom. As far as I know, there is no consistency across dunedaq. There are some repos that have a version of this, example in-file enum here and separare logging header here. Other places simply use integers example here, and these do go to high values (example here).
I just made that list according to what made sense to me, and wanted it to (a) be granural at the lower levels and (b) roughly cover the available space for levels. Open to changes. As you mentioned, we are not using standards from the industry (or at least not closely).
| @@ -34,7 +40,7 @@ TriggerCandidateMakerMichelElectron::operator()(const TriggerActivity& activity, | |||
|
|
|||
| // add_window_to_record(m_current_window); | |||
| // dump_window_record(); | |||
| // TLOG(1) << "Constructing trivial TC."; | |||
| TLOG_DEBUG(TLVL_DEBUG_LOW) << "[TCM:ME] Constructing trivial TC."; | |||
There was a problem hiding this comment.
| TLOG_DEBUG(TLVL_DEBUG_LOW) << "[TCM:ME] Constructing trivial TC."; | |
| TLOG_DEBUG(TLVL_DEBUG_LOW) << "[TCM:ME] Constructing a TC."; |
I know this is not your doing, but might as well correct this... why "Constructing trivial TC"? Why "trivial"? Are there non-trivial TCs? I think just "Constructing a TC" would be fine here, and less confusing...
There was a problem hiding this comment.
I have not been generally modifying the messages (outside of few examples in MLT where I added more information). I agree with your change.
| @@ -34,8 +40,8 @@ TriggerCandidateMakerPlaneCoincidence::operator()(const TriggerActivity& activit | |||
|
|
|||
| // add_window_to_record(m_current_window); | |||
| // dump_window_record(); | |||
| TLOG(1) << "Constructing trivial TC."; | |||
| TLOG(1) << "Activity count: " << m_activity_count; | |||
| TLOG_DEBUG(TLVL_DEBUG_LOW) << "[TCM:PC] Constructing trivial TC."; | |||
There was a problem hiding this comment.
| TLOG_DEBUG(TLVL_DEBUG_LOW) << "[TCM:PC] Constructing trivial TC."; | |
| TLOG_DEBUG(TLVL_DEBUG_LOW) << "[TCM:PC] Constructing a TC."; |
Ditto w.g. "trivial"
|
Thanks for the review. I have removed the 'trivial' messages in comment above. |
Please see details in DUNE-DAQ/trigger#285