git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Lucas Werkmeister <mail@lucaswerkmeister.de>
To: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Cc: Lucas Werkmeister <mail@lucaswerkmeister.de>
Subject: [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)
Date: Sat, 27 Jan 2018 19:31:32 +0100	[thread overview]
Message-ID: <20180127183132.19724-1-mail@lucaswerkmeister.de> (raw)
In-Reply-To: <xmqqtvvbds42.fsf@gitster.mtv.corp.google.com>

This makes it possible to use --inetd while still logging to standard
error. --syslog is retained as an alias for --send-log-to=syslog. A mode
to disable logging explicitly is also provided.

The combination of --inetd with --send-log-to=stderr is useful, for
instance, when running `git daemon` as an instanced systemd service
(with associated socket unit). In this case, log messages sent via
syslog are received by the journal daemon, but run the risk of being
processed at a time when the `git daemon` process has already exited
(especially if the process was very short-lived, e.g. due to client
error), so that the journal daemon can no longer read its cgroup and
attach the message to the correct systemd unit (see systemd/systemd#2913
[1]). Logging to stderr instead can solve this problem, because systemd
can connect stderr directly to the journal daemon, which then already
knows which unit is associated with this stream.

[1]: https://github.com/systemd/systemd/issues/2913

Signed-off-by: Lucas Werkmeister <mail@lucaswerkmeister.de>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
---

Notes:
    This was originally “daemon: add --no-syslog to undo implicit
    --syslog”, but Junio pointed out that combining --no-syslog with
    --detach isn’t especially useful and suggested --send-log-to=
    instead. Is Helped-by: the right credit for this or should it be
    something else?
    
    I’m also not quite sure if the systemd part of the commit message is
    accurate – see my comment on the linked issue [2]. TL;DR: this might
    no longer be necessary on systemd v235. (I’m experiencing the
    problem on Debian Stretch, systemd v232.) As in the last patch, feel
    free to remove that part of the commit message.
    
    [2]: https://github.com/systemd/systemd/issues/2913#issuecomment-361002589

 Documentation/git-daemon.txt | 23 +++++++++++++++++++++--
 daemon.c                     | 43 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 3c91db7be..e973f4390 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -20,6 +20,7 @@ SYNOPSIS
 	     [--inetd |
 	      [--listen=<host_or_ipaddr>] [--port=<n>]
 	      [--user=<user> [--group=<group>]]]
+	     [--send-log-to=(stderr|syslog|none)]
 	     [<directory>...]
 
 DESCRIPTION
@@ -110,8 +111,26 @@ OPTIONS
 	zero for no limit.
 
 --syslog::
-	Log to syslog instead of stderr. Note that this option does not imply
-	--verbose, thus by default only error conditions will be logged.
+	Short for `--send-log-to=syslog`.
+
+--send-log-to=<destination>::
+	Send log messages to the specified destination.
+	Note that this option does not imply --verbose,
+	thus by default only error conditions will be logged.
+	The <destination> defaults to `stderr`, and must be one of:
++
+--
+stderr::
+	Write to standard error.
+	Note that if `--detach` is specified,
+	the process disconnects from the real standard error,
+	making this destination effectively equivalent to `none`.
+syslog::
+	Write to syslog, using the `git-daemon` identifier.
+none::
+	Disable all logging.
+--
++
 
 --user-path::
 --user-path=<path>::
diff --git a/daemon.c b/daemon.c
index e37e343d0..3d8e16ede 100644
--- a/daemon.c
+++ b/daemon.c
@@ -9,7 +9,11 @@
 #define initgroups(x, y) (0) /* nothing */
 #endif
 
-static int log_syslog;
+static enum log_destination {
+	LOG_TO_NONE = -1,
+	LOG_TO_STDERR = 0,
+	LOG_TO_SYSLOG = 1,
+} log_destination;
 static int verbose;
 static int reuseaddr;
 static int informative_errors;
@@ -25,6 +29,7 @@ static const char daemon_usage[] =
 "           [--access-hook=<path>]\n"
 "           [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>]\n"
 "                      [--detach] [--user=<user> [--group=<group>]]\n"
+"           [--send-log-to=(stderr|syslog|none)]\n"
 "           [<directory>...]";
 
 /* List of acceptable pathname prefixes */
@@ -74,11 +79,14 @@ static const char *get_ip_address(struct hostinfo *hi)
 
 static void logreport(int priority, const char *err, va_list params)
 {
-	if (log_syslog) {
+	switch (log_destination) {
+	case LOG_TO_SYSLOG: {
 		char buf[1024];
 		vsnprintf(buf, sizeof(buf), err, params);
 		syslog(priority, "%s", buf);
-	} else {
+		break;
+	}
+	case LOG_TO_STDERR: {
 		/*
 		 * Since stderr is set to buffered mode, the
 		 * logging of different processes will not overlap
@@ -88,6 +96,11 @@ static void logreport(int priority, const char *err, va_list params)
 		vfprintf(stderr, err, params);
 		fputc('\n', stderr);
 		fflush(stderr);
+		break;
+	}
+	case LOG_TO_NONE: {
+		break;
+	}
 	}
 }
 
@@ -1289,7 +1302,7 @@ int cmd_main(int argc, const char **argv)
 		}
 		if (!strcmp(arg, "--inetd")) {
 			inetd_mode = 1;
-			log_syslog = 1;
+			log_destination = LOG_TO_SYSLOG;
 			continue;
 		}
 		if (!strcmp(arg, "--verbose")) {
@@ -1297,9 +1310,25 @@ int cmd_main(int argc, const char **argv)
 			continue;
 		}
 		if (!strcmp(arg, "--syslog")) {
-			log_syslog = 1;
+			log_destination = LOG_TO_SYSLOG;
 			continue;
 		}
+		if (skip_prefix(arg, "--send-log-to=", &v)) {
+			if (!strcmp(v, "syslog")) {
+				log_destination = LOG_TO_SYSLOG;
+				continue;
+			}
+			else if (!strcmp(v, "stderr")) {
+				log_destination = LOG_TO_STDERR;
+				continue;
+			}
+			else if (!strcmp(v, "none")) {
+				log_destination = LOG_TO_NONE;
+				continue;
+			}
+			else
+				die("Unknown log destination %s", v);
+		}
 		if (!strcmp(arg, "--export-all")) {
 			export_all_trees = 1;
 			continue;
@@ -1356,7 +1385,7 @@ int cmd_main(int argc, const char **argv)
 		}
 		if (!strcmp(arg, "--detach")) {
 			detach = 1;
-			log_syslog = 1;
+			log_destination = LOG_TO_SYSLOG;
 			continue;
 		}
 		if (skip_prefix(arg, "--user=", &v)) {
@@ -1402,7 +1431,7 @@ int cmd_main(int argc, const char **argv)
 		usage(daemon_usage);
 	}
 
-	if (log_syslog) {
+	if (log_destination == LOG_TO_SYSLOG) {
 		openlog("git-daemon", LOG_PID, LOG_DAEMON);
 		set_die_routine(daemon_die);
 	} else
-- 
2.16.0


  parent reply	other threads:[~2018-01-27 18:32 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-22 23:23 [PATCH] daemon: add --no-syslog to undo implicit --syslog Lucas Werkmeister
2018-01-23  0:00 ` Ævar Arnfjörð Bjarmason
2018-01-23 18:30   ` Junio C Hamano
2018-01-23 22:06     ` Lucas Werkmeister
2018-01-24 10:05       ` Lucas Werkmeister
2018-01-24 18:33       ` Junio C Hamano
2018-01-24 19:48         ` Lucas Werkmeister
2018-01-27 18:31         ` Lucas Werkmeister [this message]
2018-01-28  6:40           ` [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none) Eric Sunshine
2018-01-28 22:58             ` Lucas Werkmeister
2018-01-29  0:10               ` Eric Sunshine
2018-02-03 23:08                 ` [PATCH v3] daemon: add --log-destination=(stderr|syslog|none) Lucas Werkmeister
2018-02-04  6:36                   ` Eric Sunshine
2018-02-04 18:29                     ` Lucas Werkmeister
2018-02-04 18:30                       ` [PATCH v4] " Lucas Werkmeister
2018-02-04 18:55                         ` Ævar Arnfjörð Bjarmason
2018-02-04 18:58                           ` Lucas Werkmeister
2018-02-05 20:09                             ` Ævar Arnfjörð Bjarmason
2018-02-04 19:44                           ` Eric Sunshine
2018-02-04 19:36                         ` Eric Sunshine
2018-02-05 18:31                           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180127183132.19724-1-mail@lucaswerkmeister.de \
    --to=mail@lucaswerkmeister.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).