| Message ID | 20240214193743.963349-6-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Using l_notice for low level IWD state information | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
Hi James, > With the introduction of l_notice in IWD some guidelines need to be > set for l_info, l_warn, l_error, l_debug and l_notice. > --- > doc/coding-style.txt | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/doc/coding-style.txt b/doc/coding-style.txt > index bf0535c5..fdd9e0e3 100644 > --- a/doc/coding-style.txt > +++ b/doc/coding-style.txt > @@ -317,6 +317,30 @@ Functions that are static should not be forward-declared. The only exception > to this rule is if a circular dependency condition exists, and the forward > declaration cannot be avoided. > > +M18: Use appropriate logging levels > +=================================== > +An appropriate log level should be used depending on the type of message > +being logged. Logging is done using the l_log APIs in ELL: > + > +l_info Information that is expected during normal operation. l_info's use > + should be very limited so non-debug logs are concise > +l_warn An unexpected, but non-fatal condition ocurred > +l_error An unexpected condition ocurred. These are generally fatal to the > + current connection/protocol that is running but not generally to IWD's > + overall operation. > +l_debug General debugging. These can be used relatively freely but should > + provide some piece of useful information. > +l_notice Reserved for specific event-type notifications about IWD's internal > + state. These are logs that are mean to be both human-readable and > + parsed by tooling so they are required to be of a certain syntax. They > + should start with "event: <name>" followed by comma separated key > + value pairs containing the data of interest. Event names and their > + arguments should be consistent across the code base, i.e. two events > + called in different locations should have the same arguments. and now they are not even in order of their log level. That confuses people. > + > + For example: > + > + l_notice("event: mycondition, arg1: value1, arg2: value2, ..."); Please no semantics for the freeform text. If you want that, then better build some proper iwd_log functionality that does that. Regards Marcel
Hi Marcel, On 2/14/24 11:49 AM, Marcel Holtmann wrote: > Hi James, > >> With the introduction of l_notice in IWD some guidelines need to be >> set for l_info, l_warn, l_error, l_debug and l_notice. >> --- >> doc/coding-style.txt | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/doc/coding-style.txt b/doc/coding-style.txt >> index bf0535c5..fdd9e0e3 100644 >> --- a/doc/coding-style.txt >> +++ b/doc/coding-style.txt >> @@ -317,6 +317,30 @@ Functions that are static should not be forward-declared. The only exception >> to this rule is if a circular dependency condition exists, and the forward >> declaration cannot be avoided. >> >> +M18: Use appropriate logging levels >> +=================================== >> +An appropriate log level should be used depending on the type of message >> +being logged. Logging is done using the l_log APIs in ELL: >> + >> +l_info Information that is expected during normal operation. l_info's use >> + should be very limited so non-debug logs are concise >> +l_warn An unexpected, but non-fatal condition ocurred >> +l_error An unexpected condition ocurred. These are generally fatal to the >> + current connection/protocol that is running but not generally to IWD's >> + overall operation. >> +l_debug General debugging. These can be used relatively freely but should >> + provide some piece of useful information. >> +l_notice Reserved for specific event-type notifications about IWD's internal >> + state. These are logs that are mean to be both human-readable and >> + parsed by tooling so they are required to be of a certain syntax. They >> + should start with "event: <name>" followed by comma separated key >> + value pairs containing the data of interest. Event names and their >> + arguments should be consistent across the code base, i.e. two events >> + called in different locations should have the same arguments. > and now they are not even in order of their log level. That confuses people. You mean just the order they appear in this document or using l_notice for this purpose in general? > >> + >> + For example: >> + >> + l_notice("event: mycondition, arg1: value1, arg2: value2, ..."); > Please no semantics for the freeform text. If you want that, then better build some proper iwd_log functionality that does that. The intent was to use l_notice only for this purpose so I felt describing the specific semantics was appropriate. I can remove the example but since the intended use is very specific and has some syntax requirements I think describing those here is needed. > > Regards > > Marcel >
Hi James, >>> With the introduction of l_notice in IWD some guidelines need to be >>> set for l_info, l_warn, l_error, l_debug and l_notice. >>> --- >>> doc/coding-style.txt | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/doc/coding-style.txt b/doc/coding-style.txt >>> index bf0535c5..fdd9e0e3 100644 >>> --- a/doc/coding-style.txt >>> +++ b/doc/coding-style.txt >>> @@ -317,6 +317,30 @@ Functions that are static should not be forward-declared. The only exception >>> to this rule is if a circular dependency condition exists, and the forward >>> declaration cannot be avoided. >>> >>> +M18: Use appropriate logging levels >>> +=================================== >>> +An appropriate log level should be used depending on the type of message >>> +being logged. Logging is done using the l_log APIs in ELL: >>> + >>> +l_info Information that is expected during normal operation. l_info's use >>> + should be very limited so non-debug logs are concise >>> +l_warn An unexpected, but non-fatal condition ocurred >>> +l_error An unexpected condition ocurred. These are generally fatal to the >>> + current connection/protocol that is running but not generally to IWD's >>> + overall operation. >>> +l_debug General debugging. These can be used relatively freely but should >>> + provide some piece of useful information. >>> +l_notice Reserved for specific event-type notifications about IWD's internal >>> + state. These are logs that are mean to be both human-readable and >>> + parsed by tooling so they are required to be of a certain syntax. They >>> + should start with "event: <name>" followed by comma separated key >>> + value pairs containing the data of interest. Event names and their >>> + arguments should be consistent across the code base, i.e. two events >>> + called in different locations should have the same arguments. >> and now they are not even in order of their log level. That confuses people. > You mean just the order they appear in this document or using l_notice for this purpose in general? the order number / priority they have in reality according to syslog. >> >>> + >>> + For example: >>> + >>> + l_notice("event: mycondition, arg1: value1, arg2: value2, ..."); >> Please no semantics for the freeform text. If you want that, then better build some proper iwd_log functionality that does that. > The intent was to use l_notice only for this purpose so I felt describing the specific semantics was appropriate. I can remove the example but since the intended use is very specific and has some syntax requirements I think describing those here is needed. If you want something like that then lets enforce that with iwd_log or iwd_notice helper and not via some one line example in the coding style document. Regards Marcel
diff --git a/doc/coding-style.txt b/doc/coding-style.txt index bf0535c5..fdd9e0e3 100644 --- a/doc/coding-style.txt +++ b/doc/coding-style.txt @@ -317,6 +317,30 @@ Functions that are static should not be forward-declared. The only exception to this rule is if a circular dependency condition exists, and the forward declaration cannot be avoided. +M18: Use appropriate logging levels +=================================== +An appropriate log level should be used depending on the type of message +being logged. Logging is done using the l_log APIs in ELL: + +l_info Information that is expected during normal operation. l_info's use + should be very limited so non-debug logs are concise +l_warn An unexpected, but non-fatal condition ocurred +l_error An unexpected condition ocurred. These are generally fatal to the + current connection/protocol that is running but not generally to IWD's + overall operation. +l_debug General debugging. These can be used relatively freely but should + provide some piece of useful information. +l_notice Reserved for specific event-type notifications about IWD's internal + state. These are logs that are mean to be both human-readable and + parsed by tooling so they are required to be of a certain syntax. They + should start with "event: <name>" followed by comma separated key + value pairs containing the data of interest. Event names and their + arguments should be consistent across the code base, i.e. two events + called in different locations should have the same arguments. + + For example: + + l_notice("event: mycondition, arg1: value1, arg2: value2, ..."); O1: Shorten the name ====================