From 77ae45d22f49ee712850e3b5d4c16112dca134d0 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 27 Aug 2018 20:32:36 +0200 Subject: log: add 'last' option to 'logging print file' Allow printing the source file information *after* the log information. Add target->print_filename_pos, log_set_print_filename_pos() and enum log_filename_pos, and the optional 'last' keyword to the 'logging print file' vty command. (An enum in case we want to add other positions later.) Rationale: on log lines, the logging context is usually printed first in the logging text. Without the source file information, this logging context pretty much aligns in an easily readable way. When adding the source file information, each line's logging context info is shifted right by a pretty much random amount and it is hard to spot recurring logging contexts. One solution is to switch off source file info, of course, but that's not an option when actively hacking on bugs and new features. For example, it is unnecessarily hard to spot lchan FSM related log lines in this osmo-bsc log snippet: DRSL NOTICE mgw_endpoint_fsm.c:603 mgw-endpoint(conn0){WAIT_MGW_RESPONSE}: Freeing instance DRSL NOTICE fsm.c:381 mgw-endpoint(conn0){WAIT_MGW_RESPONSE}: Deallocated DMSC NOTICE mgw_endpoint_fsm.c:603 SUBSCR_CONN(conn0){CLEARING}: Received Event FORGET_MGW_ENDPOINT DCHAN DEBUG lchan_rtp_fsm.c:741 lchan(0-0-1-TCH_F-0){WAIT_RLL_RTP_ESTABLISH}: Received Event LCHAN_EV_RTP_RELEASED DCHAN DEBUG lchan_rtp_fsm.c:520 lchan_rtp(0-0-1-TCH_F-0){ROLLBACK}: Freeing instance DCHAN DEBUG fsm.c:381 lchan_rtp(0-0-1-TCH_F-0){ROLLBACK}: Deallocated DCHAN DEBUG lchan_rtp_fsm.c:520 lchan(0-0-1-TCH_F-0){WAIT_RLL_RTP_ESTABLISH}: Received Event LCHAN_EV_RTP_RELEASED DCHAN DEBUG lchan_fsm.c:1232 lchan(0-0-1-TCH_F-0){WAIT_RLL_RTP_ESTABLISH}: state_chg to WAIT_RF_RELEASE_ACK DRSL NOTICE handover_fsm.c:762 handover(interBSCin_conn0){WAIT_RR_HO_DETECT}: Terminating (cause = OSMO_FSM_TERM_REGULAR) DRSL NOTICE handover_fsm.c:762 handover(interBSCin_conn0){WAIT_RR_HO_DETECT}: Freeing instance DRSL NOTICE fsm.c:381 handover(interBSCin_conn0){WAIT_RR_HO_DETECT}: Deallocated DCHAN DEBUG lchan_fsm.c:1196 lchan(0-0-1-TCH_F-0){WAIT_RF_RELEASE_ACK}: state_chg to BORKEN DCHAN DEBUG lchan_fsm.c:354 lchan(0-0-1-TCH_F-0){BORKEN}: (type=TCH_F) Clearing lchan state DLMGCP DEBUG mgcp_client.c:741 Tx MGCP msg to MGCP GW: 'DLCX 3 2@mgw MGCP 1.0' DLMGCP DEBUG mgcp_client.c:743 Sending msg to MGCP GW size: 66 DRLL NOTICE mgcp_client_fsm.c:422 MGCP_CONN(to-BTS){ST_DLCX_RESP}: Received Event EV_DLCX_RESP Placing the source file info behind the log text makes it much easier to follow, while the source file info is still available: DRSL NOTICE mgw-endpoint(conn0){WAIT_MGW_RESPONSE}: Freeing instance (mgw_endpoint_fsm.c:603) DRSL NOTICE mgw-endpoint(conn0){WAIT_MGW_RESPONSE}: Deallocated (fsm.c:381) DMSC NOTICE SUBSCR_CONN(conn0){CLEARING}: Received Event FORGET_MGW_ENDPOINT (mgw_endpoint_fsm.c:603) DCHAN DEBUG lchan(0-0-1-TCH_F-0){WAIT_RLL_RTP_ESTABLISH}: Received Event LCHAN_EV_RTP_RELEASED (lchan_rtp_fsm.c:741) DCHAN DEBUG lchan_rtp(0-0-1-TCH_F-0){ROLLBACK}: Freeing instance (lchan_rtp_fsm.c:520) DCHAN DEBUG lchan_rtp(0-0-1-TCH_F-0){ROLLBACK}: Deallocated (fsm.c:381) DCHAN DEBUG lchan(0-0-1-TCH_F-0){WAIT_RLL_RTP_ESTABLISH}: Received Event LCHAN_EV_RTP_RELEASED (lchan_rtp_fsm.c:520) DCHAN DEBUG lchan(0-0-1-TCH_F-0){WAIT_RLL_RTP_ESTABLISH}: state_chg to WAIT_RF_RELEASE_ACK (lchan_fsm.c:1232) DRSL NOTICE handover(interBSCin_conn0){WAIT_RR_HO_DETECT}: Terminating (cause = OSMO_FSM_TERM_REGULAR) (handover_fsm.c:745) DRSL NOTICE handover(interBSCin_conn0){WAIT_RR_HO_DETECT}: Freeing instance (handover_fsm.c:745) DRSL NOTICE handover(interBSCin_conn0){WAIT_RR_HO_DETECT}: Deallocated (fsm.c:381) DCHAN DEBUG lchan(0-0-1-TCH_F-0){WAIT_RF_RELEASE_ACK}: state_chg to BORKEN (lchan_fsm.c:1196) DCHAN DEBUG lchan(0-0-1-TCH_F-0){BORKEN}: (type=TCH_F) Clearing lchan state (lchan_fsm.c:354) DLMGCP DEBUG Tx MGCP msg to MGCP GW: 'DLCX 3 12@mgw MGCP 1.0' (mgcp_client.c:741) DLMGCP DEBUG Sending msg to MGCP GW size: 67 (mgcp_client.c:743) DRLL NOTICE MGCP_CONN(to-BTS){ST_DLCX_RESP}: Received Event EV_DLCX_RESP (mgcp_client_fsm.c:422) Implementation: for 'last', insert source file info only when an '\n' is found at the end of the log line composition buffer, so that LOGP()...LOGPC() constructs also print source file info only when a log line actually ends. Change-Id: I393907b3c9e0cc1145e102328adad0a83ee13a9f --- include/osmocom/core/logging.h | 9 ++++++++ src/logging.c | 51 +++++++++++++++++++++++++++++++++++++----- src/vty/logging_vty.c | 10 +++++++-- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index 3cae63da..8464043f 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -228,6 +228,12 @@ enum log_filename_type { LOG_FILENAME_BASENAME, }; +/*! Where on a log line source file and line should be logged. */ +enum log_filename_pos { + LOG_FILENAME_POS_HEADER_END, + LOG_FILENAME_POS_LINE_END, +}; + /*! structure representing a logging target */ struct log_target { struct llist_head entry; /*!< linked list */ @@ -313,6 +319,8 @@ struct log_target { bool print_category_hex; /* Should we print the source file and line, and in which way? */ enum log_filename_type print_filename2; + /* Where on a log line to put the source file info. */ + enum log_filename_pos print_filename_pos; }; /* use the above macros */ @@ -335,6 +343,7 @@ void log_set_print_extended_timestamp(struct log_target *target, int); void log_set_print_timestamp(struct log_target *target, int); void log_set_print_filename(struct log_target *target, int); void log_set_print_filename2(struct log_target *target, enum log_filename_type lft); +void log_set_print_filename_pos(struct log_target *target, enum log_filename_pos pos); void log_set_print_category(struct log_target *target, int); void log_set_print_category_hex(struct log_target *target, int); void log_set_print_level(struct log_target *target, int); diff --git a/src/logging.c b/src/logging.c index edb2b56a..de0f2b0f 100644 --- a/src/logging.c +++ b/src/logging.c @@ -406,27 +406,55 @@ static void _output(struct log_target *target, unsigned int subsys, goto err; OSMO_SNPRINTF_RET(ret, rem, offset, len); } + + if (target->print_filename_pos == LOG_FILENAME_POS_HEADER_END) { + switch (target->print_filename2) { + case LOG_FILENAME_NONE: + break; + case LOG_FILENAME_PATH: + ret = snprintf(buf + offset, rem, "%s:%d ", file, line); + if (ret < 0) + goto err; + OSMO_SNPRINTF_RET(ret, rem, offset, len); + break; + case LOG_FILENAME_BASENAME: + ret = snprintf(buf + offset, rem, "%s:%d ", const_basename(file), line); + if (ret < 0) + goto err; + OSMO_SNPRINTF_RET(ret, rem, offset, len); + break; + } + } + } + ret = vsnprintf(buf + offset, rem, format, ap); + if (ret < 0) + goto err; + OSMO_SNPRINTF_RET(ret, rem, offset, len); + + /* For LOG_FILENAME_POS_LAST, print the source file info only when the caller ended the log + * message in '\n'. If so, nip the last '\n' away, insert the source file info and re-append an + * '\n'. All this to allow LOGP("start..."); LOGPC("...end\n") constructs. */ + if (target->print_filename_pos == LOG_FILENAME_POS_LINE_END + && offset > 0 && buf[offset-1] == '\n') { switch (target->print_filename2) { case LOG_FILENAME_NONE: break; case LOG_FILENAME_PATH: - ret = snprintf(buf + offset, rem, "%s:%d ", file, line); + offset --; + ret = snprintf(buf + offset, rem, " (%s:%d)\n", file, line); if (ret < 0) goto err; OSMO_SNPRINTF_RET(ret, rem, offset, len); break; case LOG_FILENAME_BASENAME: - ret = snprintf(buf + offset, rem, "%s:%d ", const_basename(file), line); + offset --; + ret = snprintf(buf + offset, rem, " (%s:%d)\n", const_basename(file), line); if (ret < 0) goto err; OSMO_SNPRINTF_RET(ret, rem, offset, len); break; } } - ret = vsnprintf(buf + offset, rem, format, ap); - if (ret < 0) - goto err; - OSMO_SNPRINTF_RET(ret, rem, offset, len); if (target->use_color) { ret = snprintf(buf + offset, rem, "\033[0;m"); @@ -677,6 +705,17 @@ void log_set_print_filename2(struct log_target *target, enum log_filename_type l target->print_filename2 = lft; } +/*! Set the position where on a log line the source file info should be logged. + * \param[in] target Log target to be affected. + * \param[in] pos A LOG_FILENAME_POS_* enum value. + * LOG_FILENAME_POS_DEFAULT logs just before the caller supplied log message. + * LOG_FILENAME_POS_LAST logs only at the end of a log line, where the caller issued an '\n' to end the + */ +void log_set_print_filename_pos(struct log_target *target, enum log_filename_pos pos) +{ + target->print_filename_pos = pos; +} + /*! Enable or disable printing of the category name * \param[in] target Log target to be affected * \param[in] print_catname Enable (1) or disable (0) filenames diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c index 11949144..8c8a3326 100644 --- a/src/vty/logging_vty.c +++ b/src/vty/logging_vty.c @@ -246,12 +246,14 @@ static const struct value_string logging_print_file_args[] = { DEFUN(logging_prnt_file, logging_prnt_file_cmd, - "logging print file (0|1|basename)", + "logging print file (0|1|basename) [last]", LOGGING_STR "Log output settings\n" "Configure log message\n" "Don't prefix each log message\n" "Prefix each log message with the source file and line\n" - "Prefix each log message with the source file's basename (strip leading paths) and line\n") + "Prefix each log message with the source file's basename (strip leading paths) and line\n" + "Log source file info at the end of a log line. If omitted, log source file info just" + " before the log text.\n") { struct log_target *tgt = osmo_log_vty2tgt(vty); @@ -259,6 +261,10 @@ DEFUN(logging_prnt_file, return CMD_WARNING; log_set_print_filename2(tgt, get_string_value(logging_print_file_args, argv[0])); + if (argc > 1) + log_set_print_filename_pos(tgt, LOG_FILENAME_POS_LINE_END); + else + log_set_print_filename_pos(tgt, LOG_FILENAME_POS_HEADER_END); return CMD_SUCCESS; } -- cgit v1.2.3