From 9c0dcd7dd147a1302d1b14c4d07ca246ac72c5b6 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Mon, 23 Oct 2023 17:00:23 +0300
Subject: [PATCH] log: make log.cfg{modules=...} work as
box.cfg{log_modules=...}
Configuring log modules work differently with log.cfg and box.cfg:
box.cfg{log_modules=...} overwrites the current config completely while
log.cfg{modules=...} overwrites the currently config only for the
specified modules. Let's fix this inconsistency by making log.cfg behave
exactly as box.cfg.
Closes #7962
NO_DOC=bug fix
(cherry picked from commit c13e59a555966c1c8a0948041233afa270957fd3)
---
.../unreleased/gh-7962-log-cfg-modules-fix.md | 6 ++
src/box/lua/load_cfg.lua | 34 +++++-----
src/lua/log.lua | 3 +-
.../gh_3211_per_module_log_level_test.lua | 63 +++++++------------
4 files changed, 43 insertions(+), 63 deletions(-)
create mode 100644 changelogs/unreleased/gh-7962-log-cfg-modules-fix.md
diff --git a/changelogs/unreleased/gh-7962-log-cfg-modules-fix.md b/changelogs/unreleased/gh-7962-log-cfg-modules-fix.md
new file mode 100644
index 0000000000..b6988ea969
--- /dev/null
+++ b/changelogs/unreleased/gh-7962-log-cfg-modules-fix.md
@@ -0,0 +1,6 @@
+## bugfix/core
+
+* Fixed the behavior of `log.cfg{modules = ...}`. Now, instead of merging the
+ new log modules configuration with the old one, it completely overwrites the
+ current configuration, which is consistent with `box.cfg{log_modules = ...}`
+ (gh-7962).
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 5199ca18c6..1813fb3936 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -771,11 +771,10 @@ local function check_cfg_option_type(template, name, value)
end
end
-local function prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
+local function prepare_cfg(cfg, old_cfg, default_cfg, template_cfg, modify_cfg)
if cfg == nil then
- return {}
- end
- if type(cfg) ~= 'table' then
+ cfg = {}
+ elseif type(cfg) ~= 'table' then
error("Error: cfg should be a table")
end
local new_cfg = {}
@@ -793,6 +792,15 @@ local function prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
end
new_cfg[k] = v
end
+ -- Use the old config for omitted options.
+ for k, v in pairs(old_cfg) do
+ -- Don't override options set to box.NULL (which equals nil but
+ -- evaluates to true) because setting an option to box.NULL must
+ -- be equivalent to resetting it to the default value.
+ if cfg[k] == nil and not cfg[k] then
+ new_cfg[k] = v
+ end
+ end
return new_cfg
end
@@ -809,16 +817,6 @@ local function apply_env_cfg(cfg, env_cfg, skip_cfg)
end
end
-local function merge_cfg(cfg, cur_cfg)
- for k,v in pairs(cur_cfg) do
- if cfg[k] == nil then
- cfg[k] = v
- elseif type(v) == 'table' then
- merge_cfg(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
@@ -907,8 +905,8 @@ end
local function reload_cfg(oldcfg, cfg)
cfg = upgrade_cfg(cfg, translate_cfg)
- local newcfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
-
+ local newcfg = prepare_cfg(cfg, {}, default_cfg, template_cfg,
+ modify_cfg)
local module_keys = {}
-- iterate over original table because prepare_cfg() may store NILs
for key in pairs(cfg) do
@@ -1004,8 +1002,7 @@ local function load_cfg(cfg)
-- Set options passed through environment variables.
apply_env_cfg(cfg, box.internal.cfg.env, pre_load_cfg_is_set)
- cfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
- merge_cfg(cfg, pre_load_cfg);
+ cfg = prepare_cfg(cfg, pre_load_cfg, default_cfg, template_cfg, modify_cfg)
-- Save new box.cfg
box.cfg = cfg
@@ -1227,7 +1224,6 @@ end
box.internal.prepare_cfg = prepare_cfg
box.internal.apply_env_cfg = apply_env_cfg
-box.internal.merge_cfg = merge_cfg
box.internal.check_cfg_option_type = check_cfg_option_type
box.internal.update_cfg = update_cfg
box.internal.env_cfg = env_cfg
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 1f8ce74ea2..13e36082df 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -445,8 +445,7 @@ local function log_configure(self, cfg, box_api)
local env_cfg = box.internal.env_cfg(box2log_keys)
box.internal.apply_env_cfg(cfg, box_to_log_cfg(env_cfg))
end
- cfg = box.internal.prepare_cfg(cfg, default_cfg, option_types)
- box.internal.merge_cfg(cfg, log_cfg);
+ cfg = box.internal.prepare_cfg(cfg, log_cfg, default_cfg, option_types)
end
log_check_cfg(cfg)
diff --git a/test/box-luatest/gh_3211_per_module_log_level_test.lua b/test/box-luatest/gh_3211_per_module_log_level_test.lua
index 8fc2e48177..391eb0cc1c 100644
--- a/test/box-luatest/gh_3211_per_module_log_level_test.lua
+++ b/test/box-luatest/gh_3211_per_module_log_level_test.lua
@@ -80,14 +80,6 @@ g2.test_per_module_log_level = function(cg)
local log = require('log')
local log_cfg = _G.log_cfg
- local function assert_log_and_box_cfg_equals()
- t.assert_equals(log.cfg.level, box.cfg.log_level)
- t.assert_equals(log.cfg.modules.mod1, box.cfg.log_modules.mod1)
- t.assert_equals(log.cfg.modules.mod2, box.cfg.log_modules.mod2)
- t.assert_equals(log.cfg.modules.mod3, box.cfg.log_modules.mod3)
- t.assert_equals(log.cfg.modules.mod4, box.cfg.log_modules.mod4)
- end
-
t.assert_error_msg_contains(
"modules': should be of type table",
log_cfg, {modules = 'hello'})
@@ -99,6 +91,14 @@ g2.test_per_module_log_level = function(cg)
"Incorrect value for option 'log_modules.mod2': " ..
"expected crit,warn,info,debug,syserror,verbose,fatal,error",
log_cfg, {modules = {mod2 = 'hello'}})
+ t.assert_error_msg_content_equals(
+ "Incorrect value for option 'log_modules.mod3': " ..
+ "expected crit,warn,info,debug,syserror,verbose,fatal,error",
+ log_cfg, {modules = {mod3 = ''}})
+ t.assert_error_msg_content_equals(
+ "Incorrect value for option 'log_modules.mod4': " ..
+ "should be one of types number, string",
+ log_cfg, {modules = {mod4 = box.NULL}})
t.assert_error_msg_content_equals(
"Incorrect value for option 'module name': should be of type string",
log_cfg, {modules = {[123] = 'debug'}})
@@ -112,40 +112,23 @@ g2.test_per_module_log_level = function(cg)
-- per-module levels.
log_cfg{level = 'warn'}
t.assert_equals(log.cfg.level, 'warn')
- t.assert_equals(log.cfg.modules.mod1, 'debug')
- t.assert_equals(log.cfg.modules.mod2, 2)
- t.assert_equals(log.cfg.modules.mod3, 'error')
- assert_log_and_box_cfg_equals()
- --[[ TODO(gh-7962)
- -- Check that log.cfg{modules = {...}} with the new modules appends them
- -- to the existing ones.
- log_cfg{modules = {mod4 = 4}}
- t.assert_equals(log.cfg.modules.mod1, 'debug')
- t.assert_equals(log.cfg.modules.mod2, 2)
- t.assert_equals(log.cfg.modules.mod3, 'error')
- t.assert_equals(log.cfg.modules.mod4, 4)
- assert_log_and_box_cfg_equals()
- -- Check that log.cfg{modules = {...}} sets levels for the specified
- -- modules, and doesn't affect other modules.
- log_cfg{modules = {mod1 = 0, mod3 = 'info'}}
- t.assert_equals(log.cfg.modules.mod1, 0)
- t.assert_equals(log.cfg.modules.mod2, 2)
- t.assert_equals(log.cfg.modules.mod3, 'info')
- t.assert_equals(log.cfg.modules.mod4, 4)
- assert_log_and_box_cfg_equals()
- -- Check that box.NULL and the empty string remove the corresponding
- -- module from the config.
- log_cfg{modules = {mod2 = box.NULL, mod4 = ''}}
- t.assert_equals(log.cfg.modules.mod1, 0)
- t.assert_equals(log.cfg.modules.mod2, nil)
- t.assert_equals(log.cfg.modules.mod3, 'info')
- t.assert_equals(log.cfg.modules.mod4, nil)
- assert_log_and_box_cfg_equals()
+ t.assert_equals(box.cfg.log_level, log.cfg.level)
+ t.assert_equals(log.cfg.modules, {
+ mod1 = 'debug', mod2 = 2, mod3 = 'error'
+ })
+ t.assert_equals(box.cfg.log_modules, log.cfg.modules)
+ -- Check that log.cfg{modules = {...}} overwrites old modules.
+ log_cfg{modules = {mod3 = 'info', mod4 = 4}}
+ t.assert_equals(log.cfg.modules, {mod3 = 'info', mod4 = 4})
+ t.assert_equals(box.cfg.log_modules, log.cfg.modules)
+ t.assert_equals(log.cfg.level, 'warn')
+ t.assert_equals(box.cfg.log_level, log.cfg.level)
-- Check that log.cfg{modules = box.NULL} removes all modules.
log_cfg{modules = box.NULL}
t.assert_equals(log.cfg.modules, nil)
t.assert_equals(box.cfg.log_modules, nil)
- --]]
+ t.assert_equals(log.cfg.level, 'warn')
+ t.assert_equals(box.cfg.log_level, log.cfg.level)
-- Check that log levels actually affect what is printed to the log.
log_cfg{level = 'info', modules = {module2 = 4,
module3 = 'debug'}}
@@ -226,8 +209,4 @@ g2.test_tarantool_module = function(cg)
end)
find_in_log(cg, 'Lua verbose message 3', false)
find_in_log(cg, 'C debug message 3', true)
-
- -- TODO(gh-7962)
- -- Check that box.NULL unsets custom Tarantool C log level.
- -- _G.log_cfg{modules = {tarantool = box.NULL}}
end
--
GitLab