From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4D107469710 for ; Tue, 2 Jun 2020 11:05:59 +0300 (MSK) References: <20200601222507.560415-1-gorcunov@gmail.com> <20200601222507.560415-11-gorcunov@gmail.com> From: Oleg Babin Message-ID: <220cea35-47f1-6a1e-5f33-94553ec5302a@tarantool.org> Date: Tue, 2 Jun 2020 11:05:57 +0300 MIME-Version: 1.0 In-Reply-To: <20200601222507.560415-11-gorcunov@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v7 10/11] log/lua: allow to specify logging level as a string List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Also. I've found that it's not allowed to set log level as string via box API: ``` tarantool> box.cfg{log_level = 'info'} --- - error: 'Incorrect value for option ''log_level'': should be of type number' ... ``` But it could be easily fixed here: https://github.com/tarantool/tarantool/blob/master/src/box/lua/load_cfg.lua#L129 On 02/06/2020 01:25, 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 repreentation of loglevels such as: `fatal`, `syserror`, > `error`, `crit`, `warn`, `info`, `verbose` and `debug`. > > Part-of #689 > > Suggested-by: Oleg Babin > Signed-off-by: Cyrill Gorcunov > --- > 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 dfdc326fe..3376932ff 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() > @@ -279,8 +299,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 c6f207908..557c31f1a 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 sees fatal') > +log.cfg({level='syserror'}) > +test:ok(log.cfg.level == 1 and box.cfg.log_level == 1, 'both sees fatal') > +log.cfg({level='error'}) > +test:ok(log.cfg.level == 2 and box.cfg.log_level == 2, 'both sees fatal') > +log.cfg({level='crit'}) > +test:ok(log.cfg.level == 3 and box.cfg.log_level == 3, 'both sees fatal') > +log.cfg({level='warn'}) > +test:ok(log.cfg.level == 4 and box.cfg.log_level == 4, 'both sees fatal') > +log.cfg({level='info'}) > +test:ok(log.cfg.level == 5 and box.cfg.log_level == 5, 'both sees fatal') > +log.cfg({level='verbose'}) > +test:ok(log.cfg.level == 6 and box.cfg.log_level == 6, 'both sees fatal') > +log.cfg({level='debug'}) > +test:ok(log.cfg.level == 7 and box.cfg.log_level == 7, 'both sees fatal') > + > +_,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 > -- >