Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{}
@ 2020-05-26 12:46 Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 01/10] core/say: drop redundant declarations Cyrill Gorcunov
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

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.

v2:
 - hide box_api symbols from users
 - initialize logger via log.cfg() call to look similar
   to box.cfg

branch gorcunov/gh-689-logger-2
issue https://github.com/tarantool/tarantool/issues/689

Cyrill Gorcunov (10):
  core/say: drop redundant declarations
  core/say: drop vsay declaration
  core/say: do not reconfig early set up logger
  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: use direct log module

 src/box/lua/load_cfg.lua     |  28 ++++--
 src/lib/core/say.c           |  18 ++--
 src/lib/core/say.h           |   8 +-
 src/lua/log.lua              | 169 ++++++++++++++++++++++++++++++-----
 test/app-tap/logger.test.lua |  28 +++++-
 5 files changed, 212 insertions(+), 39 deletions(-)

-- 
2.26.2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 01/10] core/say: drop redundant declarations
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 02/10] core/say: drop vsay declaration Cyrill Gorcunov
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

Part-of #689

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lib/core/say.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 7116883d4..e22089aac 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -244,11 +244,6 @@ say_format_by_name(const char *format)
 	return STR2ENUM(say_format, format);
 }
 
-static void
-write_to_file(struct log *log, int total);
-static void
-write_to_syslog(struct log *log, int total);
-
 /**
  * Sets O_NONBLOCK flag in case if lognonblock is set.
  */
-- 
2.26.2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 02/10] core/say: drop vsay declaration
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 01/10] core/say: drop redundant declarations Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 03/10] core/say: do not reconfig early set up logger Cyrill Gorcunov
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

The implementation has been removed in
commit ef2c171d3e8f1d675c1cab548838a03ae67a2a66

Part-of #689

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lib/core/say.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/lib/core/say.h b/src/lib/core/say.h
index 43883f0da..c50d7bbf4 100644
--- a/src/lib/core/say.h
+++ b/src/lib/core/say.h
@@ -278,10 +278,6 @@ say_logger_init(const char *init_str,
 void
 say_logger_free();
 
-CFORMAT(printf, 5, 0) void
-vsay(int level, const char *filename, int line, const char *error,
-     const char *format, va_list ap);
-
 /** \cond public */
 typedef void (*sayfunc_t)(int, const char *, int, const char *,
 		    const char *, ...);
-- 
2.26.2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 03/10] core/say: do not reconfig early set up logger
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 01/10] core/say: drop redundant declarations Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 02/10] core/say: drop vsay declaration Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 04/10] lua/log: declare say_logger_init and say_logger_initialized Cyrill Gorcunov
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

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

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] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 04/10] lua/log: declare say_logger_init and say_logger_initialized
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (2 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 03/10] core/say: do not reconfig early set up logger Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 05/10] lua/log: put string constants to map Cyrill Gorcunov
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

We gonna use it to provide a way to initialize
logger subsystem early.

Part-of #689

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lua/log.lua | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/lua/log.lua b/src/lua/log.lua
index 312c14d5e..9830b0886 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -1,5 +1,7 @@
 -- log.lua
 --
+-- vim: ts=4 sw=4 et
+
 local ffi = require('ffi')
 ffi.cdef[[
     typedef void (*sayfunc_t)(int level, const char *filename, int line,
@@ -22,6 +24,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] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 05/10] lua/log: put string constants to map
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (3 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 04/10] lua/log: declare say_logger_init and say_logger_initialized Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 06/10] lua/log: do not allow to set json for boot logger Cyrill Gorcunov
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

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

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lua/log.lua | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/lua/log.lua b/src/lua/log.lua
index 9830b0886..3fceb1d0e 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -83,6 +83,20 @@ 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 say(level, fmt, ...)
     if ffi.C.log_level < level then
         -- don't waste cycles on debug.getinfo()
@@ -104,7 +118,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)
@@ -135,16 +149,23 @@ 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 keyset = {}
+        for k,_ in pairs(fmt_str2num) do
+            keyset[#keyset+1] = k
+        end
+        local m = "log_format: expected %s"
+        error(m:format(table.concat(keyset,',')))
+    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] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 06/10] lua/log: do not allow to set json for boot logger
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (4 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 05/10] lua/log: put string constants to map Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 07/10] lua/log: declare log as separate variable Cyrill Gorcunov
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

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 | 1 -
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/lua/log.lua b/src/lua/log.lua
index 3fceb1d0e..c6e03ad78 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..7bfa06e80 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -5,7 +5,6 @@ test:plan(24)
 
 -- gh-3946: Assertion failure when using log_format() before box.cfg()
 local log = require('log')
-log.log_format('json')
 log.log_format('plain')
 
 --
-- 
2.26.2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 07/10] lua/log: declare log as separate variable
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (5 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 06/10] lua/log: do not allow to set json for boot logger Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 08/10] lua/log: use log module settings inside box.cfg Cyrill Gorcunov
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

We gonna hide some internal symbols in sake
of communication with box module, thus lets
make it clear.

Part-of #689

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 c6e03ad78..bee2851c8 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] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 08/10] lua/log: use log module settings inside box.cfg
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (6 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 07/10] lua/log: declare log as separate variable Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 09/10] lua/log: allow to configure logging without a box Cyrill Gorcunov
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

Since we're going to implement early setup of
logging module we need both.

Part-of #689

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/lua/load_cfg.lua | 21 +++++++++++++++------
 src/lua/log.lua          | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 5d818addf..9aef12840 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -1,4 +1,6 @@
 -- load_cfg.lua - internal file
+--
+-- vim: ts=4 sw=4 et
 
 local log = require('log')
 local json = require('json')
@@ -59,10 +61,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 +231,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 +468,16 @@ local function apply_default_cfg(cfg, default_cfg)
     end
 end
 
+local function apply_default_modules_cfg(cfg)
+    --
+    -- logging
+    for k,v in pairs(log.box_api.cfg) do
+        if cfg[k] == nil then
+            cfg[k] = v
+        end
+    end
+end
+
 -- Return true if two configurations are equivalent.
 local function compare_cfg(cfg1, cfg2)
     if type(cfg1) ~= type(cfg2) then
@@ -554,6 +562,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);
+    apply_default_modules_cfg(cfg)
     -- Save new box.cfg
     box.cfg = cfg
     if not pcall(private.cfg_check)  then
diff --git a/src/lua/log.lua b/src/lua/log.lua
index bee2851c8..25c919412 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -97,6 +97,22 @@ local fmt_str2num = {
     ["json"]            = ffi.C.SF_JSON,
 }
 
+--
+-- Default options. The keys are part of
+-- user API, so change with caution.
+local default_cfg = {
+    log             = nil,
+    log_nonblock    = nil,
+    log_level       = S_INFO,
+    log_format      = fmt_num2str[ffi.C.SF_PLAIN],
+}
+
+local log_cfg = setmetatable(default_cfg, {
+    __newindex = function()
+        error('log: Attempt to modify a read-only table')
+    end,
+})
+
 local function say(level, fmt, ...)
     if ffi.C.log_level < level then
         -- don't waste cycles on debug.getinfo()
@@ -146,7 +162,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, 'log_level', level)
 end
 
 local function log_format(name)
@@ -170,6 +187,7 @@ local function log_format(name)
     else
         ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
     end
+    rawset(log_cfg, 'log_format', name)
 end
 
 local function log_pid()
@@ -197,9 +215,26 @@ 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 = {
+        cfg = log_cfg,
+        set_log_level = function()
+            log_level(box.cfg.log_level)
+        end,
+        set_log_format = function()
+            log_format(box.cfg.log_format)
+        end,
+    },
 }
 
 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] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 09/10] lua/log: allow to configure logging without a box
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (7 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 08/10] lua/log: use log module settings inside box.cfg Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 10/10] test: use direct log module Cyrill Gorcunov
  2020-05-26 13:42 ` [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Oleg Babin
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

Now a user can configure logging without running `box.cfg{}`.
The syntax is the same as in `box.cfg{}` call.

Fixes #689

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/lua/load_cfg.lua |  7 +++++
 src/lua/log.lua          | 68 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 9aef12840..232a1695a 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -478,6 +478,12 @@ local function apply_default_modules_cfg(cfg)
     end
 end
 
+local function update_modules_cfg(cfg)
+    --
+    -- logging
+    log.box_api.cfg_update(cfg)
+end
+
 -- Return true if two configurations are equivalent.
 local function compare_cfg(cfg1, cfg2)
     if type(cfg1) ~= type(cfg2) then
@@ -597,6 +603,7 @@ local function load_cfg(cfg)
             end
         end
     end
+    update_modules_cfg(cfg)
     if not box.cfg.read_only and not box.cfg.replication then
         box.schema.upgrade{auto = true}
     end
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 25c919412..c82390897 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -97,6 +97,14 @@ local fmt_str2num = {
     ["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
+
 --
 -- Default options. The keys are part of
 -- user API, so change with caution.
@@ -168,12 +176,8 @@ end
 
 local function log_format(name)
     if not fmt_str2num[name] then
-        local keyset = {}
-        for k,_ in pairs(fmt_str2num) do
-            keyset[#keyset+1] = k
-        end
         local m = "log_format: expected %s"
-        error(m:format(table.concat(keyset,',')))
+        error(m:format(fmt_list()))
     end
 
     if fmt_str2num[name] == ffi.C.SF_JSON then
@@ -194,6 +198,55 @@ local function log_pid()
     return tonumber(ffi.C.log_pid)
 end
 
+--
+-- Initialize logger early (if not yet set up
+-- via box.cfg interface.
+--
+local function log_init(args)
+    if ffi.C.say_logger_initialized() == true then
+        error("log: the logger is already initialized")
+    end
+
+    args = args or {}
+
+    if args.log_format ~= nil then
+        if fmt_str2num[args.log_format] == nil then
+            local m = "log: 'log_format' must be %s"
+            error(m:format(fmt_list()))
+        end
+    else
+        args.log_format = log_cfg.log_format
+    end
+
+    args.log_level = args.log_level or log_cfg.log_level
+
+    args.log_nonblock = args.log_nonblock or (log_cfg.log_nonblock or false)
+
+    --
+    -- 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(args.log, args.log_level,
+                          args.log_nonblock, args.log_format, 0)
+
+    --
+    -- Update log_cfg vars to show them in module
+    -- configuration output.
+    rawset(log_cfg, 'log', args.log)
+    rawset(log_cfg, 'log_level', args.log_level)
+    rawset(log_cfg, 'log_nonblock', args.log_nonblock)
+    rawset(log_cfg, 'log_format', args.log_format)
+end
+
+--
+-- Reflect the changes made by box.cfg interface
+local function box_cfg_update(box_cfg)
+    rawset(log_cfg, 'log', box_cfg.log)
+    rawset(log_cfg, 'log_level', box_cfg.log_level)
+    rawset(log_cfg, 'log_nonblock', box_cfg.log_nonblock)
+    rawset(log_cfg, 'log_format', box_cfg.log_format)
+end
+
 local compat_warning_said = false
 local compat_v16 = {
     logger_pid = function()
@@ -215,6 +268,7 @@ local log ={
     pid = log_pid,
     level = log_level,
     log_format = log_format,
+    cfg = log_init,
     --
     -- Internal API to box module, not for users,
     -- names can be changed.
@@ -226,6 +280,7 @@ local log ={
         set_log_format = function()
             log_format(box.cfg.log_format)
         end,
+        cfg_update = box_cfg_update,
     },
 }
 
@@ -233,6 +288,9 @@ setmetatable(log, {
     __serialize = function(self)
         local res = table.copy(self)
         res.box_api = nil
+        if ffi.C.say_logger_initialized() == true then
+            res.cfg = log_cfg
+        end
         return setmetatable(res, {})
     end,
     __index = compat_v16;
-- 
2.26.2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Tarantool-patches] [PATCH v2 10/10] test: use direct log module
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (8 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 09/10] lua/log: allow to configure logging without a box Cyrill Gorcunov
@ 2020-05-26 12:46 ` Cyrill Gorcunov
  2020-05-26 13:42 ` [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Oleg Babin
  10 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 12:46 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

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 | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 7bfa06e80..410220494 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,23 +1,44 @@
 #!/usr/bin/env tarantool
 
 local test = require('tap').test('log')
-test:plan(24)
+test:plan(27)
 
 -- gh-3946: Assertion failure when using log_format() before box.cfg()
 local log = require('log')
 log.log_format('plain')
 
+--
+-- gh-689: Operate with logger via log module without calling box.cfg{}
+local json = require('json')
+local filename = "1.log"
+
+log.cfg({log=filename, log_format='json', log_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")
+log.log_format('plain')
+
 --
 -- Check that Tarantool creates ADMIN session for #! script
 --
-local filename = "1.log"
 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] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{}
  2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
                   ` (9 preceding siblings ...)
  2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 10/10] test: use direct log module Cyrill Gorcunov
@ 2020-05-26 13:42 ` Oleg Babin
  2020-05-26 14:00   ` Cyrill Gorcunov
  10 siblings, 1 reply; 13+ messages in thread
From: Oleg Babin @ 2020-05-26 13:42 UTC (permalink / raw)
  To: Cyrill Gorcunov, tml; +Cc: Alexander Turenko

Hi! Thanks for your changes.

I have several comments:

1. log.cfg() should be idempotent I think:

```
tarantool> box.cfg{log_level = 2}
---
...

tarantool> box.cfg{log_level = 2}
---
...

tarantool> log.cfg{level = 2}
---
- error: 'builtin/log.lua:207: log: the logger is already initialized'
...
```

2. It would be great to add some validation for input parameters:
```
tarantool> log.level()
---
- error: 'builtin/log.lua:173: bad argument #1 to ''say_set_log_level'' 
(cannot convert
     ''nil'' to ''int'')'
...

tarantool> log.level(-1)
---
...

tarantool> log.level(100000)
---
...

```
Usually functions without arguments return current value of some 
property. See space:format()/:before_replace()/:on_replace functions.
But I believe that we should support log.cfg({level = ...}) instead of 
log.level(...).


Also please don't forget about changelog entry and docbot request.

And seems I've missed this issue 
(https://github.com/tarantool/tarantool/issues/1054) when I asked about 
log.cfg. I think you could add "Closes #1054" as well.

On 26/05/2020 15:46, 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.
> 
> v2:
>   - hide box_api symbols from users
>   - initialize logger via log.cfg() call to look similar
>     to box.cfg
> 
> branch gorcunov/gh-689-logger-2
> issue https://github.com/tarantool/tarantool/issues/689
> 
> Cyrill Gorcunov (10):
>    core/say: drop redundant declarations
>    core/say: drop vsay declaration
>    core/say: do not reconfig early set up logger
>    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: use direct log module
> 
>   src/box/lua/load_cfg.lua     |  28 ++++--
>   src/lib/core/say.c           |  18 ++--
>   src/lib/core/say.h           |   8 +-
>   src/lua/log.lua              | 169 ++++++++++++++++++++++++++++++-----
>   test/app-tap/logger.test.lua |  28 +++++-
>   5 files changed, 212 insertions(+), 39 deletions(-)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{}
  2020-05-26 13:42 ` [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Oleg Babin
@ 2020-05-26 14:00   ` Cyrill Gorcunov
  0 siblings, 0 replies; 13+ messages in thread
From: Cyrill Gorcunov @ 2020-05-26 14:00 UTC (permalink / raw)
  To: Oleg Babin; +Cc: tml, Alexander Turenko

On Tue, May 26, 2020 at 04:42:52PM +0300, Oleg Babin wrote:
> Hi! Thanks for your changes.
> 
> I have several comments:
...
> 
> And seems I've missed this issue
> (https://github.com/tarantool/tarantool/issues/1054) when I asked about
> log.cfg. I think you could add "Closes #1054" as well.

Thanks! Will address in next series!

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-05-26 14:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 12:46 [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 01/10] core/say: drop redundant declarations Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 02/10] core/say: drop vsay declaration Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 03/10] core/say: do not reconfig early set up logger Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 04/10] lua/log: declare say_logger_init and say_logger_initialized Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 05/10] lua/log: put string constants to map Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 06/10] lua/log: do not allow to set json for boot logger Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 07/10] lua/log: declare log as separate variable Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 08/10] lua/log: use log module settings inside box.cfg Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 09/10] lua/log: allow to configure logging without a box Cyrill Gorcunov
2020-05-26 12:46 ` [Tarantool-patches] [PATCH v2 10/10] test: use direct log module Cyrill Gorcunov
2020-05-26 13:42 ` [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{} Oleg Babin
2020-05-26 14:00   ` Cyrill Gorcunov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox