feat: config.yaml: move instance.log* parameters to a instance.log subsection

src/config.rs

@@ -219,11 +219,11 @@ Using configuration file '{args_path}'.");
         }
 
         if let Some(log_level) = args.log_level {
-            self.instance.log_level = Some(log_level);
+            self.instance.log.level = Some(log_level);
         }
 
         if let Some(log_destination) = args.log {
-            self.instance.log = Some(log_destination);
+            self.instance.log.destination = Some(log_destination);
         }
 
         if let Some(audit_destination) = args.audit {
@@ -614,9 +614,6 @@ pub struct InstanceConfig {
     pub deprecated_script: Option<String>,
 
     pub audit: Option<String>,
-    pub log_level: Option<args::LogLevel>,
-    pub log: Option<String>,
-    pub log_format: Option<String>,
 
     // box.cfg.background currently doesn't work with our supervisor/child implementation
     // pub background: Option<bool>,
@@ -628,6 +625,10 @@ pub struct InstanceConfig {
 
     pub memtx_memory: Option<u64>,
 
+    #[serde(default)]
+    #[introspection(nested)]
+    pub log: LogSection,
+
     /// Special catch-all field which will be filled by serde with all unknown
     /// fields from the yaml file.
     #[serde(flatten)]
@@ -706,7 +707,7 @@ impl InstanceConfig {
 
     #[inline]
     pub fn log_level(&self) -> SayLevel {
-        if let Some(level) = self.log_level {
+        if let Some(level) = self.log.level {
             level.into()
         } else {
             SayLevel::Info
@@ -724,6 +725,48 @@ impl InstanceConfig {
     }
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// LogSection
+////////////////////////////////////////////////////////////////////////////////
+
+#[derive(
+    PartialEq,
+    Default,
+    Debug,
+    Clone,
+    serde::Deserialize,
+    serde::Serialize,
+    tlua::Push,
+    tlua::PushInto,
+    Introspection,
+)]
+#[serde(deny_unknown_fields)]
+pub struct LogSection {
+    pub level: Option<args::LogLevel>,
+
+    /// By default, Picodata sends the log to the standard error stream
+    /// (stderr). If `log.destination` is specified, Picodata can send the log to a:
+    /// - file
+    /// - pipe
+    /// - system logger
+    pub destination: Option<String>,
+
+    pub format: Option<LogFormat>,
+}
+
+tarantool::define_str_enum! {
+    #[derive(Default)]
+    pub enum LogFormat {
+        #[default]
+        Plain = "plain",
+        Json = "json",
+    }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// deserialize_map_forbid_duplicate_keys
+////////////////////////////////////////////////////////////////////////////////
+
 pub fn deserialize_map_forbid_duplicate_keys<'de, D, K, V>(
     des: D,
 ) -> Result<HashMap<K, V>, D::Error>
@@ -1299,13 +1342,13 @@ instance:
             );
             let mut config = PicodataConfig::read_yaml_contents(&yaml).unwrap();
             let args = args::Run::try_parse_from(["run",
-                "-c", "  instance.log_level =debug  ",
+                "-c", "  instance.log .level =debug  ",
                 "--config-parameter", "instance. memtx_memory=  0xdeadbeef",
             ]).unwrap();
             config.set_from_args(args).unwrap();
             assert_eq!(config.instance.tier.unwrap(), "ABC");
             assert_eq!(config.cluster.cluster_id.unwrap(), "DEF");
-            assert_eq!(config.instance.log_level.unwrap(), args::LogLevel::Debug);
+            assert_eq!(config.instance.log.level.unwrap(), args::LogLevel::Debug);
             assert_eq!(config.instance.memtx_memory.unwrap(), 0xdead_beef);
             assert_eq!(config.instance.audit.unwrap(), "audit.txt");
             assert_eq!(config.instance.data_dir.unwrap(), ".");

src/tarantool.rs

@@ -208,7 +208,7 @@ impl Cfg {
     }
 
     pub fn set_core_parameters(&mut self, config: &PicodataConfig) -> Result<(), Error> {
-        self.log = config.instance.log.clone();
+        self.log = config.instance.log.destination.clone();
         self.log_level = Some(config.instance.log_level() as _);
 
         self.wal_dir = config.instance.data_dir();
@@ -221,6 +221,8 @@ impl Cfg {
 
         #[rustfmt::skip]
         const MAPPING: &[(&str, &str)] = &[
+            // Other instance.log.* parameters are set explicitly above
+            ("log_format",                  "instance.log.format"),
         ];
         for (box_field, picodata_field) in MAPPING {
             let value = config

test/int/test_config_file.py

@@ -46,7 +46,8 @@ instance:
     replicaset_id: my-replicaset
     tier: deluxe
     audit: {data_dir}/audit.log
-    log_level: verbose
+    log:
+        level: verbose
 """
     )
     instance = Instance(
@@ -87,7 +88,7 @@ instance:
             config_file=instance.config_path,
             data_dir=data_dir,
             listen=dict(host=host, port=str(port)),
-            log_level="verbose",
+            log=dict(level="verbose"),
             peers=[dict(host=host, port=str(port))],
             unknown_parameters=[],
         ),
@@ -244,3 +245,59 @@ error: option `--init-replication-factor` cannot be used with `--config` simulta
     i1.fail_to_start()
 
     assert crawler.matched
+
+
+def test_config_file_box_cfg_parameters(cluster: Cluster):
+    #
+    # Check default values
+    #
+    cluster.set_config_file(
+        yaml="""
+# just the required part
+cluster:
+    cluster_id: test
+    tiers:
+        default:
+"""
+    )
+    i1 = cluster.add_instance(wait_online=False)
+    i1.start()
+    i1.wait_online()
+
+    box_cfg = i1.eval("return box.cfg")
+
+    assert box_cfg.get("log") is None  # type: ignore
+    assert box_cfg["log_level"] == 6  # means verbose -- set by our testing harness
+    assert box_cfg["log_format"] == "plain"
+
+    #
+    # Check explicitly set values
+    #
+    cluster.config_path = None
+    cluster.set_config_file(
+        yaml="""
+cluster:
+    cluster_id: test
+    tiers:
+        default:
+
+instance:
+    log:
+        destination: file:/proc/self/fd/2  # this is how you say `stderr` explicitly
+        level: debug
+        format: json
+"""
+    )
+
+    # XXX: Just pretend this value comes from the config,
+    # even though this will override any value from the config
+    i1.env["PICODATA_LOG_LEVEL"] = "debug"
+
+    i1.restart(remove_data=True)
+    i1.wait_online()
+
+    box_cfg = i1.eval("return box.cfg")
+
+    assert box_cfg["log"] == "file:/proc/self/fd/2"
+    assert box_cfg["log_level"] == 7  # means debug
+    assert box_cfg["log_format"] == "json"