[Tarantool-patches] [PATCH 0/3] log: allow json formatter in boottime logger

[Tarantool-patches] [PATCH 0/3] log: allow json formatter in boottime logger

[Cyrill Gorcunov]

For some reason we've disabled usage of json formatter in early
logging since the commit 098324556. Lets allow it back.

issue https://github.com/tarantool/tarantool/issues/5121
branch gorcunov/gh-5121-logger-boot-json

Cyrill Gorcunov (3):
  core/say: allow to use json in boot logger
  lua/log: allow to use json formatter early
  test: app-tap/logger -- test json in boottime logger

 src/lib/core/say.c           | 7 ++-----
 src/lua/log.lua              | 6 ++----
 test/app-tap/logger.test.lua | 8 +++++---
 3 files changed, 9 insertions(+), 12 deletions(-)

-- 
2.26.2

[Tarantool-patches] [PATCH 1/3] core/say: allow to use json in boot logger

[Cyrill Gorcunov]

For some reason in commit 098324556 we've disabled
to use json format in boot time logger. There is
no reason to do so.

Only syslog output format is predefined and must not
be changed, in turn json format is just a decoration
over output stream so we can use it whenever requested.

Part-of #5121

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

diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 791011e6f..b903cb03f 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -206,17 +206,14 @@ say_set_log_format(enum say_format format)
 {
 	log_format_func_t format_func;
 	/*
-	 * When logger type is SAY_LOGGER_BOOT it simply prints
-	 * every message to stdout intact and can't be formatted.
 	 * SAY_LOGGER_SYSLOG type uses the well-documented and
 	 * *recommended* format described in the RFC below:
 	 * https://tools.ietf.org/html/rfc3164#section-4.1
 	 * Thereby format can't be changed for this type either.
 	 */
-	bool unchangeable = log_default->type == SAY_LOGGER_BOOT ||
-			    log_default->type == SAY_LOGGER_SYSLOG;
-	if (unchangeable)
+	if (log_default->type == SAY_LOGGER_SYSLOG)
 		return;
+
 	switch (format) {
 	case SF_JSON:
 		format_func = say_format_json;
-- 
2.26.2

[Tarantool-patches] [PATCH 2/3] lua/log: allow to use json formatter early

[Cyrill Gorcunov]

There is no reason to not allow for json formatter
on early logging stage.

Fixes #5121

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

diff --git a/src/lua/log.lua b/src/lua/log.lua
index bed690526..66fdaca7b 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -203,10 +203,8 @@ local function verify_format(key, name)
     end
 
     if fmt_str2num[name] == ffi.C.SF_JSON then
-        if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG or
-            ffi.C.log_type() == ffi.C.SAY_LOGGER_BOOT then
-            local m = "%s can't be used with " ..
-            "syslog or boot-time logger"
+        if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG then
+            local m = "%s can't be used with syslog logger"
             return false, m:format(fmt_num2str[ffi.C.SF_JSON])
         end
     end
-- 
2.26.2

[Tarantool-patches] [PATCH 3/3] test: app-tap/logger -- test json in boottime logger

[Cyrill Gorcunov]

Make sure we're allowed to setup json formatter before
box.cfg() call, ie that named boot-time logger.

Part-of #5121

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 test/app-tap/logger.test.lua | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index ae4a3b99a..41276c3d0 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -3,11 +3,13 @@
 local test = require('tap').test('log')
 test:plan(62)
 
--- gh-3946: Assertion failure when using log_format() before box.cfg()
+--
+-- gh-5121: Allow to use 'json' output before box.cfg()
+--
 local log = require('log')
-log.log_format('plain')
 _, err = pcall(log.log_format, 'json')
-test:ok(err:find("json can\'t be used") ~= nil)
+test:ok(err == nil)
+log.log_format('plain')
 
 --
 -- gh-689: various settings change from box.cfg/log.cfg interfaces
-- 
2.26.2

Re: [Tarantool-patches] [PATCH 0/3] log: allow json formatter in boottime logger

[Oleg Babin]

Hi! Thanks for your patch. Looks good but I want to clarify one thing:


```

tarantool> log.cfg{log = 'syslog', format='json'}
---
...

```

It works fine but seems such command should throw an error:

"json can't be used with syslog logger", isn't it?

It happened because "ffi.C.log_type()" returns "0" -- boot-time logger.


On 29/06/2020 14:23, Cyrill Gorcunov wrote:
> For some reason we've disabled usage of json formatter in early
> logging since the commit 098324556. Lets allow it back.
>
> issue https://github.com/tarantool/tarantool/issues/5121
> branch gorcunov/gh-5121-logger-boot-json
>
> Cyrill Gorcunov (3):
>    core/say: allow to use json in boot logger
>    lua/log: allow to use json formatter early
>    test: app-tap/logger -- test json in boottime logger
>
>   src/lib/core/say.c           | 7 ++-----
>   src/lua/log.lua              | 6 ++----
>   test/app-tap/logger.test.lua | 8 +++++---
>   3 files changed, 9 insertions(+), 12 deletions(-)
>

Re: [Tarantool-patches] [PATCH 0/3] log: allow json formatter in boottime logger

[Cyrill Gorcunov]

On Mon, Jun 29, 2020 at 03:50:00PM +0300, Oleg Babin wrote:
> Hi! Thanks for your patch. Looks good but I want to clarify one thing:
> 
> 
> ```
> 
> tarantool> log.cfg{log = 'syslog', format='json'}
> ---
> ...
> 
> ```
> 
> It works fine but seems such command should throw an error:
> 
> "json can't be used with syslog logger", isn't it?
> 
> It happened because "ffi.C.log_type()" returns "0" -- boot-time logger.

Good catch! Due to code structure this case is not covered.
I'll update the series and the test. Thanks a lot!!!

Re: [Tarantool-patches] [PATCH 0/3] log: allow json formatter in boottime logger

[Kirill Yukhin]

Hello,

On 29 июн 14:23, Cyrill Gorcunov wrote:
> For some reason we've disabled usage of json formatter in early
> logging since the commit 098324556. Lets allow it back.
> 
> issue https://github.com/tarantool/tarantool/issues/5121
> branch gorcunov/gh-5121-logger-boot-json

I've checked your patchset into master.

--
Regards, Kirill Yukhin

[Tarantool-patches] [PATCH v2 0/3] log: allow json formatter in boottime logger

[Cyrill Gorcunov]

For some reason we've disabled usage of json formatter in early
logging since the commit 098324556. Lets allow it back.

v2:
 - add early verification of options for cases like
   box.cfg{log="syslog:", log_format="json"}
 - extend test

branch gorcunov/gh-5121-logger-boot-json-2
issue https://github.com/tarantool/tarantool/issues/5121

Cyrill Gorcunov (3):
  core/say: allow to use json in boot logger
  lua/log: allow to use json formatter early
  test: app-tap/logger -- test json in boottime logger

 src/box/lua/load_cfg.lua     |  6 ++---
 src/lib/core/say.c           |  7 ++----
 src/lua/log.lua              | 43 +++++++++++++++++++++++++++---------
 test/app-tap/logger.test.lua | 19 ++++++++++++----
 4 files changed, 53 insertions(+), 22 deletions(-)

-- 
2.26.2

[Tarantool-patches] [PATCH 1/3] core/say: allow to use json in boot logger

[Cyrill Gorcunov]

For some reason in commit 098324556 we've disabled
to use json format in boot time logger. There is
no reason to do so.

Only syslog output format is predefined and must not
be changed, in turn json format is just a decoration
over output stream so we can use it whenever requested.

Part-of #5121

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

diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 791011e6f..b903cb03f 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -206,17 +206,14 @@ say_set_log_format(enum say_format format)
 {
 	log_format_func_t format_func;
 	/*
-	 * When logger type is SAY_LOGGER_BOOT it simply prints
-	 * every message to stdout intact and can't be formatted.
 	 * SAY_LOGGER_SYSLOG type uses the well-documented and
 	 * *recommended* format described in the RFC below:
 	 * https://tools.ietf.org/html/rfc3164#section-4.1
 	 * Thereby format can't be changed for this type either.
 	 */
-	bool unchangeable = log_default->type == SAY_LOGGER_BOOT ||
-			    log_default->type == SAY_LOGGER_SYSLOG;
-	if (unchangeable)
+	if (log_default->type == SAY_LOGGER_SYSLOG)
 		return;
+
 	switch (format) {
 	case SF_JSON:
 		format_func = say_format_json;
-- 
2.26.2