change(ui): Enable the progress bar feature by default, but only show…

… progress bars when the config is enabled (#7615)

* Add a progress bar config that is disabled unless the feature is on

* Simplify the default config

* Enable the progress bar feature by default, but require the config

* Rename progress bars config to avoid merge conflicts

* Use a log file when the progress bar is activated

* Document how to configure progress bars

* Handle log files in config_tests and check config path

* Fix doc link

* Fix path check

* Fix config log matching

* Fix clippy warning

* Add tracing to config tests

* It's zebrad not zebra

* cargo fmt --all

* Update release for config file changes

* Fix config test failures

* Allow printing to stdout in a method

Cargo.lock

@@ -5779,6 +5779,7 @@ dependencies = [
  "humantime",
  "indexmap 2.0.1",
  "insta",
+ "itertools 0.11.0",
  "lazy_static",
  "once_cell",
  "owo-colors",

README.md

@@ -11,7 +11,7 @@
 - [Getting Started](#getting-started)
   - [Docker](#docker)
   - [Building Zebra](#building-zebra)
-    - [Optional Features](#optional-features)
+    - [Optional Configs & Features](#optional-features)
 - [Known Issues](#known-issues)
 - [Future Work](#future-work)
 - [Documentation](#documentation)
@@ -116,13 +116,24 @@ zebrad start
 See the [Installing Zebra](https://zebra.zfnd.org/user/install.html) and [Running Zebra](https://zebra.zfnd.org/user/run.html)
 sections in the book for more details.
 
-#### Optional Features
+#### Optional Configs & Features
+
+##### Configuring Progress Bars
+
+Configure `tracing.progress_bar` in your `zebrad.toml` to
+[show key metrics in the terminal using progress bars](https://zfnd.org/experimental-zebra-progress-bars/).
+When progress bars are active, Zebra automatically sends logs to a file.
+
+In future releases, the `progress_bar = "summary"` config will show a few key metrics,
+and the "detailed" config will show all available metrics. Please let us know which metrics are
+important to you!
+
+##### Custom Build Features
 
 You can also build Zebra with additional [Cargo features](https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options):
 
 - `getblocktemplate-rpcs` for [mining support](https://zebra.zfnd.org/user/mining.html)
 - `prometheus` for [Prometheus metrics](https://zebra.zfnd.org/user/metrics.html)
-- `progress-bar` [experimental progress bars](https://zfnd.org/experimental-zebra-progress-bars/)
 - `sentry` for [Sentry monitoring](https://zebra.zfnd.org/user/tracing.html#sentry-production-monitoring)
 - `elasticsearch` for [experimental Elasticsearch support](https://zebra.zfnd.org/user/elasticsearch.html)
 

zebra-test/Cargo.toml

@@ -19,6 +19,7 @@ hex = "0.4.3"
 indexmap = "2.0.1"
 lazy_static = "1.4.0"
 insta = "1.33.0"
+itertools = "0.11.0"
 proptest = "1.3.1"
 once_cell = "1.18.0"
 rand = "0.8.5"

zebra-test/src/command.rs

@@ -21,10 +21,11 @@ use tracing::instrument;
 
 #[macro_use]
 mod arguments;
+
 pub mod to_regex;
 
 pub use self::arguments::Arguments;
-use self::to_regex::{CollectRegexSet, ToRegex, ToRegexSet};
+use self::to_regex::{CollectRegexSet, ToRegexSet};
 
 /// A super-trait for [`Iterator`] + [`Debug`].
 pub trait IteratorDebug: Iterator + Debug {}
@@ -791,7 +792,7 @@ impl<T> TestChild<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn expect_stdout_line_matches<R>(&mut self, success_regex: R) -> Result<String>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
         self.apply_failure_regexes_to_outputs();
 
@@ -823,7 +824,7 @@ impl<T> TestChild<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn expect_stderr_line_matches<R>(&mut self, success_regex: R) -> Result<String>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
         self.apply_failure_regexes_to_outputs();
 
@@ -855,7 +856,7 @@ impl<T> TestChild<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn expect_stdout_line_matches_silent<R>(&mut self, success_regex: R) -> Result<String>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
         self.apply_failure_regexes_to_outputs();
 
@@ -887,7 +888,7 @@ impl<T> TestChild<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn expect_stderr_line_matches_silent<R>(&mut self, success_regex: R) -> Result<String>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
         self.apply_failure_regexes_to_outputs();
 
@@ -1246,9 +1247,9 @@ impl<T> TestOutput<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn stdout_matches<R>(&self, regex: R) -> Result<&Self>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
-        let re = regex.to_regex().expect("regex must be valid");
+        let re = regex.to_regex_set().expect("regex must be valid");
 
         self.output_check(
             |stdout| re.is_match(stdout),
@@ -1270,9 +1271,9 @@ impl<T> TestOutput<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn stdout_line_matches<R>(&self, regex: R) -> Result<&Self>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
-        let re = regex.to_regex().expect("regex must be valid");
+        let re = regex.to_regex_set().expect("regex must be valid");
 
         self.any_output_line(
             |line| re.is_match(line),
@@ -1300,9 +1301,9 @@ impl<T> TestOutput<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn stderr_matches<R>(&self, regex: R) -> Result<&Self>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
-        let re = regex.to_regex().expect("regex must be valid");
+        let re = regex.to_regex_set().expect("regex must be valid");
 
         self.output_check(
             |stderr| re.is_match(stderr),
@@ -1324,9 +1325,9 @@ impl<T> TestOutput<T> {
     #[allow(clippy::unwrap_in_result)]
     pub fn stderr_line_matches<R>(&self, regex: R) -> Result<&Self>
     where
-        R: ToRegex + Debug,
+        R: ToRegexSet + Debug,
     {
-        let re = regex.to_regex().expect("regex must be valid");
+        let re = regex.to_regex_set().expect("regex must be valid");
 
         self.any_output_line(
             |line| re.is_match(line),

zebra-test/src/command/to_regex.rs

@@ -2,6 +2,7 @@
 
 use std::iter;
 
+use itertools::Itertools;
 use regex::{Error, Regex, RegexBuilder, RegexSet, RegexSetBuilder};
 
 /// A trait for converting a value to a [`Regex`].
@@ -135,15 +136,17 @@ pub trait CollectRegexSet {
 impl<I> CollectRegexSet for I
 where
     I: IntoIterator,
-    I::Item: ToRegex,
+    I::Item: ToRegexSet,
 {
     fn collect_regex_set(self) -> Result<RegexSet, Error> {
-        let regexes: Result<Vec<Regex>, Error> =
-            self.into_iter().map(|item| item.to_regex()).collect();
+        let regexes: Result<Vec<RegexSet>, Error> = self
+            .into_iter()
+            .map(|item| item.to_regex_set())
+            .try_collect();
         let regexes = regexes?;
 
         // This conversion discards flags and limits from Regex and RegexBuilder.
-        let regexes = regexes.iter().map(|regex| regex.as_str());
+        let regexes = regexes.iter().flat_map(|regex_set| regex_set.patterns());
 
         RegexSet::new(regexes)
     }

zebrad/Cargo.toml

@@ -52,7 +52,7 @@ features = [
 
 [features]
 # In release builds, don't compile debug logging code, to improve performance.
-default = ["release_max_level_info"]
+default = ["release_max_level_info", "progress-bar"]
 
 # Default features for official ZF binary release builds
 default-release-binaries = ["default", "sentry"]

zebrad/src/application.rs

@@ -434,6 +434,7 @@ impl Application for ZebradApp {
             // Override the default tracing filter based on the command-line verbosity.
             tracing_config.filter = tracing_config
                 .filter
+                .clone()
                 .or_else(|| Some(default_filter.to_owned()));
         } else {
             // Don't apply the configured filter for short-lived commands.

zebrad/src/components/tracing.rs

@@ -1,6 +1,10 @@
 //! Tracing and logging infrastructure for Zebra.
 
-use std::{net::SocketAddr, path::PathBuf};
+use std::{
+    net::SocketAddr,
+    ops::{Deref, DerefMut},
+    path::PathBuf,
+};
 
 use serde::{Deserialize, Serialize};
 
@@ -16,10 +20,59 @@ pub use endpoint::TracingEndpoint;
 #[cfg(feature = "flamegraph")]
 pub use flame::{layer, Grapher};
 
-/// Tracing configuration section.
-#[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(deny_unknown_fields, default)]
+/// Tracing configuration section: outer config after cross-field defaults are applied.
+///
+/// This is a wrapper type that dereferences to the inner config type.
+///
+//
+// TODO: replace with serde's finalizer attribute when that feature is implemented.
+//       we currently use the recommended workaround of a wrapper struct with from/into attributes.
+//       https://github.com/serde-rs/serde/issues/642#issuecomment-525432907
+#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize, Serialize)]
+#[serde(
+    deny_unknown_fields,
+    default,
+    from = "InnerConfig",
+    into = "InnerConfig"
+)]
 pub struct Config {
+    inner: InnerConfig,
+}
+
+impl Deref for Config {
+    type Target = InnerConfig;
+
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
+
+impl DerefMut for Config {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.inner
+    }
+}
+
+impl From<InnerConfig> for Config {
+    fn from(mut inner: InnerConfig) -> Self {
+        inner.log_file = runtime_default_log_file(inner.log_file, inner.progress_bar);
+
+        Self { inner }
+    }
+}
+
+impl From<Config> for InnerConfig {
+    fn from(mut config: Config) -> Self {
+        config.log_file = disk_default_log_file(config.log_file.clone(), config.progress_bar);
+
+        config.inner
+    }
+}
+
+/// Tracing configuration section: inner config used to deserialize and apply cross-field defaults.
+#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
+#[serde(deny_unknown_fields, default)]
+pub struct InnerConfig {
     /// Whether to use colored terminal output, if available.
     ///
     /// Colored terminal output is automatically disabled if an output stream
@@ -108,10 +161,16 @@ pub struct Config {
     /// replaced with `.folded` and `.svg` for the respective files.
     pub flamegraph: Option<PathBuf>,
 
+    /// Shows progress bars for block syncing, and mempool transactions, and peer networking.
+    /// Also sends logs to the default log file path.
+    ///
+    /// This config field is ignored unless the `progress-bar` feature is enabled.
+    pub progress_bar: Option<ProgressConfig>,
+
     /// If set to a path, write the tracing logs to that path.
     ///
     /// By default, logs are sent to the terminal standard output.
-    /// But if the `progress-bar` feature is activated, logs are sent to the standard log file path:
+    /// But if the `progress_bar` config is activated, logs are sent to the standard log file path:
     /// - Linux: `$XDG_STATE_HOME/zebrad.log` or `$HOME/.local/state/zebrad.log`
     /// - macOS: `$HOME/Library/Application Support/zebrad.log`
     /// - Windows: `%LOCALAPPDATA%\zebrad.log` or `C:\Users\%USERNAME%\AppData\Local\zebrad.log`
@@ -131,6 +190,21 @@ pub struct Config {
     pub use_journald: bool,
 }
 
+/// The progress bars that Zebra will show while running.
+#[derive(Copy, Clone, Debug, Default, Eq, PartialEq, Deserialize, Serialize)]
+#[serde(rename_all = "lowercase")]
+pub enum ProgressConfig {
+    /// Show a lot of progress bars.
+    Detailed,
+
+    /// Show a few important progress bars.
+    //
+    // TODO: actually hide some progress bars in this mode.
+    #[default]
+    #[serde(other)]
+    Summary,
+}
+
 impl Config {
     /// Returns `true` if standard output should use color escapes.
     /// Automatically checks if Zebra is running in a terminal.
@@ -152,12 +226,10 @@ impl Config {
     }
 }
 
-impl Default for Config {
+impl Default for InnerConfig {
     fn default() -> Self {
-        #[cfg(feature = "progress-bar")]
-        let default_log_file = dirs::state_dir()
-            .or_else(dirs::data_local_dir)
-            .map(|dir| dir.join("zebrad.log"));
+        // TODO: enable progress bars by default once they have been tested
+        let progress_bar = None;
 
         Self {
             use_color: true,
@@ -166,11 +238,50 @@ impl Default for Config {
             buffer_limit: 128_000,
             endpoint_addr: None,
             flamegraph: None,
-            #[cfg(not(feature = "progress-bar"))]
-            log_file: None,
-            #[cfg(feature = "progress-bar")]
-            log_file: default_log_file,
+            progress_bar,
+            log_file: runtime_default_log_file(None, progress_bar),
             use_journald: false,
         }
     }
 }
+
+/// Returns the runtime default log file path based on the `log_file` and `progress_bar` configs.
+fn runtime_default_log_file(
+    log_file: Option<PathBuf>,
+    progress_bar: Option<ProgressConfig>,
+) -> Option<PathBuf> {
+    if let Some(log_file) = log_file {
+        return Some(log_file);
+    }
+
+    // If the progress bar is active, we want to use a log file regardless of the config.
+    // (Logging to a terminal erases parts of the progress bars, making both unreadable.)
+    if progress_bar.is_some() {
+        return default_log_file();
+    }
+
+    None
+}
+
+/// Returns the configured log file path using the runtime `log_file` and `progress_bar` config.
+///
+/// This is the inverse of [`runtime_default_log_file()`].
+fn disk_default_log_file(
+    log_file: Option<PathBuf>,
+    progress_bar: Option<ProgressConfig>,
+) -> Option<PathBuf> {
+    // If the progress bar is active, and we've likely substituted the default log file path,
+    // don't write that substitute to the config on disk.
+    if progress_bar.is_some() && log_file == default_log_file() {
+        return None;
+    }
+
+    log_file
+}
+
+/// Returns the default log file path.
+fn default_log_file() -> Option<PathBuf> {
+    dirs::state_dir()
+        .or_else(dirs::data_local_dir)
+        .map(|dir| dir.join("zebrad.log"))
+}