* [Tarantool-patches] [PATCH v8 01/12] core/say: do not reconfig early set up logger
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 02/12] core/say: use say_logger_initialized in say_logger_free Cyrill Gorcunov
` (12 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
We gonna support logger configuration on its own
without requirement to call `box.cfg{}`. Thus lets
say_logger_init() to skip processing if we already
did.
Note it is preparatory for next patches. Currently
the init called once.
Part-of #689
Reviewed-by: Oleg Babin <olegrok@tarantool.org>
Reviewed-by: Leonid Vasiliev <lvasiliev@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lib/core/say.c | 13 +++++++++++++
src/lib/core/say.h | 4 ++++
2 files changed, 17 insertions(+)
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index e22089aac..d8f59fb9b 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -684,10 +684,23 @@ log_create(struct log *log, const char *init_str, int nonblock)
return 0;
}
+bool
+say_logger_initialized(void)
+{
+ return log_default == &log_std;
+}
+
void
say_logger_init(const char *init_str, int level, int nonblock,
const char *format, int background)
{
+ /*
+ * The logger may be early configured
+ * by hands without configuing the whole box.
+ */
+ if (say_logger_initialized())
+ return;
+
if (log_create(&log_std, init_str, nonblock) < 0)
goto fail;
diff --git a/src/lib/core/say.h b/src/lib/core/say.h
index c50d7bbf4..857145465 100644
--- a/src/lib/core/say.h
+++ b/src/lib/core/say.h
@@ -274,6 +274,10 @@ say_logger_init(const char *init_str,
const char *log_format,
int background);
+/** Test if logger is initialized. */
+bool
+say_logger_initialized(void);
+
/** Free default logger */
void
say_logger_free();
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 02/12] core/say: use say_logger_initialized in say_logger_free
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 01/12] core/say: do not reconfig early set up logger Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 03/12] lua/log: declare say_logger_init and say_logger_initialized Cyrill Gorcunov
` (11 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
To eliminate opencoded comparision.
Reviewed-by: Leonid Vasiliev <lvasiliev@tarantool.org>
Reviewed-by: Oleg Babin <olegrok@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lib/core/say.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index d8f59fb9b..791011e6f 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -749,7 +749,7 @@ say_logger_init(const char *init_str, int level, int nonblock,
void
say_logger_free()
{
- if (log_default == &log_std)
+ if (say_logger_initialized())
log_destroy(&log_std);
}
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 03/12] lua/log: declare say_logger_init and say_logger_initialized
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 01/12] core/say: do not reconfig early set up logger Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 02/12] core/say: use say_logger_initialized in say_logger_free Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 04/12] lua/log: put string constants to map Cyrill Gorcunov
` (10 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
We gonna use it to provide a way to initialize
logger subsystem early.
Part-of #689
Reviewed-by: Oleg Babin <olegrok@tarantool.org>
Reviewed-by: Leonid Vasiliev <lvasiliev@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lua/log.lua | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 312c14d5e..23529b30e 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -22,6 +22,12 @@ ffi.cdef[[
void
say_set_log_format(enum say_format format);
+ extern void
+ say_logger_init(const char *init_str, int level, int nonblock,
+ const char *format, int background);
+
+ extern bool
+ say_logger_initialized(void);
extern sayfunc_t _say;
extern struct ev_loop;
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 04/12] lua/log: put string constants to map
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (2 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 03/12] lua/log: declare say_logger_init and say_logger_initialized Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger Cyrill Gorcunov
` (9 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
This allows us to reuse them instead of copying
opencoded constants. They are highly bound to
ones compiled into the C code.
Part-of #689
Reviewed-by: Oleg Babin <olegrok@tarantool.org>
Reviewed-by: Leonid Vasiliev <lvasiliev@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lua/log.lua | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 23529b30e..94d8ca46c 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -81,6 +81,26 @@ local special_fields = {
"error_msg"
}
+-- Map format number to string.
+local fmt_num2str = {
+ [ffi.C.SF_PLAIN] = "plain",
+ [ffi.C.SF_JSON] = "json",
+}
+
+-- Map format string to number.
+local fmt_str2num = {
+ ["plain"] = ffi.C.SF_PLAIN,
+ ["json"] = ffi.C.SF_JSON,
+}
+
+local function fmt_list()
+ local keyset = {}
+ for k in pairs(fmt_str2num) do
+ keyset[#keyset + 1] = k
+ end
+ return table.concat(keyset, ',')
+end
+
local function say(level, fmt, ...)
if ffi.C.log_level < level then
-- don't waste cycles on debug.getinfo()
@@ -102,7 +122,7 @@ local function say(level, fmt, ...)
fmt = json.encode(fmt)
if ffi.C.log_format == ffi.C.SF_JSON then
-- indicate that message is already encoded in JSON
- format = "json"
+ format = fmt_num2str[ffi.C.SF_JSON]
end
elseif type_fmt ~= 'string' then
fmt = tostring(fmt)
@@ -133,16 +153,19 @@ local function log_level(level)
return ffi.C.say_set_log_level(level)
end
-local function log_format(format_name)
- if format_name == "json" then
+local function log_format(name)
+ if not fmt_str2num[name] then
+ local m = "log_format: expected %s"
+ error(m:format(fmt_list()))
+ end
+
+ if fmt_str2num[name] == ffi.C.SF_JSON then
if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG then
error("log_format: 'json' can't be used with syslog logger")
end
ffi.C.say_set_log_format(ffi.C.SF_JSON)
- elseif format_name == "plain" then
- ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
else
- error("log_format: expected 'json' or 'plain'")
+ ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
end
end
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (3 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 04/12] lua/log: put string constants to map Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-03 6:59 ` Oleg Babin
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 06/12] lua/log: declare log as separate variable Cyrill Gorcunov
` (8 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
For some reason we've missed that say_set_log_format
doesn't support json format not only for syslog but
for boottime logging as well.
Part-of #689
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lua/log.lua | 7 +++++--
test/app-tap/logger.test.lua | 5 +++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 94d8ca46c..d5a99076d 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -160,8 +160,11 @@ local function log_format(name)
end
if fmt_str2num[name] == ffi.C.SF_JSON then
- if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG then
- error("log_format: 'json' can't be used with syslog logger")
+ if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG or
+ ffi.C.log_type() == ffi.C.SAY_LOGGER_BOOT then
+ local m = "log_format: %s can't be used with " ..
+ "syslog or boot-time logger"
+ error(m:format(fmt_num2str[ffi.C.SF_JSON]))
end
ffi.C.say_set_log_format(ffi.C.SF_JSON)
else
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 492d5ea0b..222343908 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,12 +1,13 @@
#!/usr/bin/env tarantool
local test = require('tap').test('log')
-test:plan(24)
+test:plan(25)
-- gh-3946: Assertion failure when using log_format() before box.cfg()
local log = require('log')
-log.log_format('json')
log.log_format('plain')
+_, err = pcall(log.log_format, 'json')
+test:ok(err:find("log_format: json can\'t be used") ~= nil)
--
-- Check that Tarantool creates ADMIN session for #! script
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger Cyrill Gorcunov
@ 2020-06-03 6:59 ` Oleg Babin
2020-06-03 7:34 ` Cyrill Gorcunov
0 siblings, 1 reply; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 6:59 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Hi! Thanks for your changes. LGTM.
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> For some reason we've missed that say_set_log_format
> doesn't support json format not only for syslog but
> for boottime logging as well.
>
> Part-of #689
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> src/lua/log.lua | 7 +++++--
> test/app-tap/logger.test.lua | 5 +++--
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/lua/log.lua b/src/lua/log.lua
> index 94d8ca46c..d5a99076d 100644
> --- a/src/lua/log.lua
> +++ b/src/lua/log.lua
> @@ -160,8 +160,11 @@ local function log_format(name)
> end
>
> if fmt_str2num[name] == ffi.C.SF_JSON then
> - if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG then
> - error("log_format: 'json' can't be used with syslog logger")
> + if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG or
> + ffi.C.log_type() == ffi.C.SAY_LOGGER_BOOT then
> + local m = "log_format: %s can't be used with " ..
> + "syslog or boot-time logger"
> + error(m:format(fmt_num2str[ffi.C.SF_JSON]))
> end
> ffi.C.say_set_log_format(ffi.C.SF_JSON)
> else
> diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
> index 492d5ea0b..222343908 100755
> --- a/test/app-tap/logger.test.lua
> +++ b/test/app-tap/logger.test.lua
> @@ -1,12 +1,13 @@
> #!/usr/bin/env tarantool
>
> local test = require('tap').test('log')
> -test:plan(24)
> +test:plan(25)
>
> -- gh-3946: Assertion failure when using log_format() before box.cfg()
> local log = require('log')
> -log.log_format('json')
> log.log_format('plain')
> +_, err = pcall(log.log_format, 'json')
> +test:ok(err:find("log_format: json can\'t be used") ~= nil)
>
> --
> -- Check that Tarantool creates ADMIN session for #! script
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger
2020-06-03 6:59 ` Oleg Babin
@ 2020-06-03 7:34 ` Cyrill Gorcunov
2020-06-03 9:44 ` Kirill Yukhin
0 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-03 7:34 UTC (permalink / raw)
To: Kirill Yukhin; +Cc: tml
On Wed, Jun 03, 2020 at 09:59:38AM +0300, Oleg Babin wrote:
> Hi! Thanks for your changes. LGTM.
>
Kirill, can we please merge first 5 patches (they are not changing
mutch but prepa the ground for others) so I won't have to continue
resending them.
I've stacked into a separate branch gorcunov/gh-689-logger-8-merge
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger
2020-06-03 7:34 ` Cyrill Gorcunov
@ 2020-06-03 9:44 ` Kirill Yukhin
0 siblings, 0 replies; 23+ messages in thread
From: Kirill Yukhin @ 2020-06-03 9:44 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
Hello,
On 03 июн 10:34, Cyrill Gorcunov wrote:
> On Wed, Jun 03, 2020 at 09:59:38AM +0300, Oleg Babin wrote:
> > Hi! Thanks for your changes. LGTM.
> >
>
> Kirill, can we please merge first 5 patches (they are not changing
> mutch but prepa the ground for others) so I won't have to continue
> resending them.
>
> I've stacked into a separate branch gorcunov/gh-689-logger-8-merge
Done.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Tarantool-patches] [PATCH v8 06/12] lua/log: declare log as separate variable
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (4 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 05/12] lua/log: do not allow to set json for boot logger Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 07/12] lua/log: use log module settings inside box.cfg Cyrill Gorcunov
` (7 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
We gonna hide some internal symbols in sake
of communication with box module, thus lets
make it clear.
Part-of #689
Reviewed-by: Oleg Babin <olegrok@tarantool.org>
Reviewed-by: Leonid Vasiliev <lvasiliev@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lua/log.lua | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index d5a99076d..746e0d82f 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -187,16 +187,20 @@ local compat_v16 = {
end;
}
-return setmetatable({
- warn = say_closure(S_WARN);
- info = say_closure(S_INFO);
- verbose = say_closure(S_VERBOSE);
- debug = say_closure(S_DEBUG);
- error = say_closure(S_ERROR);
- rotate = log_rotate;
- pid = log_pid;
- level = log_level;
- log_format = log_format;
-}, {
+local log = {
+ warn = say_closure(S_WARN),
+ info = say_closure(S_INFO),
+ verbose = say_closure(S_VERBOSE),
+ debug = say_closure(S_DEBUG),
+ error = say_closure(S_ERROR),
+ rotate = log_rotate,
+ pid = log_pid,
+ level = log_level,
+ log_format = log_format,
+}
+
+setmetatable(log, {
__index = compat_v16;
})
+
+return log
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 07/12] lua/log: use log module settings inside box.cfg
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (5 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 06/12] lua/log: declare log as separate variable Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-03 7:00 ` Oleg Babin
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 08/12] lua/log: allow to configure logging without a box Cyrill Gorcunov
` (6 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
Currently box module carries configuration settings in box.cfg
variable which is created dinamically on demand.
The default values are kept in default_cfg variable. Since we're
going to make the log module to work on its own, we need it to
provide default settings to the box.cfg interface.
For this sake we export log:box_api table which the main box
module use when needed.
Part-of #689
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/lua/load_cfg.lua | 26 ++++++++++++++----
src/lua/log.lua | 59 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 78 insertions(+), 7 deletions(-)
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 5d818addf..7612deb90 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -59,10 +59,6 @@ local default_cfg = {
vinyl_range_size = nil, -- set automatically
vinyl_page_size = 8 * 1024,
vinyl_bloom_fpr = 0.05,
- log = nil,
- log_nonblock = nil,
- log_level = 5,
- log_format = "plain",
io_collect_interval = nil,
readahead = 16320,
snap_io_rate_limit = nil, -- no limit
@@ -233,8 +229,8 @@ end
local dynamic_cfg = {
listen = private.cfg_set_listen,
replication = private.cfg_set_replication,
- log_level = private.cfg_set_log_level,
- log_format = private.cfg_set_log_format,
+ log_level = log.box_api.set_log_level,
+ log_format = log.box_api.set_log_format,
io_collect_interval = private.cfg_set_io_collect_interval,
readahead = private.cfg_set_readahead,
too_long_threshold = private.cfg_set_too_long_threshold,
@@ -470,6 +466,21 @@ local function apply_default_cfg(cfg, default_cfg)
end
end
+-- Fetch default settings from modules.
+local function modules_apply_default_cfg(cfg)
+ log.box_api.on_apply_default_cfg(cfg)
+end
+
+-- Propagate settings to modules.
+local function modules_load_cfg(cfg)
+ log.box_api.on_load_cfg(cfg)
+end
+
+-- Propagate settings to modules.
+local function modules_reload_cfg(cfg)
+ log.box_api.on_reload_cfg(cfg)
+end
+
-- Return true if two configurations are equivalent.
local function compare_cfg(cfg1, cfg2)
if type(cfg1) ~= type(cfg2) then
@@ -517,6 +528,7 @@ local function reload_cfg(oldcfg, cfg)
json.encode(val))
end
end
+ modules_reload_cfg(cfg)
if type(box.on_reload_configuration) == 'function' then
box.on_reload_configuration()
end
@@ -554,6 +566,7 @@ local function load_cfg(cfg)
cfg = upgrade_cfg(cfg, translate_cfg)
cfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
apply_default_cfg(cfg, default_cfg);
+ modules_apply_default_cfg(cfg)
-- Save new box.cfg
box.cfg = cfg
if not pcall(private.cfg_check) then
@@ -573,6 +586,7 @@ local function load_cfg(cfg)
end,
__call = locked(reload_cfg),
})
+ modules_load_cfg(box.cfg)
private.cfg_load()
for key, fun in pairs(dynamic_cfg) do
local val = cfg[key]
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 746e0d82f..87ce313e5 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -101,6 +101,24 @@ local function fmt_list()
return table.concat(keyset, ',')
end
+-- Default options. The keys are part of
+-- user API, so change with caution.
+local log_cfg = {
+ log = nil,
+ nonblock = false,
+ level = S_INFO,
+ format = fmt_num2str[ffi.C.SF_PLAIN],
+}
+
+-- Name mapping from box to log module.
+-- Make sure all required fields are covered!
+local log2box_keys = {
+ ['log'] = 'log',
+ ['nonblock'] = 'log_nonblock',
+ ['level'] = 'log_level',
+ ['format'] = 'log_format',
+}
+
local function say(level, fmt, ...)
if ffi.C.log_level < level then
-- don't waste cycles on debug.getinfo()
@@ -150,7 +168,8 @@ local function log_rotate()
end
local function log_level(level)
- return ffi.C.say_set_log_level(level)
+ ffi.C.say_set_log_level(level)
+ rawset(log_cfg, 'level', level)
end
local function log_format(name)
@@ -170,12 +189,31 @@ local function log_format(name)
else
ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
end
+ rawset(log_cfg, 'format', name)
end
local function log_pid()
return tonumber(ffi.C.log_pid)
end
+-- Apply defaut config to the box module
+local function box_on_apply_default_cfg(box_cfg)
+ for k, v in pairs(log2box_keys) do
+ if box_cfg[v] == nil then
+ box_cfg[v] = log_cfg[k]
+ end
+ end
+end
+
+-- Update own values from box interface.
+local function box_on_load_cfg(box_cfg)
+ for k, v in pairs(log2box_keys) do
+ if box_cfg[v] ~= log_cfg[k] then
+ log_cfg[k] = box_cfg[v]
+ end
+ end
+end
+
local compat_warning_said = false
local compat_v16 = {
logger_pid = function()
@@ -197,9 +235,28 @@ local log = {
pid = log_pid,
level = log_level,
log_format = log_format,
+
+ -- Internal API to box module, not for users,
+ -- names can be changed.
+ box_api = {
+ set_log_level = function()
+ log_level(box.cfg.log_level)
+ end,
+ set_log_format = function()
+ log_format(box.cfg.log_format)
+ end,
+ on_load_cfg = box_on_load_cfg,
+ on_reload_cfg = box_on_load_cfg,
+ on_apply_default_cfg = box_on_apply_default_cfg,
+ },
}
setmetatable(log, {
+ __serialize = function(self)
+ local res = table.copy(self)
+ res.box_api = nil
+ return setmetatable(res, {})
+ end,
__index = compat_v16;
})
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 07/12] lua/log: use log module settings inside box.cfg
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 07/12] lua/log: use log module settings inside box.cfg Cyrill Gorcunov
@ 2020-06-03 7:00 ` Oleg Babin
0 siblings, 0 replies; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 7:00 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Hi! Thanks for your changes see one comment below.
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> Currently box module carries configuration settings in box.cfg
> variable which is created dinamically on demand.
>
> The default values are kept in default_cfg variable. Since we're
> going to make the log module to work on its own, we need it to
> provide default settings to the box.cfg interface.
>
> For this sake we export log:box_api table which the main box
> module use when needed.
>
> Part-of #689
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> src/box/lua/load_cfg.lua | 26 ++++++++++++++----
> src/lua/log.lua | 59 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 78 insertions(+), 7 deletions(-)
>
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 5d818addf..7612deb90 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -59,10 +59,6 @@ local default_cfg = {
> vinyl_range_size = nil, -- set automatically
> vinyl_page_size = 8 * 1024,
> vinyl_bloom_fpr = 0.05,
> - log = nil,
> - log_nonblock = nil,
> - log_level = 5,
> - log_format = "plain",
> io_collect_interval = nil,
> readahead = 16320,
> snap_io_rate_limit = nil, -- no limit
> @@ -233,8 +229,8 @@ end
> local dynamic_cfg = {
> listen = private.cfg_set_listen,
> replication = private.cfg_set_replication,
> - log_level = private.cfg_set_log_level,
> - log_format = private.cfg_set_log_format,
> + log_level = log.box_api.set_log_level,
> + log_format = log.box_api.set_log_format,
Seems after that private.cfg_set_log_format/cfg_set_log_format functions
will be unused/redundant.
And seems they should be dropped
https://github.com/tarantool/tarantool/blob/5a1a220ee84a32841b0834a282fe43201343258a/src/box/lua/cfg.cc#L91
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Tarantool-patches] [PATCH v8 08/12] lua/log: allow to configure logging without a box
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (6 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 07/12] lua/log: use log module settings inside box.cfg Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-03 7:01 ` Oleg Babin
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 09/12] test: logger -- use log module directly Cyrill Gorcunov
` (5 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
We bring in an ability to configure logger early without
calling box.cfg{}. For this sake log.cfg({}) becomes a
callable table. One can provide arguments of the looger
which later will be propagated back to box.cfg table on
demand.
Another notable change is that the settings in log module
become defaults value, thus if one configured log before
the box then new settings get propagated to box's default
logging settings (previously they are silently reset to
defaults).
Fixes #689
@TarantoolBot documnent
Title: Module log
To configure log module eary without initializing box module
at all call the `log.cfg({})` method, where the following
arguments are acceptable:
- `log` to specify file, pipe or syslog (same as
`box.cfg{log}` option); the default value is nil
and stderr is used;
- `level` to specify logging level (1-7); the default
value is 5;
- `format` to specify output encoding ('plain' or 'json');
the default value is 'plain';
- `nonblock` to open logging output in nonblocking mode
(`true` or `false`); the default value is `false`.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/lua/load_cfg.lua | 9 +++
src/lua/log.lua | 132 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 139 insertions(+), 2 deletions(-)
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 7612deb90..a9024ee64 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -481,6 +481,12 @@ local function modules_reload_cfg(cfg)
log.box_api.on_reload_cfg(cfg)
end
+-- Test if modules support dynamic change
+-- of the key provided.
+local function modules_may_dynamic_cfg(key, val)
+ return log.box_api.on_may_dynamic_cfg(key, val)
+end
+
-- Return true if two configurations are equivalent.
local function compare_cfg(cfg1, cfg2)
if type(cfg1) ~= type(cfg2) then
@@ -506,6 +512,9 @@ local function reload_cfg(oldcfg, cfg)
local ordered_cfg = {}
-- iterate over original table because prepare_cfg() may store NILs
for key, val in pairs(cfg) do
+ if not modules_may_dynamic_cfg(key, val) then
+ box.error(box.error.RELOAD_CFG, key);
+ end
if dynamic_cfg[key] == nil and oldcfg[key] ~= val then
box.error(box.error.RELOAD_CFG, key);
end
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 87ce313e5..33f7a5892 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -119,6 +119,27 @@ local log2box_keys = {
['format'] = 'log_format',
}
+local box_module_cfg = nil
+
+-- Reflect changes in box.cfg (if
+-- box.cfg been initialized).
+local function update_box_cfg(k)
+ if box_module_cfg ~= nil then
+ local update = function(k, v)
+ if box_module_cfg[v] ~= log_cfg[k] then
+ box_module_cfg[v] = log_cfg[k]
+ end
+ end
+ if k ~= nil then
+ update(k, log2box_keys[k])
+ else
+ for k, v in pairs(log2box_keys) do
+ update(k, v)
+ end
+ end
+ end
+end
+
local function say(level, fmt, ...)
if ffi.C.log_level < level then
-- don't waste cycles on debug.getinfo()
@@ -170,6 +191,7 @@ end
local function log_level(level)
ffi.C.say_set_log_level(level)
rawset(log_cfg, 'level', level)
+ update_box_cfg('level')
end
local function log_format(name)
@@ -190,17 +212,47 @@ local function log_format(name)
ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
end
rawset(log_cfg, 'format', name)
+ update_box_cfg('format')
end
local function log_pid()
return tonumber(ffi.C.log_pid)
end
--- Apply defaut config to the box module
+-- The keys to be assigned once either
+-- from log.cfg interface or from box.cfg.
+local box2log_static_keys = {
+ ['log'] = 'log',
+ ['log_nonblock']= 'nonblock',
+}
+
+local function box_on_may_dynamic_cfg(box_key, box_val)
+ local log_key = box2log_static_keys[box_key]
+ -- Skip non ours keys (the box_keys iterate
+ -- over the global cummulative config).
+ if log_key ~= nil then
+ if box_val ~= log_cfg[log_key] then
+ return false
+ end
+ end
+ return true
+end
+
+-- Apply defaut config to the box module.
local function box_on_apply_default_cfg(box_cfg)
for k, v in pairs(log2box_keys) do
if box_cfg[v] == nil then
box_cfg[v] = log_cfg[k]
+ else
+ -- In case if we're already initialized
+ -- via the log.cfg() interface make
+ -- the key may be changed dynamically.
+ if ffi.C.say_logger_initialized() == true then
+ if not box_on_may_dynamic_cfg(v, box_cfg[v]) then
+ local m = "log: '%s' can't be set dynamically"
+ error(m:format(v))
+ end
+ end
end
end
end
@@ -208,10 +260,82 @@ end
-- Update own values from box interface.
local function box_on_load_cfg(box_cfg)
for k, v in pairs(log2box_keys) do
- if box_cfg[v] ~= log_cfg[k] then
+ if box_cfg[v] ~= nil and box_cfg[v] ~= log_cfg[k] then
log_cfg[k] = box_cfg[v]
end
end
+ -- Remember the backreference to box
+ -- module config table.
+ box_module_cfg = box_cfg
+end
+
+-- Setup dynamic parameters.
+local function dynamic_cfg(cfg)
+ for _,v in pairs(box2log_static_keys) do
+ if cfg[v] ~= nil and cfg[v] ~= log_cfg[v] then
+ local m = "log: '%s' can't be set dynamically"
+ error(m:format(v))
+ end
+ end
+
+ if cfg.level ~= nil then
+ log_level(cfg.level)
+ end
+
+ if cfg.format ~= nil then
+ log_format(cfg.format)
+ end
+
+ -- The log_x helpers above update box
+ -- settings on their own, so no need
+ -- for more update_box_cfg calls.
+end
+
+-- Load or reload configuration via log.cfg({}) call.
+local function load_cfg(oldcfg, cfg)
+ cfg = cfg or {}
+
+ if cfg.format ~= nil then
+ if fmt_str2num[cfg.format] == nil then
+ local m = "log: 'format' must be %s"
+ error(m:format(fmt_list()))
+ end
+ end
+
+ if cfg.level ~= nil then
+ if type(cfg.level) ~= 'number' then
+ error("log: 'level' option must be a number")
+ end
+ end
+
+ if cfg.nonblock ~= nil then
+ if type(cfg.nonblock) ~= 'boolean' then
+ error("log: 'nonblock' option must be 'true' or 'false'")
+ end
+ end
+
+ if ffi.C.say_logger_initialized() == true then
+ return dynamic_cfg(cfg)
+ end
+
+ cfg.level = cfg.level or log_cfg.level
+ cfg.format = cfg.format or log_cfg.format
+ cfg.nonblock = cfg.nonblock or log_cfg.nonblock
+
+ -- We never allow confgure the logger in background
+ -- mode since we don't know how the box will be configured
+ -- later.
+ ffi.C.say_logger_init(cfg.log, cfg.level,
+ cfg.nonblock, cfg.format, 0)
+
+ -- Update log_cfg vars to show them in module
+ -- configuration output.
+ rawset(log_cfg, 'log', cfg.log)
+ rawset(log_cfg, 'level', cfg.level)
+ rawset(log_cfg, 'nonblock', cfg.nonblock)
+ rawset(log_cfg, 'format', cfg.format)
+
+ update_box_cfg()
end
local compat_warning_said = false
@@ -235,6 +359,9 @@ local log = {
pid = log_pid,
level = log_level,
log_format = log_format,
+ cfg = setmetatable(log_cfg, {
+ __call = load_cfg,
+ }),
-- Internal API to box module, not for users,
-- names can be changed.
@@ -247,6 +374,7 @@ local log = {
end,
on_load_cfg = box_on_load_cfg,
on_reload_cfg = box_on_load_cfg,
+ on_may_dynamic_cfg = box_on_may_dynamic_cfg,
on_apply_default_cfg = box_on_apply_default_cfg,
},
}
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 09/12] test: logger -- use log module directly
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (7 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 08/12] lua/log: allow to configure logging without a box Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-03 7:02 ` Oleg Babin
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 10/12] log/lua: allow to specify logging level as a string Cyrill Gorcunov
` (4 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
To test if we can setup logging module before the box.cfg{}.
Part-of #689
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
test/app-tap/logger.test.lua | 56 ++++++++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 222343908..bc301d720 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,7 +1,7 @@
#!/usr/bin/env tarantool
local test = require('tap').test('log')
-test:plan(25)
+test:plan(34)
-- gh-3946: Assertion failure when using log_format() before box.cfg()
local log = require('log')
@@ -10,16 +10,66 @@ _, err = pcall(log.log_format, 'json')
test:ok(err:find("log_format: json can\'t be used") ~= nil)
--
--- Check that Tarantool creates ADMIN session for #! script
+-- gh-689: Operate with logger via log module without calling box.cfg{}
--
+local json = require('json')
local filename = "1.log"
+
+-- Test json output
+log.cfg({log=filename, format='json', level=5})
+local m = "info message"
+
+local file = io.open(filename)
+while file:read() do
+end
+
+log.info(m)
+local line = file:read()
+local message = json.decode(line)
+file:close()
+
+test:is(type(message), 'table', "(log) json valid in log.info")
+test:is(message.level, "INFO", "(log) check type info")
+test:is(message.message, m, "(log) check message content")
+
+-- Test runtime change to plain format
+log.log_format('plain')
+local file = io.open(filename)
+while file:read() do
+end
+log.info(m)
+local line = file:read()
+file:close()
+test:is(line:sub(-m:len()), m, m)
+
+-- Test wrong arguments
+_, err = pcall(log.cfg, {format = 'unknown'})
+test:ok(err:find("log: \'format\' must be json,plain") ~= nil)
+
+-- Test that changes in log module are propagated
+-- back to the box module correctly
+box.cfg{
+ log=filename,
+ log_format='json',
+ log_level=6,
+ memtx_memory=107374182,
+}
+test:is(box.cfg.log_format, 'json', 'box in json format')
+test:is(box.cfg.log_level, 6, 'box level 6')
+log.cfg({format='plain', level=5})
+test:is(box.cfg.log_format, 'plain', 'box sees plain format')
+test:is(box.cfg.log_level, 5, 'box sees level change')
+
+--
+-- Check that Tarantool creates ADMIN session for #! script
+--
local message = "Hello, World!"
box.cfg{
log=filename,
+ log_format='plain',
memtx_memory=107374182,
}
local fio = require('fio')
-local json = require('json')
local fiber = require('fiber')
local file = io.open(filename)
while file:read() do
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 09/12] test: logger -- use log module directly
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 09/12] test: logger -- use log module directly Cyrill Gorcunov
@ 2020-06-03 7:02 ` Oleg Babin
0 siblings, 0 replies; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 7:02 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Hi! Thanks for your patch see one comment below.
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> +-- Test that changes in log module are propagated
> +-- back to the box module correctly
> +box.cfg{
> + log=filename,
> + log_format='json',
> + log_level=6,
> + memtx_memory=107374182,
> +}
> +test:is(box.cfg.log_format, 'json', 'box in json format')
> +test:is(box.cfg.log_level, 6, 'box level 6')
It's quite strange. You change box.cfg options and check that box.cfg
options are changed. I suppose you should check log.cfg options here.
Anyway if you cover that box.cfg change propagates to log.cfg you should
test that log.cfg changes propagate to box.cfg as well.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Tarantool-patches] [PATCH v8 10/12] log/lua: allow to specify logging level as a string
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (8 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 09/12] test: logger -- use log module directly Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-03 7:03 ` Oleg Babin
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 11/12] lua/log: use log_cfg instead of ffi's instances Cyrill Gorcunov
` (3 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
Allow to specify logging level as a string in log.cfg() call.
@TarantoolBot document
Title: Module log
The `log.cfg({level='string'})` accepts not only number but
symbolic representation of loglevels such as: `fatal`, `syserror`,
`error`, `crit`, `warn`, `info`, `verbose` and `debug`.
Part-of #689
Suggested-by: Oleg Babin <olegrok@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lua/log.lua | 36 ++++++++++++++++++++++++++++++++++--
test/app-tap/logger.test.lua | 29 ++++++++++++++++++++++++++++-
2 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 33f7a5892..50c96a8d2 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -140,6 +140,26 @@ local function update_box_cfg(k)
end
end
+-- Logging levels symbolic representation.
+local log_level_keys = {
+ ['fatal'] = ffi.C.S_FATAL,
+ ['syserror'] = ffi.C.S_SYSERROR,
+ ['error'] = ffi.C.S_ERROR,
+ ['crit'] = ffi.C.S_CRIT,
+ ['warn'] = ffi.C.S_WARN,
+ ['info'] = ffi.C.S_INFO,
+ ['verbose'] = ffi.C.S_VERBOSE,
+ ['debug'] = ffi.C.S_DEBUG,
+}
+
+local function log_level_list()
+ local keyset = {}
+ for k in pairs(log_level_keys) do
+ keyset[#keyset + 1] = k
+ end
+ return table.concat(keyset, ',')
+end
+
local function say(level, fmt, ...)
if ffi.C.log_level < level then
-- don't waste cycles on debug.getinfo()
@@ -303,8 +323,20 @@ local function load_cfg(oldcfg, cfg)
end
if cfg.level ~= nil then
- if type(cfg.level) ~= 'number' then
- error("log: 'level' option must be a number")
+ if type(cfg.level) == 'number' then
+ if cfg.level < ffi.C.S_FATAL or
+ cfg.level > ffi.C.S_DEBUG then
+ local m = "log: 'level' must be in range [%d;%d]"
+ error(m:format(ffi.C.S_FATAL, ffi.C.S_DEBUG))
+ end
+ elseif type(cfg.level) == 'string' then
+ if log_level_keys[cfg.level] == nil then
+ local m = "log: 'level' must be one of [%s]"
+ error(m:format(log_level_list()))
+ end
+ cfg.level = log_level_keys[cfg.level]
+ else
+ error("log: 'level' must be a number or string")
end
end
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index bc301d720..48dc8af3e 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,7 +1,7 @@
#!/usr/bin/env tarantool
local test = require('tap').test('log')
-test:plan(34)
+test:plan(44)
-- gh-3946: Assertion failure when using log_format() before box.cfg()
local log = require('log')
@@ -60,6 +60,33 @@ log.cfg({format='plain', level=5})
test:is(box.cfg.log_format, 'plain', 'box sees plain format')
test:is(box.cfg.log_level, 5, 'box sees level change')
+-- Test symbolic names for loglevels
+log.cfg({level='fatal'})
+test:ok(log.cfg.level == 0 and box.cfg.log_level == 0, 'both got fatal')
+log.cfg({level='syserror'})
+test:ok(log.cfg.level == 1 and box.cfg.log_level == 1, 'both got syserror')
+log.cfg({level='error'})
+test:ok(log.cfg.level == 2 and box.cfg.log_level == 2, 'both got error')
+log.cfg({level='crit'})
+test:ok(log.cfg.level == 3 and box.cfg.log_level == 3, 'both got crit')
+log.cfg({level='warn'})
+test:ok(log.cfg.level == 4 and box.cfg.log_level == 4, 'both got warn')
+log.cfg({level='info'})
+test:ok(log.cfg.level == 5 and box.cfg.log_level == 5, 'both got info')
+log.cfg({level='verbose'})
+test:ok(log.cfg.level == 6 and box.cfg.log_level == 6, 'both got verbose')
+log.cfg({level='debug'})
+test:ok(log.cfg.level == 7 and box.cfg.log_level == 7, 'both got debug')
+
+_,err = pcall(log, {level = -1})
+test:ok(err ~= nil, 'level = -1 error')
+
+_,err = pcall(log, {level = 'unknown'})
+test:ok(err ~= nil, "level = 'unknown' error")
+
+-- restore to info for next tests
+log.cfg({level='info'})
+
--
-- Check that Tarantool creates ADMIN session for #! script
--
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 10/12] log/lua: allow to specify logging level as a string
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 10/12] log/lua: allow to specify logging level as a string Cyrill Gorcunov
@ 2020-06-03 7:03 ` Oleg Babin
0 siblings, 0 replies; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 7:03 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Hi! Thanks for changes, see one comment below.
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> Allow to specify logging level as a string in log.cfg() call.
>
> @TarantoolBot document
> Title: Module log
>
> The `log.cfg({level='string'})` accepts not only number but
> symbolic representation of loglevels such as: `fatal`, `syserror`,
> `error`, `crit`, `warn`, `info`, `verbose` and `debug`.
>
> Part-of #689
>
> Suggested-by: Oleg Babin <olegrok@tarantool.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> src/lua/log.lua | 36 ++++++++++++++++++++++++++++++++++--
> test/app-tap/logger.test.lua | 29 ++++++++++++++++++++++++++++-
> 2 files changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/src/lua/log.lua b/src/lua/log.lua
> index 33f7a5892..50c96a8d2 100644
> --- a/src/lua/log.lua
> +++ b/src/lua/log.lua
> @@ -140,6 +140,26 @@ local function update_box_cfg(k)
> end
> end
>
> +-- Logging levels symbolic representation.
> +local log_level_keys = {
> + ['fatal'] = ffi.C.S_FATAL,
> + ['syserror'] = ffi.C.S_SYSERROR,
> + ['error'] = ffi.C.S_ERROR,
> + ['crit'] = ffi.C.S_CRIT,
> + ['warn'] = ffi.C.S_WARN,
> + ['info'] = ffi.C.S_INFO,
> + ['verbose'] = ffi.C.S_VERBOSE,
> + ['debug'] = ffi.C.S_DEBUG,
> +}
> +
> +local function log_level_list()
> + local keyset = {}
> + for k in pairs(log_level_keys) do
> + keyset[#keyset + 1] = k
> + end
> + return table.concat(keyset, ',')
> +end
> +
> local function say(level, fmt, ...)
> if ffi.C.log_level < level then
> -- don't waste cycles on debug.getinfo()
> @@ -303,8 +323,20 @@ local function load_cfg(oldcfg, cfg)
> end
>
> if cfg.level ~= nil then
> - if type(cfg.level) ~= 'number' then
> - error("log: 'level' option must be a number")
> + if type(cfg.level) == 'number' then
> + if cfg.level < ffi.C.S_FATAL or
> + cfg.level > ffi.C.S_DEBUG then
> + local m = "log: 'level' must be in range [%d;%d]"
> + error(m:format(ffi.C.S_FATAL, ffi.C.S_DEBUG))
> + end
> + elseif type(cfg.level) == 'string' then
> + if log_level_keys[cfg.level] == nil then
> + local m = "log: 'level' must be one of [%s]"
> + error(m:format(log_level_list()))
> + end
> + cfg.level = log_level_keys[cfg.level]
> + else
> + error("log: 'level' must be a number or string")
> end
> end
>
> diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
> index bc301d720..48dc8af3e 100755
> --- a/test/app-tap/logger.test.lua
> +++ b/test/app-tap/logger.test.lua
> @@ -1,7 +1,7 @@
> #!/usr/bin/env tarantool
>
> local test = require('tap').test('log')
> -test:plan(34)
> +test:plan(44)
>
> -- gh-3946: Assertion failure when using log_format() before box.cfg()
> local log = require('log')
> @@ -60,6 +60,33 @@ log.cfg({format='plain', level=5})
> test:is(box.cfg.log_format, 'plain', 'box sees plain format')
> test:is(box.cfg.log_level, 5, 'box sees level change')
>
> +-- Test symbolic names for loglevels
> +log.cfg({level='fatal'})
> +test:ok(log.cfg.level == 0 and box.cfg.log_level == 0, 'both got fatal')
> +log.cfg({level='syserror'})
> +test:ok(log.cfg.level == 1 and box.cfg.log_level == 1, 'both got syserror')
> +log.cfg({level='error'})
> +test:ok(log.cfg.level == 2 and box.cfg.log_level == 2, 'both got error')
> +log.cfg({level='crit'})
> +test:ok(log.cfg.level == 3 and box.cfg.log_level == 3, 'both got crit')
> +log.cfg({level='warn'})
> +test:ok(log.cfg.level == 4 and box.cfg.log_level == 4, 'both got warn')
> +log.cfg({level='info'})
> +test:ok(log.cfg.level == 5 and box.cfg.log_level == 5, 'both got info')
> +log.cfg({level='verbose'})
> +test:ok(log.cfg.level == 6 and box.cfg.log_level == 6, 'both got verbose')
> +log.cfg({level='debug'})
> +test:ok(log.cfg.level == 7 and box.cfg.log_level == 7, 'both got debug')
> +
> +_,err = pcall(log, {level = -1})
> +test:ok(err ~= nil, 'level = -1 error')
> +
> +_,err = pcall(log, {level = 'unknown'})
> +test:ok(err ~= nil, "level = 'unknown' error")
> +
```
tarantool> pcall(log, {level = -1})
---
- false
- attempt to call a table value
...
tarantool> pcall(log, {level = 'unknown'})
---
- false
- attempt to call a table value
...
```
It should be pcall(log.cfg, ...). Also it would be nice to check that
error message is "log: 'level' must be one of...".
> +-- restore to info for next tests
> +log.cfg({level='info'})
> +
> --
> -- Check that Tarantool creates ADMIN session for #! script
> --
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Tarantool-patches] [PATCH v8 11/12] lua/log: use log_cfg instead of ffi's instances
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (9 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 10/12] log/lua: allow to specify logging level as a string Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 12/12] test: logger -- consider more cases Cyrill Gorcunov
` (2 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
To reduce number of ffi calls.
Part-of #689
Suggested-by: Leonid Vasiliev <lvasiliev@tarantool.org>
Reviewed-by: Oleg Babin <olegrok@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/lua/log.lua | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 50c96a8d2..161f403db 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -161,7 +161,7 @@ local function log_level_list()
end
local function say(level, fmt, ...)
- if ffi.C.log_level < level then
+ if log_cfg.level < level then
-- don't waste cycles on debug.getinfo()
return
end
@@ -179,7 +179,7 @@ local function say(level, fmt, ...)
fmt[field] = nil
end
fmt = json.encode(fmt)
- if ffi.C.log_format == ffi.C.SF_JSON then
+ if log_cfg.format == fmt_num2str[ffi.C.SF_JSON] then
-- indicate that message is already encoded in JSON
format = fmt_num2str[ffi.C.SF_JSON]
end
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [Tarantool-patches] [PATCH v8 12/12] test: logger -- consider more cases
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (10 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 11/12] lua/log: use log_cfg instead of ffi's instances Cyrill Gorcunov
@ 2020-06-02 22:18 ` Cyrill Gorcunov
2020-06-03 7:03 ` Oleg Babin
2020-06-03 6:59 ` [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Oleg Babin
2020-06-03 7:09 ` Oleg Babin
13 siblings, 1 reply; 23+ messages in thread
From: Cyrill Gorcunov @ 2020-06-02 22:18 UTC (permalink / raw)
To: tml
Part-of #689
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
test/app-tap/logger.test.lua | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 48dc8af3e..6ad15724e 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,7 +1,7 @@
#!/usr/bin/env tarantool
local test = require('tap').test('log')
-test:plan(44)
+test:plan(49)
-- gh-3946: Assertion failure when using log_format() before box.cfg()
local log = require('log')
@@ -46,6 +46,19 @@ test:is(line:sub(-m:len()), m, m)
_, err = pcall(log.cfg, {format = 'unknown'})
test:ok(err:find("log: \'format\' must be json,plain") ~= nil)
+-- Test static arguments
+_, err = pcall(box.cfg, {log = "newname"})
+test:ok(err:find("log: \'log\' can\'t be set dynamically") ~= nil)
+
+_, err = pcall(box.cfg, {log_nonblock = true})
+test:ok(err:find("log: \'log_nonblock\' can\'t be set dynamically") ~= nil)
+
+_, err = pcall(log.cfg, {nonblock = true})
+test:ok(err:find("log: \'nonblock\' can\'t be set dynamically") ~= nil)
+
+_, err = pcall(log.cfg, {log = "newname"})
+test:ok(err:find("log: \'log\' can\'t be set dynamically") ~= nil)
+
-- Test that changes in log module are propagated
-- back to the box module correctly
box.cfg{
@@ -59,6 +72,7 @@ test:is(box.cfg.log_level, 6, 'box level 6')
log.cfg({format='plain', level=5})
test:is(box.cfg.log_format, 'plain', 'box sees plain format')
test:is(box.cfg.log_level, 5, 'box sees level change')
+test:ok(log.cfg.log == filename and box.cfg.log == filename, 'filename match')
-- Test symbolic names for loglevels
log.cfg({level='fatal'})
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 12/12] test: logger -- consider more cases
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 12/12] test: logger -- consider more cases Cyrill Gorcunov
@ 2020-06-03 7:03 ` Oleg Babin
0 siblings, 0 replies; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 7:03 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Thanks for your patch! LGTM
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> Part-of #689
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> test/app-tap/logger.test.lua | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
> index 48dc8af3e..6ad15724e 100755
> --- a/test/app-tap/logger.test.lua
> +++ b/test/app-tap/logger.test.lua
> @@ -1,7 +1,7 @@
> #!/usr/bin/env tarantool
>
> local test = require('tap').test('log')
> -test:plan(44)
> +test:plan(49)
>
> -- gh-3946: Assertion failure when using log_format() before box.cfg()
> local log = require('log')
> @@ -46,6 +46,19 @@ test:is(line:sub(-m:len()), m, m)
> _, err = pcall(log.cfg, {format = 'unknown'})
> test:ok(err:find("log: \'format\' must be json,plain") ~= nil)
>
> +-- Test static arguments
> +_, err = pcall(box.cfg, {log = "newname"})
> +test:ok(err:find("log: \'log\' can\'t be set dynamically") ~= nil)
> +
> +_, err = pcall(box.cfg, {log_nonblock = true})
> +test:ok(err:find("log: \'log_nonblock\' can\'t be set dynamically") ~= nil)
> +
> +_, err = pcall(log.cfg, {nonblock = true})
> +test:ok(err:find("log: \'nonblock\' can\'t be set dynamically") ~= nil)
> +
> +_, err = pcall(log.cfg, {log = "newname"})
> +test:ok(err:find("log: \'log\' can\'t be set dynamically") ~= nil)
> +
> -- Test that changes in log module are propagated
> -- back to the box module correctly
> box.cfg{
> @@ -59,6 +72,7 @@ test:is(box.cfg.log_level, 6, 'box level 6')
> log.cfg({format='plain', level=5})
> test:is(box.cfg.log_format, 'plain', 'box sees plain format')
> test:is(box.cfg.log_level, 5, 'box sees level change')
> +test:ok(log.cfg.log == filename and box.cfg.log == filename, 'filename match')
>
> -- Test symbolic names for loglevels
> log.cfg({level='fatal'})
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{}
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (11 preceding siblings ...)
2020-06-02 22:18 ` [Tarantool-patches] [PATCH v8 12/12] test: logger -- consider more cases Cyrill Gorcunov
@ 2020-06-03 6:59 ` Oleg Babin
2020-06-03 7:09 ` Oleg Babin
13 siblings, 0 replies; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 6:59 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Hi! Thanks for your patchset. I've found another example of
inconsistency between log.cfg and box.cfg:
```
Tarantool 2.5.0-91-ga70675e0e
type 'help' for interactive help
tarantool> box.cfg{log = ''}
2020-06-03 09:57:50.356 [54011] main/103/interactive C> Tarantool
2.5.0-91-ga70675e0e
2020-06-03 09:57:50.357 [54011] main/103/interactive C> log level 5
...
2020-06-03 09:57:50.408 [54011] main/103/interactive I> set 'log_level'
configuration option to 5
2020-06-03 09:57:50.408 [54011] main/105/checkpoint_daemon I> scheduled
next checkpoint for Wed Jun 3 11:45:13 2020
2020-06-03 09:57:50.408 [54011] main/103/interactive I> set 'log_format'
configuration option to "plain"
```
And
```
Tarantool 2.5.0-91-ga70675e0e
type 'help' for interactive help
tarantool> log = require('log')
---
...
tarantool> log.cfg({log = ''})
SystemError can't open log file: /Users/o.babin/Projects/tarantool/: Is
a directory
failed to initialize logging subsystem
```
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> In the series we add an ability to configure logger early
> without calling box.cfg{}. The syntax is the same as
> in box.cfg{} call.
>
> There was an idea to implement something similar via triggers but
> I think this will require a way more efforts and code redesign,
> so at first lets stick to simplier solution.
>
> Guys, please don't Ack/LGTM the messages which already have Reviewed-by tags.
> Hopefully this is the last series and code is ready for merging. Though
> I should confess I dont like it much even this "simple" solution looks
> too complex.
>
> v2 by Oleg:
> - hide box_api symbols from users
> - initialize logger via log.cfg() call to look similar to box.cfg
> v3 by Oleg:
> - add parametesr verification
> - allow to reconfig via log.cfg() call
> v4 by Oleg:
> - lua style fixes
> - extent test (I didn't find a way to test wrong params verification
> inside tap test, so I did it manually. Still we might extent the
> test on top of the series).
> v5..v6:
> - left for internal use
> v7 by Oleg and Leonid:
> - add bidirectional sync between log and box settings
> - symbolic names for levels
> - test extension
> - reduce ffi usage
> v7 by Oleg:
> - add verification for `log` being immutable once set via
> box or log interface
> - more precise check for dynamic configs
> - test enhansion for new cases
>
> branch gorcunov/gh-689-logger-8
> issue https://github.com/tarantool/tarantool/issues/689
>
> Cyrill Gorcunov (12):
> core/say: do not reconfig early set up logger
> core/say: use say_logger_initialized in say_logger_free
> lua/log: declare say_logger_init and say_logger_initialized
> lua/log: put string constants to map
> lua/log: do not allow to set json for boot logger
> lua/log: declare log as separate variable
> lua/log: use log module settings inside box.cfg
> lua/log: allow to configure logging without a box
> test: logger -- use log module directly
> log/lua: allow to specify logging level as a string
> lua/log: use log_cfg instead of ffi's instances
> test: logger -- consider more cases
>
> src/box/lua/load_cfg.lua | 35 ++++-
> src/lib/core/say.c | 15 +-
> src/lib/core/say.h | 4 +
> src/lua/log.lua | 297 ++++++++++++++++++++++++++++++++---
> test/app-tap/logger.test.lua | 100 +++++++++++-
> 5 files changed, 418 insertions(+), 33 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{}
2020-06-02 22:18 [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
` (12 preceding siblings ...)
2020-06-03 6:59 ` [Tarantool-patches] [PATCH v8 00/12] lua/log: add an ability to setup logger without box.cfg{} Oleg Babin
@ 2020-06-03 7:09 ` Oleg Babin
13 siblings, 0 replies; 23+ messages in thread
From: Oleg Babin @ 2020-06-03 7:09 UTC (permalink / raw)
To: Cyrill Gorcunov, tml
Also note that CI is red -
https://gitlab.com/tarantool/tarantool/-/jobs/578131122
Some log options appeared in test result files, please fix it.
On 03/06/2020 01:18, Cyrill Gorcunov wrote:
> In the series we add an ability to configure logger early
> without calling box.cfg{}. The syntax is the same as
> in box.cfg{} call.
>
> There was an idea to implement something similar via triggers but
> I think this will require a way more efforts and code redesign,
> so at first lets stick to simplier solution.
>
> Guys, please don't Ack/LGTM the messages which already have Reviewed-by tags.
> Hopefully this is the last series and code is ready for merging. Though
> I should confess I dont like it much even this "simple" solution looks
> too complex.
>
> v2 by Oleg:
> - hide box_api symbols from users
> - initialize logger via log.cfg() call to look similar to box.cfg
> v3 by Oleg:
> - add parametesr verification
> - allow to reconfig via log.cfg() call
> v4 by Oleg:
> - lua style fixes
> - extent test (I didn't find a way to test wrong params verification
> inside tap test, so I did it manually. Still we might extent the
> test on top of the series).
> v5..v6:
> - left for internal use
> v7 by Oleg and Leonid:
> - add bidirectional sync between log and box settings
> - symbolic names for levels
> - test extension
> - reduce ffi usage
> v7 by Oleg:
> - add verification for `log` being immutable once set via
> box or log interface
> - more precise check for dynamic configs
> - test enhansion for new cases
>
> branch gorcunov/gh-689-logger-8
> issue https://github.com/tarantool/tarantool/issues/689
>
> Cyrill Gorcunov (12):
> core/say: do not reconfig early set up logger
> core/say: use say_logger_initialized in say_logger_free
> lua/log: declare say_logger_init and say_logger_initialized
> lua/log: put string constants to map
> lua/log: do not allow to set json for boot logger
> lua/log: declare log as separate variable
> lua/log: use log module settings inside box.cfg
> lua/log: allow to configure logging without a box
> test: logger -- use log module directly
> log/lua: allow to specify logging level as a string
> lua/log: use log_cfg instead of ffi's instances
> test: logger -- consider more cases
>
> src/box/lua/load_cfg.lua | 35 ++++-
> src/lib/core/say.c | 15 +-
> src/lib/core/say.h | 4 +
> src/lua/log.lua | 297 ++++++++++++++++++++++++++++++++---
> test/app-tap/logger.test.lua | 100 +++++++++++-
> 5 files changed, 418 insertions(+), 33 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread