Hi all again,
Here is the updated patch set after changes based on feedback received.
The change is now split across 2 patches.
Patch 0001 adding:
- tune.lua.log { on | off } (defaults to 'on') for usage of loggers
- tune.lua.log-stderr { on | auto | off } (defaults to 'on') for usage
of stderr
Effectively no change to current behaviour, hence fit for backport.
Patch 0002 changing:
- tune.lua.log-stderr default from 'on' to 'auto'
Changes behaviour, hence no backport.
I guess my only remaining question is whether we want regtests for this?
I'm be happy to write some if yes, but I know regtests runtime is a
concern and it'd require a bit of test infra work to make capturing
stderr output, so I just want to be sure it is desirable before :-)
Regards,
Tristan
From 45e6f271404d20479284fc3e0e1f7448e260e016 Mon Sep 17 00:00:00 2001
From: Tristan <tris...@mangadex.org>
Date: Fri, 20 Oct 2023 16:31:58 +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 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.txt | 4 +++
src/hlua.c | 80 +++++++++++++++++++++++++++++++++++++++++--
3 files changed, 103 insertions(+), 3 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 50fe882d0..7c4a585ec 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
+ - 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 { 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 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.
+
+ 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.txt b/doc/lua.txt
index 8d5561668..8d244ab3a 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 and tune.lua.log-stderr for more information.
+
Default path and libraries
--------------------------
diff --git a/src/hlua.c b/src/hlua.c
index e94325727..70a25e762 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -69,6 +69,20 @@
#include <haproxy/check.h>
#include <haproxy/mailers.h>
+/* Global LUA flags */
+/* log handling */
+#define HLUA_TUNE_LOG 0x00000001 /* send to current logger */
+#define HLUA_TUNE_LOG_STDERR_ON 0x00000010 /* send to stderr */
+#define HLUA_TUNE_LOG_STDERR_AUTO 0x00000020 /* send to stderr if no logger in
use */
+#define HLUA_TUNE_LOG_STDERR_OFF 0x00000040 /* never send to stderr */
+#define HLUA_TUNE_LOG_STDERR_MASK 0x00000070
+
+/* default flag values */
+#define HLUA_TUNE_FLAGS_DFLT 0x00000011
+
+/* flags made of HLUA_TUNE_* */
+static uint hlua_tune_flags = HLUA_TUNE_FLAGS_DFLT;
+
/* 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_tune_flags & HLUA_TUNE_LOG)
+ send_log(px, level, "%s\n", trash.area);
+
if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE |
MODE_STARTING))) {
+ if (hlua_tune_flags & HLUA_TUNE_LOG_STDERR_OFF)
+ 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_tune_flags & HLUA_TUNE_LOG_STDERR_AUTO) &&
(hlua_tune_flags & HLUA_TUNE_LOG)) {
+ /* 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(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_tune_flags |= HLUA_TUNE_LOG;
+ else if (strcmp(args[1], "off") == 0)
+ hlua_tune_flags &= ~HLUA_TUNE_LOG;
+ 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_tune_flags = (hlua_tune_flags &
~HLUA_TUNE_LOG_STDERR_MASK) | HLUA_TUNE_LOG_STDERR_ON;
+ else if (strcmp(args[1], "auto") == 0)
+ hlua_tune_flags = (hlua_tune_flags &
~HLUA_TUNE_LOG_STDERR_MASK) | HLUA_TUNE_LOG_STDERR_AUTO;
+ else if (strcmp(args[1], "off") == 0)
+ hlua_tune_flags = (hlua_tune_flags &
~HLUA_TUNE_LOG_STDERR_MASK) | HLUA_TUNE_LOG_STDERR_OFF;
+ 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", hlua_cfg_parse_log },
+ { CFG_GLOBAL, "tune.lua.log-stderr", hlua_cfg_parse_log_stderr },
{ 0, NULL, NULL },
}};
--
2.41.0
From ba137e51049fb4049240712a296baaced0be198f Mon Sep 17 00:00:00 2001
From: Tristan <tris...@mangadex.org>
Date: Fri, 20 Oct 2023 16:49:46 +0100
Subject: [PATCH] MINOR: lua: change tune.lua.log-stderr default from 'on' to
'auto'
After making it configurable in commit 45e6f27140 ("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.txt | 4 ++--
src/hlua.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 7c4a585ec..ad4bac7a8 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.
- 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.txt b/doc/lua.txt
index 8d244ab3a..465400a16 100644
--- a/doc/lua.txt
+++ b/doc/lua.txt
@@ -631,8 +631,8 @@ 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 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.
Default path and libraries
--------------------------
diff --git a/src/hlua.c b/src/hlua.c
index 70a25e762..cca96ec0b 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -78,7 +78,7 @@
#define HLUA_TUNE_LOG_STDERR_MASK 0x00000070
/* default flag values */
-#define HLUA_TUNE_FLAGS_DFLT 0x00000011
+#define HLUA_TUNE_FLAGS_DFLT 0x00000021
/* flags made of HLUA_TUNE_* */
static uint hlua_tune_flags = HLUA_TUNE_FLAGS_DFLT;
--
2.41.0
