Hi Willy,
On 23/10/2023 10:16, Willy Tarreau wrote:
No more comments, overall this looks good to me. Thus in summary, let's
try to avoid the ambiguity of "tune.lua.log" alone, and re-adjust the
bitfields. By the way, if we're having the same prefix "tune.lua.log"
for both options, it's an indication that we should likely have a dot
after to enable loggers (or any other name) and stderr:
tune.lua.log.loggers on|off
tune.lua.log.stderr on|auto|off
One being prefix of the other was also not great. So even if
"log.loggers" is a little awkward, this is probably for the best after all.
So here's the 2 patches again. Hopefully they match what you wanted.
Two notes:
- I saw various forms of enum declarations in the codebase (naming it or
not and typedef-ing or not) and I don't practice C enough to have an
opinion on the matter, so I just picked the one I think looks nicest...
- For the regtests, if we don't want them to tee stderr to check it,
then there's little point, and chance of breaking this all is also
rather low anyway as you said, so none added
Regards,
Tristan
From 7ef333b8803c213ab1bd0a33a73faa30336ab55e Mon Sep 17 00:00:00 2001
From: Tristan <tris...@mangadex.org>
Date: Mon, 23 Oct 2023 13:07:39 +0100
Subject: [PATCH] MINOR: lua: Add flags to configure logging behaviour
Until now, messages printed from LUA log functions were sent both to
the any logger configured for the current proxy, and additionally to
stderr (in most cases)
This introduces two flags to configure LUA log handling:
- tune.lua.log.loggers to use standard loggers or not
- tune.lua.log.stderr to use stderr, or not, or only conditionally
This addresses github feature request #2316
This can be backported to 2.8 as it doesn't change previous behaviour.
---
doc/configuration.txt | 22 ++++++++++++
doc/lua-api/index.rst | 12 ++++---
doc/lua.txt | 4 +++
src/hlua.c | 80 +++++++++++++++++++++++++++++++++++++++++--
4 files changed, 111 insertions(+), 7 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index bb599bca9..433f15baf 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -1196,6 +1196,8 @@ The following keywords are supported in the "global"
section :
- tune.lua.service-timeout
- tune.lua.session-timeout
- tune.lua.task-timeout
+ - tune.lua.log.loggers
+ - tune.lua.log.stderr
- tune.max-checks-per-thread
- tune.maxaccept
- tune.maxpollevents
@@ -3192,6 +3194,26 @@ tune.lua.task-timeout <timeout>
remain alive during of the lifetime of HAProxy. For example, a task used to
check servers.
+tune.lua.log.loggers { on | off }
+ Enables ('on') or disables ('off') logging the output of LUA scripts via the
+ loggers applicable to the current proxy, if any.
+
+ Defaults to 'on'.
+
+tune.lua.log.stderr { on | auto | off }
+ Enables ('on') or disables ('off') logging the output of LUA scripts via
+ stderr.
+ When set to 'auto', logging via stderr is conditionally 'on' if any of:
+
+ - tune.lua.log.loggers is set to 'off'
+ - the script is executed in a non-proxy context with no global logger
+ - the script is executed in a proxy context with no logger attached
+
+ Please note that, when enabled, this logging is in addition to the logging
+ configured via tune.lua.log.loggers.
+
+ Defaults to 'on'.
+
tune.max-checks-per-thread <number>
Sets the number of active checks per thread above which a thread will
actively try to search a less loaded thread to run the health check, or
diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index 8a29bc5b5..90802cfd4 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -267,8 +267,10 @@ Core class
**context**: body, init, task, action, sample-fetch, converter
This function sends a log. The log is sent, according with the HAProxy
- configuration file, on the default syslog server if it is configured and on
- the stderr if it is allowed.
+ configuration file, to the loggers relevant to the current context and
+ to stderr if it is allowed.
+
+ The exact behaviour depends on tune.lua.log.loggers and tune.lua.log.stderr.
:param integer loglevel: Is the log level associated with the message. It is
a
number between 0 and 7.
@@ -2648,8 +2650,10 @@ TXN class
.. js:function:: TXN.log(TXN, loglevel, msg)
This function sends a log. The log is sent, according with the HAProxy
- configuration file, on the default syslog server if it is configured and on
- the stderr if it is allowed.
+ configuration file, to the loggers relevant to the current context and
+ to stderr if it is allowed.
+
+ The exact behaviour depends on tune.lua.log.loggers and tune.lua.log.stderr.
:param class_txn txn: The class txn object containing the data.
:param integer loglevel: Is the log level associated with the message. It is
diff --git a/doc/lua.txt b/doc/lua.txt
index 8d5561668..25db8a304 100644
--- a/doc/lua.txt
+++ b/doc/lua.txt
@@ -630,6 +630,10 @@ It displays a log during the HAProxy startup:
[alert] 285/083533 (14465) : Hello World !
+Note: By default, logs originating from a LUA script are sent to the loggers
+applicable to the current context, if any, and additionally to stderr. See
+tune.lua.log.loggers and tune.lua.log.stderr for more information.
+
Default path and libraries
--------------------------
diff --git a/src/hlua.c b/src/hlua.c
index e94325727..7c83237ff 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -69,6 +69,20 @@
#include <haproxy/check.h>
#include <haproxy/mailers.h>
+/* Global LUA flags */
+
+enum hlua_log_opt {
+ /* tune.lua.log.loggers */
+ HLUA_LOG_LOGGERS_ON = 0x00000001, /* forward logs to current
loggers */
+
+ /* tune.lua.log.stderr */
+ HLUA_LOG_STDERR_ON = 0x00000010, /* forward logs to stderr */
+ HLUA_LOG_STDERR_AUTO = 0x00000020, /* forward logs to stderr if no
loggers */
+ HLUA_LOG_STDERR_MASK = 0x00000030,
+};
+/* default log options, made of flags in hlua_log_opt */
+static uint hlua_log_opts = HLUA_LOG_LOGGERS_ON | HLUA_LOG_STDERR_ON;
+
/* Lua uses longjmp to perform yield or throwing errors. This
* macro is used only for identifying the function that can
* not return because a longjmp is executed.
@@ -1366,8 +1380,9 @@ const char *hlua_show_current_location(const char *pfx)
return NULL;
}
-/* This function is used to send logs. It try to send on screen (stderr)
- * and on the default syslog server.
+/* This function is used to send logs. It tries to send them to:
+ * - the log target applicable in the current context, AND
+ * - stderr if not in quiet mode or explicitly disabled
*/
static inline void hlua_sendlog(struct proxy *px, int level, const char *msg)
{
@@ -1392,8 +1407,28 @@ static inline void hlua_sendlog(struct proxy *px, int
level, const char *msg)
}
*p = '\0';
- send_log(px, level, "%s\n", trash.area);
+ if (hlua_log_opts & HLUA_LOG_LOGGERS_ON)
+ send_log(px, level, "%s\n", trash.area);
+
if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE |
MODE_STARTING))) {
+ if (!(hlua_log_opts & HLUA_LOG_STDERR_MASK))
+ return;
+
+ /* when logging via stderr is set to 'auto', it behaves like
'off' unless one of:
+ * - logging via loggers is disabled
+ * - this is a non-proxy context and there is no global logger
configured
+ * - this is a proxy context and the proxy has no logger
configured
+ */
+ if ((hlua_log_opts & (HLUA_LOG_STDERR_MASK |
HLUA_LOG_LOGGERS_ON)) == (HLUA_LOG_STDERR_AUTO | HLUA_LOG_LOGGERS_ON)) {
+ /* AUTO=OFF in non-proxy context only if at least one
global logger is defined */
+ if ((px == NULL) && (!LIST_ISEMPTY(&global.loggers)))
+ return;
+
+ /* AUTO=OFF in proxy context only if at least one
logger is configured for the proxy */
+ if ((px != NULL) && (!LIST_ISEMPTY(&px->loggers)))
+ return;
+ }
+
if (level == LOG_DEBUG && !(global.mode & MODE_DEBUG))
return;
@@ -12431,6 +12466,43 @@ static int hlua_parse_maxmem(char **args, int
section_type, struct proxy *curpx,
return 0;
}
+static int hlua_cfg_parse_log_loggers(char **args, int section_type, struct
proxy *curpx,
+ const struct proxy *defpx, const char *file, int
line,
+ char **err)
+{
+ if (too_many_args(1, args, err, NULL))
+ return -1;
+
+ if (strcmp(args[1], "on") == 0)
+ hlua_log_opts |= HLUA_LOG_LOGGERS_ON;
+ else if (strcmp(args[1], "off") == 0)
+ hlua_log_opts &= ~HLUA_LOG_LOGGERS_ON;
+ else {
+ memprintf(err, "'%s' expects either 'on' or 'off' but got
'%s'.", args[0], args[1]);
+ return -1;
+ }
+ return 0;
+}
+
+static int hlua_cfg_parse_log_stderr(char **args, int section_type, struct
proxy *curpx,
+ const struct proxy *defpx, const char
*file, int line,
+ char **err)
+{
+ if (too_many_args(1, args, err, NULL))
+ return -1;
+
+ if (strcmp(args[1], "on") == 0)
+ hlua_log_opts = (hlua_log_opts & ~HLUA_LOG_STDERR_MASK) |
HLUA_LOG_STDERR_ON;
+ else if (strcmp(args[1], "auto") == 0)
+ hlua_log_opts = (hlua_log_opts & ~HLUA_LOG_STDERR_MASK) |
HLUA_LOG_STDERR_AUTO;
+ else if (strcmp(args[1], "off") == 0)
+ hlua_log_opts &= ~HLUA_LOG_STDERR_MASK;
+ else {
+ memprintf(err, "'%s' expects either 'on', 'auto', or 'off' but
got '%s'.", args[0], args[1]);
+ return -1;
+ }
+ return 0;
+}
/* This function is called by the main configuration key "lua-load". It loads
and
* execute an lua file during the parsing of the HAProxy configuration file.
It is
@@ -12680,6 +12752,8 @@ static struct cfg_kw_list cfg_kws = {{ },{
{ CFG_GLOBAL, "tune.lua.burst-timeout", hlua_burst_timeout },
{ CFG_GLOBAL, "tune.lua.forced-yield", hlua_forced_yield },
{ CFG_GLOBAL, "tune.lua.maxmem", hlua_parse_maxmem },
+ { CFG_GLOBAL, "tune.lua.log.loggers", hlua_cfg_parse_log_loggers },
+ { CFG_GLOBAL, "tune.lua.log.stderr", hlua_cfg_parse_log_stderr },
{ 0, NULL, NULL },
}};
--
2.41.0
From b036bc3047efbfc2d526a153f528520ddc7cb7cd Mon Sep 17 00:00:00 2001
From: Tristan <tris...@mangadex.org>
Date: Mon, 23 Oct 2023 13:35:40 +0100
Subject: [PATCH] MINOR: lua: change tune.lua.log.stderr default from 'on' to
'auto'
After making it configurable in commit 7ef333b880 ("MINOR: lua: Add flags
to configure logging behaviour"), this patch changes the default value
of tune.lua.log.stderr from 'on' (unconditionally forward LUA logs to
stderr) to 'auto' (only forward LUA logs to stderr if logging via a
standard logger is disabled, or none is configured for the current context)
Since this is a change in behaviour, it shouldn't be backported
---
doc/configuration.txt | 2 +-
doc/lua-api/index.rst | 4 ++--
doc/lua.txt | 5 +++--
src/hlua.c | 6 +++---
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 433f15baf..4014fb8f6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3212,7 +3212,7 @@ tune.lua.log.stderr { on | auto | off }
Please note that, when enabled, this logging is in addition to the logging
configured via tune.lua.log.loggers.
- Defaults to 'on'.
+ Defaults to 'auto'.
tune.max-checks-per-thread <number>
Sets the number of active checks per thread above which a thread will
diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index 90802cfd4..a71eb6fe4 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -267,7 +267,7 @@ Core class
**context**: body, init, task, action, sample-fetch, converter
This function sends a log. The log is sent, according with the HAProxy
- configuration file, to the loggers relevant to the current context and
+ configuration file, to the loggers relevant to the current context and/or
to stderr if it is allowed.
The exact behaviour depends on tune.lua.log.loggers and tune.lua.log.stderr.
@@ -2650,7 +2650,7 @@ TXN class
.. js:function:: TXN.log(TXN, loglevel, msg)
This function sends a log. The log is sent, according with the HAProxy
- configuration file, to the loggers relevant to the current context and
+ configuration file, to the loggers relevant to the current context and/or
to stderr if it is allowed.
The exact behaviour depends on tune.lua.log.loggers and tune.lua.log.stderr.
diff --git a/doc/lua.txt b/doc/lua.txt
index 25db8a304..edc400558 100644
--- a/doc/lua.txt
+++ b/doc/lua.txt
@@ -631,8 +631,9 @@ It displays a log during the HAProxy startup:
[alert] 285/083533 (14465) : Hello World !
Note: By default, logs originating from a LUA script are sent to the loggers
-applicable to the current context, if any, and additionally to stderr. See
-tune.lua.log.loggers and tune.lua.log.stderr for more information.
+applicable to the current context, if any. If none are configured for use,
+logs are instead sent to stderr. See tune.lua.log.loggers and
tune.lua.log.stderr
+for more information.
Default path and libraries
--------------------------
diff --git a/src/hlua.c b/src/hlua.c
index 7c83237ff..7e0580d35 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -81,7 +81,7 @@ enum hlua_log_opt {
HLUA_LOG_STDERR_MASK = 0x00000030,
};
/* default log options, made of flags in hlua_log_opt */
-static uint hlua_log_opts = HLUA_LOG_LOGGERS_ON | HLUA_LOG_STDERR_ON;
+static uint hlua_log_opts = HLUA_LOG_LOGGERS_ON | HLUA_LOG_STDERR_AUTO;
/* Lua uses longjmp to perform yield or throwing errors. This
* macro is used only for identifying the function that can
@@ -1381,8 +1381,8 @@ const char *hlua_show_current_location(const char *pfx)
}
/* This function is used to send logs. It tries to send them to:
- * - the log target applicable in the current context, AND
- * - stderr if not in quiet mode or explicitly disabled
+ * - the log target applicable in the current context, OR
+ * - stderr when no logger is in use for the current context
*/
static inline void hlua_sendlog(struct proxy *px, int level, const char *msg)
{
--
2.41.0
