On Mon, Dec 23, 2013 at 08:01:15AM -0500, Stephen Gallagher wrote:
> On Dec 21, 2013, at 1:25 PM, Lukas Slebodnik <lslebodn(a)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.
ACK
>>
>>
>> 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.
ACK to the code but I wonder if it would be nicer to put the whole block
that actually prints to journal into its own function?
>>
>>
>> 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 agree that not removing -f is the right thing to do. At the very
least, I think we should amend our wiki and document how the
administrator should obtain the debug logs.
>> 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.
ACK to this patch. Let me know your preference about splitting debug_fn,
otherwise I think this patch is good to go along with re-adding the
PRINTF_ATTRIBUTE.