Feature #5034
openlogging: Enable category name instead of hex val by default
90%
Description
Discussion started in this patch:
https://gerrit.osmocom.org/c/libosmocore/+/12095
Summary/outcome:
As a reference, I think it makes more sense to change it for all log targets this way:diff --git a/src/logging.c b/src/logging.c
index a40008e9..f878be5a 100644
--- a/src/logging.c
+++ b/src/logging.c
@@ -918,7 +918,8 @@ struct log_target *log_target_create(void)
target->use_color = 1;
target->print_timestamp = 0;
target->print_filename2 = LOG_FILENAME_PATH;
- target->print_category_hex = true;
+ target->print_category_hex = false;
+ target->print_category = true;
So here actually by default we disable the hex output and we enable the string representation. Once this is applied, some tests will need to be adapted since they may not set those fields explicitly and use default ones.
Steps:
- Apply change in libosmocore locally
- Build other projects and see if a test in any projects fails. For each test failing, make sure to set explicit values for log_set_print_category{_hex} to match current expectancies.
- Once those patches are merged, submit + merge libosmocore patch changing the default.
Related issues
Updated by pespin about 3 years ago
First bunch of related patches (libosmocore only):
remote: https://gerrit.osmocom.org/c/libosmocore/+/22961 tests: Set print_category values explicitly [NEW]
remote: https://gerrit.osmocom.org/c/libosmocore/+/22962 Drop use of log_set_print_filename() API inside libosmocore [NEW]
remote: https://gerrit.osmocom.org/c/libosmocore/+/22963 logging: Deprecate API log_set_print_filename [NEW]
remote: https://gerrit.osmocom.org/c/libosmocore/+/12095 logging: Enable category name instead of hex val by default [WIP]
Updated by pespin about 3 years ago
- Status changed from New to Stalled
- % Done changed from 0 to 90
I submitted a bunch of patches to explictly set category printing in all tests to avoid breakage starting from current master if we even change the default value.
However, the libosmocore patch changing the default from hex to string (https://gerrit.osmocom.org/c/libosmocore/+/12095) cannot be merged because it would prevent older test versions of different projects to build against newer libosmocore, which is something we want to avoid.
So the idea here is then to keep all tests always setting the category printing settings for a while until we decide backward compatibility is good enough and we can merge the patch.
Updated by neels almost 3 years ago
there are also other changes i would assume are a much better default logging config:
- actually print the log level (DEBUG, NOTICE, ERROR, ...)
logging print level 1
target->print_level = true;
- printing the source file and line after the log message, not before. And only print the basename, not the full path:
logging print file basename last
target->print_filename2 = LOG_FILENAME_PATH;
target->print_filename_pos = LOG_FILENAME_POS_LINE_END;
Is it too late now?
Updated by laforge almost 3 years ago
On Thu, Jun 10, 2021 at 03:42:03PM +0000, neels [REDMINE] wrote:
there are also other changes i would assume are a much better default logging config:
I agree.
Is it too late now?
Probably needs some thought. For sure we should make that dependent on the log output,
e.g. for syslog target, reasonable defaults may very well be different than for console or file.
