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 c13e59a)

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).

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

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)

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