Project

General

Profile

Actions

Feature #5034

open

logging: Enable category name instead of hex val by default

Added by pespin 9 months ago. Updated 6 months ago.

Status:
Stalled
Priority:
Low
Assignee:
Category:
libosmocore
Target version:
-
Start date:
02/18/2021
Due date:
% Done:

90%

Spec Reference:

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.
Actions #1

Updated by pespin 9 months 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]

Actions #2

Updated by pespin 9 months 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.

Actions #3

Updated by pespin 9 months ago

  • Priority changed from Normal to Low
Actions #4

Updated by laforge 8 months ago

  • Category set to libosmocore
Actions #5

Updated by neels 6 months 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?

Actions #6

Updated by laforge 6 months 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.

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)