Re: [Tarantool-patches] [PATCH v7 07/11] lua/log: use log module settings inside box.cfg

[Oleg Babin <olegrok@tarantool.org>]

Hi! Thanks for your patch.

I've found a strange case:
```
tarantool> log.cfg({log = '1.txt'})
---
...

tarantool> log.cfg({log = '2.txt'}) -- error - OK
---
- error: 'builtin/log.lua:273: log: ''log'' can''t be set dynamically'
...

tarantool> box.cfg{log = '2.txt'} -- no error (why?)
---
...

tarantool> log.error('test')
---
...
```

log.cfg and box.cfg shows:
```
tarantool> log.cfg.log
---
- 2.txt
...

tarantool> box.cfg.log
---
- 2.txt
...
```

But actually "1.txt" is my log file.
Also I suggest to cover such situation with a test :)

On 02/06/2020 01:25, 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,
>       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..33e98fd84 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        = nil,
> +    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;
>   })
>   
>