Re: [Tarantool-patches] [PATCH v4 7/8] lua/log: allow to configure logging without a box

[Oleg Babin <olegrok@tarantool.org>]

Hi! Thanks for your patch. See one comment below.

On 28/05/2020 13:07, Cyrill Gorcunov wrote:
> In the commit we allow to configure logger without
> calling box.cfg{}.
> 
> Fixes #689
> 
> @TarantoolBot document
> Title: Module log
> 
> Module log allows to setup logger early without calling ``box.cfg()``.
> Configuration parameters are same as in ``box.cfg()`` call.
> 
> Example
> ```
>> log = require('log')
>> log.cfg({log='filename', log_format='plain', log_level=6})
> ...
>> log.cfg({log_format='json', log_level=5})
> ```
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>   src/box/lua/load_cfg.lua |  13 +++--
>   src/lua/log.lua          | 117 +++++++++++++++++++++++++++++++++++++--
>   2 files changed, 119 insertions(+), 11 deletions(-)
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 9aef12840..00e424156 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -471,11 +471,13 @@ 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
> +    log.box_api.cfg_apply_default(cfg)
> +end
> +
> +local function update_modules_cfg(cfg)
> +    --
> +    -- logging
> +    log.box_api.cfg_update(cfg)
>   end
>   
>   -- Return true if two configurations are equivalent.
> @@ -597,6 +599,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 ce546f21e..31a80e18a 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.
> @@ -162,18 +170,17 @@ local function log_rotate()
>   end
>   
>   local function log_level(level)
> +    if type(level) ~= 'number' then
> +        error("log: 'level' (or 'log_level' option) expects a number")
> +    end

Seems we could easily add an ability to specify log level as string.
log.cfg{log_level = 'info'} - it will be really more user-friendly than 
currently.

Also it's possible to specify "invalid" numbers:
```
tarantool> require('log').cfg({log_level = 0/0})
---
...

tarantool> require('log').cfg
---
- log_format: json
   log_level: nan
   log_nonblock: false
...
```
I think we could restict levels from 0/1 to 7.
All available log_levels are exposed as `enum say_level` and you can use 
it as you've done it for `say_format`s.