From d4261b553d171a4ba56f0c54be90df6c69b20a29 Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 12:02:19 -0800 Subject: [PATCH 01/11] journal probe: fix error handling of read_new_entries() The error case is negative, so fix the return code conditional check appropriately. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index 5f5fcde..8c20338 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -1,7 +1,7 @@ /* * This program is part of the Clear Linux Project * - * Copyright 2015 Intel Corporation + * Copyright 2015-2017 Intel Corporation * * This program is free software; you can redistribute it and/or modify it under * the terms and conditions of the GNU Lesser General Public License, as @@ -148,7 +148,8 @@ static bool process_existing_entries(sd_journal *journal) return false; } - if (!read_new_entries(journal)) { + ret = read_new_entries(journal); + if (ret < 0) { return false; } From 8780f3c32cd6314352b91f39bac26b19d842576e Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 12:21:00 -0800 Subject: [PATCH 02/11] journal probe: match all messages ERR or higher Previous behavior was to only filter LOG_ERR messages from the telemetrics crashprobe, but it will be helpful to make this probe more generic to capture log messages with the highest log levels. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index 8c20338..5e9eb52 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -202,7 +202,13 @@ static bool add_filters(sd_journal *journal) } free(data); - r = sd_journal_add_match(journal, "SYSLOG_IDENTIFIER=crashprobe", 0); + r = sd_journal_add_match(journal, "PRIORITY=1", 0); + if (r < 0) { + tm_journal_match_err(r); + return false; + } + + r = sd_journal_add_match(journal, "PRIORITY=2", 0); if (r < 0) { tm_journal_match_err(r); return false; From 1f2083b086ae47a5cb120d5e2d9f15a367f9c2b7 Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 12:28:24 -0800 Subject: [PATCH 03/11] journal probe: add newlines for to each message in the payload The make payloads more legible, make sure separate messages are newline separated. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index 5e9eb52..830fb34 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -61,11 +61,11 @@ static inline void tm_journal_match_err(int ret) static void add_to_payload(const void *data, size_t length) { if (payload != NULL) { - g_string_append_printf(payload, "%.*s", (int)length, + g_string_append_printf(payload, "%.*s\n", (int)length, (char *)data); } else { payload = g_string_new(NULL); - g_string_printf(payload, "%.*s", (int)length, + g_string_printf(payload, "%.*s\n", (int)length, (char *)data); } } From 0adfc636782b8f89d4453370cc11bc7e50e5ae6a Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 12:54:49 -0800 Subject: [PATCH 04/11] journal probe: fix formatting of some log messages Signed-off-by: Patrick McCarty --- src/probes/journal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index 830fb34..eae891a 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -77,7 +77,7 @@ static bool send_data(char *class) if ((ret = tm_create_record(&handle, severity, class, payload_version)) < 0) { - telem_log(LOG_ERR, "Failed to create record: %s", + telem_log(LOG_ERR, "Failed to create record: %s\n", strerror(-ret)); goto fail; } @@ -86,7 +86,7 @@ static bool send_data(char *class) payload = NULL; if ((ret = tm_set_payload(handle, (char *)payload_str)) < 0) { - telem_log(LOG_ERR, "Failed to set payload: %s", strerror(-ret)); + telem_log(LOG_ERR, "Failed to set payload: %s\n", strerror(-ret)); free(payload_str); tm_free_record(handle); goto fail; @@ -95,7 +95,7 @@ static bool send_data(char *class) free(payload_str); if ((ret = tm_send_record(handle)) < 0) { - telem_log(LOG_ERR, "Failed to send record: %s", strerror(-ret)); + telem_log(LOG_ERR, "Failed to send record: %s\n", strerror(-ret)); tm_free_record(handle); goto fail; } From 9dcdc0d6475c8008a7bfcff8a2238f9d107f171a Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 13:06:09 -0800 Subject: [PATCH 05/11] journal probe: add macros to help with using the journal API To avoid having to read the repetitive error handling when adding journal filters, add some helper macros. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/probes/journal.c b/src/probes/journal.c index eae891a..aeace2f 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -186,6 +186,33 @@ static bool get_boot_id(char **data) return true; } +#define JOURNAL_MATCH(data) \ + do { \ + r = sd_journal_add_match(journal, data, 0); \ + if (r < 0) { \ + tm_journal_match_err(r); \ + return false; \ + } \ + } while (0); + +#define JOURNAL_AND \ + do { \ + r = sd_journal_add_conjunction(journal); \ + if (r < 0) { \ + tm_journal_match_err(r); \ + return false; \ + } \ + } while (0); + +#define JOURNAL_OR \ + do { \ + r = sd_journal_add_disjunction(journal); \ + if (r < 0) { \ + tm_journal_match_err(r); \ + return false; \ + } \ + } while (0); + static bool add_filters(sd_journal *journal) { char *data = NULL; From 9834e9f4dfcdd79e9661650a201ff081423a0e5d Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 13:09:00 -0800 Subject: [PATCH 06/11] journal probe: use the new macros Signed-off-by: Patrick McCarty --- src/probes/journal.c | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index aeace2f..e100f24 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -222,30 +222,12 @@ static bool add_filters(sd_journal *journal) return false; } - r = sd_journal_add_match(journal, data, 0); - if (r < 0) { - tm_journal_match_err(r); - return false; - } + JOURNAL_MATCH(data); free(data); - - r = sd_journal_add_match(journal, "PRIORITY=1", 0); - if (r < 0) { - tm_journal_match_err(r); - return false; - } - - r = sd_journal_add_match(journal, "PRIORITY=2", 0); - if (r < 0) { - tm_journal_match_err(r); - return false; - } - - r = sd_journal_add_match(journal, "PRIORITY=3", 0); - if (r < 0) { - tm_journal_match_err(r); - return false; - } + // The three highest log levels, all indicating errors + JOURNAL_MATCH("PRIORITY=1"); + JOURNAL_MATCH("PRIORITY=2"); + JOURNAL_MATCH("PRIORITY=3"); return true; } From 7dee7644b2c94d6bdee24777c72efd4c36d3f572 Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 13:09:54 -0800 Subject: [PATCH 07/11] journal probe: filter on EXIT_CODE as well This field appears to be set when services fail, so filter on it as well. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/probes/journal.c b/src/probes/journal.c index e100f24..d043974 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -224,10 +224,14 @@ static bool add_filters(sd_journal *journal) JOURNAL_MATCH(data); free(data); + JOURNAL_AND; // The three highest log levels, all indicating errors JOURNAL_MATCH("PRIORITY=1"); JOURNAL_MATCH("PRIORITY=2"); JOURNAL_MATCH("PRIORITY=3"); + JOURNAL_OR; + // Only set for service-level error conditions + JOURNAL_MATCH("EXIT_CODE=exited"); return true; } From 4e79ea10df711ce3c97ff31af223565ab1101dc0 Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 13:27:50 -0800 Subject: [PATCH 08/11] journal probe: send records for every log message The previous logic concatenates log messages on initial startup, when the entire journal is read to process existing messages. But doing so might run into the payload size limit (8KB), and thus fail to create a record. Sending one record per log message will ensure that the payload size remains relatively small, almost always below the 8KB size limit. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index d043974..f871989 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -122,6 +122,16 @@ static int read_new_entries(sd_journal *journal) add_to_payload(data, length); + // For now, we send one record per log message, in case the + // there is a large backlog of messages and we exceed the + // payload size limit (8KB). And ignore errors, hoping that it's + // a transient problem. + + if (!send_data(error_class)) { + telem_log(LOG_ERR, "Failed to send data. Ignoring.\n"); + return num_entries; + } + num_entries++; } @@ -151,17 +161,11 @@ static bool process_existing_entries(sd_journal *journal) ret = read_new_entries(journal); if (ret < 0) { return false; - } - - if (!payload) { + } else if (ret == 0) { telem_log(LOG_DEBUG, "No existing entries found\n"); return true; } - if (!send_data(error_class)) { - return false; - } - return true; } @@ -278,10 +282,6 @@ static bool process_journal(void) } else if (r < 0) { return false; } - - if (!send_data(error_class)) { - return false; - } } } } From bf3dc47d9d020442f944bf94650167bf8511257b Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 20 Jan 2017 13:46:10 -0800 Subject: [PATCH 09/11] journal probe: filter LOG_EMERG logs as well I omitted log level 0 from the filter, so add it here. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index f871989..aa0fbc3 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -229,7 +229,8 @@ static bool add_filters(sd_journal *journal) JOURNAL_MATCH(data); free(data); JOURNAL_AND; - // The three highest log levels, all indicating errors + // The four highest log levels, all indicating errors + JOURNAL_MATCH("PRIORITY=0"); JOURNAL_MATCH("PRIORITY=1"); JOURNAL_MATCH("PRIORITY=2"); JOURNAL_MATCH("PRIORITY=3"); From 15bdcf0147ff1c55e0f243c65ab2848b841a29ce Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Mon, 13 Feb 2017 11:16:31 -0800 Subject: [PATCH 10/11] Document the logical expression for journal entry matching Signed-off-by: Patrick McCarty --- src/probes/journal.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/probes/journal.c b/src/probes/journal.c index aa0fbc3..4d49055 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -226,6 +226,18 @@ static bool add_filters(sd_journal *journal) return false; } + /* The semantics of how journal entry matching works is described in + * detail in sd_journal_add_match(3). + * + * The matches declared here correspond to the logical expression: + * + * BOOTID && ((P0 || P1 || P2 || P3) || EXITED) + * + * BOOTID is short for _BOOT_ID=VAL, where VAL is the boot ID for the + * current boot. P0, P1, etc stand for PRIORITY=0, etc. And EXITED is + * short for EXIT_CODE=exited. + */ + JOURNAL_MATCH(data); free(data); JOURNAL_AND; From 35b6ca2e36098020cf11cc8858d3b26da78e9e68 Mon Sep 17 00:00:00 2001 From: Patrick McCarty Date: Fri, 17 Feb 2017 11:32:04 -0800 Subject: [PATCH 11/11] journal probe: remove obsolete LOG_DEBUG message A return value of 0 can also indicate send_data() failure, so remove this log message. Signed-off-by: Patrick McCarty --- src/probes/journal.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/probes/journal.c b/src/probes/journal.c index 4d49055..bce0be2 100644 --- a/src/probes/journal.c +++ b/src/probes/journal.c @@ -162,7 +162,6 @@ static bool process_existing_entries(sd_journal *journal) if (ret < 0) { return false; } else if (ret == 0) { - telem_log(LOG_DEBUG, "No existing entries found\n"); return true; }