From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Wed, 5 Mar 2025 10:28:14 +0000 (+0100)
Subject: MEDIUM: log: postpone the decision to send or not log with empty messages
X-Git-Tag: v3.2-dev7~31
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c7abe7778e3a89f914dfecbff6eb8f6c19b9d17f;p=thirdparty%2Fhaproxy.git

MEDIUM: log: postpone the decision to send or not log with empty messages

As reported by Nick Ramirez in GH #2891, it is currently not possible to
use log-profile without a log-format set on the proxy.

This is due to historical reason, because all log sending functions avoid
trying to send a log with empty message. But now with log-profile which
can override log-format, it is possible that some loggers may actually
end up generating a valid log message that should be sent! Yet from the
upper logging functions we don't know about that because loggers are
evaluated in lower API functions.

Thus, to avoid skipping potentially valid messages (thanks to log-profile
overrides), in this patch we postpone the decision to send or not empty
log messages in lower log API layer, ie: _process_send_log_final(), once
the log-profile settings were evaluated for a given logger.

A known side-effect of this change is that fe->log_count statistic may
be increased even if no log message is sent because the message was empty
and even the log-profile didn't help to produce a non empty log message.
But since configurations lacking proxy log-format are not supposed to be
used without log-profile (+ log steps combination) anyway it shouldn't be
an issue.
---

diff --git a/src/log.c b/src/log.c
index 4557c181d..a93f72ab9 100644
--- a/src/log.c
+++ b/src/log.c
@@ -2912,6 +2912,9 @@ struct process_send_log_ctx {
 static inline void _process_send_log_final(struct logger *logger, struct log_header hdr,
                                            char *message, size_t size, int nblogger)
 {
+	if (!size)
+		return; // don't try to send empty message
+
 	if (logger->target.type == LOG_TARGET_BACKEND) {
 		__do_send_log_backend(logger->target.be, hdr, nblogger, logger->maxlen, message, size);
 	}
@@ -5188,6 +5191,7 @@ out:
  */
 void do_log(struct session *sess, struct stream *s, struct log_orig origin)
 {
+	struct process_send_log_ctx ctx;
 	int size;
 	int sd_size = 0;
 	int level = -1;
@@ -5222,15 +5226,12 @@ void do_log(struct session *sess, struct stream *s, struct log_orig origin)
 	}
 
 	size = sess_build_logline_orig(sess, s, logline, global.max_syslog_len, &sess->fe->logformat, origin);
-	if (size > 0) {
-		struct process_send_log_ctx ctx;
 
-		ctx.origin = origin;
-		ctx.sess = sess;
-		ctx.stream = s;
-		__send_log(&ctx, &sess->fe->loggers, &sess->fe->log_tag, level,
-			   logline, size, logline_rfc5424, sd_size);
-	}
+	ctx.origin = origin;
+	ctx.sess = sess;
+	ctx.stream = s;
+	__send_log(&ctx, &sess->fe->loggers, &sess->fe->log_tag, level,
+		   logline, size, logline_rfc5424, sd_size);
 }
 
 /*
@@ -5239,6 +5240,7 @@ void do_log(struct session *sess, struct stream *s, struct log_orig origin)
  */
 void strm_log(struct stream *s, struct log_orig origin)
 {
+	struct process_send_log_ctx ctx;
 	struct session *sess = s->sess;
 	int size, err, level;
 	int sd_size = 0;
@@ -5280,17 +5282,14 @@ void strm_log(struct stream *s, struct log_orig origin)
 	}
 
 	size = build_logline_orig(s, logline, global.max_syslog_len, &sess->fe->logformat, origin);
-	if (size > 0) {
-		struct process_send_log_ctx ctx;
 
-		_HA_ATOMIC_INC(&sess->fe->log_count);
-		ctx.origin = origin;
-		ctx.sess = sess;
-		ctx.stream = s;
-		__send_log(&ctx, &sess->fe->loggers, &sess->fe->log_tag, level,
-			   logline, size, logline_rfc5424, sd_size);
-		s->logs.logwait = 0;
-	}
+	_HA_ATOMIC_INC(&sess->fe->log_count);
+	ctx.origin = origin;
+	ctx.sess = sess;
+	ctx.stream = s;
+	__send_log(&ctx, &sess->fe->loggers, &sess->fe->log_tag, level,
+		   logline, size, logline_rfc5424, sd_size);
+	s->logs.logwait = 0;
 }
 
 /*
@@ -5308,6 +5307,7 @@ void strm_log(struct stream *s, struct log_orig origin)
  */
 void _sess_log(struct session *sess, int embryonic)
 {
+	struct process_send_log_ctx ctx;
 	int size, level;
 	int sd_size = 0;
 	struct log_orig orig;
@@ -5349,17 +5349,14 @@ void _sess_log(struct session *sess, int embryonic)
 		session_embryonic_build_legacy_err(sess, &buf);
 		size = buf.data;
 	}
-	if (size > 0) {
-		struct process_send_log_ctx ctx;
 
-		_HA_ATOMIC_INC(&sess->fe->log_count);
-		ctx.origin = orig;
-		ctx.sess = sess;
-		ctx.stream = NULL;
-		__send_log(&ctx, &sess->fe->loggers,
-		           &sess->fe->log_tag, level,
-			   logline, size, logline_rfc5424, sd_size);
-	}
+	_HA_ATOMIC_INC(&sess->fe->log_count);
+	ctx.origin = orig;
+	ctx.sess = sess;
+	ctx.stream = NULL;
+	__send_log(&ctx, &sess->fe->loggers,
+	           &sess->fe->log_tag, level,
+		   logline, size, logline_rfc5424, sd_size);
 }
 
 void app_log(struct list *loggers, struct buffer *tag, int level, const char *format, ...)