[SSSD] [PATCHES] Enable sending DEBUG logs to journald

Stephen Gallagher sgallagh at redhat.com
Mon Dec 23 13:01:15 UTC 2013



> On Dec 21, 2013, at 1:25 PM, Lukas Slebodnik <lslebodn at redhat.com> wrote:
> 
>> On (20/12/13 17:05), Stephen Gallagher wrote:
>> All of these patches require Nikolai's "DEBUG Macro Refactoring v3"
>> patches to be applied first.
>> 
>> 
>> 
>> 
>> Patch 0001: Allow debug_fn to process __FILE__ and __LINE__
>> 
>> In preparation for enabling journald support for the DEBUG logs, we
>> will need to be able to pass in certain additional arguments that will
>> be required, specifically the code file and line number.
>> 
>> We will be able to optionally enable this in the file-based logs as well
>> if we so choose, but for right now we will avoid breaking the log
>> format on disk.
>> 
>> 
>> Patch 0002: Enable sending structured debug logs to journald
>> 
>> We are now able to send structured debug logs to journald, tagged with
>> the code file, line number and domain that the log pertains to. To
>> enable this functionality, SSSD must be configured at build-time with
>> --with-syslog=journald and must be launched without -f/--debug-to-files
>> 
>> This behavior is nearly identical to how SSSD will function today on a
>> systemd-based system if --debug-to-files is disabled, since it will
>> redirect stdout and stderr into journald. This patch merely enhances
>> the situation to send structured logs instead of simple string messages.
>> 
>> 
>> Patch 0003: BUILD: Build with journald support by default on Fedora
>> 
>> The journal provided by systemd gives us structured logging
>> capabilities that we should be taking advantage of.
>> 
>> Note: this patch explicitly does not change the systemd unit file for
>> SSSD. Right now, an administrator will need to manually remove the
>> '-f' from ExecStart in the unit file to send debug logs to journald. I
>> suspect we'll want to discuss this before we make it the default. This
>> patch DOES change the default for sss_log messages to use
>> sd_journal_send() instead of straight log() for those messages that we
>> traditionally sent to the syslog (such as login events). This is code
>> that has been in place for some time now, but has not been the default
>> because we hadn't build with --with-syslog=journald.
> 
>> From e52fdecc33b0e8b2df8e983ee145ab421a6fffc2 Mon Sep 17 00:00:00 2001
>> From: Stephen Gallagher <sgallagh at redhat.com>
>> Date: Fri, 20 Dec 2013 15:17:19 -0500
>> Subject: [PATCH 1/3] DEBUG: Allow debug_fn to process __FILE__ and __LINE__
>> 
>> In preparation for enabling journald support for the DEBUG logs, we
>> will need to be able to pass in certain additional arguments that will
>> be required, specifically the code file and line number.
>> 
>> We will be able to optionally enable this in the file-based logs as well
>> if we so choose, but for right now we will avoid breaking the log
>> format on disk.
>> ---
>> src/tools/selinux.c |  2 +-
>> src/util/debug.c    |  8 ++++++--
>> src/util/util.h     | 14 ++++++++++----
>> 3 files changed, 17 insertions(+), 7 deletions(-)
>> 
>> diff --git a/src/tools/selinux.c b/src/tools/selinux.c
>> index b3565de3de1f35218afbb7e3bf3e3362ae8122ec..5905461ca1d09673409cc05d519b6c00a83ca3d9 100644
>> --- a/src/tools/selinux.c
>> +++ b/src/tools/selinux.c
>> @@ -121,7 +121,7 @@ static void sss_semanage_error_callback(void *varg,
>>        return;
>>    }
>> 
>> -    debug_fn("libsemanage", level, "%s\n", message);
>> +    debug_fn(__FILE__, __LINE__, "libsemanage", level, "%s\n", message);
>>    free(message);
>> }
>> 
>> diff --git a/src/util/debug.c b/src/util/debug.c
>> index 0aff2da9e5eaeb468c9502d2686f78da9cc31056..b3304a859e91b2509746c5ffbd101cf73b711fce 100644
>> --- a/src/util/debug.c
>> +++ b/src/util/debug.c
>> @@ -128,7 +128,11 @@ static void debug_printf(const char *format, ...)
>>    va_end(ap);
>> }
>> 
>> -void debug_fn(const char *function, int newlevel, const char *format, ...)
>> +void debug_fn(const char *file,
>> +              long line,
>> +              const char *function,
>> +              int newlevel,
>> +              const char *format, ...)
>> {
>>    va_list ap;
>>    if (debug_timestamps) {
>> @@ -190,7 +194,7 @@ void ldb_debug_messages(void *context, enum ldb_debug_level level,
>>        return;
>>    }
>> 
>> -    debug_fn("ldb", loglevel, "%s\n", message);
>> +    debug_fn(__FILE__, __LINE__, "ldb", loglevel, "%s\n", message);
>> 
>>    free(message);
>> }
>> diff --git a/src/util/util.h b/src/util/util.h
>> index 7d9a5264d461e3bcf511187320050936f44da70f..b3bb740f7780ce71f6726099d51ccac0065d3c9d 100644
>> --- a/src/util/util.h
>> +++ b/src/util/util.h
>> @@ -68,8 +68,11 @@ extern int debug_timestamps;
>> extern int debug_microseconds;
>> extern int debug_to_file;
>> extern const char *debug_log_file;
>> -void debug_fn(const char *function, int newlevel, const char *format, ...)
>> -                                SSS_ATTRIBUTE_PRINTF(3, 4);
>> +void debug_fn(const char *file,
>> +              long line,
>> +              const char *function,
>> +              int newlevel,
>> +              const char *format, ...);
>> int debug_convert_old_level(int old_level);
>> errno_t set_debug_file_from_fd(const int fd);
> 
> NACK
> 
> Why do you removed SSS_ATTRIBUTE_PRINTF ?
> 
> We had some crashes in sssd because of wrong debug message.
> For example: e9ea740625881291893b006ffad906d4861c668d
>             "Fix segfault in DYNDNS"
> 



I think that was probably me hitting ctrl-d one too many times and not noticing that I removed it. It wasn't intentional. I'll send a new version of the patch with that added back in when I am back from vacation in the new year (or it can be fixed at push, if someone else could test and verify it doesn't break anything.)


More information about the sssd-devel mailing list