Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Journal probe rework #7

Merged
merged 11 commits into from
Feb 17, 2017
68 changes: 44 additions & 24 deletions src/probes/journal.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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;
}
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -185,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; \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always a little scary when the macro can return in some cases but in this case it seems reasonable enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In this case, sd_journal_add_match and friends returning an error is very unlikely, so I figured the returns here would be acceptable for unlikely conditions.

} \
} 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;
Expand All @@ -194,24 +222,16 @@ 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, "SYSLOG_IDENTIFIER=crashprobe", 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;
}
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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this confuses me a little. I'd expect exited to be a possible issue assuming non zero exit but looking at:
journalctl -F EXIT_CODE

dumped
exited
killed

I'd assume we may want to look at the dumped and killed case potentially too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. In my testing, I only encountered "exited", but I didn't inspect the source to see what other values it accepts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed an issue for this (#13)


return true;
}
Expand Down