-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 01/07/2014 10:22 AM, Jakub Hrozek wrote: > On Mon, Dec 23, 2013 at 08:01:15AM -0500, Stephen Gallagher wrote: >> >> >>> On Dec 21, 2013, at 1:25 PM, Lukas Slebodnik >>> <lsleb...@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? >
Done. I created a new static function 'journal_send()' in debug.c >>>> >>>> >>>> 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. > I've added a new patch 0004 that will create a systemd service file override in /etc/systemd/system/sssd.service.d/journal.conf This file will have its contents commented out by default, but if it is uncommented, the effect will be to redirect output to the journal. This should vastly simplify the effort required. Simo recommended that we may want to reverse the behavior (have the journal be the standard destination if --with-syslog=journald was passed to configure) and make the override capable of reverting to the old behavior. This might have some real value (particularly for RHEL 7.0) so that we can migrate people easier while giving them an easy way to get the old behavior back. If we prefer to do it that way, I can quickly amend patch 0004 to do this. >>>> 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. I've fixed the PRINTF_ATTRIBUTE and split the debug_fn. New patches attached. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlLMoh4ACgkQeiVVYja6o6O6UACbBbYp6gRSl6vJj04Di39kbAiE zFAAn0wm2tc3yYQ/GhjTQzUSnIA0PO62 =4E/N -----END PGP SIGNATURE-----
>From 77f2a66a875f1a7bf5e10463151599f3c76c324f Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Fri, 20 Dec 2013 15:17:19 -0500
Subject: [PATCH 1/4] 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 df7c71f459593b33fd112319b8b38d89d1011e50..5ca42e7c7864a42f8904f7a4999fff2ead127986 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 9ec86ced519cab1038bc8b280404d37ae5c436f5..601641c2ddf2f8b3d8b6ceb6499cfd4dfd5c2e9d 100644
--- a/src/util/debug.c
+++ b/src/util/debug.c
@@ -129,7 +129,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) {
@@ -191,7 +195,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..2b639dd57146030780b80b7bf9a74d1129463f07 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, ...) SSS_ATTRIBUTE_PRINTF(5,6);
int debug_convert_old_level(int old_level);
errno_t set_debug_file_from_fd(const int fd);
@@ -127,8 +130,11 @@ errno_t set_debug_file_from_fd(const int fd);
*/
#define DEBUG(level, format, ...) do { \
int __debug_macro_level = level; \
- if (DEBUG_IS_SET(__debug_macro_level)) \
- debug_fn(__FUNCTION__, __debug_macro_level, format, ##__VA_ARGS__); \
+ if (DEBUG_IS_SET(__debug_macro_level)) { \
+ debug_fn(__FILE__, __LINE__, __FUNCTION__, \
+ __debug_macro_level, \
+ format, ##__VA_ARGS__); \
+ } \
} while (0)
/** \def DEBUG_IS_SET(level)
--
1.8.4.2
>From 425058f3b187a3afe6339306bd6f2fca9ec3fbfd Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Fri, 20 Dec 2013 16:14:25 -0500
Subject: [PATCH 2/4] DEBUG: 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.
---
src/util/debug.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/src/util/debug.c b/src/util/debug.c
index 601641c2ddf2f8b3d8b6ceb6499cfd4dfd5c2e9d..d17e388c2f840bde01fdd5020182d20d8aaa60b5 100644
--- a/src/util/debug.c
+++ b/src/util/debug.c
@@ -30,6 +30,10 @@
#include <sys/stat.h>
#include <sys/time.h>
+#ifdef WITH_JOURNALD
+#include <systemd/sd-journal.h>
+#endif
+
#include "util/util.h"
const char *debug_prg_name = "sssd";
@@ -129,6 +133,69 @@ static void debug_printf(const char *format, ...)
va_end(ap);
}
+#ifdef WITH_JOURNALD
+static errno_t journal_send(const char *file,
+ long line,
+ const char *function,
+ int newlevel,
+ const char *format,
+ va_list ap)
+{
+ errno_t ret;
+ int res;
+ char *message = NULL;
+ char *code_file = NULL;
+ char *code_line = NULL;
+ const char *domain;
+
+ /* First, evaluate the message to be sent */
+ ret = vasprintf(&message, format, ap);
+ if (ret == -1) {
+ /* ENOMEM, just return */
+ return ENOMEM;
+ }
+
+ res = asprintf(&code_file, "CODE_FILE=%s", file);
+ if (res == -1) {
+ ret = ENOMEM;
+ goto journal_done;
+ }
+
+ res = asprintf(&code_line, "CODE_LINE=%ld", line);
+ if (res == -1) {
+ ret = ENOMEM;
+ goto journal_done;
+ }
+
+ /* If this log message was sent from a provider,
+ * track the domain.
+ */
+ domain = getenv(SSS_DOM_ENV);
+ if (domain == NULL) {
+ domain = "";
+ }
+
+ /* Send the log message to journald, specifying the
+ * source code location and other tracking data.
+ */
+ res = sd_journal_send_with_location(
+ code_file, code_line, function,
+ "MESSAGE=%s", message,
+ "PRIORITY=%i", LOG_DEBUG,
+ "SSSD_DOMAIN=%s", domain,
+ "SSSD_PRG_NAME=%s", debug_prg_name,
+ "SSSD_DEBUG_LEVEL=%x", newlevel,
+ NULL);
+ ret = -res;
+
+journal_done:
+ free(code_line);
+ free(code_file);
+ free(message);
+ return ret;
+}
+#endif /* WiTH_JOURNALD */
+
void debug_fn(const char *file,
long line,
const char *function,
@@ -136,6 +203,29 @@ void debug_fn(const char *file,
const char *format, ...)
{
va_list ap;
+
+#ifdef WITH_JOURNALD
+ errno_t ret;
+ if (!debug_file) {
+ /* If we are not outputting logs to files, we should be sending them
+ * to journald.
+ * NOTE: on modern systems, this is where stdout/stderr will end up
+ * from system services anyway. The only difference here is that we
+ * can also provide extra structuring data to make it more easily
+ * searchable.
+ */
+ va_start(ap, format);
+ ret = journal_send(file, line, function, newlevel, format, ap);
+ if (ret != EOK) {
+ /* Emergency fallback, send to STDERR */
+ debug_vprintf(format, ap);
+ debug_fflush();
+ }
+ va_end(ap);
+ return;
+ }
+#endif
+
if (debug_timestamps) {
struct timeval tv;
struct tm *tm;
--
1.8.4.2
>From 1938f07ef98d9e37737bce4a2c7577a1effbc3cd Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Fri, 20 Dec 2013 15:34:43 -0500
Subject: [PATCH 3/4] 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.
---
contrib/fedora/bashrc_sssd | 1 +
contrib/sssd.spec.in | 2 ++
2 files changed, 3 insertions(+)
diff --git a/contrib/fedora/bashrc_sssd b/contrib/fedora/bashrc_sssd
index e2132ef7aa93a6144959b436f56e0ec3a02e834e..ccad5ddd13321b3b259024196b3414a17d14c7b6 100644
--- a/contrib/fedora/bashrc_sssd
+++ b/contrib/fedora/bashrc_sssd
@@ -44,6 +44,7 @@ alias fedconfig='../configure \
--enable-pammoddir=/$SSS_LIB/security \
--with-krb5-rcache-dir=/var/cache/krb5rcache \
--with-initscript=systemd \
+ --with-syslog=journald \
--with-test-dir=/dev/shm \
--enable-all-experimental-features \
--cache-file=/tmp/fedconfig.cache \
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index 6bc2aa2a41bf100cfadda63ad83850e4b980647f..ec75b53a4f73ed40f14cf215bcc9fba849ed7e88 100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -24,6 +24,7 @@
%if (0%{?use_systemd} == 1)
%global with_initscript --with-initscript=systemd --with-systemdunitdir=%{_unitdir}
+ %global with_syslog --with-syslog=journald
%else
%global with_initscript --with-initscript=sysv
%endif
@@ -421,6 +422,7 @@ autoreconf -ivf
--disable-rpath \
%{?with_ccache} \
%{with_initscript} \
+ %{?with_syslog} \
%{?with_cifs_utils_plugin_option} \
%{?experimental}
--
1.8.4.2
>From 5ec510d74936d5fd0a7cafe46f087fe7507066b0 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Tue, 7 Jan 2014 15:43:13 -0500
Subject: [PATCH 4/4] BUILD: Simplify enabling journald on installed systems
systemd supports overrides of the standard service file to be placed in
/etc/systemd/system/<service>.service.d/
With this patch, we will install a commented-out override file to /etc
that will instruct the user on how to enable logging to journald.
---
Makefile.am | 9 +++++++++
configure.ac | 5 +++--
contrib/sssd.spec.in | 2 ++
src/conf_macros.m4 | 20 ++++++++++++++++++++
src/sysv/systemd/journal.conf.in | 7 +++++++
5 files changed, 41 insertions(+), 2 deletions(-)
create mode 100644 src/sysv/systemd/journal.conf.in
diff --git a/Makefile.am b/Makefile.am
index 4015dadd78e26ec285b7010c3e39b22458fa878b..d015f0cfe0cdfe5bead322163830c6393c61d7d8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3,6 +3,10 @@ if HAVE_DEVSHM
extra_distcheck_flags += --with-test-dir=/dev/shm
endif
+if HAVE_SYSTEMD_UNIT
+ extra_distcheck_flags += --with-syslog=journald
+endif
+
DISTCHECK_CONFIGURE_FLAGS = --with-ldb-lib-dir="$$dc_install_base"/lib/ldb \
--enable-all-experimental-features \
$(extra_distcheck_flags)
@@ -51,6 +55,7 @@ pipepath = @pipepath@
mcpath = @mcpath@
initdir = @initdir@
systemdunitdir = @systemdunitdir@
+systemdconfdir = @systemdconfdir@/sssd.service.d
logpath = @logpath@
pubconfpath = @pubconfpath@
pkgconfigdir = $(libdir)/pkgconfig
@@ -2007,9 +2012,12 @@ endif
dist_init_SCRIPTS =
dist_systemdunit_DATA =
+dist_systemdconf_DATA =
if HAVE_SYSTEMD_UNIT
dist_systemdunit_DATA += \
src/sysv/systemd/sssd.service
+ dist_systemdconf_DATA += \
+ src/sysv/systemd/journal.conf
else
if HAVE_SUSE
dist_init_SCRIPTS += \
@@ -2107,6 +2115,7 @@ endif
if HAVE_SYSTEMD_UNIT
mkdir -p $(DESTDIR)$(systemdunitdir)
+ mkdir -p $(DESTDIR)$(systemdconfdir)
else
mkdir -p $(DESTDIR)$(initdir)
endif
diff --git a/configure.ac b/configure.ac
index 93038204fad6f2f52bc741cfcc3aebc9bdecb988..82951396aa8990f15e468b0e919437e7d066e14d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -181,6 +181,7 @@ fi
WITH_INITSCRIPT
if test x$initscript = xsystemd; then
WITH_SYSTEMD_UNIT_DIR
+ WITH_SYSTEMD_CONF_DIR
fi
PKG_CHECK_MODULES([DBUS],[dbus-1])
@@ -313,8 +314,8 @@ AC_DEFINE_UNQUOTED([ABS_BUILD_DIR], ["$abs_build_dir"], [Absolute path to the bu
AC_SUBST([abs_builddir], $abs_build_dir)
AC_CONFIG_FILES([Makefile contrib/sssd.spec src/examples/rwtab src/doxy.config
- src/sysv/systemd/sssd.service src/sysv/sssd
- src/sysv/gentoo/sssd src/sysv/SUSE/sssd
+ src/sysv/systemd/sssd.service src/sysv/systemd/journal.conf
+ src/sysv/sssd src/sysv/gentoo/sssd src/sysv/SUSE/sssd
po/Makefile.in src/man/Makefile
src/providers/ipa/ipa_hbac.pc src/providers/ipa/ipa_hbac.doxy
src/lib/idmap/sss_idmap.pc src/lib/idmap/sss_idmap.doxy
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index ec75b53a4f73ed40f14cf215bcc9fba849ed7e88..e0830f8f863a9a03b00793b9b324ae0ffebf4188 100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -585,6 +585,8 @@ rm -rf $RPM_BUILD_ROOT
%attr(750,root,root) %dir %{_var}/log/%{name}
%attr(711,root,root) %dir %{_sysconfdir}/sssd
%ghost %attr(0600,root,root) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf
+%attr(755,root,root) %dir %{_sysconfdir}/systemd/system/sssd.service.d
+%config(noreplace) %{_sysconfdir}/systemd/system/sssd.service.d/journal.conf
%config(noreplace) %{_sysconfdir}/logrotate.d/sssd
%config(noreplace) %{_sysconfdir}/rwtab.d/sssd
%dir %{_datadir}/sssd
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index 4be819d4a3389dfcf89a0452aab8165c0ec216e4..71118593c81ba81804b077a0a51719b9c71574c7 100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -221,6 +221,26 @@ AC_DEFUN([WITH_SYSTEMD_UNIT_DIR],
AC_SUBST(systemdunitdir)
])
+dnl A macro to configure the directory to install the systemd unit file
+dnl overrides to
+AC_DEFUN([WITH_SYSTEMD_CONF_DIR],
+ [ AC_ARG_WITH([systemdconfdir],
+ [ AC_HELP_STRING([--with-systemdconfdir=DIR],
+ [Directory for systemd service file overrides [Auto]]
+ ),
+ ],
+ )
+ if test x"$with_systemdconfdir" != x; then
+ systemdconfdir=$with_systemdconfdir
+ else
+ systemdconfdir=$($PKG_CONFIG --variable=systemdsystemconfdir systemd)
+ if test x"$systemdconfdir" = x; then
+ AC_MSG_ERROR([Could not detect systemd config directory])
+ fi
+ fi
+ AC_SUBST(systemdconfdir)
+ ])
+
AC_DEFUN([WITH_MANPAGES],
[ AC_ARG_WITH([manpages],
[AC_HELP_STRING([--with-manpages],
diff --git a/src/sysv/systemd/journal.conf.in b/src/sysv/systemd/journal.conf.in
new file mode 100644
index 0000000000000000000000000000000000000000..d89325e0872881e3e8485102d9971871101098f3
--- /dev/null
+++ b/src/sysv/systemd/journal.conf.in
@@ -0,0 +1,7 @@
+[Service]
+# Uncomment *both* of the following lines to enable debug logging
+# to go to journald instead of /var/log/sssd. You will need to
+# run 'systemctl daemon-reload' and then restart the SSSD service
+# for this to take effect
+#ExecStart=
+#ExecStart=@sbindir@/sssd -D
--
1.8.4.2
0001-DEBUG-Allow-debug_fn-to-process-__FILE__-and-__LINE_.patch.sig
Description: PGP signature
0002-DEBUG-Enable-sending-structured-debug-logs-to-journa.patch.sig
Description: PGP signature
0003-BUILD-Build-with-journald-support-by-default-on-Fedo.patch.sig
Description: PGP signature
0004-BUILD-Simplify-enabling-journald-on-installed-system.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
